Commit graph

394 commits

Author SHA1 Message Date
Niels De Graef
7cf4ab6229 Don't pass default GLib marshallers for signals
By passing `NULL` to `g_signal_new` instead of a marshaller, GLib will
actually internally optimize the signal (if the marshaller is available
in GLib itself) by also setting the valist marshaller. This makes the
signal emission a bit more performant than the regular marshalling,
which still needs to box into `GValue` and call libffi in case of a
generic marshaller.

Note that for custom marshallers, one would use
`g_signal_set_va_marshaller()` with the valist marshaller instead.
2019-11-17 15:32:30 +00:00
Nicolas Dufresne
db187eec19 rtpjitterbuffer: Check the exit condition after executing timers
The do_expected_timeout() function may release the JBUF_LOCK, so we need
to check if nothing wanted the timer thread to exit after this call.
The side effect was that we may endup going back into waiting for a timer
which will cause arbitrary delay on tear down (or deadlock when test
clock is used).

Fixes #653
2019-11-14 17:52:16 -05:00
Nicolas Dufresne
fd6cd6f545 rtpjitterbuffer: Check exit condition immediately after JBUF_WAIT
JBUF_WAIT_QUEUE drops the JBUF_LOCK, which means the stop condition
for the chain function may have changed (change_state to NULL). Check
this immediately after the wait so that we don't delay shutting down.
2019-11-14 17:51:31 -05:00
Aaron Boxer
46989dca96 documentation: fix a number of typos 2019-10-05 22:38:11 +00:00
Simon Arnling Bååth
8173596ed2 gstrtpjitterbuffer: Custom messages when dropping packets
This commit adds custom element messages for when gstrtpjitterbuffer
drops an incoming rtp packets due to for example arriving too late.
Applications can listen to these messages on the bus which enables
actions to be taken when packets are dropped due to for example high
network jitter.

Two properties has been added, one to enable posting drop messages and
one to set a minimum time between each message to enable throttling the
posting of messages as high drop rates.
2019-10-04 20:31:56 +00:00
Olivier Crête
a24596423a rtpjitterbuffer: Cancel timers instead of just unlocking loop thread
When the queue is full (and adding more packets would risk a seqnum
roll-over), the best approach is to just start pushing out packets
from the other side.  Just pushing out the packets results in the
timers being left hanging with old seqnums, so it's safer to just
execute them immediately in this case. It does limit the timer space
to the time it takes to receiver about 32k packets, but without
extended sequence number, this is the best RTP can do.

This also results in the test no longer needed to have timeouts or
timers as pushing packets in drives everything.

Fixes #619
2019-09-28 07:47:54 -04:00
Nicolas Dufresne
4a9f42430a rtpjitterbuffer: Optimize offset update
As we are applying the same offset over all timers, there timer
ordering won't change, so we can safely skip time-reordering.
2019-09-27 17:34:04 -04:00
Nicolas Dufresne
1897c1fbe6 rtpjitterbuffer: Fix a typo in comment 2019-09-27 17:34:04 -04:00
Nicolas Dufresne
9ebcadb349 rtpjitterbuffer: Don't use stats timer on the timers queue
The timer passed to update_timers may be from the stats timer. At the
moment, we could endup rescheduling (reusing) that timer onto the normal
timer queue, unschedul it as if it was from the normal timer queue or
duplicate it into the stats timer queue again. This was protected before
as the with the fact the stats timer didn't have a valid idx.
2019-09-27 17:34:04 -04:00
Nicolas Dufresne
81bffb5e5c rtpjitterbuffer: Update timers on ts-offset changes
As the offset is already applied now, we need to update and reschedule
all timers each time the offset is changed. I'm not sure who expect this
to be retro-actively applied, but there was a unit test for it.
2019-09-27 17:34:04 -04:00
Nicolas Dufresne
d4c6c335c5 rtpjitterbuffer: No need to wake the timer thread on head changes
If the jitterbuffer head change, there is no need to systematically
wakeup the timer thread. The timer thread will be waken up on if
an earlier timeout has been pushed. This prevent some more spurious
wakeup when the system is loaded. As a side effect, cranking the clock
may set the clock at an earlier position.
2019-09-27 17:34:04 -04:00
Nicolas Dufresne
36771b75e9 rtpjittterbuffer: Port timers array to RtpTimerQueue
In this patch we now make use of the new RtpTimerQueue instead of the
old GArray. This required a lot of changes all over the place, some of
the important changes are that `timer->timeout` is no longer a PTS but
the actual timeout. This was required to get the RtpTimerQueue sorting
right. The applied offset is saved as `timer->offset`, this allow
retreiving back the PTS when needed.

The clockid updates only happens once per incoming packet. If the
currently schedule timer is before the earliest timer in the queue, we
no longer wakeup the thread. This way, if other timers get setup in the
meantime, this will reduce the number of wakup.

The timer loop code has been mostly rewritten, though the behaviour of
running the lost timers first has been kept (even though there is no
test to show what would be the side effect of doing this differently).

Fixes #608
2019-09-27 17:34:04 -04:00
Nicolas Dufresne
d4b2231de2 rtpjittterbuffer: Port from TimerQueue to RtpTimerQueue 2019-09-27 17:34:04 -04:00
Nicolas Dufresne
f5e3280dbe rtpjitterbuffer: Port use the new RtpTimer structure
First iteration toward porting to the new timer queue.
2019-09-27 17:34:04 -04:00
Nicolas Dufresne
37742cd36d rtptimerqueue: Consolidate a data structure for timers
Implement a single timer queue for all timers. The goal is to always use
ordered queues for storing timers. This way, extracting timers for
execution becomes O(1). This also allow separating the clock wait
scheduling from the timer itself and ensure that we only wake up the
timer thread when strictly needed.

The knew data structure is still O(n) on insertions and reschedule,
but we now use proximity optimization so that normal cases should be
really fast. The GList structure is also embeded intot he RtpTimer
structure to reduce the number of allocations.
2019-09-27 17:34:04 -04:00
Nicolas Dufresne
c917f11ae8 rtpjitterbuffer: Move item structure outside of the element
This moves the RtpJitterBufferStructure type, alloc, free into
rtpjitterbuffer.c/h implementation. jitterbuffer.c strictly rely on
the fact this structure is compatible with GList, and so it make more
sense to keep encapsulate it. Also, anything that could possibly
reduce the amount of code in the element is a win.

In order to support that move, a function pointer to free the data
was added. This also allow making the free function option when
flushing the jitterbuffer.
2019-09-27 13:02:16 -04:00
Nicolas Dufresne
9b706b6220 rtpjitterbuffer: Constify timer pointers where possible
This helps understanding which function modify the Timerdata
and which one does not. This is not always obvious from thelper
name considering recalculate_timer() does not.
2019-09-27 13:02:16 -04:00
Olivier Crête
37d22186ff rtpjitterbuffer: Unlock output if the queue is full 2019-07-03 18:03:42 +00:00
Thomas Bluemel
080eba64de rtpjitterbuffer: Ignore unsolicited rtx packets.
If an rtx packet arrives that hasn't been requested (it might
have been requested from prior to a reset), ignore it so that
it doesn't inadvertently trigger a clock skew.
2019-07-03 06:23:07 -06:00
Thomas Bluemel
8d955fc32b rtpjitterbuffer: Only calculate skew or reset if no gap.
In the case of reordered packets, calculating skew would cause
pts values to be off. Only calculate skew when packets come
in as expected. Also, late RTX packets should not trigger
clock skew adjustments.

Fixes #612
2019-07-03 06:23:07 -06:00
Olivier Crête
af618cb081 rtpjitterbuffer: max-dropout-time gets cast to int32
So any value over MAXINT32 gets considered as negative and is silently ignored.
2019-07-02 19:59:49 +00:00
Mathieu Duponchelle
ebe2756434 jitterbuffer: unset DTS on output buffers 2019-06-14 16:02:59 +02:00
Mikhail Fludkov
ec5fa49631 rtpjitterbuffer: late packets shouldn't affect PTS of the following packet
If, say, a rtx-packet arrives really late, this can have a dramatic
effect on the jitterbuffer clock-skew logic, having it being reset
and losing track of the current dts-to-pts calculations, directly affecting
the packets that arrive later.

This is demonstrated in the test, where a RTX packet is pushed in really
late, and without this patch the last packet will have its PTS affected
by this, where as a late RTX packet should be redundant information, and
not affect anything.
2019-06-13 11:55:10 +02:00
Mikhail Fludkov
b9c3e354ee rtpjitterbuffer: fix rtx delay calulation when large packet spacing 2019-06-12 11:39:32 +02:00
Stian Selnes
6269ed49ab rtpjitterbuffer: Fix delay for EXPECTED timers added by gaps
This patch corrects the delay set on EXPECTED timers that are added when
processing gaps. Previously the delay could be too small so that
'timout + delay' was much less than 'now', causing the following retries
to be scheduled too early. (They were sent earlier than
rtx-retry-timeout after the previous timeout.)
2019-06-12 11:39:32 +02:00
Vivia Nikolaidou
987230a759 rtpjitterbuffer: Print GstClockTimeDiff as GST_STIME_FORMAT 2019-05-26 17:46:06 +03:00
Thibault Saunier
0a6a62aa76 docs: Port all docstring to gtk-doc markdown 2019-05-13 10:24:40 -04:00
Antonio Ospite
435f67debf docs: fix typo s/abonormally/abnormally/ 2019-04-03 16:42:26 +02:00
Antonio Ospite
d6939c4031 docs: fix typo s/incomming/incoming/ 2019-04-03 16:38:56 +02:00
Olivier Crête
7ecbd7271d rtpmanager: Register chain functions to debug 2019-03-22 16:44:41 +00:00
Olivier Crête
bf00ee46de rtpjitterbuffer: Limit size to 2^15 packets
If it goes over 2^15 packets, it will think it has rolled over
and start dropping all packets. So make sure the seqnum distance is not too big.

