[Bug 640859] basesink incorrectly categorizes timestamp jitter as drift

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=640859
  GStreamer | gst-plugins-base | git

--- Comment #34 from Håvard Graff (hgr) <havard.graff at tandberg.com> 2011-05-02 00:37:20 UTC ---
(In reply to comment #33)
> (In reply to comment #32)
> > We (Tandberg/Cisco) recently changed this value from 125ms to 225ms based on
> > simulations of real-life data from users experiencing glitches in their audio.
> 
> Did you try my patch in attachment #179691 [details]?
> 

I am very happy to try out your patch, but could you make it apply against
current git?

I also think there is a bit of a syntax-mixup here. As I pointed out, the
drift-tolerance variable is actually (IMO) used for two different things. One
is the clock-slaving, where I mean it belongs, and the other is for a
"aligment-threshold" when trying to figure out whether to align the current
buffer with the previous one.

Now, for the sake of argument, lets say that we make another configurable
variable and name it "alignment-threshold", and it will be the maximum amount
of time you will displace a buffer from its rightful timestamp-location in the
ringbuffer in order to align it with the previous buffer. 

Now you could see how this variable is very useful for us. We don´t want to
drift out of lipsync too much, and if we allow the "alignment-threshold" to be
very big, we could potentially be accepting to play out buffers that are either
too early or too late compared to video.

However, since we deal with jittery networks, the timestamps on the buffers are
sometimes very differently inter-spaced, and if we always placed the buffers
according to their timestamps, we would have glitches-galore.

Now I believe the playback-case is similar. You don´t want to allow yourself to
drift too far from your timestamp (sync with video), but you certainly don´t
want glitches in the sound.

If I read your patch correctly, you are allowing for the timestamp to do a sort
of a "back-and-forth" jump, without resynching on both accounts. This I think
is a good idea. We often see one positive and one negative resync shortly after
eachother in the logs. Starvation or bursting are typical causes for this.

I have written a baseaudiosink testsuite where I feed it with actual real-life
"problematic" data. (people who have had problems with audio, and logged it for
me) If you clean up your patch, I could try to run it against the scenarios we
have, and see if there is any improvements.

Also, could you consider making the "jitter-window" configurable with getters
and setters, as well as maybe creating this second "alignment-threshold"
variable so that the confusion drift/jitter stops... :)

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.