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

Maybe removeTrack should be a no-op on [[Stopping]] as well as [[Stopped]]? #2874

Closed
jan-ivar opened this issue May 31, 2023 · 3 comments · Fixed by #2875
Closed

Maybe removeTrack should be a no-op on [[Stopping]] as well as [[Stopped]]? #2874

jan-ivar opened this issue May 31, 2023 · 3 comments · Fixed by #2875
Assignees

Comments

@jan-ivar
Copy link
Member

jan-ivar commented May 31, 2023

Similar to #2820 but for removeTrack (and no-op instead of rejection which is how removeTrack works).

The rationale would be the same that this method serves no purpose after stop() which "will immediately cause the transceiver's sender to no longer send, and its receiver to no longer receive."

While it might seem like a no-brainer to align with our earlier decision on those similar methods, implementations aren't 100% aligned, so we need a decision. https://wpt.fyi/results/webrtc/RTCPeerConnection-removeTrack.https.html
image
The red is Safari following the current spec, which says to proceed as long as the sender is in CollectSenders. The others aren't.

@jan-ivar
Copy link
Member Author

In case your next question is who follows the spec on collectSenders and collectReceivers, this is (perhaps surprisingly) tested in https://wpt.fyi/results/webrtc/RTCDtlsTransport-state.html
image
The red is browsers following the current spec, the green is Firefox not (the test is from Mozilla and seems backwards). *

Given this, I think it makes sense to leave the sender in getSenders() on [[Stopping]] (current spec), ditto receivers, and fix the second test, but change the removeTrack spec language to abort explicitly if transceiver.[[Stopping]]


*) The test tests both getReceivers() and getSenders() but it tests getReceivers() first which is why the error message says getReceivers(). Presumably they work the same for senders and receivers.

@jan-ivar
Copy link
Member Author

This means fixing Safari and Firefox.

@jan-ivar
Copy link
Member Author

(perhaps surprisingly)

We can remove those asserts, as this is instead correctly tested in RTCRtpTransceiver.https.html (wpt.fyi):
image
(Safari here appears to fail for an unrelated reason)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant