Commit graph

14 commits

Author SHA1 Message Date
François Laignel
b9f7ab6052 rtpmanager/rtsession: data race leading to critical warnings
This is a fix for a data race leading to:

> GLib-CRITICAL: g_hash_table_foreach:
>   assertion 'version == hash_table->version' failed

Identified sequence:

* `rtp_session_on_timeout` acquires the lock on `session` and proceeds with its
  processing.
* `rtp_session_process_rtcp` is called (debug log : received RTCP packet) and
  attempts to acquire the lock on `session`, which is still held by
  `rtp_session_on_timeout`.
* as part of an hash table iterator, `rtp_session_on_timeout` transitively
  invokes `source_caps` which releases the lock on `session` so as to call
  `session->callbacks.caps`.
* Since `rtp_session_process_rtcp` was waiting for the lock to be released, it
  succeeds in acquiring it and proceeds with `rtp_session_process_rr` which
  transitively calls `g_hash_table_insert` via `add_source`.
* After `source_caps` re-acquires the lock and gives the control flow back to
  `rtp_session_on_timeout`, the hash table iterator is changed, resulting in the
  assertion failure.

This commits copies `sess->ssrcs[sess->mask_idx]` and iterates on the copy so
the iterator is not affected by a concurrent change due to the lock being
released in the `source_caps` callback.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/4585>
2023-05-09 22:35:23 +00:00
François Laignel
943a53cc51 rtpmanager/rtsession: race conditions leading to critical warnings
While testing the [implementation for insertable streams] in `webrtcsink` &
`webrtcsrc`, I encountered critical warnings, which turned out to result from
two race conditions in `rtpsession`. Both race conditions produce:

> GLib-CRITICAL: g_hash_table_foreach:
>   assertion 'version == hash_table->version' failed

This commit fixes one of the race conditions observed.

In its simplest form, the test consists in 2 pipelines and a Signalling server:

* pipelines_sink: audiotestsrc ! webrtcsink
* pipelines_src: webrtcsrc ! appsrc

1. Set `pipelines_sink` to `Playing`.
2. The Signalling server delivers the `producer_id`.
3. Initialize `pipelines_src` to establish a session with `producer_id`.
4. Set `pipelines_src` to `Playing`.
5. Wait for a buffer to be received by the `appsrc`.
6. Set `pipelines_src` to `Null`.
7. Set `pipelines_sink` to `Null`.

The race condition happens in the following sequence:

* `webrtcsink` runs a task to periodically retrieve statistics from `webrtcbin`.
  This transitively ends up executing `rtp_session_create_stats`.
* `pipelines_sink` is set to `Null`.
* In `Paused` to `Ready`, `gst_rtp_session_change_state()` calls
  `rtp_session_reset()`.
* The assertion failure occurs when `rtp_session_reset` is called while
  `rtp_session_create_stats` is executing.

This is because `rtp_session_create_stats` acquires the lock on `session` prior
to calling `g_hash_table_foreach`, but `rtp_session_reset` doesn't acquire the
lock before calling `g_hash_table_remove_all`.

Acquiring the lock in `rtp_session_reset` fixes the issue.

[implementing insertable streams support]: https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/merge_requests/1176

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/4532>
2023-05-03 09:59:22 +01:00
Matt Crane
ca7f66f9b5 rtpsession: Support disabling late adjustment of ntp-64 header ext
Currently in rtp_session_send_rtp(), the existing ntp-64 RTP header
extension timestamp is updated with the actual NTP time before sending
the packet. However, there are some circumstances where we would like
to preserve the original timestamp obtained from reference timestamp
buffer metadata.

This commit provides the ability to configure whether or not to update
the ntp-64 header extension timestamp with the actual NTP time via the
update-ntp64-header-ext boolean property. The property is also exposed
via rtpbin. Default property value of TRUE will preserve existing
behavior (update ntp-64 header ext with actual NTP time).

Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/1580

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/3451>
2022-11-24 08:23:03 +00:00
Johan Sternerup
9794c9bfd0 Use the correct SSRC(s) when routing a RTCB FB FIR
Previously we tried to route an incoming RTCP FB FIR to the correct ssrc
using the "media source" component of the RTCP FB message. However,
according to RFC5104 (section 4.3.1.2) the "media source" SHALL be set
to 0. Instead the ssrc(s) in use are propagated via the FCI data. Now
a specific GstForceKeyUnit event is sent for every ssrc.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/3292>
2022-11-23 11:31:23 +00:00
Sebastian Dröge
bd5a4d321b rtpsource: Don't do probation for RTX sources
Disable probation for RTX sources as packets will arrive very
irregularly and waiting for a second packet usually exceeds the deadline
of the retransmission.

Fixes https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/issues/181

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/3112>
2022-10-10 14:56:18 +00:00
Sebastian Dröge
72b6dabd32 rtpsession: Remember the corresponding media SSRC for RTX sources
This allows timing out the RTX source and sending BYE for it when the
actual media source belonging to it is timed out.

This change only applies to sending sources from this session.

Fixes https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/360

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/3112>
2022-10-10 14:56:17 +00:00
Sebastian Dröge
d5c072fadd rtpsource: Rename rtp_source_update_caps to rtp_source_update_send_caps
To make it clear that this is only used for sending RTP sources.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/3112>
2022-10-10 14:56:17 +00:00
Marc Leeman
db5a4b490d rtpsession: properly initialise favor-new property
Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/2625>
2022-06-17 13:05:18 +00:00
Thibault Saunier
d673a90aea rtpsession: Emit "notify::stats" when we update stats from RR or SR
Sensibily optimizing caching the pspecs and using them directly

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/2266>
2022-04-26 08:49:42 +00:00
Sebastian Dröge
02115a5efc rtpmanager: Move some duplicated constant and helper function to a single place
Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/2132>
2022-04-20 14:40:25 +00:00
Sebastian Dröge
ce38614e1a rtpsession: Handle RTCP-SR-REQ (RFC6051) RTCP feedback message
This causes an RTCP SR to be sent at the earliest possible time.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/2132>
2022-04-20 14:40:25 +00:00
Sebastian Dröge
7ffc830959 rtpsession: Update 64-bit NTP header extensions with the actual NTP time in senders
Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/2132>
2022-04-20 14:40:25 +00:00
Nirbheek Chauhan
8c2ef0f025 twcc: Add some logging to debug TWCC feedback
This should allow people to debug when TWCC feedback is not enabled
because they haven't set the extmap in the caps.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/1952>
2022-03-15 22:32:07 +00:00
Thibault Saunier
5ff769d731 Move files from gst-plugins-good into the "subprojects/gst-plugins-good/" subdir 2021-09-24 16:13:50 -03:00
Renamed from gst/rtpmanager/rtpsession.c (Browse further)