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

What is the intended behavior of rollback of remote simulcast offer? #2764

Closed
docfaraday opened this issue Aug 17, 2022 · 6 comments · Fixed by #2797
Closed

What is the intended behavior of rollback of remote simulcast offer? #2764

docfaraday opened this issue Aug 17, 2022 · 6 comments · Fixed by #2797
Assignees
Labels
Ready for PR TPAC 2022 For discussion at TPAC

Comments

@docfaraday
Copy link
Contributor

Right now, it looks like rollback of a remote simulcast offer does not undo any modifications made to RTCRtpSender.[[SendEncodings]] (unless the transceiver is removed, making the point moot). Is this the intent?

@aboba
Copy link
Contributor

aboba commented Aug 17, 2022

That doesn't seem consistent with the description of Rollback in RFC 8829:

4.1.10.2. Rollback

In certain situations, it may be desirable to "undo" a change made to
setLocalDescription or setRemoteDescription. Consider a case where a
call is ongoing and one side wants to change some of the session
parameters; that side generates an updated offer and then calls
setLocalDescription. However, the remote side, either before or
after setRemoteDescription, decides it does not want to accept the
new parameters and sends a reject message back to the offerer. Now,
the offerer, and possibly the answerer as well, needs to return to a
"stable" state and the previous local/remote description. To support
this, we introduce the concept of "rollback", which discards any
proposed changes to the session, returning the state machine to the
"stable" state. A rollback is performed by supplying a session
description of type "rollback" with empty contents to either
setLocalDescription or setRemoteDescription.

@aboba aboba added the TPAC 2022 For discussion at TPAC label Aug 25, 2022
@jan-ivar
Copy link
Member

jan-ivar commented Sep 7, 2022

I've tested this and Chrome removes the encodings while Safari doesn't.

Removing encodings on rollback seems in keeping with the spirit of RFC 8829, so I think we should align the spec with Chrome here.

@docfaraday
Copy link
Contributor Author

Similarly, do we want a rollback to restore encodings that were removed by a sRD(offer)? Including any scaleResolutionDownBy etc that was set by JS?

@jan-ivar
Copy link
Member

jan-ivar commented Oct 7, 2022

Similarly, do we want a rollback to restore encodings that were removed by a sRD(offer)?

If we do #2762 (comment) then sRD(offer) won't have removed anything, and there's nothing to restore.

@jan-ivar
Copy link
Member

jan-ivar commented Oct 8, 2022

I've tested this and Chrome removes the encodings while Safari doesn't.

To clarify, all I could test until crbug 944821 is fixed was the moot "the transceiver is removed' case, where the transceiver is stopped and removed from pc.getTransceivers() (JS can still observe the difference by holding a reference to the sender, which the fiddle does).

The only non-moot case that seems important here is addTrack, and it's not possible to remove its first encoding.

it looks like rollback of a remote simulcast offer does not undo any modifications made to RTCRtpSender.[[SendEncodings]]

Re-reading the OP, by "modifications" do you mean modifications made by sRD itself, or do you also include calls to setParameters() in have-remote-offer in your question?

While investigating the latter, I found a bunch of interesting pathological cases: calling sRD back-to-back, first without rollback in-between and then with.

In Chrome, back-to-back sRDs affect each other (even with rollback in-between), producing the same narrowing effect as if O/A completed between each call. This seems wrong. Also, parameter changes from JS survive when rollback isn't used in-between, but not when it is (which seems... right? wrong? Why not leave them alone always?) cc @Orphis

Safari seems to exhibit the same behavior when rollback isn't used in-between. When rollback is used in-between the test fails because a second transceiver appears (!) A bug? cc @youennf

@jan-ivar
Copy link
Member

jan-ivar commented Nov 1, 2022

With #2758 merged, the only remaining trouncing done by sRD(simulcastOffer) is of ridless unicast [[SendEncodings]].

I'll add a PR to roll that back when necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready for PR TPAC 2022 For discussion at TPAC
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants