Skip to content

Use RTCIceCandidatePair interface in RTCIceTransport#205

Merged
jan-ivar merged 6 commits intow3c:mainfrom
sam-vi:2930-candidatepair-interface
Jun 5, 2025
Merged

Use RTCIceCandidatePair interface in RTCIceTransport#205
jan-ivar merged 6 commits intow3c:mainfrom
sam-vi:2930-candidatepair-interface

Conversation

@sam-vi
Copy link
Copy Markdown
Contributor

@sam-vi sam-vi commented Apr 18, 2024

Addresses: w3c/webrtc-pc#2930.

This change is built on top of w3c/webrtc-pc#2961, which converts RTCIceCandidatePair from a dictionary to an interface.

  • Change RTCIceCandidatePairEvent to contain a single RTCIceCandidatePair attribute.
  • Modify RTCIceTransport algo so that getSelectedCandidatePair() returns an item from [[CandidatePairs]].
  • Validate RTCIceCandidatePair inputs using object identity instead of candidate pair match algo.

Preview | Diff

@sam-vi
Copy link
Copy Markdown
Contributor Author

sam-vi commented Aug 6, 2024

Filed #217 to address the validation errors.

Probably best to merge #218 or fix #217 another way before proceeding with this PR.

Addresses: w3c/webrtc-pc#2930.

As an interface, RTCIceCandidatePair can be used as an attribute of
other interfaces.

So simplify RTCIceCandidatePairEvent by having a single candidatePair
attribute instead of individual local and remote candidates.
An application only acquires objects of RTCIceCandidatePair through
RTCIceTransport events and getSelectedCandidatePair().

So simplify input validation where RTCIceCandidatePair is used as an
argument by directly comparing object identity instead of using the
candidate pair match algorithm.
This ensures that each candidate pair has a unique identity, simplifying
the validation of RTCIceCandidatePair arguments to RTCIceTransport
methods.
@dontcallmedom dontcallmedom force-pushed the 2930-candidatepair-interface branch from 2de721f to cd26c2a Compare October 15, 2024 11:24
@alvestrand
Copy link
Copy Markdown
Contributor

@sam-vi this has been hanging around for a while - can you check that it's still up to date?

dictionary RTCIceCandidatePairEventInit : EventInit {
required RTCIceCandidate local;
required RTCIceCandidate remote;
required RTCIceCandidatePair candidatePair;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Was this change discussed?
It looks ok to me but this reduces the ability of creating event to candidate pairs created by the UA

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is in line with the goals of w3c/webrtc-pc#2930, namely restricting JS's ability to create invalid pairs and simplifying validation steps in specs. It reduces the burden of validation in the RTCIceTransport extension API specs, and provides web developers with a loose guarantee that events will signal candidate pairs that can be provided as inputs to other RTCIceTransport methods (barring state errors).

I hadn't considered this particular change to be especially significant, but we could discuss in the next interim if needed.

My thoughts are:

  • It is a good idea to limit the general purpose RTCIceCandidatePairEventInit to valid candidate pairs created by the UA.
  • If in the future there is a need for an event to signal candidate pairs not created by the UA, it should be a more specific event (similar to RTCPeerConnectionIceErrorEvent) that is used in that limited scope.
    • If this is found to be too restrictive, then it could be discussed whether it makes sense to introduce a JS-accessible constructor for RTCIceCandidatePair.

@sam-vi
Copy link
Copy Markdown
Contributor Author

sam-vi commented May 19, 2025

@alvestrand made another pass over the changes, and added a small fix. Otherwise looks ready to me.

@jan-ivar jan-ivar merged commit 5ea95ee into w3c:main Jun 5, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants