Skip to content
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.

Sign up for GitHub

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

Jump to bottom

update MID to be random values when not received in offer #535

Merged
merged 15 commits into from Apr 14, 2016
Merged

update MID to be random values when not received in offer #535

Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
2d68b45
update MID to be random values when not received in offer
fluffy Mar 4, 2016
d0bbe42
Merge branch 'master' into randomMid1
fluffy Mar 14, 2016
76d7fab
deal with re-offer case for MID value creation
fluffy Mar 14, 2016
d137c43
update jsep map
fluffy Mar 14, 2016
e33ea0c
reformat and move out map
fluffy Mar 14, 2016
565fa1f
change whitespace
fluffy Mar 14, 2016
4fa52fa
Editorial: Fix broken links to TCRtpReceiver mid
adam-be Mar 16, 2016
a9e1fb7
update MID to be random values when not received in offer
fluffy Mar 4, 2016
9fa7da3
deal with re-offer case for MID value creation
fluffy Mar 14, 2016
b5ba57e
update jsep map
fluffy Mar 14, 2016
59f0dfb
reformat and move out map
fluffy Mar 14, 2016
58a227b
change whitespace
fluffy Mar 14, 2016
93c3348
revert jsep map
fluffy Mar 17, 2016
8f53855
Merge branch 'randomMid1' of github.com:w3c/webrtc-pc into randomMid1
fluffy Mar 17, 2016
11bb175
revert jsep map
fluffy Mar 17, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 12 additions & 5 deletions webrtc.html
View file
Open in desktop
Expand Up @@ -2382,17 +2382,17 @@ <h4>RTCIceCandidate Interface</h4>
takes a dictionary argument, <var>candidateInitDict</var>, whose content is used
to initialize the new
<a class="idlType" href="#idl-def-RTCIceCandidate"><code>RTCIceCandidate</code></a>
object. If either <var>sdpMid</var> or <var>sdpMLineIndex</var> is not present in
object. If the <var>sdpMLineIndex</var> is not present in
<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>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;


<dt>readonly attribute DOMString candidate</dt>

<dd>This carries the <code>candidate-attribute</code> as defined in
section 15.1 of [[!ICE]].</dd>

<dt>readonly attribute DOMString? sdpMid</dt>
<dt>readonly attribute DOMString sdpMid</dt>

<dd>If not <code>null</code>, this contains the identifier of the "media stream
identification" as defined in [[!RFC5888]] for the media component this
Expand Down Expand Up @@ -4096,8 +4096,15 @@ <h3>RTCRtpTransceiver Interface</h3>
"dom-rtptransceiver-mid"><code>mid</code></dfn>
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,
<span data-jsep="initial-offers
initial-answers">[[!JSEP]]</span>.
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point - updated to deal with that.

<span data-jsep="ice-candidate-format"> [[!JSEP]] </span>
when the remote SDP is
set. After rollbacks,
the value may change from a non-null value to null.</p>
</dd>

Expand Down