[RAOP] [PATCH] Fix audio synchronisation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06 September 2017 at 14h59, Tanu Kaskinen wrote:

Hi Tanu, 

> > +    latency += 2000000; /* RAOP default latency */
> > +    latency += pa_raop_client_get_rtp_diff(u->raop) * 1000; /*
> > plus last packet length in usec */  
> 
> pa_raop_client_get_rtp_diff() returns the size of the last audio chunk
> in bytes divided by four, but you treat that as a time value. That's a
> clear bug.

You may well be right! I'm not quite used to the RTP protocol, but it
seemed to me that rtptime is fed into the 'timestamp' part of the
RTP header and that it represented a time - from what I gathered from
the spec.

I derived rtpdiff from the previous way of incrementing rtptime:

> -    c->rtptime += length / 4;
> +    c->rtpdiff = length / 4;
> +    c->rtptime += c->rtpdiff;

it is indeed incremented in a way that I don't understand, though, so
I'm not sure about that.

> Adding the size of the last packet doesn't make sense to me anyway,
> though. The size of the last packet is already taken into account in
> u->write_count. Now you're counting it twice.  

My reasoning was that the "last" (I should have commented "current",
maybe) packet doesn't get played before the next one is received, in
which case it makes sense that it adds an "packet duration" latency.

I did find the audio to be better synchronised to the video with this
patch, than with stock PulseAudio and shifting audio -2.3s in mplayer, 
which is what made me think that calculation made sense.

> If the 2 second constant results in too low latency, I think the
> constant should be made bigger.

If this seems too much like guesswork to you, I can understand that and
change the patch to set the latency empirically to a bigger value
(around 2.3 seconds). I can also, if we go this route, set the default
latency empirically and add a parameter to the module to allow changing
it.

One way or another is great anyway, compared to having to shift audio
in the media player (and being left with unsynced audio on Youtube :) ).

Thanks for your help :)
-- 
Colin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/pulseaudio-discuss/attachments/20170906/c6e738c9/attachment-0001.sig>


[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux