GNOME Bugzilla – Bug 745539
rtpjitterbuffer: Applying ts-offset to deadline timer can result in a stalled
Last modified: 2015-03-18 22:18:12 UTC
Discovered in 1.4.1 but, based on code inspection, I see no reason it won't happen in master. Two scenarios illustrate the issue (assume no lost packets for simplicity), retransmissions enabled. 1. Simple case: No upstream timestamping, so no expected timers are queued. - First packet comes in, dts zero, deadline timer timeout is equal to the latency. Because there are no expected timers, this is the only timer. - wait_next_timeout() syncs on the deadline timer. - SR comes in, resulting in a modification to ts-offset. - Sync completes, wait_next_timeout() returns to the top of the loop and 'now = latency'. - When get_timeout() is called for the timer that was just used for sync, the timeout value is now different (and wrong) because ts-offset is added. - timer_timeout !< now (even though we just sync'd on this timer) stuck 2. Upstream timestamping, expected timers are queued. - First packet comes in, dts zero, deadline timer timeout is equal to the latency. Because there are no expected timers, this is the only timer. - wait_next_timeout() syncs on the deadline timer. - Next packet comes in, sync on deadline is unscheduled. Best timer is now the expected timer for the next packet (ok because latency has not been exceeded yet). - SR comes in, resulting in adjustment to ts-offset. From this point forward, EXPECTED timers will outrank the deadline timer until the DTS on incoming packets exceeds the deadline timer's timeout (pretty sure until ts-offset elapses).
I'm testing a patch that adds the latency, but not ts-offset, to the deadline timeout. It resolves scenario 1, will try scenario 2 and then attempt to determine what other issues may arise. If desired I can post the patch here.
Please provide a patch if you can, yes :) Thanks!
I just made a testcase for 1) and it works fine, the timer times out, with the added ts-offset it goes waiting some more and then eventually times out on the final deadline. Try to make a unit test (or improve the one I just made) to show the problem. commit 13804eab7d1361c5c8496613ff3e2a26ceac5238 Author: Wim Taymans <wtaymans@redhat.com> Date: Fri Mar 6 11:39:39 2015 +0100 check: add jitterbuffer unit test See https://bugzilla.gnome.org/show_bug.cgi?id=745539
The test demonstrates the problem, but from your reply it sounds like waiting for the new deadline is expected behavior? I considered the second wait to be incorrect (which can be substantial in some cases) because I inferred that the purpose of the deadline timer was to enforce the initial latency. By sync'ing on a new timeout, data is buffered for longer than the latency. Additionally, if ts-offset is large (say seconds), the additional time buffering may be substantial. For example (Assume the jitterbuffer is configured for buffer-mode=0 where the timeline is controlled by the RTP timestamps): When a client joins a session already in progress, it won't be sync'd to the media timeline until it receives the first SR RTCP packet. Once the SR comes in and is processed, the ts-offset can be added to the PTS of each outgoing buffer to ensure it's timestamp reflects its correct position in the media timeline. If the case where the SR comes in before the latency expires, the deadline timer is extended, delaying the first buffer longer than the desired latency. Is that the desired effect? If so then my understanding is wrong and it's likely that at least #1 is a non-issue and can be closed out.
(In reply to Jason Litzinger from comment #4) > The test demonstrates the problem, but from your reply it sounds like > waiting for the new deadline is expected behavior? It is, after all the latency of the jitterbuffer was increased so we have more time to wait for missing packets. > > I considered the second wait to be incorrect (which can be substantial in > some cases) because I inferred that the purpose of the deadline timer was to > enforce the initial latency. By sync'ing on a new timeout, data is buffered > for longer than the latency. > > Additionally, if ts-offset is large (say seconds), the additional time > buffering may be substantial. > > For example (Assume the jitterbuffer is configured for buffer-mode=0 where > the timeline is controlled by the RTP timestamps): > > When a client joins a session already in progress, it won't be sync'd to the > media timeline until it receives the first SR RTCP packet. Once the SR > comes in and is processed, the ts-offset can be added to the PTS of each > outgoing buffer to ensure it's timestamp reflects its correct position in > the media timeline. > > If the case where the SR comes in before the latency expires, the deadline > timer is extended, delaying the first buffer longer than the desired > latency. Is that the desired effect? yes, if you don't do that, your buffer will be out-of-sync.
I can see how, in some modes, the buffers would be out of sync, but in mode 0, where the timestamp is derived from the RTP timestamp directly (with ts-offset applied to correct PTS), how would a long or short delay cause the buffers to be out of sync? The timestamp should independent of the delay in that mode, correct? Also, given the delay can be arbitrarily long, that will result in no data flowing downstream for a potentially long time. I've actually observed minutes of valid data held up and later discarded. That said, once the old data is gone, everything is sync'd up as expected. Regardless of the above, I suspect my understanding (which predicated the bug) of the intended functionality is flawed. Given that should probably be closed as "Not an issue" and I have something more specific I'll open another bug.
Based on the above, I also think case #2 is working as expected. The EXPECTED timers will outrank the deadline timer until they catch up, at which point the deadline timer will fire and data will begin flowing. Granted, as stated previously, the delay can be long, it all depends on how ts-offset is modified. All in all, this bug is probably a non-issue, shall I mark it as such?