-
Notifications
You must be signed in to change notification settings - Fork 14
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
Fix no supported codecs scenario #61
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -122,27 +122,30 @@ defmodule ExWebRTC.RTPTransceiver do | |
@doc false | ||
@spec to_answer_mline(t(), ExSDP.Media.t(), Keyword.t()) :: ExSDP.Media.t() | ||
def to_answer_mline(transceiver, mline, opts) do | ||
offered_direction = SDPUtils.get_media_direction(mline) | ||
direction = get_direction(offered_direction, transceiver.direction) | ||
opts = Keyword.put(opts, :direction, direction) | ||
|
||
# Reject mline. See RFC 8829 sec. 5.3.1 and RFC 3264 sec. 6. | ||
# We could reject earlier (as RFC suggests) but we generate | ||
# answer mline at first to have consistent fingerprint, ice_ufrag and | ||
# ice_pwd values across mlines. | ||
# We also set direction to inactive, even though JSEP doesn't require it. | ||
# See see https://github.com/w3c/webrtc-pc/issues/2927 | ||
cond do | ||
transceiver.codecs == [] -> | ||
# there has to be at least one format so take it from the offer | ||
codecs = SDPUtils.get_rtp_codec_parameters(mline) | ||
transceiver = %__MODULE__{transceiver | codecs: codecs} | ||
opts = Keyword.put(opts, :direction, :inactive) | ||
mline = to_mline(transceiver, opts) | ||
%ExSDP.Media{mline | port: 0} | ||
|
||
transceiver.stopping == true or transceiver.stopped == true -> | ||
opts = Keyword.put(opts, :direction, :inactive) | ||
mline = to_mline(transceiver, opts) | ||
%ExSDP.Media{mline | port: 0} | ||
|
||
true -> | ||
offered_direction = SDPUtils.get_media_direction(mline) | ||
direction = get_direction(offered_direction, transceiver.direction) | ||
opts = Keyword.put(opts, :direction, direction) | ||
Comment on lines
+146
to
+148
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was this previously unhandled? How did we handle the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only change is that now, when we reject an mline we also set its direction as inactive |
||
to_mline(transceiver, opts) | ||
end | ||
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.
TBH I don't know what we want to do with this but personally I don't think functions like
is_supported
is_mandatory
etc are bad. Credo suggests usingsupported?
andmandatory?
which is imo less readable. See https://hexdocs.pm/credo/Credo.Check.Readability.PredicateFunctionNames.htmlThere 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.
I quite like the
something?
convention, I think the question mark makes it quite obvious as to what the function returns and it's an established convention.is_something
, on the other hand, suggests that it can be used in guards, but in reality it cannot.