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

PC.local_description and friends - snapshot views or dynamic views? #2977

Open
alvestrand opened this issue May 30, 2024 · 4 comments · May be fixed by #2983
Open

PC.local_description and friends - snapshot views or dynamic views? #2977

alvestrand opened this issue May 30, 2024 · 4 comments · May be fixed by #2983

Comments

@alvestrand
Copy link
Contributor

In the current specification, current_local_description is described as an attribute that returns the content of the [[CurrentLocalDescription]] slot.
Returning an object in Javascript is basically passing a reference, not cloning the value.

So - in the current model, if we have:

x = pc.current_local_description
... time passes
a new candidate is added
... time passes
what is the value of x.sdp? does it include the new candidate or not?

This came up while hunting for a thread access issue inside WebRTC; it made it clear to me that life would be much simpler if those accessors returned a copy of the current state of [[LocalDescription]], and did not change after returning. But that is not the way attributes usually work.

What should we intend here?
(all the attributes of an RTCSessionDescription are readonly, so the Javascript side can't change it. The question is whether the platform can change it.)
@youennf
Copy link
Contributor

youennf commented May 30, 2024

It seems worth writing some tests and checking results for all browsers.

I think the spec mentions when new objects should be created and when the objects should be updated.
Surfacing new ICE candidates should probably not create new objects from what I can see in the spec.
Looking quickly at Safari, it seems new objects are created when surfacing new ICE candidates :(

@jan-ivar
Copy link
Member

jan-ivar commented Jun 5, 2024

A quick test https://jsfiddle.net/jib1/gawh7eno/20/ shows the following in all browsers:

  1. pc.localDescription === pc.localDescription
    • caveat: use Firefox Nightly or media.peerconnection.description.legacy.enabled=false in about:config
  2. pc.localDescription replaced by a new RTCSessionDescription upon onicecandidate with a candidate

This seems reasonable to me. pc.localDescription changes but x.sdp does not (which would seem like action at a distance).

In contrast the spec says: "add candidate to connection.[[PendingLocalDescription]].sdp."

I vote we change the spec to match implementations.

@alvestrand
Copy link
Contributor Author

So this would replace all the places with "add candidate to connection.[[PendingLocalDescription]].sdp" and similar with
"replace connection.[[PendingLocalDescription]] with a new object where 'candidate' has been added to the sdp"?

This works for me; x = pc.localDescription; x === pc.localDescription; await addCandidate(); x !== pc.LocalDescription

@dontcallmedom-bot
Copy link

This issue had an associated resolution in WebRTC June 18 2024 meeting – 18 June 2024 (WebRTC: PC.local_description and friends - snapshot views or dynamic views?):

RESOLUTION: no objection to proposal to align spec with implementations

@jan-ivar jan-ivar linked a pull request Jul 12, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update localDescription etc. attributes rather than just their sdp. jan-ivar/webrtc-pc
4 participants
@alvestrand @jan-ivar @youennf @dontcallmedom-bot