summaryrefslogtreecommitdiff
path: root/gst/rtpmanager/gstrtpjitterbuffer.c
Commit message (Collapse)AuthorAgeFilesLines
* rtpjitterbuffer: Don't always reset PTS to 0 after a gapAndrew2017-02-271-4/+7
| | | | | | | | | | | | | In function rtp_jitter_buffer_calculate_pts: If gap in incoming RTP timestamps is more than (3 * jbuf->clock_rate) we call rtp_jitter_buffer_reset_skew which resets pts to 0. So components down the pipeline (playes, mixers) just skip frames/samples until pts becomes equal to pts before gap. In version 1.10.2 and before this checking was bypassed for packets with "estimated dts", and gaps were handled correctly. https://bugzilla.gnome.org/show_bug.cgi?id=778341
* jitterbuffer: Don't leak duplicate itemsEdward Hervey2016-12-051-1/+3
| | | | | | | | When providing items with a seqnum, there is a (very small) probability that an element with the same seqnum already exists. Don't forget to free that item if it wasn't inserted. And avoid returning undefined values when dealing with duplicate items
* rtpjitterbuffer: fix timer-reuse bugHavard Graff2016-12-051-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | When doing rtx, the jitterbuffer will always add an rtx-timer for the next sequence number. In the case of the packet corresponding to that sequence number arriving, that same timer will be reused, and simply moved on to wait for the following sequence number etc. Once an rtx-timer expires (after all retries), it will be rescheduled as a lost-timer instead for the same sequence number. Now, if this particular sequence-number now arrives (after the timer has become a lost-timer), the reuse mechanism *should* now set a new rtx-timer for the next sequence number, but the bug is that it does not change the timer-type, and hence schedules a lost-timer for that following sequence number, with the result that you will have a very early lost-event for a packet that might still arrive, and you will never be able to send any rtx for this packet. Found by Erlend Graff - erlend@pexip.com https://bugzilla.gnome.org/show_bug.cgi?id=773891
* rtpjitterbuffer: fix lost-event using dts instead of ptsHavard Graff2016-12-051-180/+184
| | | | | | | | | | | | | | | | | | | | | | | | The lost-event was using a different time-domain (dts) than the outgoing buffers (pts). Given certain network-conditions these two would become sufficiently different and the lost-event contained timestamp/duration that was really wrong. As an example GstAudioDecoder could produce a stream that jumps back and forth in time after receiving a lost-event. The previous behavior calculated the pts (based on the rtptime) inside the rtp_jitter_buffer_insert function, but now this functionality has been refactored into a new function rtp_jitter_buffer_calculate_pts that is called much earlier in the _chain function to make pts available to various calculations that wrongly used dts previously (like the lost-event). There are however two calculations where using dts is the right thing to do: calculating the receive-jitter and the rtx-round-trip-time, where the arrival time of the buffer from the network is the right metric (and is what dts in fact is today). The patch also adds two tests regarding B-frames or the “rtptime-going-backwards”-scenario, as there were some concerns that this patch might break this behavior (which the tests shows it does not).
* rtpjitterbuffer: fix bug in reschedule_timerHavard Graff2016-12-051-7/+12
| | | | | | | | | | | | | | The new timeout is always going to be (timeout + delay), however, the old behavior compared the current timeout to just (timeout), basically being (delay) off. This would happen if rtx-delay == rtx-retry-timeout, with the result that a second rtx attempt for any buffers would be scheduled immediately instead of after rtx-delay ms. Simply calculate (new_timeout = timeout + delay) and then use that instead. https://bugzilla.gnome.org/show_bug.cgi?id=773905
* rtpjitterbuffer: Fix calculating next_seqnum when dropping old buffers from ↵Thomas Bluemel2016-09-141-1/+1
| | | | | | | | | a full queue. Fixes calculating the next sequence number when a ITEM_TYPE_LOST with more than one definitely lost packets is encountered. https://bugzilla.gnome.org/show_bug.cgi?id=769757
* rtpjitterbuffer: improved rtx-rtt averagingHavard Graff2016-09-141-4/+27
| | | | | | | | | | | | | | | | | | The basic idea is this: 1. For *larger* rtx-rtt, weigh a new measurement as before 2. For *smaller* rtx-rtt, be a bit more conservative and weigh a bit less 3. For very large measurements, consider them "outliers" and count them a lot less The idea being that reducing the rtx-rtt is much more harmful then increasing it, since we don't want to be underestimating the rtt of the network, and when using this number to estimate the latency you need for you jitterbuffer, you would rather want it to be a bit larger then a bit smaller, potentially losing rtx-packets. The "outlier-detector" is there to prevent a single skewed measurement to affect the outcome too much. On wireless networks, these are surprisingly common. https://bugzilla.gnome.org/show_bug.cgi?id=769768
* rtpjitterbuffer: Detect whether to assume equidistant spacing when lossStian Selnes2016-09-141-44/+66
| | | | | | | | | | | | Assuming equidistant packet spacing when that's not true leads to more loss than necessary in the case of reordering and jitter. Typically this is true for video where one frame often consists of multiple packets with the same rtp timestamp. In this case it's better to assume that the missing packets have the same timestamp as the last received packet, so that the scheduled lost timer does not time out too early causing the packets to be considered lost even though they may arrive in time. https://bugzilla.gnome.org/show_bug.cgi?id=769768
* rtpjitterbuffer: Don't request rtx if 'now' is past retry periodStian Selnes2016-09-141-1/+2
| | | | | | | | | | | There is no need to schedule another EXPECTED timer if we're already past the retry period. Under normal operation this won't happen, but if there are more timers than the jitterbuffer is able to process in real-time, scheduling more timers will just make the situation worse. Instead, consider this packet as lost and move on. This scenario can occur with high loss rate, low rtt and high configured latency. https://bugzilla.gnome.org/show_bug.cgi?id=769768
* rtpjitterbuffer: Fix lost duration when gap after lost timerStian Selnes2016-09-141-1/+3
| | | | | | | | | | | | This patch fixes an issue with the estimated gap duration when there is a gap immediately after a lost timer has been processed. Previously there was a discrepancy beteen the gap in seqnum and gap in dts which would cause wrong calculated duration. The issue would only be seen with retranmission enabled since when it's disabled lost timers are only created when a packet is received and the actual gap length and last dts is known. https://bugzilla.gnome.org/show_bug.cgi?id=769768
* rtpjitterbuffer: Expose rtx-deadline as a propertyHavard Graff2016-09-141-1/+33
| | | | | | The default -1 gives the old behavior. https://bugzilla.gnome.org/show_bug.cgi?id=769768
* rtpjitterbuffer: Improved expected-timer handling when gap > 0Havard Graff2016-09-141-4/+6
| | | | https://bugzilla.gnome.org/show_bug.cgi?id=769768
* rtpjitterbuffer: Major improvements for RTX statsStian Selnes2016-09-141-61/+252
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Stats should also be collected for unsuccessful packets. rtx-rtt is very important for determining the necessary configured latency on the jitterbuffer. It's especially important to be able to increase the latency when retransmitted packets arrive too late and are considered lost. This patch includes these late packets in the calculation of the various rtx stats, making them more correct and useful. Also in the case where the original packet arrives after a NACK is sent, the received RTX packet should update the stats since it provides useful information about RTT. The RTT is only updated if and only if all requested retranmissions are received. That way the RTT is guaranteed to make sense. If not we don't know which request the packet is a response to and the RTT may be bogus. A consequence of this patch is that RTT is not updated for a request when one of the RTX packets for that seqnum is lost, but that since measured RTT will be more accurate. The implementation store the RTX information from the timed out timers and use this when the retransmitted packet arrives. For performance these timers are stored separately from the "normal" timers in order to not impact performance (see attached performance test). https://bugzilla.gnome.org/show_bug.cgi?id=769768
* rtpjitterbuffer: Add and expose more stats and increase testing of itHavard Graff2016-09-141-7/+44
| | | | | | | Add num-pushed and num-lost. Expose num-late, num-duplicates and avg-jitter. https://bugzilla.gnome.org/show_bug.cgi?id=769768
* rtpjitterbuffer: Option to disable rtx-delay-reorderHavard Graff2016-09-141-33/+35
| | | | | | | | | | | | | | | | | | | | | | | | | When disabled we can save some iterations over timers. There is probably an argument for rtx-delay-reorder to exist, but for normal operations, handling jitter (reordering) is something a jitterbuffer should do, and this variable feels like functionality that is not "in-sync" with what the jitterbuffer is trying to achieve. Example: You have 50ms jitter on your network, and are receiving audio packets with 10ms durations. An audio packet should not be considered late until its rtx-timeout has expired (and hence a rtx-event is sent), but with rtx-delay-reorder, events will be sent pretty much all the time due to the jitter on the network. Point being: The jitterbuffer should adapt its size to the measured network jitter, and then rtx-delay-reorder needs to adapt as well, or simply get out of the way and let the other (better) rtx-mechanisms do their job. Also change find_timer to only use seqnum as an argument, since there will only ever be one timer per seqnum at any given time. In the one case where the type matters, the caller simply checks the type. https://bugzilla.gnome.org/show_bug.cgi?id=769768
* rtpjitterbuffer: Actually calculate the packet rate for max-dropout and ↵Thomas Bluemel2016-08-101-0/+4
| | | | | | max-misorder calculations. https://bugzilla.gnome.org/show_bug.cgi?id=751311
* rtpjitterbuffer: avoid unref of null bufferThiago Santos2016-08-041-1/+3
| | | | | | The current 'l' pointer will be NULL when the loop is interrupted with a 'break' statement. Need to have it advance to the next list item before interrupting.
* rtpjitterbuffer: Work with non-TIME segmentsOlivier Crête2016-06-081-13/+13
| | | | | | | | With non-time segments, it now assumes that the arrival time of packets is not relevant and that only the RTP timestamp matter and it produces an output segment start at running time 0. https://bugzilla.gnome.org/show_bug.cgi?id=766438
* jitterbuffer: Upgrade debug message to errorOlivier Crête2016-05-141-1/+1
| | | | It causes the entire pipeline to fail, it should be easier to find.
* rtpjitterbuffer: Fix stall when receiving already lost packetHavard Graff2016-05-061-1/+36
| | | | | | | | | | | | | | | | | | When a packet arrives that has already been considered lost as part of a large gap the "lost timer" for this will be cancelled. If the remaining packets of this large gap never arrives, there will be missing entries in the queue and the loop function will keep waiting for these packets to arrive and never push another packet, effectively stalling the pipeline. The proposed fix conciders parts of a large gap definitely lost (since they are calculated from latency) and ignores the late arrivals. In practice the issue is rare since large gaps are scheduled immediately, and for the stall to happen the late arrival needs to be processed before this times out. https://bugzilla.gnome.org/show_bug.cgi?id=765933
* rtpjitterbuffer: Ensure to not take caps with the wrong pt for getting the ↵Sebastian Dröge2016-04-271-5/+18
| | | | | | | | | clock-rate Especially the caps on the pad might be out of date, and the new caps would be provided for the current pt via the request-pt-map signal. https://bugzilla.gnome.org/show_bug.cgi?id=765689
* jiterbuffer: Move assertion to the right locationEdward Hervey2016-04-071-3/+3
| | | | We shouldn't have "late" lost timers at that point
* jitterbuffer: Speed up lost timeout handlingEdward Hervey2016-04-071-28/+47
| | | | | | | | | | | | | | | | | | | | | | | When downstream blocks, "lost" timers are created to notify the outgoing thread that packets are lost. The problem is that for high packet-rate streams, we might end up with a big list of lost timeouts (had a use-case with ~1000...). The problem isn't so much the amount of lost timeouts to handle, but rather the way they were handled. All timers would first be iterated, then the one selected would be handled ... to re-iterate the list again. All of this is being done while the jbuf lock is taken, which in some use-cases would return in holding that lock for 10s... blocking any buffers from being accepted in input... which would then arrive late ... which would create plenty of lost timers ... which would cause the same issue. In order to avoid that situation, handle the lost timers immediately when iterating the list of pending timers. This modifies the complexity from a quadratic to a linear complexity. https://bugzilla.gnome.org/show_bug.cgi?id=762988
* jitterbuffer: Don't create lost events if we don't need themEdward Hervey2016-04-071-16/+20
| | | | | | | When "do-lost" is set to FALSE we don't use/send the lost events. In that case, don't create them to start with :) https://bugzilla.gnome.org/show_bug.cgi?id=762988
* jitterbuffer: Add tracing of lock usageEdward Hervey2016-04-071-2/+9
| | | | | | Helps with debugging lock usage https://bugzilla.gnome.org/show_bug.cgi?id=762988
* rtpjitterbuffer: Add RFC7273 media clock handlingSebastian Dröge2016-04-031-6/+113
| | | | https://bugzilla.gnome.org/show_bug.cgi?id=762259
* good: use new gst_element_class_add_static_pad_template()Vineeth TM2016-03-241-6/+6
| | | | https://bugzilla.gnome.org/show_bug.cgi?id=763076
* Revert "rtpjitterbuffer: don't forget to unlock mutex in error code path in ↵Sebastian Dröge2016-03-021-2/+0
| | | | | | | | | two cases" This reverts commit a7fb7b53592d87f7983544debb74d364fc3257ad. The mutex is taken by the caller, we should keep it locked when returning so the caller can unlock it again.
* rtpjitterbuffer: don't forget to unlock mutex in error code path in two casesTim-Philipp Müller2016-03-011-0/+2
|
* rtpmanager: Don't warn for duplicate/reordered packetsStian Selnes2016-02-211-2/+2
| | | | | | This is a normal scenario and should not be a warning. https://bugzilla.gnome.org/show_bug.cgi?id=762208
* Revert "WIP: rtpjitterbuffer: Add RFC7273 media clock handling"Sebastian Dröge2016-01-181-86/+0
| | | | | | This reverts commit 271501f6576de4d141e7c2f618e28b9e3b1e5b38. It wasn't meant to be pushed yet as the commit message indicates.
* WIP: rtpjitterbuffer: Add RFC7273 media clock handlingSebastian Dröge2016-01-181-0/+86
|
* rtpjitterbuffer: Fix packet dropping after a big discontSebastian Dröge2015-12-091-1/+11
| | | | | | | We would queue 5 consective packets before considering a reset and a proper discont here. Instead of expecting the next output packet to have the current seqnum (i.e. the fifth), expect it to have the first seqnum. Otherwise we're going to drop all queued up packets.
* rtpmanager: switch G_GINT64_FORMAT for GST_STIME_ARGSLuis de Bethencourt2015-11-031-2/+3
| | | | | | | | No need to use G_GINT64_FORMAT for potentially negative values of GstClockTimeDiff. Since 1.6 these can be handled with GST_STIME_ARGS. Plus it creates more readable values in the logs. https://bugzilla.gnome.org/show_bug.cgi?id=757480
* rtpmanager: Take into account packet rate for max-dropout and max-misorder ↵Miguel París Díaz2015-10-071-16/+34
| | | | | | calculations https://bugzilla.gnome.org/show_bug.cgi?id=751311
* rtpmanager: add "max-dropout-time" and "max-misorder-time" propsMiguel París Díaz2015-10-071-1/+41
| | | | https://bugzilla.gnome.org/show_bug.cgi?id=751311
* gst: Don't use deprecated gst_segment_to_position()Sebastian Dröge2015-09-261-2/+4
|
* rtpbin/rtpjitterbuffer/rtspsrc: Add property to set maximum ms between RTCP ↵Sebastian Dröge2015-09-251-2/+35
| | | | | | SR RTP time and last observed RTP time https://bugzilla.gnome.org/show_bug.cgi?id=755125
* rtpjitterbuffer: reset just a bit more upon flush_stopMark Nauwelaerts2015-09-131-0/+1
|
* rtpjitterbuffer: remove dead struct memberMark Nauwelaerts2015-09-131-3/+0
|
* rtpjitterbuffer: Keep the DTS estimate if we got no DTS after a jitterbuffer ↵Sebastian Dröge2015-08-131-1/+7
| | | | | | | | | reset Otherwise we will just output buffers without timestamps after a reset if no timestamps are provided by upstream, e.g. when using RTSP over TCP. https://bugzilla.gnome.org/show_bug.cgi?id=749536
* rtpjitterbuffer: Fix indentionSebastian Dröge2015-07-101-1/+1
|
* rtpjitterbuffer: Always estimate DTS from the current clock timeSebastian Dröge2015-07-101-56/+47
| | | | | | | | | | Estimating it from the RTP time will give us the PTS, so in cases of PTS!=DTS we would produce wrong DTS. As now the estimated DTS is based on the clock, don't store it in the jitterbuffer items as it would otherwise be used in the skew calculations and would influence the results. We only really need the DTS for timer calculations. https://bugzilla.gnome.org/show_bug.cgi?id=749536
* rtpjitterbuffer: Calculate DTS from the clock if we had none for the first ↵Sebastian Dröge2015-07-081-0/+22
| | | | | | packet after a reset https://bugzilla.gnome.org/show_bug.cgi?id=749536
* rtpjitterbuffer: fix gap-time calculation and remove "late"Havard Graff2015-07-081-15/+19
| | | | | | | | | | | | | | | | | | | The amount of time that is completely expired and not worth waiting for, is the duration of the packets in the gap (gap * duration) - the latency (size) of the jitterbuffer (priv->latency_ns). This is the duration that we make a "multi-lost" packet for. The "late" concept made some sense in 0.10 as it reflected that a buffer coming in had not been waited for at all, but had a timestamp that was outside the jitterbuffer to wait for. With the rewrite of the waiting (timeout) mechanism in 1.0, this no longer makes any sense, and the variable no longer reflects anything meaningful (num > 0 is useless, the duration is what matters) Fixed up the tests that had been slightly modified in 1.0 to allow faulty behavior to sneak in, and port some of them to use GstHarness. https://bugzilla.gnome.org/show_bug.cgi?id=738363
* Revert "rtpjitterbuffer: Fix expected_dts calc in calculate_expected"Stian Selnes2015-07-081-2/+2
| | | | | | | | This reverts commit 05bd708fc5e881390fe839803b53144393d95ab0. The reverted patch is wrong and introduces a regression because there may still be time to receive some of the packets included in the gap if they are reordered.
* rtpjitterbuffer: Calculate receive time if we don't have anySebastian Dröge2015-07-081-2/+31
| | | | | | | This is required to properly schedule packet loss timers and make sure all our calculations work properly. https://bugzilla.gnome.org/show_bug.cgi?id=749536
* rtpjitterbuffer: Handle seqnum gaps in TCP streams without erroring out or ↵Sebastian Dröge2015-07-081-16/+6
| | | | | | | | overflowing calculations That is, handle DTS==GST_CLOCK_TIME_NONE correctly. https://bugzilla.gnome.org/show_bug.cgi?id=749536
* rtpjitterbuffer: Consider timers len to compare with RTP_MAX_DROPOUTMiguel París Díaz2015-07-021-3/+17
| | | | | | | When there are a lot of small gaps, we can consider that there is a big gap (too losses) to reset the buffer. https://bugzilla.gnome.org/show_bug.cgi?id=751636
* rtpjitterbuffer: If possible, always update the current time before looping ↵Sebastian Dröge2015-07-021-1/+13
| | | | | | | | | | | | | | over all timers If we have a clock, update "now" now with the very latest running time we have. If timers are unscheduled below we otherwise wouldn't update now (it's only updated when timers expire), and also for the very first loop iteration now would otherwise always be 0. Also the time is used for the timeout functions, e.g. to calculate any times for the next timeouts and we would otherwise pass too old times there. https://bugzilla.gnome.org/show_bug.cgi?id=751636