-
Notifications
You must be signed in to change notification settings - Fork 8
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
Adjustments for BUNDLE group. Update ex_ice. #102
Conversation
6bb1a9a
to
f5d76ea
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #102 +/- ##
==========================================
+ Coverage 87.13% 87.20% +0.06%
==========================================
Files 33 33
Lines 1625 1626 +1
==========================================
+ Hits 1416 1418 +2
+ Misses 209 208 -1
Continue to review full report in Codecov by Sentry.
|
d1c605b
to
246eb8a
Compare
f709e2d
to
67944d1
Compare
@@ -78,6 +66,26 @@ defmodule ExWebRTC.SDPUtils do | |||
end | |||
end | |||
|
|||
@spec get_media_direction(ExSDP.Media.t()) :: | |||
:sendrecv | :sendonly | :recvonly | :inactive | nil | |||
def get_media_direction(media) do |
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.
Moved to be close to other get_*
functions
desc_cands = | ||
desc.sdp | ||
|> String.split("\r\n") | ||
|> Enum.find(&String.starts_with?(&1, "a=candidate:")) | ||
|> Enum.filter(&String.starts_with?(&1, "a=candidate:")) | ||
|
||
assert desc_cand == cand.candidate | ||
assert ("a=" <> cand.candidate) in desc_cands |
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.
We no longer can relay on candidates order as they are stored as map in ex_ice
Besides fixes listed below, this PR also update deps and adjusts code to the newest ex_ice.
Fixes:
bundle-only
attribute, see here. This is used by Firefox when PC is configured withbundlePolicy: "maxBundle"
maxBundle
policy (the only one we support), candidates are only included in the first m-line, see here. This seems to be true even if the first m-line is stopped in subsequent offers, at least until ICE restart, which I left untouched i.e. we always include candidates in the first mlineQuestionable:
in
maxBundle
policy, all mlines but the first one should havebundle-only
attribute added, and port in the m-section set to 0, see here. However, there are differences between JSEP RFC, BUNDLE RFC, and browser implementations (see RFC 8829. sec. 1.3 and because of that, we still don't use neither port 0 nor bundle-only attributedepending on the
bundle-policy
, browsers may send candidates for every m-section, only for the first m-section, or for every type of m-section. There is a question, which candidates we should add to the ICE agent on the answerer side (as we only have one ICE stream and one ICE component) - all of them or only those from the first m-section. Looking at RFC 8829, sec. 5.11:so we could take only those candidates that belong to the first m-section as other ICE
components on the offerer side should be closed anyway. However, because current solution seems to work, I would
go back to this question once we notice any problems.
depending on the
rtcpMuxPolicy
browsers may generate candidates only for one ICE component (RTP) or for two ICE components (RTP and RTCP) doubling the number of ICE candidates. There is the same question as in the point above, and we do exactly the same thing i.e. we add all candidates regardless of their ICE components.