But let's not limit it to a number that is too small to avoid emptying it
needlessly if there is a spurious huge sequence number, let's allow at
least 10k packets in any case.
2019-02-11 23:41:14 +00:00
Olivier Crête
086bad4643 rtpjitterbuffer: There is no automatic reorder threshold 2019-02-11 11:33:36 -05:00
Mathieu Duponchelle
a6d681ad09 rtpjitterbuffer: use the correct segment seqnum 2019-02-04 13:14:37 +00:00
Olivier Crête
d857522237 rtpjitterbuffer: Run all timers immediately on EOS
When the EOS event is received, run all timers immediately and avoid
pushing the EOS downstream before this has been run. This ensures that
the lost packet statistics are accurate.
2018-12-14 12:10:16 +00:00
Nicolas Dufresne
3de2c28fc1 rtpjitterbuffer: Stop waiting after EOS
After EOS is received, it is pointless to wait for further events,
specially waiting on timers. This patches fixes two cases where we could
wait instead of returning GST_FLOW_EOS and trigger a spin of the loop
function when EOS is queued, regardless if this EOS is the queue head or
not.
2018-12-14 12:10:16 +00:00
Tim-Philipp Müller
238a37295c Update for g_type_class_add_private() deprecation in recent GLib
https://gitlab.gnome.org/GNOME/glib/merge_requests/7
2018-06-23 23:44:19 +02:00
Patrick Radizi
364dbb5fc7 rtpjitterbuffer: allow timestamps to move backwards
The original solution for #784002 incorrectly assumed that timestamps
may not move backwards and changed timestamps that did so.

https://bugzilla.gnome.org/show_bug.cgi?id=784002
2018-02-15 10:05:39 +02:00
Tim-Philipp Müller
6cb51bd8cf rtpjitterbuffer: fix debug message on pt mismatch 2017-10-08 00:07:43 +01:00
Tim-Philipp Müller
a802f5df42 rtpjitterbuffer: implement basic chain_list function
Doesn't do anything fancy yet, but still avoids lots of
unnecessary locking/unlocking that would happen if the
default chain_list fallback function in GstPad got invoked.
2017-09-17 16:33:15 +01:00
Patrick Radizi
23f7739ba4 rtpbin: add option for increasing ts_offset gradually
Instant large changes to ts_offset may cause timestamps to move
backwards and also cause visible effects in media playback. The new
option max-ts-offset-adjustment lets the application control the rate to
apply changes to ts_offset.

https://bugzilla.gnome.org/show_bug.cgi?id=784002
2017-09-14 13:15:56 +03:00
Nicolas Dufresne
bbe0053f8a rtpjitterbuffer: Add a faststart-min-packets property
When set this property will allow the jitterbuffer to start delivering
packets as soon as N most recent packets have consecutive seqnum. A
faststart-min-packets of zero disables this feature. This heuristic is
also used in rtpsource which implements the probation mechanism and a
similar heuristic is used to handle long gaps.

https://bugzilla.gnome.org/show_bug.cgi?id=769536
2017-06-28 11:51:10 -04:00
Andrew
76792a5c20 rtpjitterbuffer: Don't always reset PTS to 0 after a gap
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
2017-02-26 12:41:19 +02:00
Edward Hervey
e5158ca496 jitterbuffer: Don't leak duplicate items
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
2016-12-02 09:01:57 +01:00
Havard Graff
1a4393fb4d rtpjitterbuffer: fix timer-reuse bug
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
2016-11-04 16:56:56 +02:00
Havard Graff
fb9c75db36 rtpjitterbuffer: fix lost-event using dts instead of pts
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).
2016-11-04 16:51:20 +02:00
Havard Graff
bea35f97c8 rtpjitterbuffer: fix bug in reschedule_timer
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
2016-11-04 16:40:14 +02:00
Thomas Bluemel
567afdd4d3 rtpjitterbuffer: Fix calculating next_seqnum when dropping old buffers from 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
2016-09-14 19:47:28 -04:00
Havard Graff
f440b074b1 rtpjitterbuffer: improved rtx-rtt averaging
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
2016-09-14 19:37:50 -04:00
Stian Selnes
f8238f0a9f rtpjitterbuffer: Detect whether to assume equidistant spacing when loss
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
2016-09-14 19:37:50 -04:00
Stian Selnes
2eb7383816 rtpjitterbuffer: Don't request rtx if 'now' is past retry period
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
2016-09-14 19:37:50 -04:00