polimediaupv / paella-core

Paella Player core library
Educational Community License v2.0
20 stars 15 forks source link

Add fallback-fullscreen-mode (mostly for iOS) #355

Closed LukasKalbertodt closed 7 months ago

LukasKalbertodt commented 7 months ago

Compare #346

The idea is: if there is no proper fullscreen support, then Paella can just span the whole viewport. It's not as good but can make the difference between barely usable and "fine". That's especially true when Paella is embedded in another page, like Tobira or an LMS.

You can test this patch here: https://pr1108.tobira.opencast.org/

karendolan commented 7 months ago

@LukasKalbertodt this makes the full screen button functional for directly embedded players. My objection is that non-embedded players are changed to showing a non-functional full-screen button, taking up space on the control bar. As I mentioned on the other ticket, it would be more optimal if a site could override by config.

Something like:

    isFullScreenSupported() {
       // Override this check in config in order to enable fullscreen-like behaviour for iOS embedded players
       if (config.overrideFullScreenCheck) {
          return true;
       }
       return this.containerElement.requestFullscreen ||
            this.containerElement.webkitRequestFullScreen;
    }
LukasKalbertodt commented 7 months ago

@karendolan That's a very good point that I forgot to consider! I just pushed a commit that hides the fullscreen button in cases where the "fake fullscreen" doesn't do anything, i.e. when the Paella player already spans the whole website. This includes Opencast's paella7/ui/watch.html?id= route, but also cases where the player is embedded via <iframe>.

I'm not opposed to an additional configuration key for that though! We can also add that, but I think the check I just added should already be able to deal with most of these cases.

ferserc1 commented 7 months ago

For the specific case of a portal with embedded videos, this approach is adequate, but it seems to me that this is not the right place to implement it. A function to switch to full screen should not do anything other than switch to full screen, by simple principle of separation of responsibilities. But it seems to me a very interesting option for the full screen plugin in paella-basic-plugins:

https://github.com/polimediaupv/paella-basic-plugins

I will modify the plugin so that it can work like this.

PS: @LukasKalbertodt, I have not been able to test this option in operation because in the link you have attached I have not seen any video that works in Safari (neither on mac nor on iPhone), but from what I see in the code I think I understand your idea. I'm going to add a 'fullscreen' class in the player element, and add the settings in CSS. This would also allow the portal developer to customize the full screen options, for example to set a different z-index level or to modify the left, top, right or bottom values.

@karendolan, I think that I can do something to avoid the button to be present if the player is not an embedded player without any configuration.

LukasKalbertodt commented 7 months ago

I will modify the plugin so that it can work like this.

If I can then implement this feature as a separate plugin, that sounds good to me!

I have not been able to test this option in operation because in the link you have attached I have not seen any video that works in Safari (neither on mac nor on iPhone)

That's odd. It works perfectly with all videos on my iPhone 13 Mini. I don't have a Mac personally, but on browerstack it also works in mac (though it uses the standard "good" fullscreen method, not the fallback).

I'm going to add a 'fullscreen' class in the player element, and add the settings in CSS. This would also allow the portal developer to customize the full screen options, for example to set a different z-index level or to modify the left, top, right or bottom values.

If I understand this correctly, this could work well. Again, as long as exactly this functionality can be implemented as external plugin or just in the integrating app (e.g. Tobira), then I'm totally fine with this.

ferserc1 commented 7 months ago

As Apple has accustomed us to, they have broken HLS playback (again) with the latest iOS update.

I have the problem pinpointed and will be releasing a patch this morning to fix it. The problem not only affects HLS, but also mp4 (and in general all formats) and has to do with the fact that paella is a multi stream player, and when loading a video it can't start playing until the next one is loaded.

In order not to make off topic, I'll comment something else in relation to this in pull request #352

LukasKalbertodt commented 7 months ago

I assume you will ping me once you made the necessary changes to allow this fake-fullscreen feature as a plugin? Feel free to also ping me so that I can test, for example.

EDIT: oh I just saw that there were some commits regarding this already. Neat.

miesgre commented 6 months ago

@LukasKalbertodt Commit https://github.com/polimediaupv/paella-basic-plugins/commit/200a87fd902c220ea272f9f0df1c8f54aa5d96d0 adds the fake-fullscreen. Please read the documentation. You may need to change some CSS to fit your site.

Comments are welcome.

LukasKalbertodt commented 6 months ago

We updated this in Tobira and it works as expected. Letting the embedding application define the CSS rules is a good idea. From now on, this can be found on https://tobira.opencast.org