-
Notifications
You must be signed in to change notification settings - Fork 16
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 #185
Conversation
82d0d65
to
f03b4c2
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #185 +/- ##
==========================================
- Coverage 84.56% 84.52% -0.05%
==========================================
Files 49 49
Lines 2482 2507 +25
==========================================
+ Hits 2099 2119 +20
- Misses 383 388 +5
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
|> Enum.map(fn seq_no -> | ||
{seq_no, Map.get(responder.packets, rem(seq_no, @max_packets))} | ||
end) | ||
|> Enum.filter(fn {seq_no, packet} -> | ||
packet != nil and packet.sequence_number == seq_no | ||
end) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔
Formatter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I understand the logic, everything looks good 👍 👍 👍
{:ok, pc} = PeerConnection.start_link() | ||
@config PeerConnection.get_configuration(pc) | ||
:ok = PeerConnection.close(pc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
O_O is this even legal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeap, but acutally I am curious what is the process, peer connection will be linked to 🤔
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, including ssrc 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:
a=ssrc:1234
attribute, it should also always has cname attr i.e.a=ssrc:1234 cname:qW34eced
(RFC 5567)