Fix no supported codecs scenario #61
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #61 +/- ##
==========================================
- Coverage 88.85% 88.57% -0.29%
==========================================
Files 25 25
Lines 1059 1068 +9
==========================================
+ Hits 941 946 +5
- Misses 118 122 +4
Continue to review full report in Codecov by Sentry.
|
mickel8
force-pushed
the
failing-negotiation
branch
4 times, most recently
from
cdcaf49
to
f373f2d
Compare
.credo.exs
Outdated
| @@ -104,7 +104,7 @@ | |||
| {Credo.Check.Readability.ParenthesesInCondition, []}, | |||
| {Credo.Check.Readability.ParenthesesOnZeroArityDefs, []}, | |||
| {Credo.Check.Readability.PipeIntoAnonymousFunctions, []}, | |||
| {Credo.Check.Readability.PredicateFunctionNames, []}, | |||
| {Credo.Check.Readability.PredicateFunctionNames, [exit_status: 0]}, | |||
There was a problem hiding this comment.
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 using supported? and mandatory? which is imo less readable. See https://hexdocs.pm/credo/Credo.Check.Readability.PredicateFunctionNames.html
There was a problem hiding this comment.
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.
mickel8
force-pushed
the
failing-negotiation
branch
from
f373f2d
to
cd90b29
Compare
mickel8
force-pushed
the
failing-negotiation
branch
from
cd90b29
to
1f2edb0
Compare
mickel8
changed the title
.credo.exs
Outdated
| @@ -104,7 +104,7 @@ | |||
| {Credo.Check.Readability.ParenthesesInCondition, []}, | |||
| {Credo.Check.Readability.ParenthesesOnZeroArityDefs, []}, | |||
| {Credo.Check.Readability.PipeIntoAnonymousFunctions, []}, | |||
| {Credo.Check.Readability.PredicateFunctionNames, []}, | |||
| {Credo.Check.Readability.PredicateFunctionNames, [exit_status: 0]}, | |||
There was a problem hiding this comment.
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.
| offered_direction = SDPUtils.get_media_direction(mline) | ||
| direction = get_direction(offered_direction, transceiver.direction) | ||
| opts = Keyword.put(opts, :direction, direction) |
There was a problem hiding this comment.
Was this previously unhandled? How did we handle the sRD(offer) -> tr.direction=inactive -> sLD(answer) scenario then?
There was a problem hiding this comment.
The only change is that now, when we reject an mline we also set its direction as inactive
Following Firefox behaviour, when there are no supported codecs in the offer, we will end up with empty transceivers lists on both sides after finishing negotiation.
See w3c/webrtc-pc#2927