[pulseaudio-discuss] [PATCH] Do not use tsched watermark if tsched is disabled

[David Henningsson]

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