Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

Update video projection for currently playing video #3466

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

keianhzo
Copy link
Contributor

@keianhzo keianhzo commented Jun 4, 2020

We just set the video projection when we enter full screen so when videos are playing in a list or next video is played, the previous video projection remains set. This PR should address that issue and always set the currently playing video projection. If the user changes the projection for the currently playing video, that would be used until the next video is played or full-screen mode is exited.

STRs:

  • Create a playlist with 360 and flat videos
  • Play the first video fullscreen
  • Click the next button to play the next one or let the video end.

The next video should be played and the correct projection should be used.

Note: This will only work when in immersive/full-screen.

Related to #3394 (comment)

@keianhzo keianhzo self-assigned this Jun 4, 2020
@bluemarvin
Copy link
Contributor

One thing I noticed is if I start from a video in the feed, since it sets the projection in the URL it always uses that even when a video does not have a projection. In my testing where I didn't start from the feed so it didn't have the extra data in the URL, it would work as expected for an ad and drop out of immersive mode, but if the content after the add was 360, it would stay in fullscreen and never return to immersive mode.

@bluemarvin bluemarvin added this to the #11 polish milestone Jun 4, 2020
@keianhzo
Copy link
Contributor Author

keianhzo commented Jun 5, 2020

@bluemarvin We set the auto projection based on the video title/description and those doesn't change when an ad is playing so it's not easy to detect and update the projection. In your case it is probably exiting immersive mode when an ad plays because the next video is flat. Also it's quite hard for some reason to get ads, I guess it must be because of the UA?

@bluemarvin
Copy link
Contributor

I don't know if there is a good solution. I just wasn't sure if what I was seeing was expected. I think I would rather it not drop out of immersive mode for ads if we can't go back into immersive mode when the next video starts.

@keianhzo
Copy link
Contributor Author

keianhzo commented Jun 8, 2020

That should be the behavior of this PR. If the next video is 3D, it will stay in immersive mode regardless of whether or not there is an ad before the video.

@keianhzo
Copy link
Contributor Author

@bluemarvin do you think that this is worth landing?

@bluemarvin
Copy link
Contributor

@keianhzo Maybe a few other people should try it and see what they think. I don't know if there is a right answer without meta data for the video. I'll try it again.

@bluemarvin
Copy link
Contributor

I guess I still don't understand the expected behavior. From the STR it seems like it should be able to switch between flat fullscreen and immersive mode as the videos change but I only ever see it drop out of immersive mode and it never goes back. Is that the expected behavior?

@keianhzo
Copy link
Contributor Author

@bluemarvin I see it switching from flat to 360 as long as the next video is 360. Can you share the playlist that you are using?

@bluemarvin
Copy link
Contributor

I think we should move this to v12. I'm not sure I understand how it is supposed to work yet.

@bluemarvin bluemarvin modified the milestones: #11 polish, #12 features Jun 26, 2020
@keianhzo keianhzo changed the base branch from master to main June 30, 2020 16:56
@cknowles-admin cknowles-admin added the ARCHIVED CLOSED at time of archiving label Jul 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ARCHIVED CLOSED at time of archiving
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants