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

Add ssrc attributes to the SDP #183

Closed
wants to merge 0 commits into from
Closed

Add ssrc attributes to the SDP #183

wants to merge 0 commits into from

Conversation

mickel8
Copy link
Member

@mickel8 mickel8 commented Jan 15, 2025

Some implementations still rely on ssrc attributes in SDP offer/answer when demuxing incoming RTP packets (e.g. Pion). While including a=ssrc attribute in the SDP is obsoleted and we should demux by mid, this is still correct and supported by web browsers.

This PR adds ssrc attributes to the SDP offer/answer when a transceiver is sendonly or sendrecv.

A couple of notes:

  • in theory, if we add a=ssrc:1234 attribute, it should also always has cname attr i.e. a=ssrc:1234 cname:qW34eced
  • we don't use cnames so we don't do this. Instead we always add msid attribute as we already have it
  • we stop sending app_data in msid according to what JSEP says. This app_data does not make any sense anyway as its value is MediaStreamTrack.id, which can change without renegotiation (e.g. when calling rtpsender.replaceTrack)

@mickel8 mickel8 force-pushed the ssrc branch 2 times, most recently from 05fe01a to 008b334 Compare January 15, 2025 12:12
Copy link

codecov bot commented Jan 15, 2025

Codecov Report

Attention: Patch coverage is 87.27273% with 7 lines in your changes missing coverage. Please review.

Project coverage is 84.78%. Comparing base (e0ff731) to head (008b334).

Files with missing lines Patch % Lines
lib/ex_webrtc/rtp_sender.ex 70.83% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #183      +/-   ##
==========================================
+ Coverage   84.75%   84.78%   +0.02%     
==========================================
  Files          49       49              
  Lines        2479     2510      +31     
==========================================
+ Hits         2101     2128      +27     
- Misses        378      382       +4     
Files with missing lines Coverage Δ
lib/ex_webrtc/peer_connection.ex 86.39% <100.00%> (+0.22%) ⬆️
lib/ex_webrtc/peer_connection/configuration.ex 94.89% <ø> (ø)
lib/ex_webrtc/rtp_transceiver.ex 90.75% <100.00%> (-0.21%) ⬇️
lib/ex_webrtc/rtp_sender.ex 85.88% <70.83%> (-7.57%) ⬇️

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e0ff731...008b334. Read the comment docs.

@mickel8 mickel8 changed the base branch from master to fix-extensions January 15, 2025 12:36
Base automatically changed from fix-extensions to master January 15, 2025 14:16
@mickel8 mickel8 closed this Jan 15, 2025
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.

1 participant