[pulseaudio-discuss] [Patch] RAOP: fix audio synchronisation, take two

[Georg Chini]

On 09.09.2017 19:16, Tanu Kaskinen wrote:
> On Sat, 2017-09-09 at 18:07 +0200, Colin Leroy wrote:
>> On 09 September 2017 at 18h00, Tanu Kaskinen wrote:
>>
>> Hi Tanu,
>>
>> Your mail's empty :)
> Well I definitely wrote things, but Evolution did act strange while
> composing the message, and my "sent" folder confirms your
> observation... I supposed I'll have to write it all again :(
>
>> Anyway, after this patch, there still is a bug left (which was not
>> visible with the previous patch where I did
>>
>> if (latency < 0) {
>>      latency = 0;
>> }
>>
>> in sink_get_latency().
>>
>> The bug is that latency's all wrong (like, -10 to -5 seconds instead of
>> +2.3) after a pause (that is, after switching to PA_SINK_PA_SINK_IDLE
>> and then back to PA_SINK_RUNNING). At the first playback, it's fine.
>>
>> I don't understand why, and if you have some pointers on how pa_rtpoll
>> and pa_smoother work, I guess that'd help.
> pa_rtpoll is just a mechanism for sleeping until the set timeout or
> until something happens (pa_rtpoll users can register fds to listen). I
> don't think it's really relevant for latency.
>
> pa_smoother is primarily used for translating between a sound card
> clock and the system clock (they usually run at different speeds). I
> think the name comes from the fact that it also helps to smooth out
> jumps in the sound card clock (e.g. the sound card clock may work in
> steps: it "jumps forward" when a buffer if consumed, and between the
> jumps the time appears to not change at all).

The smoother is used to extrapolate the sound card time between
buffer writes. I guess this is what you mean with "smooth out jumps
in the sound card clock".

>
> I don't quite understand what the "oob" thing is in raop-sink. If I'm
> guessing correctly, the "oob" flag determines whether the sink relies
> solely on the system clock to decide when to write more data, or the
> sink uses the POLLOUT event on the TCP stream to trigger writes. Maybe
> the system clock approach is used with UDP and the POLLOUT approach is
> used with TCP? That would make sense to me.
>
> If the sink is only using the system clock, then there's no need for
> the smoother. Yet it seems to be used unconditionally?

It will do no harm to use the smoother, in that case both times
are just equal.

>
> When waiting for POLLOUT events, then the smoother is useful, because
> the rate at which the data is consumed will not exactly match the
> system clock.
>
> I'm not super confident with my understanding of pa_smoother either,
> but here's how I imagine it's supposed to be used:
>
> Whenever you get a time update from the sound card (I'll call the raop
> device a sound card here), you tell the smoother the current system
> time and the time of the sound card. In this case the "time update" is
> the POLLOUT event: the TCP buffer has been consumed enough that you can
> write more data. I guess the "time" of the sound card is the total
> amount of data (converted to time units) that has been written to the
> TCP socket before the POLLOUT event.
>
> Based on the timing events, the smoother creates an estimate of the
> speed difference between the system clock and the sound card clock.
> This allows you to ask the smoother the time of the sound card (i.e.
> the amount of data it has consumed) right now, given the current system
> time.
>
> When you temporarily stop playing, the smoother has to be told that
> playback has stopped. pa_smoother_pause() and pa_smoother_resume() can
> be used for this. Or maybe pa_smoother_reset()? I really don't know the
> nitty-gritty details here.
>
> It might be easiest to just totally throw away the old smoother when
> pausing, and recreate the smoother in the same way you do when the sink
> is initially created.

It should be fully sufficient to use pa_smoother_reset(). It does a
complete new initialization.

> Then the behaviour will at least be consistent.
> The downside is that you lose the information in the old smoother: it
> has already worked out the speed difference between the system clock
> and the sound card clock, and the new smoother will have to figure that
> out from scratch, meaning that the initial latency reports after a
> pause might be less accurate.

pa_smoother_pause() and pa_smoother_resume() do not seem to
work as expected from my experience. After a smoother_resume()
the initial error is high again, so using pause()/resume() is no big
advantage over reset().

>
> It would be great to have some more documentation in pulsecore/time-
> smoother.h... Like what is the "time offset" used for? What does
> pa_smoother_fix_now() do?
>
The time_offset is the reference point for the system time at
sound_card_time = 0. It is subtracted from the time passed
to smoother_put() or smoother_get().
pa_smoother_fix_now() is only used internally within the
smoother, don't know why it ever was exported. It is used
after a pause to fix up internal variables so that the pause
time does not get counted. Not sure why there is an option
to skip that fix-up.

I have written a patch series that adds an alternate smoother
(not submitted yet, also needs to be re-based). I hope it is
documented somewhat better.