FreeTubeApp
Stay in Picture in Picture and full window mode when using autoplay or wathing a playlist
Stay in Picture in Picture and Full Window mode when using autoplay or watching a playlist in PiP
Pull Request Type
- [x] Feature Implementation
Related issue #927 #1192
Description If a video is in Picture in Picture mode when it has ended and the user has autoplay enabled or is watching a playlist, the next video will continue playing in that same Picture in Picture view.
This replaces the current functionality which just keeps the ended video in the PiP view and plays the next video in the regular video mode essentially creating two videos on screen.
Update: Same applies for Full Window Mode
Screenshots (if appropriate) Video (Hosted on flickr)
Testing Manually Tested
Open a video in Picture in Picture mode and make sure Autoplay is on Watch, or skip, to the end of the video and wait for the 5 second countdown to finish The next video in the Up Next tab should then play in the Picture in Picture view replacing the old video
Desktop (please complete the following information):
- OS: Mac OS
- OS Version: 10.15.7
- FreeTube version: latest Development branch
Update: All of this is true for PiP and Full Window mode Update2: Added another related issue
Could u take a look at #1192? I think u could easily reuse some of ur code for this.
Could u take a look at #1192? I think u could easily reuse some of ur code for this.
I think rather than saving a boolean for each possibility, maybe we just store a single string variable called lastPlayedVideoMode and store an empty string for a regular mode, pip for Picture in Picture, and fullscreen for fullscreen.
I am curious though, the linked issue differentiates fullscreen and full window. What would the difference there be? Did you mean Theater Mode and Fullscreen?
Edit: I apologize, the application clearly shows the difference. This is something i should be able to add.
Np🙂 at this moment in time when autoplay is enabled the only 2 states that are being remembered are the default one and when theater mode is enabled.
The difference with theater mode is that it persists regardless of if autoplay is on or not. So if you view a video in theater mode and then manually click another video, that video will also play in theater mode. This will be slightly different since all these tickets have to do with autoplay or playing a playlist. I just want to make sure that difference is made clear.
I added functionality to also remember the full window mode when using autoplay or watching a playlist, but unfortunately fullscreen capabilities will not work in this way.
Note: This method must be called while responding to a user interaction or a device orientation change; otherwise it will fail.
[Source] (https://developer.mozilla.org/en-US/docs/Web/API/Element/requestFullScreen)
If anyone knows a way around this feel free to mention it here. I do remember the state to see if it was in fullscreen or not, but I am not able to implement anything to actually open up the player in fullscreen mode without user interaction.
Life can never be easy lol. Thanks for implementing it for Fullwindow tho.
The fullscreen ability could probably be implemented using RobotJS since it uses electron. That being said, I'm not sure if the maintainers of this application would approve that because it would not be compliant, and just outright not work, if this application is ported to a native web application.
Would u like me to edit issue #1192 to just fullscreen so this can be approved for merge?
I would keep it as is so someone else can add their feedback on it. There may be a way to accomplish this that I am not aware of. Honestly, the entire triggering fullscreen mode seems to be a bit of a mess. Sometimes it would work for me and enter fullscreen mode, but other times it would throw an error related to the issue I added above.
I can see the PR #954 was able to achieve this but the PR was never merged. I'll take another look at it today to see if I can figure anything out.
So, finally getting to this. Short notice, we still have quite a backlog, so sorry if it takes some time to get to every PR.
My opinion on this is that #954 definitely has a good way to use it. The requestFullscreen() function is also mentioned in the VideosJS documentation. But as PrestoN mentioned over there, a problem with the implementation of #954 exists that due to the reload of the player when the source (legacy, audio) is changed, it reloads the player, which then re-enables the fullscreen. So this must be prevented. But I do not see what the problem was over there. Because one has to leave fullscreen in order to click one of these buttons. Therefore, the flag to check whether it was in fullscreen should be set to false anyways at that point.
RobotJS is definitely not applicable here as it simply is too much of an overkill.
I think rather than saving a boolean for each possibility
This could also be done simply by using one index, where each number corresponds to one option each.
The difference with theater mode is that it persists regardless of if auto-play is on or not.
This is true. However, it would be an opportunity to unify and streamline this. Because if we now go with your solution for the fullwindow and pip, then we are creating multiple solutions for basically the same problem. This makes the whole code less maintainable and much more complex than it needs to be.
Due to the merge conflicts the PR has, because of the new settings system and the addition of external players, some changes are required anyways. So we could now opt for unifying the whole settings for keeping the way a video was played before when the next plays.
I would also suggest that no differentiation between the modes should be made in the settings. So you either activate it for all or for none. My reasoning behind this is that a user would probably already be in the correct format he/she wants to watch the video. So enabling and disabling it for different modes seems kinda pointless and will clutter the settings even more.
Let me know whether you have any objections / do not want to do this. I'd take over this if you rather leave your code as it is and not work on reworking the whole thing. At the moment we have quite a few refactors going on at the same time, so things move rapidly and changes can happen to major parts of the code, creating new merge conflicts.
Best Regards, Gilgus
To be clear, do you want this to be an option in the settings page (the way #954 seemed to implement it) or would you prefer it just remembers what the most recent state a user put the video player in?
Edit: I see that theater mode actually has two ways of being enabled. One is in the settings, and another is that it just remembers the state when watching videos in succession. I'm going to guess you want this functionality to stay the same, but for all the possible viewing modes?
Okay, so we discussed this a bit in the team how to handle this. I am just gonna copy @Svallinn's description as it is the best approach to it:
ok, so the basic setup would be sth like this:
move all needed settings from state to stateWithSideEffects, the side effect being saving to sessionStorage under a predictable key (theater mode falls under this category) for the things that don't have settings (fullscreen and PiP fall under this category), do one of the following:
make new settings and do the same as the above bullet point (do this only if it makes sense to have a setting for it) simply save their state to the session storage when they change set up the player with the values of the session storage if they exist, and fall back to the default values in the store if they don't
if you want to make this specific to certain navigations (autoplay, playlists, etc), you'll have to play either with the router, if it can somehow provide you with the previous route (if it can't, you can just provide it has a param to indicate it came from one of those routes, maybe a boolean) OR with the history (not sure how that would work) after you figured that out, you just have to incorporate that into the whole session storage/default store values logic
I'm not really sure what the difference between state and stateWithSideEffects is to be completely honest. From my existing understanding of looking at how the volume state works, a stateWithSideEffects just means there is a default state set in the settings page but there is also a session state that exists to persist changes made outside the settings page for the duration of the session. Is that correct?
I also have a few more questions,
- How do you want the settings page to look for this?
- I was initially thinking that using a single selection drop down, but then I saw that #954 allowed having fullscreen and theater mode to be on by default. I see why you would want fullscreen and theater mode enabled (default to theater mode when leaving full screen mode), but you can't really have theater mode and full window mode enabled at once.
- Should there instead be a primary (normal, theater, full window) and secondary (none, full screen, PiP) option?
- Enabling full screen playback by default will not work with playlists/autoplay as far as I know. Per the documentation:
Note: This method must be called while responding to a user interaction or a device orientation change; otherwise it will fail.
I have verified this to be the case in my own implementation, as well as the #954 implementation. Verified in the logs on Mac OS and Windows:
It will work if you are manually navigating from one video to another, but it will not work if a page refresh/navigation occurs that the user did not initiate.
From previous comment:
if you want to make this specific to certain navigations (autoplay, playlists, etc), you'll have to play either with the router, if it can somehow provide you with the previous route (if it can't, you can just provide it has a param to indicate it came from one of those routes, maybe a boolean) OR with the history (not sure how that would work) after you figured that out, you just have to incorporate that into the whole session storage/default store values logic
I don't see why you would need to distinguish between certain navigations. I think the best way to do this would be to follow either the way that Theater Mode works right now or the way volume works. Personally, I prefer the way the volume stays persistent since it's session based rather than based on watching videos concurrently.
Currently, if you have Theater Mode on by default, watch a video and disable Theater Mode, go to your subscription list, watch a new video, it will default back to Theater Mode. Wouldn't it be more intuitive if it saved your request to not be in Theater Mode for the duration of the session? I'm fine with trying to implement it either way, but this is just a suggestion.
Currently, if you have Theater Mode on by default, watch a video and disable Theater Mode, go to your subscription list, watch a new video, it will default back to Theater Mode. Wouldn't it be more intuitive if it saved your request to not be in Theater Mode for the duration of the session? I'm fine with trying to implement it either way, but this is just a suggestion.
This is basically where side effects come in. The player currently doesn't fetch the value of the default volume in the store, it grabs the session one and updates that one too. But, to begin with. the session volume is first given a value when the volume is first loaded from the store, that's the side effect. Also, if the user changes the default volume in the settings, they're most certainly gonna expect the session volume to change along with it, that's another reason why that side effect exists.
So, you could do the same for theatre mode, by moving it to stateWithSideEffects, the side effect being attaching the value to the session storage whenever the default changes, but only when the default changes. Otherwise, the session value is left untouched.
You just have to change the logic of the Watch page to retrieve and update only the hypothetical session value rather than the default value and leave updating the default value (together with the session value due to the side effect in place) to the settings toggle.
Thank you for the explanation! I personally think that implementing the video playback experience mode this way (persistent to the session) is the better approach, but I don't want to change it if the current implementation is the way it is for a reason. Either way, thank you for the clarification!
@GilgusMaximus When you get a chance, any input on the questions I asked above would be super helpful.
I also have a few more questions,
- How do you want the settings page to look for this?
- I was initially thinking that using a single selection drop down, but then I saw that #954 allowed having fullscreen and theater mode to be on by default. I see why you would want fullscreen and theater mode enabled (default to theater mode when leaving full screen mode), but you can't really have theater mode and full window mode enabled at once.
- Should there instead be a primary (normal, theater, full window) and secondary (none, full screen, PiP) option?
- Enabling full screen playback by default will not work with playlists/autoplay as far as I know. Per the documentation:
By the way u have to add the word "closes" before the issue number to link the issue. After this PR is merged the issue will close automatically instead of staying open and clutter the backlog.
@SebTota Do you plan to continue development on this (i.e., resolve the merge conflicts and implement the suggested changes)?
@jasonhenriquez Thanks for bringing this back to my attention. I haven't been actively working on it since I never got a response from my previous comment. If you'd like to take over and fix the merge conflicts and make any other changes you see necessary feel free to do so.
@jasonhenriquez Thanks for bringing this back to my attention. I haven't been actively working on it since I never got a response from my previous comment. If you'd like to take over and fix the merge conflicts and make any other changes you see necessary feel free to do so.
I believe that @GilgusMaximus probably missed your comment. I would be glad to take over this PR, but I am not fully attuned to your implementation or what needs to be changed. Given that knowledge gap, I can resolve the merge conflicts and you can finish implementing the recommended changes. As another alternative, given that I may know some convenient tricks to help implement @Svallinn's suggestion, we can work on this together.
Looking over this again it seems like the first point I made is unnecessary since we wont be able to default to full window mode anyway.
I think the best way of doing this may be to have a drop down that allows user to choose ['normal', 'theater', 'full-window']. Then we add a checkbox for 'start video in PiP'. That way we can set the view mode of the player to what the user indicated in the drop down (regardless of if PiP is enabled or not), and then enter PiP if the checkbox was marked since PiP will work in any of those viewing modes.
This does remove the option to allow users to start videos in full-screen mode, but automatically enabling full-screen mode will only work on videos that a user manually clicks on anyway (which isn't really the point of this PR) . All videos after that that are autoplayed will not enter full-screen mode, because enabling full screen must come from a user interaction.
Does that sound good?
Looking over this again it seems like the first point I made is unnecessary since we wont be able to default to full window mode anyway.
I think the best way of doing this may be to have a drop down that allows user to choose ['normal', 'theater', 'full-window']. Then we add a checkbox for 'start video in PiP'. That way we can set the view mode of the player to what the user indicated in the drop down (regardless of if PiP is enabled or not), and then enter PiP if the checkbox was marked since PiP will work in any of those viewing modes.
This does remove the option to allow users to start videos in full-screen mode, but automatically enabling full-screen mode will only work on videos that a user manually clicks on anyway (which isn't really the point of this PR) . All videos after that that are autoplayed will not enter full-screen mode, because enabling full screen must come from a user interaction.
Does that sound good?
I thought the idea was to change the default behavior, not to add new settings. I am not saying this in opposition to what you say, I'm just confused as to what the goal is.
@jasonhenriquez AFAIK there will be two things
-
There will be options in the settings menu to play videos by default in theather, fullscreen, fullwidow mode
-
And if the user clicks on lets say theater mode and is going to watch the next video, the next video will play in theater mode.
This PR fixes the second one.
Looking back at this, i think it doesn't make sense to even implement 1. I think its kind of unnecessary.
From the previous comments, it seems like @GilgusMaximus and @Svallinn wanted this to behave like theater mode currently does. Meaning you have a setting to enable a default view preference, and then the user behavior is saved on top of that for the duration of a viewing session. (Ex. You enable theater mode by default, play a video, turn theater mode off while watching the video, go to another video from the recommended list, the next video plays with theater mode disabled)
@efb4f5ff-1298-471a-8973-3d47447115dc
There will be options in the settings menu to play videos by default in theather, fullscreen, fullwidow mode
Fullscreen mode is not going to be possible as explained previously unfortunately. But yes, my idea/understanding was just to change the current theater mode on/off to a drop down where you can choose ['normal', 'theater', 'full-window'].
PiP is sort of in it's own category since you can have a video in any of the viewing modes, ['normal', 'theater', 'full-window'], and still have a video playing in PiP. This is why I recommended having a separate setting for that instead.
I may be misinterpreting the precious comments made in this PR though so feel free to disagree with this. I'll have some time over this weekend to take another look at this if you'd like me to continue working on it.
From the previous comments, it seems like @GilgusMaximus and @Svallinn wanted this to behave like theater mode currently does. Meaning you have a setting to enable a default view preference, and then the user behavior is saved on top of that for the duration of a viewing session. (Ex. You enable theater mode by default, play a video, turn theater mode off while watching the video, go to another video from the recommended list, the next video plays with theater mode disabled)
Ah, okay. This is mostly correct to my understanding, but after clarifying with @Svallinn myself, I believe they also want to include the idea of a viewing session to include the entire session, not just from Watch.js sessions. We will have to modify theater mode's implementation, as well, to account for this nuance. But altogether, yes, I think maintaining a specific scope for playlists/autoplay changes is probably too narrow and we should just implement these features at a broader level.
Maybe some day in the future we can implement customizable hierarchical granularity levels for the scope of changes to a particular setting. For example:
- One configuration has setting changes that persist only for that given video.
- Another configuration has setting changes that persist with autoplayed videos / videos within the same playlist but are reset to the default if the next video is non-autoplayed / in a different playlist.
- Another has setting changes persist within the same viewing session but not in different viewing sessions (i.e., how theater mode works currently).
- Another has setting changes that persist throughout the entire session but not in different sessions (i.e., how volume works currently).
- Finally, the broadest scope option has all setting changes enacted persist in perpetuity.
And then you could change these on a per-setting basis (e.g., one hierarchy option for volume but a different one for theater mode), but you could also have a higher option to change the hierarchy option for all settings at once to save time. We can set the defaults to match the behavior users would expect / prefer. And lol, yes, I realize as I'm typing this post that implementing this would feel like recreating a RBAC policy language.
I'm also just realizing, on a completely different note, that we need to have an option to have settings changes on a particular profile not affect the global settings. But that's neither here nor there!
Fullscreen mode is not going to be possible as explained previously unfortunately.
It's not that I don't believe you about this, but I would be interested into seeing if there is some kind of workaround for this. In any case, default fullwindow would serve about the same purpose as long as that is possible.
But yes, my idea/understanding was just to change the current theater mode on/off to a drop down where you can choose ['normal', 'theater', 'full-window'].
I do think that this would be a fine implementation, but as I said in this discussion, I still think there is merit to keeping default theater mode as its own separate, non- mutually exclusive setting.
PiP is sort of in it's own category since you can have a video in any of the viewing modes, ['normal', 'theater', 'full-window'], and still have a video playing in PiP. This is why I recommended having a separate setting for that instead.
Makes sense, and as I said earlier, unless we want to think about all of these levels of granularity, we might as well just keep this an on-off toggle for one preset hierarchy level.
I may be misinterpreting the precious comments made in this PR though so feel free to disagree with this. I'll have some time over this weekend to take another look at this if you'd like me to continue working on it.
Yes, I would very much appreciate that!
@jasonhenriquez
I do think that this would be a fine implementation, but as I said in this discussion, I still think there is merit to keeping default theater mode as its own separate, non- mutually exclusive setting.
I agree this would be the better use case, and is similar to what I suggested when I first started on this PR, but this only makes sense if defaulting to fullscreen mode would work. Then we could have two drop downs:
Primary: ['regular', 'full-window', 'theater'] Secondary: ['full-screen', 'pip']
This way if you are in Full Screen or PiP and exit that viewing mode, we default it to the primary mode. This is actually why I still want PiP to be it's own setting. Again though, this only makes sense if Full Screen would work.
It's not that I don't believe you about this, but I would be interested into seeing if there is some kind of workaround for this. In any case, default fullwindow would serve about the same purpose as long as that is possible.
I suggested using RobotJS before, but it seems like the devs did not want to go in that direction. If you can find a different solution I'd be more than glad to try it out! The issue is that the event must come from a user triggered click and not a programmatically triggered event.
I'm also just realizing, on a completely different note, that we need to have an option to have settings changes on a particular profile not affect the global settings.
I'm not sure I full understand this.
I agree this would be the better use case, and is similar to what I suggested when I first started on this PR, but this only makes sense if defaulting to fullscreen mode would work. Then we could have two drop downs:
Primary: ['regular', 'full-window', 'theater'] Secondary: ['full-screen', 'pip']
This way if you are in Full Screen or PiP and exit that viewing mode, we default it to the primary mode. This is actually why I still want PiP to be it's own setting. Again though, this only makes sense if Full Screen would work.
This makes more sense to me now, thank you. I think it's a good idea. However, I do believe that for the reasons you stated, default fullscreen is unlikely. Fullwindow is functionally very similar to fullscreen and can serve that same purpose. For that reason, it would make more sense to fill in for 'fullscreen' as a secondary. Additionally, the verbiage of primary & secondary "falling back" is a bit of a mismatched with the primary settings, as disabling theater mode would itself be a third layer of falling back.
All this considered, I think the clearest visual way to represent this information would be to keep default theater mode's toggle (as is visually shown to the user) unchanged but add two separate toggles for default PiP and default fullwindow. This implementation makes a natural matroyshka without any modifications to how these features normally work in unison. I really think this might be the simplest way to do it in a way that makes intuitive sense to users.
I'm also just realizing, on a completely different note, that we need to have an option to have settings changes on a particular profile not affect the global settings.
I'm not sure I full understand this.
Ah, sorry, this was an unclear & tangential point. I think we should allow an option such that going into the Settings tab and making some changes under the profile 'Bob' does not affect the settings as laid out on the profile 'Alice'. Under this paradigm, one would have to switch to All Channels to make settings changes that affect all profiles.
All this considered, I think the clearest visual way to represent this information would be to keep default theater mode's toggle (as is visually shown to the user) unchanged but add two separate toggles for default PiP and default fullwindow. This implementation makes a natural matroyshka without any modifications to how these features normally work in unison. I really think this might be the simplest way to do it in a way that makes intuitive sense to users.
So the reason ['regular', 'full-window', 'theater'] are in a drop down is because those are actually states the video editor can be in without modifying the application window. PiP and Full Screen, on the other hand, modify the actual way the video is being played back. This means that the video player in the application can be in ['regular', 'full-window', 'theater'] regardless of full-screen or PiP being enabled or not. So you can have a full-window and be in full-screen, but you can not have full-window and theater mode enabled since those both modify the same application window.
So if you have a separate setting for PiP, full window, and theater mode like you suggest, how do you enable full window and theater at the same time?
Does that make sense? It's a bit difficult to explain, so let me know if you need me to explain it in a different way.
So if you have a separate setting for PiP, full window, and theater mode like you suggest, how do you enable full window and theater at the same time?
You cannot use them at the same time, of course, but the idea is that theater mode will automatically be set to active after exiting fullwindow mode. It's a matryoshka of regular > theater > fullwindow > PiP, if you were to have all of the default aforementioned toggles active. I may generally want to start my stuff in full window, but if I don't at some point, I also prefer having theater mode on than not. This is also how its implementation works; theater mode is still active regardless of whether fullwindow/fullscreen/PiP is on or off, it's right there for you when you exit from those. It's a more intuitive idea than is coming through with my communication.
So the reason
['regular', 'full-window', 'theater']are in a drop down is because those are actually states the video editor can be in without modifying the application window. PiP and Full Screen, on the other hand, modify the actual way the video is being played back. This means that the video player in the application can be in['regular', 'full-window', 'theater']regardless of full-screen or PiP being enabled or not. So you can have a full-window and be in full-screen, but you can not have full-window and theater mode enabled since those both modify the same application window.
You can have full-window and theater as I stated earlier. ~~I am not sure if you can be in full-window and full-screen at the same time, but that's not implemented in FT as of this moment.~~ In fact, currently, the "full window" button is intentionally hidden when you are in fullscreen.
Edit: You can access full-screen when the full-window button is active. Full-screen is the only visible mode once clicked, and full-window returns as the only visible mode it is exited, meaning it takes its place in the layer above full-window in the matryoshka. My main point is that none of these features (theater mode, full-window, PiP) are truly incapable of being active at the same time; they just take effect in layers necessarily. Users should be able to have the choice of which layers they want on (i.e., active by default).
Alright, now I understand what you are trying to say. I think, while this idea is great in theory, it is going to be incredibly confusing for the end user in my opinion. When a user sees the ability to set full window AND theater mode by default, they will not understand the hierarchy of which one will actually be on by default.
To make 100% certain I understand your logic, let's say we have a toggle for full window and theater mode.
Case 1: A user has both of the toggles on. When the user opens a video, the window will play in full window since that has a higher priority than theater mode. Then, if a user leaves full window mode, it will switch to theater mode.
Case 2: A user has full window on and theater mode off. When the user opens a video, the window will play in full window. If the user then leaves full window mode, it will switch the video player to the regular viewing mode.
Case 3: A user has full window off and theater mode on. The logic is essentially what is already implemented as theater mode today.
Is that correct?