So far we assumed that if all sources are bye, this meant we needed to
send an EOS on the RTCP sink. The problem is that this case may happens
if we only had one internal source and it detected a collision.
So now we limit the EOS forwarding to when there is a send_rtp_sink pad
and that this pad has received EOS. We don'tcheck the recv_rtp_sink
since the code does not wait for the bye to be send before sending EOS
to the RTCP src pad.
This is useful when implementing custom retransmission mechanism like
RIST to prevent RTCP from being produces for the retransmitted SSRC.
This would also be used in general for various purpose when customizing
an RTP base pipeline.
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.
This reverts commit dcd3ce9751.
This functionality was implemented for gstopenwebrtc, but it
turned out this was not actually needed for webrtc bundling
support, as shown in webrtcbin. It also doesn't correspond
to any standards.
This is an API break, but nothing should actually depend on
this, at least not for its initial purpose.
Changes in rtpbin.c were reverted manually, to preserve some
refactoring that had occurred in the original commit.
Fixes#537
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.
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.
This is an extra internal recurisve lock use to avoid having to take
both sink pad streams lock all the time. This patch renamed it
INTERLNAL_STREAM_LOCK/UNLOCK() to avoid confusion with possible upstream
GST_PAD API.
This reverts "6f3734c305 rtpssrcdemux: Only forward stick events while
holding the sinkpad stream lock" and actually hold on the internal
stream lock. This prevents in some needed case having a second
streaming thread poping in and messing up event ordering.
While forwarding serialized event, we use gst_pad_forward() function.
In the forward callback (GstPadForwardFunction) we always return
TRUE. Returning true there will stop the dispatching procedure. As a
side effect, only one events is receiving the events. This breaks
when sending EOS from the applicaiton, it also breaks the latency
tracer.
Reset RTPSession when rtpsession changes state from PAUSED to READY.
Without this change, a stored last_rtptime in RTPSource could interfere
with RTP timestamp generation in RTCP Sender Report.
Fixes#510
Always wait with starting the RTCP thread until either a RTP or RTCP
packet is sent or received. Special handling is needed to make sure the
RTCP thread is started when requesting an early RTCP packet.
We want to wait with starting the RTCP thread until it's needed in order
to not send RTCP packets for an inactive source.
https://bugzilla.gnome.org/show_bug.cgi?id=795139
The code before copied GstStructure twice. The first time inside
gst_value_set_structure and the second time in g_value_array_append.
Optimized version does no copies, just transfers ownership to
GValueArray. It takes advantage of the fact that array has already
enough elements preallocated and the memory is zero initialized.
https://bugzilla.gnome.org/show_bug.cgi?id=795139
If obtain_internal_source() returns a source that is not internal it
means there exists a non-internal source with the same ssrc. Such an
ssrc collision should be handled by sending a GstRTPCollision event
upstream and choose a new ssrc, but for now we simply drop the packet.
Trying to process the packet further will cause it to be pushed
usptream (!) since the source is not internal (see source_push_rtp()).
https://bugzilla.gnome.org/show_bug.cgi?id=795139
If there is an external source which is about to timeout and be removed
from the source hashtable and we receive feedback RTCP packet with the
media ssrc of the source, we unlock the session in
rtp_session_process_feedback before emitting 'on-feedback-rtcp' signal
allowing rtcp timer to kick in and grab the lock. It will get rid of
the source and rtp_session_process_feedback will be left with RTPSource
with ref count 0.
The fix is to grab the ref to the RTPSource object in
rtp_session_process_feedback.
https://bugzilla.gnome.org/show_bug.cgi?id=795139
These are the sources we send from, so there is no reason to
report receive statistics for them (as we do not receive on them,
and the remote side has no knowledge of them).
https://bugzilla.gnome.org/show_bug.cgi?id=795139
The code responsible for creating retransmitted buffers
assumed the stored buffer had been created with
rtp_buffer_new_allocate when copying the extension data,
which isn't necessarily the case, for example when
the rtp buffers come from a udpsrc.
https://bugzilla.gnome.org/show_bug.cgi?id=794958
Similar to the get-session and get-internal-session signals,
we expose a get-storage signal in addition to the
get-internal-storage signal to give access to the actual
element for applications that need to set properties on the
element, in particular "size-time"
https://bugzilla.gnome.org/show_bug.cgi?id=794910
Packets with these payload types will be dropped. A use case
for this is FEC, where we want FEC packets to go through the
jitterbuffer, but not be output by rtpbin.
https://bugzilla.gnome.org/show_bug.cgi?id=792696
When the signal returns a floating reference, as its return type
is transfer full, we need to sink it ourselves before passing
it to gst_bin_add (which is transfer floating).
This allows us to unref it in bin_remove_element later on, and
thus to also release the reference we now own if the signal
returns a non-floating reference as well.
As we now still hold a reference to the element when removing it,
we also need to lock its state and setting it to NULL before
unreffing it
Also update the request_aux_sender test.
https://bugzilla.gnome.org/show_bug.cgi?id=792543
When XR packet is detected, warning message leads to misunderstandings.
Until RFC3611 is implemented in gst-plugins-base, the level needs to
be downgraded to avoid confusion.
https://bugzilla.gnome.org/show_bug.cgi?id=789746
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.
Timestamp offsets needs to be checked to detect unrealistic values
caused for example by NTP clocks not in sync. The new parameter
max-ts-offset lets the user decide an upper offset limit. There
are two different cases for checking the offset based on if
ntp-sync is used or not:
1) ntp-sync enabled
Only negative offsest are allowed since a positive offset would
mean that the sender and receiver clocks are not in sync.
Default vaule of max-ts-offset = 0 (disabled)
2) ntp-sync disabled
Both positive and negative offsets are allowed.
Default vaule of max-ts-offset = 3000000000
The reason for different default values is to be backwards
compatible.
https://bugzilla.gnome.org/show_bug.cgi?id=785733
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
* use INFO/DEBUG/LOG/TRACE equaly and meaningfully;
previously rtprtxsend:LOG and rtprtxreceive:LOG would generate
a totally different amount of log traffic and sometimes it was
impossible to see the information you wanted without useless
spam being printed around
* improve the wording, give a reasonable and self-explanatory
amount of information
* print SSRCs in hex
* avoid G_FOO_FORMAT for readability (we are just printing integers)
If one requests the send_rtcp_src_%u pad before a recv_rtcp_sink_%u pad,
the session/pad would never be created and NULL was returned.
Switching the request order would work.
https://bugzilla.gnome.org/show_bug.cgi?id=786718
Callers of the API (rtpsource, rtpjitterbuffer) pass clock_rate
as a signed integer, and the comparison "<= 0" is used against
it, leading me to think the intention was to have the field
be typed as gint32, not guint32.
This led to situations where we could call scale_int with
a MAX_UINT32 (-1) guint32 as the denom, thus raising an
assertion.
https://bugzilla.gnome.org/show_bug.cgi?id=785991
gst_util_uint64_scale_int takes a gint as denom parameter
whereas ctx->clock_rate is a guint32.
It happens when gst_rtp_packet_rate_ctx_reset set clock_rate
to -1.
So just define clock_rate as gint like it is done in rtpsource.h
https://bugzilla.gnome.org/show_bug.cgi?id=784250
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
This debug statement is meant to print the time since the last (early)
RTCP transmission, not the last regular RTCP transmission (which also
happens to be set a few lines above to current_time, so the debug output
is just confusing)
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
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
If an element queries the number of retransmission buffers pushed
*while* the push is still taking place (and before the object lock
is taken just after) it would end up with the wrong statistic
being reported.
Increment it just before the push, avoids races when getting statistics
https://bugzilla.gnome.org/show_bug.cgi?id=768723
A new signal named on-bundled-ssrc is provided and can be
used by the application to redirect a stream to a different
GstRtpSession or to keep the RTX stream grouped within the
GstRtpSession of the same media type.
https://bugzilla.gnome.org/show_bug.cgi?id=772740
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.comhttps://bugzilla.gnome.org/show_bug.cgi?id=773891
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).
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
Instead of sending EOS when a source byes we have to wait for
all the sources to be gone, which means they already sent BYE and
were removed from the session. We now handle the EOS in the rtcp
loop checking the amount of sources in the session.
https://bugzilla.gnome.org/show_bug.cgi?id=773218
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
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
