When connected to an upstream rtpfunnel element, payload-type,
ssrc and clock-rate will not be present in the received caps.
rtprtxsend can already deal with only the clock rate being
present there, a new property is exposed to allow users to
provide a payload-type -> clock-rate map, this enables the
use of the max-size-time property for bundled streams.
The key is to make sure the jitterbuffer is set to NULL *before* the
ptdemux.
The race that existed would basically happen when ptdemux had reached
READY, and the jitterbuffer would then push a buffer, triggering a new
pad with a new payloadtype being added and ghosted to the rtpbin itself.
However, the srcpad of the ptdemux would now be inactive, and all the
sticky-event pushed on it would be swallowed, not allowing any to reach
the ghost-pad. Then the buffer in-flight would come to the ghostpad,
and we would assert that a buffer arrived before the necessary
events.
By simply re-ordering the state-changes, we ensure that there will be
no buffer racing into the ptdemux while its state is being changed,
and the problem disappears completely.
Notice also that there is not point in disconnecting the signals on the
ptdemux before this point, since we need the push-thread to settle
down before we can do this in a non-racy way.
One of the jitterbuffers functions is to try and make sense of weird
network behavior.
It is quite unhelpful for the jitterbuffer to start dropping packets
itself when what you are trying to achieve is better network resilience.
In the case of a skew, this could often mean the sender has restarted
in some fashion, and then dropping the very first buffer of this "new"
stream could often mean missing valuable information, like in the case
of video and I-frames.
This patch simply reverts back to the old behavior, prior to 8d955fc32b
and includes the simplest test I could write to demonstrate the behavior,
where a single packet arrives "perfectly", then a 50ms gap happens,
and then two more packets arrive in perfect order after that.
# Conflicts:
# tests/check/elements/rtpjitterbuffer.c
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.
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
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.
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.
Add a property which explicitly maps splitmuxsink pads to the
muxer pads they should connect to, overriding the implicit logic
that tries to match pads but yields arbitrary names.
RTP and RTCP packets can be muxed together on the same channel (see
RFC5761) and can arrive in the same buffer list.
The GStreamer rtpsession element support RFC5761, so add a test to cover
this case for buffer lists too.
Buffers with different timestamps (e.g. packets belonging to different
frames) can arrive together in the same buffer list,
Add a test to cover this case.
When a new source fails to pass the probation period (i.e. new packets
have non-consecutive sequence numbers), then no buffer shall be pushed
downstream. Add a test to validate this case.
Add a test to verify that stats about received packets are correct when
using buffer lists in the rtpsession receive path.
Split get_session_source_stats() in two to be able to get stats from
a GstRtpSession object directly.
The primary video stream is used to select fragment cut points
at keyframe boundaries. Auxilliary video streams may be
broken up at any packet - so fragments may not start with a keyframe
for those streams.
If the conflict is detected when sending a packet, then also send an
upstream event to tell the source to reconfigure itself.
Also ignore the collision if we see more than one collision from the same
remote source to avoid problems on loops.
Add a new property "do-aggregate"* to the H.264 RTP payloader which
enables STAP-A aggregation as per [RFC-6184][1]. With aggregation enabled,
packets are bundled instead of sent immediately, up until the MTU size.
Bundles also end at access unit boundaries or when packets have to be
fragmented.
*: The property-name is kept generic since it might apply more widely,
e.g. STAP-B or MTAP.
[1]: https://tools.ietf.org/html/rfc6184#section-5.7
Closes https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/434
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
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.
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.)
Turns out that the "big-gap"-logic of the jitterbuffer has been horribly
broken.
For people using lost-events, an RTP-stream with a gap in sequencenumbers,
would produce exactly that many lost-events immediately.
So if your sequence-numbers jumped 20000, you would get 20000 lost-events
in your pipeline...
The test that looks after this logic "test_push_big_gap", basically
incremented the DTS of the buffer equal to the gap that was introduced,
so that in fact this would be more of a "large pause" test, than an
actual gap/discontinuity in the sequencenumbers.
Once the test was modified to not increment DTS (buffer arrival time) with
a similar gap, all sorts of crazy started happening, including adding
thousands of timers, and the logic that should have kicked in, the
"handle_big_gap_buffer"-logic, was not called at all, why?
Because the number max_dropout is calculated using the packet-rate, and
the packet-rate logic would, in this particular test, report that
the new packet rate was over 400000 packets per second!!!
I believe the right fix is to don't try and update the packet-rate if
there is any jumps in the sequence-numbers, and only do these calculations
for nice, sequential streams.
sethostent() seems to be using a global state and we endup with leaks from
that API when called through shout_init(). We had the option to only
ignore the shout case, but the impression is that if we have shout and
another sethostend user, as it's a global state, we may endup with a
different stack trace for the same leak. So in the end, we just ignore
memory allocated by sethostent in general.
In this change we now protect the internal srcpads list using the
stream lock and limit usage of the internal stream lock to
preventing data flowing on the other src pad type while creating
and signalling the new pad.
This fixes a deadlock with RTPBin shutdown lock. These two locks would
end up being taken in two different order, which caused a deadlock. More
generally, we should not rely on a streamlock when handling out-of-band
data, so as a side effect, we should not take a stream lock when
iterating internal links.
We recently added code to remove outdate NACK to avoid using bandwidth
for packet that have no chance of arriving on time. Though, this had a
side effect, which is that it was to get an early RTCP packet with no
feedback into it. This was pretty useless but also had a side effect,
which is that the RTX RTT value would never be updated. So we we stared
having late RTX request due to high RTT, we'd never manage to recover.
This fixes the regression by making sure we keep at least one NACK in
this situation. This is really light on the bandwidth and allow for
quick recover after the RTT have spiked higher then the jitterbuffer
capacity.
Right now, we may call on-new-ssrc after we have processed the first
RTP packet. This prevents properly configuring the source as some
property like "probation" are copied internally for use as a
decreasing counter. For this specific property, it prevents the
application from disabling probation on auxiliary sparse stream.
Probation is harmful on sparse streams since the probation algorithm
assume frequent and contiguous RTP packets.
We used to split the NACK if a smaller seqnum of a range of seqnum was
submited. This test also make sure that the three operations (append,
prepend, update) works properly.
Calling rtp_session_send_rtcp before marking the source as requiring a
pli/fir/nack meant the rtcp_thread could be scheduled and start running
before the source was updated. This meant the request would not be sent
early but instead was transmitted with the next regular RTCP packet.
Add test for nack generation.
Add a test to verify that stats about sent and received packets are
correct even when using buffer lists.
NOTE: the newly introduced get_session_source_stats() selects the
desired source (sender or receiver) by filtering them by type (using the
get_sender parameter) rather than by ssrc because this simplifies the
code and it's good enough for testing purposes as there is usually one
source per type in the test setup.
Filtering by ssrc would have required handling asynchronous signals like
"on-new-sender-ssrc", with the relative locking, just to retrieve the
actual ssrc of the sender.
The tests create a buffer list and then use the chain_list callback to
verify that the correct packets have been pushed.
Move the creation and validation code next to each other so that the
reader can more easily understand what is going on.
While at it add some comments to introduce the two related functions.
Make it possible to differentiate between the position in the list and
the packet index in the global structures in check_packet, in some
future case the list may change, in case some element removes a buffer
from the list, and the two indices may not coincide.
Port the rtpbin_buffer_list test to GStreamer 1.0 and re-enable it.
Some other changes include:
- the check on the caps has been moved from the buffer level to the
pad level;
- remove underscore prefix from static functions names, this is not
idiomatic in C and rarely used in the other tests;
- the unused header_buffer variable has been removed;
- check_group() has been renamed to check_packet() because in
GStreamer 1.0 there is no concept of "group" anymore, the comments
have also been updated to reflect this.
Tests might take a bit longer, esp. when run under valgrind
and/or they're running on the CI with other things going on,
so let's just bump the timeout to something higher and let
the test runner time us out if needed.
Allow fallback to orc subproject if any.
Additionally 'dependencies' keyword is removed from find_library,
because it's invalid keyword for find_library.
False positive for the three variables but some warnings like:
../tests/check/elements/matroskamux.c:875:10:
warning: 'chapters_offset' may be used uninitialized in this function [-Wmaybe-uninitialized]
*index = chapters_offset;
~~~~~~~^~~~~~~~~~~~~~~~~
The above is false positive as there is a gboolean to check if it was
initialized or not (found_chapters_declaration).
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
Bus message handler of tags checking unit test uses gst_message_parse_error()
in case of GST_MESSAGE_ERROR and GST_MESAGE_WARNING.
If gst_message_parse_error() is called in case of GST_MESSAGE_WARNING, assert occurs.
So modified to use gst_message_parse_warning() in case of GST_MESSAGE_WARNING.
Allow run some unit tests on Windows.
* Remove hardcoded path separator in whitelist env for Meson to choose
OS-specific separator automatically (i.e., ';' for windows and ':' for *nix)
* Add dependency explicitly for some test cases, otherwise plugins couldn't be
loaded on uninstalled environment of Windows.
gstreamer!55 makes some changes to how the `error-after` counter works
which breaks this test. This change makes the test not rely on the
ability to alter `error-after` at runtime and explicitly stops and
starts the harness before pushing data.
An alternative would be to add another argument to
`harness_rtpulpfecdec` to set `error-after` on construction but that's
slightly more long-winded. so I went for this approach instead.
Fixes#532, even though that's already closed.
The initial mission statement for this test was:
* demonstrate usage of the request-aux-* signals in rtpbin
* test the rtx elements
We have examples that serve the first use case, and better
(harnessed) tests for the second use case.
This test is slow and racy, it served its purpose but can now
be removed.
Fixes#533
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.
The teardown of the pads checks the refcount, but there are timers
inside the jitterbuffer that can push things, so if we're not lucky,
things could be pushed while the pads are being shut down. Putting the
jitterbuffer to NULL first avoids this.
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
This mode is useful for muxers that can take a long time to finalize a
file. Instead of blocking the whole upstream pipeline while the muxer is
doing its stuff, we can unlink it and spawn a new muxer+sink combination
to continue running normally.
This requires us to receive the muxer and sink (if needed) as factories,
optionally accompanied by their respective properties structures. Also
added the muxer-added and sink-added signals, in case custom code has to
be called for them.
https://bugzilla.gnome.org/show_bug.cgi?id=783754
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
ULP FEC, as defined in RFC 5109, has the protected and protection
packets sharing the same ssrc, and a different payload type, and
implies rewriting the seqnums of the protected stream when encoding
the protection packets. This has the unfortunate drawback of not
being able to tell whether a lost packet was a protection packet.
rtpbasedepayload relies on gaps in the seqnums to set the DISCONT
flag on buffers it outputs. Before that commit, this created two
problems:
* The protection packets don't make it as far as the depayloader,
which means it will mark buffers as DISCONT every time the previous
packets were protected
* While we could work around the previous issue by looking at
the protection packets ignored and dropped in rtpptdemux, we
would still mark buffers as DISCONT when a FEC packet was lost,
as we cannot know that it was indeed a FEC packet, even though
this should have no impact on the decoding of the stream
With this commit, we consider that when using ULPFEC, gaps in
the seqnums are not a reliable indicator of whether buffers should
be marked as DISCONT or not, and thus rewrite the seqnums on
the decoding side as well to form a perfect sequence, this
obviously doesn't prevent the jitterbuffer from doing its job
as the ulpfec decoder is downstream from it.
https://bugzilla.gnome.org/show_bug.cgi?id=794909
This reverts commit af273b4de9.
While RFC 3264 (SDP) says that sendonly/recvonly are from the point of view of
the requester, the actual RTSP RFCs (RFC 2326 / 7826) disagree and say
the opposite, just like the ONVIF standard.
Let's follow those RFCs as we're doing RTSP here, and add a property at
a later time if needed to switch to the SDP RFC behaviour.
https://bugzilla.gnome.org/show_bug.cgi?id=793964
Tested on linux with X11/wayland and semi-tested on Windows.
Windows crashes on item destruction however this is better than nothing.
Fix up some win32 build issues on the way with mismatched {} and
G_STMT_{START,END}
Looping the test 500 times to only execute the test once every
33 times means we inited and deinited gstreamer 467 times
for no reason at all, which was annoying when running the test
with valgrind.
