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

Add "connecting" to the RemotePlaybackState enum #3

Closed
avayvod opened this issue Jan 14, 2016 · 7 comments
Closed

Add "connecting" to the RemotePlaybackState enum #3

avayvod opened this issue Jan 14, 2016 · 7 comments

Comments

@avayvod
Copy link
Contributor

avayvod commented Jan 14, 2016

Represents the state between the time when a remote playback device has been picked by the user and when the playback of the media is ready to start on the device.

The justifications:

  • to be consistent with the sibling Presentation API
  • the state allows the website to show some interstitial UI for the state transitioning between disconnected to connected

The draft summary of state changes:

  • the initial state is disconnected
  • once start() is resolved, the state changes to connecting; while in this state all media commands on the video element (play, pause, seek, volume, etc) throw exceptions
  • if the remote playback is ready to start on the remote device, the state changes to connected
  • if the remote playback fails to initialize, the state changes to disconnected
@mounirlamouri
Copy link
Member

/CC @jernoble @foolip

I'm all in favour of adding connecting. It is even a TODO in the current draft. I think we need to figure out the behaviour when connecting though. Throwing exceptions sounds a bit too strong but we could reject promises or create a new error state.

@foolip
Copy link
Member

foolip commented Jan 26, 2016

Adding this sounds good, but I agree that throwing exceptions sounds weird. Can't the handover be atomic, so that all commands are applied either locally or sent to the remote?

Also, is "once start() is resolved, the state changes to connecting" correct? Shouldn't the state be connecting until start() is resolved, and connected once resolved? Make sure to define the precise timing of when the state changes and promise is resolved :) Ideally it shouldn't be possible to observe any inconsistent state, i.e. the promise is resolved exactly as the observable state changes.

@mounirlamouri
Copy link
Member

I think we could rename start() to connect() and resolve the promise as soon as there is a device to connect to. When the promise is resolved, the state moves from disconnected to connecting then from connecting to connected. The benefit for the page is that they know that when connect() resolves, they can show some spinner UI waiting for the connection to happen (ie. connecting state) but connect() might reject if the user, for example, cancel the call. The page might not want to show a connecting UI while the user sees a device picker.

The spec might want to specify if we must have a connecting state or if we allow connecting to be skipped if the device was already connected (ie. at an app/system level).

I think the connect() method can be used for implementation to show UI if they want to (like webkitShowPlaybackTargetPicker() or the Cast device picker) or simply connect to the default device if there is one (maybe the case on iOS if the default route is already a remote device?).

We could then add a disconnect() method. We might need your input on this @jernoble in order to make sure it does make sense on iOS too. Is this something you would like to be a no-op there or should it revert the decision made in the connect() UI if the user picked a device?

@foolip
Copy link
Member

foolip commented Jan 27, 2016

Should the spec enforce that local playback stops while connecting? That's a common behavior, but should it not be possible to continue local playback until connected, then pause local playback and start remote playback? (If one waits for the local pause to finish, there could be no overlap in time, which could be a concern.)

@mounirlamouri
Copy link
Member

I guess what we do while connecting is going to be tightly related to what we do with regards to media commands while connecting. We need to offer a good developer experience. I would suggest the following:

  • While the remote state is disconnected or connecting, the media commands (ie. play(), pause(), etc.) go to the local playback.
  • When the state is connected, the media commands go to the remote playback.
  • When going to/from connected, we switch from local to remote playback. When switching playback, the new playback inherits from the state of the previous playback (ie. if it was playing, it keeps playing from the same position). The playback being switched from will be paused.

If we specify that, it will allow a decent default behaviour: when connected, the local playback will stop, and the remote playback will start from the same position. Otherwise, while connecting, the local player will be allowed to pause, show a spinner and play the remote player when it is connected.

WDYT?

@foolip
Copy link
Member

foolip commented Jan 28, 2016

If "allowed to pause" means that the web developer can implement that behavior but could also continue to play back locally for a bit longer, then that SGTM.

I realize that an issue with the handover is that if you continue playing locally as the remote buffers while connecting, then it's not really possible to guarantee that the local currentTime will be close to the remote currentTime once connected. I assume that's why the pause-buffer-play flow is used.

I'm not sure if a better method is likely to be employed, but if it doesn't complicate things implementation-wise then perhaps it's a door that can be left open for experimentation. I'm not adamant about it though.

@avayvod
Copy link
Contributor Author

avayvod commented May 10, 2016

Opened #25 to track the transition specification. Closing this issue as the state is in the spec now.

@avayvod avayvod closed this as completed May 10, 2016
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

3 participants