GNOME Bugzilla – Bug 770081
rtpjitterbuffer: do not use an old buffer's receive time to update jitter
Last modified: 2016-09-01 07:21:45 UTC
Created attachment 333555 [details] [review] rtpjitterbuffer: do not use an old buffer's reveive time to update jitter This avoids the delta jumping when receiving such a buffer.
Comment on attachment 333555 [details] [review] rtpjitterbuffer: do not use an old buffer's reveive time to update jitter This means you wouldn't consider reordered packets either. Isn't the "jitter" added by reordering also to be considered in the calculated jitter?
Ideally, we'd do that only for the retransmitted packets. Short of keeping a list of those, though, I don't see a way to tell which are retransmitted, and which are merely out of order. I suppose keeping a list should be OK since it's not going to happen too often. From what I can see, the delta is used to calculate the min in a window, so it can get the "best case" to estimate the clock drift, and using late buffers will not hit the min. So I don't think it matters in that case.
That seems to be a usecase for the RETRANSMITTED flag from bug #769771 with the code from bug #769768
Indeed. And I forgot to upload another patch instead, let me upload it now. Authored not by me, but the author does not want attribution.
Created attachment 333692 [details] [review] do not use an old buffer's reveive time to update jitter
Just a heads up that this patch will conflict heavily with https://bugzilla.gnome.org/show_bug.cgi?id=769768 where you get the same outcome (non-rtx jitter-calulations) but without guessing, since you use the buffer-flag to differentiate rtx and non-rtx. Also, there are lots of tests for this, where as here there are none.
specifically, this one-liner does what this whole patch tries to do: https://bugzilla.gnome.org/attachment.cgi?id=333139&action=diff#a/gst/rtpmanager/gstrtpjitterbuffer.c_sec23
It looks like this bug can be closed as obsolete, assuming that patchset is accepted. I'll leave it open for now, and close when the other is merged.
*** This bug has been marked as a duplicate of bug 769768 ***