Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configuration of outgoing RTX streams #607

Merged
merged 3 commits into from
Sep 7, 2024

Conversation

anders-avos
Copy link
Contributor

Makes RTCRtpSender capable of configuring RTX streams.

Incoming RTX streams are already handled at the SDP/SDES layer and then processed by RTCRtpReceiver::receive_for_rtx. This means interceptors can bind to remote RTX streams. This PR aims to do the same for local streams. This is related to, but not a full fix for #295 since it doesn't implement the retransmission, but it allows the separate RTX SSRCs to be configured, negotiated, and then bound to interceptors that can do the rest.

First, a field is added to StreamInfo to pass information about associated streams from the SDP to the interceptor chain. This is populated for both senders and receivers. Typically the interceptor can check if the MIME type is */rtx and know that the associated stream is the original stream, but this concept can also used for other RTP extensions.

The main commit adds support for establishing RTX streams in RTCRtpSender. Since it's easy to unknowingly have */rtx codecs configured, this feature is gated behind a new enable_sender_rtx setting (this can be removed if it's thought safe to always enable). Whenever an encoding is added, the sender checks if the media engine contains a suitable RTX codec. If so, a SSRC is allocated, a flow identification ssrc-group is added to the SDP, and interceptors are bound for the RTX streams on start().

The last commit fixes an issue where repair_ssrc is only populated if the a=ssrc-group:FID line appears before the a=ssid: lines.

Let me know what you think about the approach!

Copy link

codecov bot commented Sep 2, 2024

Codecov Report

Attention: Patch coverage is 70.53942% with 71 lines in your changes missing coverage. Please review.

Project coverage is 60.60%. Comparing base (23bbc1f) to head (15b907d).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
.../src/rtp_transceiver/rtp_sender/rtp_sender_test.rs 60.57% 18 Missing and 23 partials ⚠️
webrtc/src/peer_connection/sdp/sdp_test.rs 80.73% 2 Missing and 19 partials ⚠️
...tc/src/peer_connection/peer_connection_internal.rs 33.33% 7 Missing and 1 partial ⚠️
webrtc/src/rtp_transceiver/rtp_codec.rs 92.30% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master     #607    +/-   ##
========================================
  Coverage   60.59%   60.60%            
========================================
  Files         482      471    -11     
  Lines       47981    48169   +188     
  Branches    12478    12513    +35     
========================================
+ Hits        29075    29192   +117     
- Misses       9665     9669     +4     
- Partials     9241     9308    +67     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@anders-avos
Copy link
Contributor Author

Do you think the new setting is necessary? If so I'm open to suggestions on how to get the setting into RTCRtpSender without tripping clippy parameter count lint. Maybe a new RTCRtpSenderInit struct?

@rainliu rainliu merged commit dd7b283 into webrtc-rs:master Sep 7, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants