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.

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

Default ufrag for addIceCandidate when there are different ufrags in SDP #1219

Closed
soareschen opened this Issue May 17, 2017 · 8 comments

Comments

Projects
None yet
5 participants
@soareschen
Copy link
Contributor

soareschen commented May 17, 2017

Is it right to say that an SDP for remote description may have different ufrag for each media stream? i.e. when bundling is not supported.

If so, when calling addIceCandidate with undefined ufrag, does "the most recent ICE generation" means that it is associated with the ufrag of the last media stream?

And does this mean that in this case when adding ICE candidate for earlier media streams, the ufrag must be specified or it would result in error?

@taylor-b

This comment has been minimized.

Copy link
Contributor

taylor-b commented May 17, 2017

addIceCandidate requires an sdpMid or sdpMLineIndex, which identifies which media stream the candidate is being added for. So if ufrag is absent, the candidate is assumed to belong to the last ICE generation used by that "m=" section.

For example, if SRD is called with:

m=video ...
a=mid:video
a=ice-ufrag:video_ufrag_1
...
m=audio ...
a=mid:audio
a=ice-ufrag:audio_ufrag_1

And later with:

m=video ...
a=mid:video
a=ice-ufrag:video_ufrag_2
...
m=audio ...
a=mid:audio
a=ice-ufrag:audio_ufrag_2

Then if addIceCandidate is called with {sdpMid: "audio", candidate: "..."} with no ufrag, the candidate is assumed to belong to the ICE generation identified by "audio_ufrag_2".

Does this make sense? Is there a way the spec could be reworded to make this more clear?

@soareschen

This comment has been minimized.

Copy link
Contributor Author

soareschen commented May 18, 2017

Thanks for clarifying. The description for the terms "ICE generation" and "ICE session" are pretty short so I wasn't very sure if different ufrags, e.g. video_ufrag_1 and audio_ufrag_1, implies different ICE generation.

The thing that isn't very clear in the spec is that the following call to addIceCandidate would have been invalid because the ufrag and sdpMid mismatch:

addIceCandidate({
  candidate: '...',
  sdpMid: 'video',
  ufrag: 'audio_ufrag_1'
});

However if the ufrag is left empty, it is not clear how the implementation figure that the correct ufrag for sdpMid: video is video_ufrag_1 and not audio_ufrag_1.

This issue don't usually arose because current browsers generate the same ufrag for all media streams in an SDP. So it is not very clear how they should behave if they receive remote SDP containing different ufrags.

I think to fix this, we just need to update step 6 of addIceCandidate from:

Use candidate.ufrag to identify the ICE generation; if the ufrag is null, process the candidate for the most recent ICE generation. If candidate.candidate is an empty string, process candidate as an end-of-candidates indication for the corresponding media description and ICE candidate generation.

to:

Use candidate.ufrag to identify the the corresponding media description and ICE generation; if the ufrag is null, process the candidate for the corresponding media description and the most recent ICE generation. If candidate.candidate is an empty string, process candidate as an end-of-candidates indication for the corresponding media description and ICE candidate generation.

@soareschen

This comment has been minimized.

Copy link
Contributor Author

soareschen commented May 18, 2017

Just realized the latest tip of tree draft has already updated the description, which is much clearer.

Use candidate.ufrag to identify the ICE generation; if the ufrag is null, process the candidate for the most recent ICE generation. If candidate.candidate is an empty string, process candidate as an end-of-candidates indication for the corresponding media description and ICE candidate generation.

So then one more clarification needed is that "corresponding media description" is determined through sdpMid or sdpMLineIndex in step 3 or step 4.

@aboba

This comment has been minimized.

Copy link
Contributor

aboba commented Jul 18, 2017

Yes, each m-line could have a distinct ufrag. For example, in ORTC, each RTCIceGatherer has a unique ufrag. However, not all implementations will have a distinct ufrag for each m-line.

@soareschen Can you submit a PR?

@soareschen

This comment has been minimized.

Copy link
Contributor Author

soareschen commented Jul 19, 2017

Yes will do.

@stefhak

This comment has been minimized.

Copy link
Contributor

stefhak commented Jul 10, 2018

@soareschen what's the status of this one now? Are you planning to author a PR?

@aboba aboba assigned stefhak and unassigned soareschen Aug 14, 2018

@stefhak

This comment has been minimized.

Copy link
Contributor

stefhak commented Aug 27, 2018

Ping @soareschen !

@aboba aboba assigned pthatcherg and unassigned stefhak Oct 6, 2018

@aboba aboba added the TPAC 2018 label Oct 6, 2018

@aboba

This comment has been minimized.

Copy link
Contributor

aboba commented Oct 12, 2018

Closing this issue due to lack of a response.

@aboba aboba closed this Oct 12, 2018

@aboba aboba removed the TPAC 2018 label Oct 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.