update MID to be random values when not received in offer #535
Conversation
| Before negotiation is complete, | ||
| the <code>mid</code> value may be null. If there is no | ||
| MID value in the the remote SDP, a random value will be | ||
| created for the <code>mid</code> as described in |
There was a problem hiding this comment.
What happens if the offerer includes a MID in the offer, the answer comes back without a MID? According to above this should mean that a new (random) value is created, but should not the original MID be retained?
There was a problem hiding this comment.
Good point - updated to deal with that.
|
@dontcallmedom - can you take a look at this and figure out why CI is failing. CI also fails if I add the data-jsep to the ref (See the TODO). Thanks |
|
@fluffy this is not an issue with your pull request but with the current state of the repo; hopefully it will be fixed soon |
thank you |
|
The fix has landed in 4fa52fa ; you'll need to rebase if you want to benefit from it. I would suggest removing the change to the JSEP mapping since it doesn't help and in fact might create issues down the line. |
|
OK - I think this is ready to merge |
|
@adam-be please review the ICECandidate validity change, since you recently touched this. |
|
It would be nice to squash this history and rebase to get rid of the diffs that are not part of this change. |
| <var>candidateInitDict</var>, the corresponding attribute will be initialized to | ||
| <code>null</code>. If neither is present, a <code>TypeError</code> exception | ||
| <code>null</code>. If <var>sdpMid</var> is not present, a <code>TypeError</code> exception | ||
| will be thrown.</dd> |
There was a problem hiding this comment.
It it safe to require sdpMid here? We accept remote descriptions without mid and generate one in that case.
This behaviour (throw if not present) can also be expressed with the "required" WebIDL keyword on the member. And should we keep sdpMLineIndex if we require sdpMid?
There was a problem hiding this comment.
On Mar 24, 2016, at 3:27 AM, Adam Bergkvist notifications@github.com wrote:
In webrtc.html:
<var>candidateInitDict</var>, the corresponding attribute will be initialized to
<code>null</code>. If neither is present, a <code>TypeError</code> exception<code>null</code>. If <var>sdpMid</var> is not present, a <code>TypeError</code> exception will be thrown.</dd>It it safe to require sdpMid here? We accept remote descriptions without mid and generate one in that case.
This behaviour (throw if not present) can also be expressed with the "required" WebIDL keyword on the member. And should we keep sdpMLineIndex if we require sdpMid?
—
I don't think we can require it here because it should not be retuned if it was not in remote description as that would cause something to fetched it to think the remote side supported mid when it does not.
We have to keep the mLineIndiex
There was a problem hiding this comment.
Throwing a TypeError if a member is not present, as the prose says, is the same behaviour as requiring it. We can also have a default value on the sdpMLineIndex member to have it initialized to null if not present.
required DOMString sdpMid;
unsigned short? sdpMLineIndex = null;
| value was previously assigned, a random value will be created for | ||
| the <code>mid</code> as described in <span | ||
| data-jsep="ice-candidate-format">[[!JSEP]]</span> when the remote | ||
| SDP is set. After rollbacks, the value may change from a non-null |
There was a problem hiding this comment.
It would be nice to integrate the generation of mid into the 'set an RTCSessionDescription' algorithm since it would define when it time it should be done in relation to all the state updating and event dispatching.
There was a problem hiding this comment.
On Mar 24, 2016, at 3:28 AM, Adam Bergkvist notifications@github.com wrote:
In webrtc.html:
attribute is the <code>mid</code> negotatiated and present in the local and remote descriptions as defined in
<span data-jsep="initial-offers initial-answers">[[!JSEP]]</span>. Before negotiation is complete,the <code>mid</code> value may be null. After rollbacks,the value may change from a non-null value to null.</p><span>[[!JSEP]]</span>.<!--TODO add data-jsep="initial-offers initial-answers" to the span-->Before negotiation is complete, the <code>mid</code> value may benull. If there is no MID value in the the remote SDP, and no MIDvalue was previously assigned, a random value will be created forthe <code>mid</code> as described in <spandata-jsep="ice-candidate-format">[[!JSEP]]</span> when the remoteSDP is set. After rollbacks, the value may change from a non-nullIt would be nice to integrate the generation of mid into the 'set an RTCSessionDescription' algorithm since it would define when it time it should be done in relation to all the state updating and event dispatching.
I don't think it is really generated it then - its more when the ICE stuff happens
There was a problem hiding this comment.
We should figure out when and document it.
|
This merges cleanly and it not easy to rebase. I do not plan to rebase it as it is simply a waste of time. |
Fixes #227