2010-08-19 07:32, Tanu Kaskinen skrev: > On Wed, 2010-08-18 at 22:55 +0200, David Henningsson wrote: >>> Yes the safeguard is needed in both cases, timer scheduling or good >>> ol' audio interrupts. This comes from limitations of the >>> snd_pcm_rewind() routine, you can rewind the appl_ptr all the way to >>> the hardware pointer, but you may have DMA transfers in flight that >>> cannot be rewound. For example HDaudio can fetch up to 128bytes, I >>> added a default a safeguard of 256bytes to be super safe, but in >>> theory this should be adjusted depending on how the DMA operates. >> >> Assuming your reasoning is correct (I'm not that deep into DMA yet), >> this should be fixed in the kernel - by not allowing rewinds further >> back than 128 (or 256) bytes ahead of actual position. >> You say HDA can transfer up to 128 bytes in advance, but what about all >> the other drivers? Could there be other drivers having a lot larger DMA >> fetches? > > What's the role of snd_pcm_rewindable()[1]? The documentation says "Get > safe count of frames which can be rewinded", which sounds to me like the > function should always be called before snd_pcm_rewind(). Currently PA > doesn't call _rewindable(). Good question. Looking at the code in alsa-lib, it seems like both snd_pcm_avail and snd_pcm_rewindable call snd_pcm_mmap_hw_avail, so in practice it probably doesn't matter, at least not for the common cases. That said, it would likely be a good thing to change it anyway, since the alsa-lib implementation might change. >>> The >>> DMA knows nothing about milliseconds, it behaves the same way no >>> matter the sampling rate it, which is the reason why bytes make more >>> sense, it's easier to correlate with the way the hardware works. >> >> So your idea is to prevent DMA transfers being modified, but I'm >> thinking of the maximum duration between the rewind and the point it >> gets filled up again by PA - all of that time we risc getting an XRUN >> because there is nothing in the buffer. And so I assume that the >> duration is never longer than 20 ms. >> >> I don't think it is much of a deal though. Rewind is not something used >> every second or so (or at least, shouldn't be). > > A comment on the last statement: I don't think the average frequency of > rewinds is important, because there are cases where multiple rewinds do > happen really quickly - for example when dragging a volume slider (IIRC > pavucontrol ratelimits the volume change events to minimum of 100ms > between each event sent to the daemon - that's still ten times a > second). I don't remember hearing complaints about that the volume is > crackling when changing volumes, but at least in theory this seems like > a possible problem with the current implementation. Ah, are you saying we might have fixed that crackling as well? That would be even better :-) For volume slider changes I don't think there is a problem with waiting 20 ms before the volume change takes effect. -- David Henningsson, Canonical Ltd. http://launchpad.net/~diwic
