videojs / video.js

Video.js - open source HTML5 video player
https://videojs.com
Other
37.81k stars 7.43k forks source link

player.currentSources() - When the video is playing, missing all the sources when the first source is not an absolute URL #8712

Open unixfox opened 4 months ago

unixfox commented 4 months ago

Description

Hello,

When you declare multiple sources inside the <video> HTML element and the first source do not have an absolute URL, then you can't get all the other sources using player.currentSources() when the video is playing.

This bug exists since video.js 8.0.0, and I can't replicate it on video.js 7.21.5. I think this is a regression from video.js 7 to video.js 8.

HTML demo URLs:

GitHub repository source files: https://github.com/unixfox/videojs-currentsources-issue

Due to this bug, silvermine/videojs-quality-selector does not work without absolute URLs: https://github.com/silvermine/videojs-quality-selector/issues/107

Thank you for taking a look at this bug.

Reduced test case

https://unixfox.github.io/videojs-currentsources-issue/videojs8.html

Steps to reproduce

  1. Create an HTML test file with two working video files named source1.mp4 and source2.mp4 (can use https://sample-videos.com/ for demo videos)
    
    <head>
    <link href="https://vjs.zencdn.net/8.13.0/video-js.css" rel="stylesheet" />
    <script src="https://vjs.zencdn.net/8.13.0/video.min.js"></script>
    </head>
2. Start a local webserver with `python -m http.server 8888`.
3. Open the HTML page with your browser (http://localhost:8888) and open the browser console from devtools.
4. Check the console and see that in the logs, source2.mp4 is missing from the listed sources. Only source1.mp4 is here.
    ![image](https://github.com/videojs/video.js/assets/4016501/e668c24b-b34e-4f88-93ef-e2eb11103239)

5. Edit the HTML page and add the absolute URL to the first source. And see that now all the sources are listed.
    ```html
    <source src="http://localhost:8888/source1.mp4" type="video/mp4" />
    <source src="/source2.mp4" type="video/mp4" />
![image](https://github.com/videojs/video.js/assets/4016501/9b382d1e-2542-4f28-81c0-10dbd93bef73)

Errors

No response

What version of Video.js are you using?

8.13.0

Video.js plugins used.

No response

What browser(s) including version(s) does this occur with?

Chrome 124.0.6367.78 and Firefox 125.0.2

What OS(es) and version(s) does this occur with?

ArchLinux

welcome[bot] commented 4 months ago

👋 Thanks for opening your first issue here! 👋

If you're reporting a 🐞 bug, please make sure you include steps to reproduce it. We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can. To help make it easier for us to investigate your issue, please follow the contributing guidelines.

mister-ben commented 4 months ago

The difference is that the video el sourceset event is handled by default in v8, when the sources cache is updated here the absolute URL from the sourceset event is compared to the source els. If the test here used getAbsoluteURL(sourceObj.src) === getAbsoluteURL(src) (from /src/js/utils.js) currentSources() would be as before - but that causes tests in /test/unit/sourceset.test.js to fail.

unixfox commented 4 months ago

@mister-ben Sorry I'm not that familiar with how the inner of video.js works, but does this mean it's considered a valid regression compared to video.js 7? Or is it considered as a bug? Right now https://github.com/silvermine/videojs-quality-selector doesn't work due to this issue because it thinks that currentSources() will always give all the existing source URLs, not in a case of absolute URLs or not.

mister-ben commented 2 months ago

@unixfox there's quite a bit to unpick here. You could disable the sourceset behaviour by using enableSourceset: false in the player options. Hopefully that can unblock you.

unixfox commented 1 month ago

Sorry for the late reply. Thank you @mister-ben, setting enableSourceset indeed fixes the issue: https://unixfox.github.io/videojs-currentsources-issue/videojs8-enableSourceset-false.html