Re-introduce ability to change quality for progressive download sources (e.g. mp4 files) #352
Conversation
Fixes polimediaupv#342 I based this on db5dfce1 [1] but had to add additional changes. A few notes. For one, it is not sufficient to pause & resume each video on its own when changing the quality. Everything has to be stopped, all videos have to be loaded in the new quality settings, and then all have to be resumed. That's why changes in `StreamProvider` were necessary. I also changed that all videos are loaded simultaneously instead of serially, like before. This should make quality switching a bit faster. Another change I had to do is move the `autoplay` attribute thingy. I first thought that autoplay couldn't possible be important if it is stopped immediately anyway. But I too found weird behaviors in Chrome and FF when first playing the video. The time sync was triggered a few times, making one of the two videos jump a few times. I am not 100% sure why that happens, but with the autoplay it is indeed fixed. However, the `.pause()` in `waitForLoaded` posed a problem for quality changing. It always confused the "playing state" after changing the quality, as some videos got randomly paused. I solved this by just moving the autoplay attribute to `loadStreamData` and also resetting it there. One final change is that in `loadStreamData`, `waitForLoad` is not used anymore, but the `canplay` event is used, which is more appropriate for that use case. [1]: miesgre/opencast@db5dfce
I am not sure why, but without autoplay mobile Safari doesn't properly load the video.
Before this commit, the highest quality was always chosen. That's often not a good idea for multiple reasons: - Increases server bandwidth - Makes video load slower - Eat lots of mobile bandwidth (if on mobile) - For mobile devices, drains the battery more quickly or even completely overwhelms the devices. I'm a video quality snob and even I don't mind 720p videos on my iPhone (the YouTube default), despite having a screen resolution of 2340×1080. The logic in this commit is simply: take the largest video that completely fits in the (potentially rotated) screen. For mobile phones, use 1600x900 as screen resolution for the described check.
|
Thanks for your work, Lukas! This is a really important fix that will allow us to easily migrate to Paella 7 without having to convert all of our 75000+ videos to adaptive HLS. For us at the University of Bern, this fix is very much appreciated. |
|
Since the same Playwright tests fail on the main branch right now, I assume that my PR is not responsible for those failures. |
|
My problem with the mp4 quality change, and the reason why it was not added in this version of paella-core is that I have had it working several times over the years, but in each new version of iOS, Safari would introduce some change that caused it to stop working properly. I don't want to introduce this feature in paella-core because it is my responsibility to maintain the code that is introduced, and therefore I don't want to have to take over the maintenance of a plugin that, in addition to this, is nothing more than a hack that is not officially supported by any browser. If you need me to add the I suggest you create your own repository to add the plugin and publish it on your own. You have a complete tutorial on how to create a plugin repository for paella-core: |
I have a lot of problems with Playwright testing. Sometimes it fails, I execute them again and it works without changing anything |
|
We are not comfortable with this functionality in the core directly for the reasons expressed by @ferserc1. Could you make a PR with the necessary changes in |
I don't think this is correct. Changing the
My understanding of the spec is that this is indeed intended behavior that web apps can take advantage of. And indeed many do. Just to throw one example out there, with Plyr I can also change the quality on my iPhone. And that's also using mp4 files, not HLS.
I'm also maintaining several pieces of software and I can totally understand this concern, really. On some occasion, we talked about someone else taking over the maintenance of that part of the code, i.e. if it breaks in newer Safari versions, someone else from the Opencast community would fix it, without you having to do anything. Is that still an option that would be ok for you? I will answer the remaining points later. |
It is true that the src attribute can be modified and the player must reload the video, but what we are trying to do here is to emulate the behavior of HLS, ie, modify the source, replay the video and jump to the last instant before changing source. To do this the browser has the Media Source Extensions APIs, with which you could implement this with MP4 sources, but that has certain requirements in the encoding. In fact, here we serve m3u8 playlists that are made up of mp4 files, but we still had to re-encode the files to meet the encoding requirements. I'm not saying it can't be done, in fact I have been maintaining it in previous versions of paella player, my problem is that it is a feature that often fails due to changes in browsers, and also we no longer use this feature in my organization, so it is much more complicated to maintain it.
Our policy is to separate the functionalities in different libraries that have different responsibilities, and in fact for paella-core 2.0 we are considering to extract the HLS part to an external plugin library. In the case of the dynamic quality change in mp4 I see that it is very justified to separate that feature in a different library. In fact, in our internal video repository we used this functionality, but that plugin was included outside paella-core. That repository is not public, but if you are interested I can send you the code of this plugin. We stopped maintaining it because we switched to HLS, since we had complaints from iOS users from time to time, but if you are interested I can pass you the code. Also, to make it work it is not necessary to modify any function of the core. As the About the changes in StreamProvider, I noticed in your pull request that you do a parallel asynchronous loading of the videos in StreamProvider, which is much more efficient than the way it is being done now. The problem is that for a while we were having problems on some devices when changing quality, and it was managed to be corrected by loading the videos in series. It was mostly on iPad, and had to do with browser tab closures due to excess memory consumption. I'm not sure if that was the problem, but loading the videos serially helped. I think with current devices we won't have those issues, but before modifying that part I'd like to test it. |
I don't think "emulate" is the appropriate word here. Both, HLS and progressive download with
I tried that first and for single stream videos, that works just fine. But for dual stream videos, both streams should be paused until both streams are loaded enough to play. So in my opinion and experience, there needs to be some synchronization between those.
Yes, true. If that really causes problems, it can easily be changed to serially load those.
As long as it's still possible to properly change quality for progressive download (see the "only resume both streams when both are loaded" above), I'm fine wiht splitting player functionality in whatever way. I personally still think this feature should be part of default Paella player, given that Paella seemingly wants to be the Opencast player and that a vast number of people in the OC community need this feature. But splitting this off into plugins seems fine. Again, as long as the ability to properly change progressive video quality isn't restricted. |
|
The mp4 player has stopped working in the latest iOS beta update, and I don't know if it happens with the stable version as well. On some videos it has started returning ReadyState=2 instead of 3, and that makes the video get stuck syncing streams. Since the stream sync is always done, even if there is only one video, the problem happens every time. I have to make some changes that are going to pause the video when reloading the stream always, then I think this functionality makes it unnecessary to add the API. I will keep you updated, for now I will leave everything else paused, because I would like to have the patch before they release this iOS version for everyone. |
|
This pull request has conflicts ☹ |
|
Good day! What's the status here now btw? You said you wanted to patch for the new iOS version, but that seems to have already landed? So do I have to do something now or am I still waiting on something?
I'm sure that causes all kinds of problems but it also reminded me: maybe one should replace the
Did those changes already make it to the main branch or to a release? Does that mean that changing the quality of HLS streams manually also pauses the stream? |
|
The waitForLoad was implemented because the canplay callback was not appropiate (this implementation is inherited from paella player 6 and is quite old). Here the problem was that calling the callback canplay means that the video can start playing, but it may not start playing immediately. Having multiple videos that have to be synchronized, the callback was not a good method. I suspect that having changed the radyState now the current method does the same as using the callback, but I don't intend to change it right now because I'm quite busy with paella-core 2.0 development. Surely this code will be revised in version 2. I published the version with the fix for iOS last week. This has nothing to do with the quality change in HLS, because in this case the streams are not modified (the stream quality changed is made using the MediaSourceExtensions API). The problem is that the video never started playing, so it never got to the point of the quality change. Here is the code of the plugin we use in the UPV video server with support for mp4 quality change. We don't use it anymore, and I haven't tested it for a long time, but it has worked without problems for about two years: https://gist.github.com/ferserc1/5526f07d9d194e2af651f5d2f5bb50a2 |
|
New MP4 multi-quality plugin: https://github.com/polimediaupv/paella-mp4multiquality-plugin Thanks @LukasKalbertodt! |
Closes #342
I think I managed to get this highly requested feature working. This was tested in many different browsers and devices and everything seems to work as expected. You can test this PR here (try the videos from "Mixed Test Series").
Unfortunately, this cannot be achieved by just having a separate video format plugin: the
StreamProviderhas to coordinate pausing and resuming. That cannot be done by each video format individually. So I was forced to touchpaella-corecode anyway. See the commits for details on how this was solved.I think this works robustly enough to be integrated into
paella-coreso that all users can just benefit from this patch.The last commit in this PR changes the way the initial quality is selected. See the commit message for more information.
I am aware that the UX could be improved still by showing a spinner while the new quality is loading, but this PR focuses on the functionality. The spinner can easily be added later, if requested.