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

Should be more clear about what states/methods allow a "rollback" #760

Closed
taylor-b opened this issue Jul 20, 2017 · 9 comments
Closed

Should be more clear about what states/methods allow a "rollback" #760

taylor-b opened this issue Jul 20, 2017 · 9 comments
Assignees

Comments

@taylor-b
Copy link
Collaborator

We have these rules in setLocalDescription (and similar rules in setRemoteDescription).

  *  If the type is "offer", the PeerConnection state MUST be either
     "stable" or "have-local-offer".

  *  If the type is "pranswer" or "answer", the PeerConnection state
     MUST be either "have-remote-offer" or "have-local-pranswer".

But they don't mention "rollback". They should. Related WebRTC issue: w3c/webrtc-pc#1470

taylor-b added a commit to taylor-b/jsep that referenced this issue Jul 20, 2017
Fixes rtcweb-wg#760.

This raises a question I haven't seen discussed before: can you roll
back a provisional answer? I'd been assuming "no" but I could be wrong.
taylor-b added a commit to taylor-b/jsep that referenced this issue Jul 20, 2017
Fixes rtcweb-wg#760.

This raises a question I haven't seen discussed before: can you roll
back a provisional answer? I'd been assuming "no" but I could be wrong.
@ekr
Copy link
Contributor

ekr commented Jul 28, 2017

Long discussion on the editor's call today. Consensus was it would be easier if set{Local,Remote}Description("rollback") always did the same thing no matter what state you were in: roll you back to the stable state.

@taylor-b
Copy link
Collaborator Author

Just to clarify, are these legal?:

  1. Rolling back PR-answer; for example, setRemoteDescription("rollback") in "have-remote-pranswer" state.
  2. Doing a rollback using a method that doesn't match the last description you applied. For example, setRemoteDescription("rollback") in state "have-local-offer".

If it doesn't matter whether you call setLocalDescription or setRemoteDescription, then why go through those methods in the first place? Why not just have a separate "rollback" method?

@taylor-b
Copy link
Collaborator Author

taylor-b commented Aug 3, 2017

@ekr, @fluffy or @juberti, can you answer my question above? Just want to make sure we get this right in webrtc-pc.

@fluffy
Copy link
Contributor

fluffy commented Aug 6, 2017

Yes, both are legal. We actually discussed just having a rollback method. The net effect will be the same as calling a single rollback method. The only reasons for this are 1) match existing code 2) if the far side is instigating the rollback, some apps can use a just take what you get over signalling and stuff it into setRemoteDescritption without even knowing if it is a rollback or something else.

@ibc
Copy link

ibc commented Aug 6, 2017

Both 1) and 2) above seem crazy to me. Not sure what they are suppose to do.

I understand that setLocalDescription({ type:'rollback' }) should just be used if:

  • state is have-local-offer.

And setRemoteDescription({ type:'rollback' }) should just be used if:

  • state is have-remote-offer.

@fluffy
Copy link
Contributor

fluffy commented Aug 6, 2017

Rollback, no matter what state you are in, takes you back to the last stable state. The stable state if a function of both local and remote so it does not matter if you call the rollback on the local or remote API.

@juberti juberti self-assigned this Aug 11, 2017
@fluffy
Copy link
Contributor

fluffy commented Aug 17, 2017

I think this can be closed now

@fluffy
Copy link
Contributor

fluffy commented Aug 17, 2017

This has been fixed.

@fluffy fluffy closed this as completed Aug 17, 2017
@taylor-b
Copy link
Collaborator Author

Fixed by 6f5dc41, specifically, which was based on #761

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

No branches or pull requests

5 participants