| Message ID | 5753FFF0.3050200@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2016-06-05 18:33 GMT+08:00 Alan Young <consult.awy@gmail.com>: > On 05/06/16 02:14, Raymond Yau wrote: > > the point only increment in DMA brust size , so it is not sample accurate > > > https://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/include/linux/dmaengine.h > > * @DMA_RESIDUE_GRANULARITY_BURST: Residue is updated after each transferred > > * burst. This is typically only supported if the hardware has a progress * register of some sort (E.g. a register with the current read/write address * or a register with the amount of bursts/beats/bytes that have been * transferred or still need to be transferred). > > if the driver point callback does not read from hardware register , it should use > > DMA_RESIDUE_GRANULARITY_DESCRIPTOR: Residue reporting is not support. The * DMA channel is only able to tell whether a descriptor has been completed or * not, which means residue reporting is not supported by this channel. The * residue field of the dma_tx_state field will always be 0. > > > > Yes, I understand that. And that is exactly my point. Because of this a > pcm_status() result is only accurate to a granularity of period in most > cases. > > In fact, some drivers, for example imx sdma, declare > DMA_RESIDUE_GRANULARITY_BURST accuracy because sometimes they may have such > an accuracy but in practice, at least for audio, they only actually achieve > period accuracy. > > Regardless of what value of DMA_RESIDUE_GRANULARITY_xxx that a driver > claims to support, it is not really defined how fine a burst might be. So > the end result is, from the point of view of audio, that the resulting > position obtained by the pointer() call is pretty inaccurate. Hence my > proposal to attempt to improve the accuracy of the pcm_status() result > given the above constraints. > http://git.alsa-project.org/?p=alsa-lib.git;a=commitdiff;h=cf40ea169aad366b222283f431addafea6327149;hp=4bdb09126a32feb4394eaeb1d400d87e7c968770 HDA has hardware specific feature (audio wallclock) for the timestamp and period wakeup can be disabled only a few pci drivers which read the residue value from hardware register (e.g. hda-intel, oxygen , .) you have to measure the granularity since the unit may be different for usb, pcie and firewire sound card as the application is based on the pointer for read/write, you still need to use small period size and CPU power if you want to determine the value returned by snd_pcm_rewindable is safe or not > >
On Sun, 05 Jun 2016 12:33:20 +0200, Alan Young wrote: > > Regardless of what value of DMA_RESIDUE_GRANULARITY_xxx that a driver > claims to support, it is not really defined how fine a burst might > be. So the end result is, from the point of view of audio, that the > resulting position obtained by the pointer() call is pretty > inaccurate. Hence my proposal to attempt to improve the accuracy of > the pcm_status() result given the above constraints. Well, the subject appears misleading. What you want isn't the audio timestamp accuracy. From API POV, the accurate position is calculated via the (additional) delay. So, what you want is rather the accurate position delay accounting, and the audio timestamp is merely one of the ways to achieve that. Takashi
On 06/06/16 02:24, Raymond Yau wrote: > http://git.alsa-project.org/?p=alsa-lib.git;a=commitdiff;h=cf40ea169aad366b222283f431addafea6327149;hp=4bdb09126a32feb4394eaeb1d400d87e7c968770 > > HDA has hardware specific feature (audio wallclock) for the timestamp > and period wakeup can be disabled > > > only a few pci drivers which read the residue value from hardware > register (e.g. hda-intel, oxygen , .) you have to measure the > granularity since the unit may be different for usb, pcie and firewire > sound card > Thank you Raymond. Yes, (I think) I understand all that. Let me restate my understanding of the situation. The pointer position will generally be inaccurate by up to a period size. Even when a driver claims to support DMA_RESIDUE_GRANULARITY_BURST, the reported data is still unlikely to be sample accurate (as the size of a burst is undefined). For most drivers the reported position will be inaccurate and the actual accuracy cannot be determined. The delay is also likely to be inaccurate. It is intended that this could include things such as codec delay but for most drivers this is not included. A few drivers, and in particular USB via an estimate, do try to include the in-flight transfer delay. In some ways this is the worst case: the delay may or may not include the transfer delay AND may or may not include the codec delay; what it actually includes is unknown. The result of these is that both the position and delay may be inaccurate and the degree to which this is the case is not known. We can tell that the position has not changed since the last call to snd_pcm_update_hw_ptr0() because new_hw_ptr == old_hw_ptr. We can use this knowledge to adjust the audio_tstamp returned by the time elapsed since the previous call (for SNDRV_PCM_AUDIO_TSTAMP_TYPE_DEFAULT). We can also make such an adjustment even if the position has changed to better deal with the inaccuracy of position reporting. Because of the 2 different types of unknown accuracy in delay, we cannot do the same trick with this. In many ways being able to update the delay in this way would be ideal. If we could, then we could leave audio_tstamp alone and let audio_tstamp_config.report_delay determine if the adjusted delay is also included in that. Since, in practice, most drivers either contain no codec delay value of a constant one, we can still use such a mechanim for most practical cases. Have I got anything wrong above? I'm going to test a revised patch based on these assumptions. > > as the application is based on the pointer for read/write, you still > need to use small period size and CPU power if you want to determine > the value returned by snd_pcm_rewindable is safe or not My point is that I want to find a way to get reported delay and/or audio timestamps that are more accurate that a period time even in the absence of hardware that has specific support for this. Alan.
On 06/06/16 09:34, Takashi Iwai wrote: > On Sun, 05 Jun 2016 12:33:20 +0200, > Alan Young wrote: >> Regardless of what value of DMA_RESIDUE_GRANULARITY_xxx that a driver >> claims to support, it is not really defined how fine a burst might >> be. So the end result is, from the point of view of audio, that the >> resulting position obtained by the pointer() call is pretty >> inaccurate. Hence my proposal to attempt to improve the accuracy of >> the pcm_status() result given the above constraints. > Well, the subject appears misleading. What you want isn't the audio > timestamp accuracy. From API POV, the accurate position is calculated > via the (additional) delay. So, what you want is rather the accurate > position delay accounting, and the audio timestamp is merely one of > the ways to achieve that. > Well, yes, you could put it that way. Whether an accurate delay, combined with the associated timestamp, or an accurate audio delay, I would have the data needed to track audio drift from wallclock time. See my response to Raymond for more detail. Alan.
On 6/6/16 4:42 AM, Alan Young wrote: > On 06/06/16 09:34, Takashi Iwai wrote: >> On Sun, 05 Jun 2016 12:33:20 +0200, >> Alan Young wrote: >>> Regardless of what value of DMA_RESIDUE_GRANULARITY_xxx that a driver >>> claims to support, it is not really defined how fine a burst might >>> be. So the end result is, from the point of view of audio, that the >>> resulting position obtained by the pointer() call is pretty >>> inaccurate. Hence my proposal to attempt to improve the accuracy of >>> the pcm_status() result given the above constraints. >> Well, the subject appears misleading. What you want isn't the audio >> timestamp accuracy. From API POV, the accurate position is calculated >> via the (additional) delay. So, what you want is rather the accurate >> position delay accounting, and the audio timestamp is merely one of >> the ways to achieve that. >> > > Well, yes, you could put it that way. Whether an accurate delay, > combined with the associated timestamp, or an accurate audio delay, I > would have the data needed to track audio drift from wallclock time. I probably need more coffee but how is this patch helping track audio v. wallclock drift? The additional precision is based on wallclock deltas... > > See my response to Raymond for more detail. > > Alan. > > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On 06/06/16 15:53, Pierre-Louis Bossart wrote: > I probably need more coffee but how is this patch helping track audio > v. wallclock drift? The additional precision is based on wallclock > deltas... Of course, it wouldn't, due to being buggy. I think I needed more coffee too. But the basic point is that if the difference between tstamp and (trigger_tstamp + audio_tstamp) varies over time, this will be the drift between the audio output and wallclock time. Depending upon the specific goal, a calculation based on tstamp + delay may be appropriate. This is true today with a granularity of about period time. The idea behind my change is to make the granularity much smaller, preferably < ~1ms. I'll work on developing and testing a patch for consideration before coming back to the last. It will be easier to discuss the merits or otherwise of my proposal with a concrete, working patch to consider. Alan.
On 6/7/16 2:44 AM, Alan Young wrote: > On 06/06/16 15:53, Pierre-Louis Bossart wrote: >> I probably need more coffee but how is this patch helping track audio >> v. wallclock drift? The additional precision is based on wallclock >> deltas... > > > Of course, it wouldn't, due to being buggy. I think I needed more coffee > too. > > But the basic point is that if the difference between tstamp and > (trigger_tstamp + audio_tstamp) varies over time, this will be the drift > between the audio output and wallclock time. You want to track (tstamp - trigger_tstamp) v. (audio_tstamp - trigger_tstamp) > > Depending upon the specific goal, a calculation based on tstamp + delay > may be appropriate. > > This is true today with a granularity of about period time. The idea > behind my change is to make the granularity much smaller, preferably < ~1ms. > > I'll work on developing and testing a patch for consideration before > coming back to the last. It will be easier to discuss the merits or > otherwise of my proposal with a concrete, working patch to consider. > > Alan. >
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index 6b5a811..ea5b525 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -234,7 +234,8 @@ int snd_pcm_update_state(struct snd_pcm_substream *substream, static void update_audio_tstamp(struct snd_pcm_substream *substream, struct timespec *curr_tstamp, - struct timespec *audio_tstamp) + struct timespec *audio_tstamp, + unsigned int adjust_existing_audio_tstamp) { struct snd_pcm_runtime *runtime = substream->runtime;