Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(player): load method fails to reset the media element #8274

Merged
merged 2 commits into from
May 31, 2023

Conversation

amtins
Copy link
Contributor

@amtins amtins commented May 13, 2023

Description

This PR provides a workaround for the load method that fails to reset the media element to its initial state when the VHS is used.

This issue affects all devices using VHS tech to play Dash and HLS. It does not affect the playback of media that does not require VHS, such as mp4 or HLS on Apple devices.

This fix does not cover the case when the load method is called directly from the mediaElement.

Screencast.from.13.05.23.21.00.59.webm

How to reproduce

player.src('https://d2zihajmogu5jn.cloudfront.net/bipbop-advanced/bipbop_16x9_variant.m3u8');

player.cache_.src;
// "https://d2zihajmogu5jn.cloudfront.net/bipbop-advanced/bipbop_16x9_variant.m3u8"
player.cache_.source;
// { src: "https://d2zihajmogu5jn.cloudfront.net/bipbop-advanced/bipbop_16x9_variant.m3u8", type: "application/x-mpegURL" }
player.cache_.sources;
// [{ src: "https://d2zihajmogu5jn.cloudfront.net/bipbop-advanced/bipbop_16x9_variant.m3u8", type: "application/x-mpegURL" }]

player.load();

player.cache_.src;
// "blob:http://localhost:9999/bc47fd5c-f3eb-4be9-8fea-40444743105e"
player.cache_.source
// { src: "blob:http://localhost:9999/bc47fd5c-f3eb-4be9-8fea-40444743105e", type: "" }
player.cache_.sources
//[{ src: "blob:http://localhost:9999/bc47fd5c-f3eb-4be9-8fea-40444743105e", type: "" }]

player.src()
// "blob:http://localhost:9999/bc47fd5c-f3eb-4be9-8fea-40444743105e"

Specific Changes proposed

  • load method falls back to the src method when vhs tech is used because the specification requirements are met, see mdn load

Appropriate events will be sent to the media element itself as the load process proceeds:

  • If the element is already in the process of loading media, that load process is aborted and the abort event is sent.
  • If the element has already been initialized with media, the emptied event is sent.
  • If resetting the playback position to the beginning of the media actually changes the playback position (that is, it was not already at the beginning), a timeupdate event is sent.
  • Once media has been selected and loading is ready to begin, the loadstart event is delivered.
  • From this point onward, events are sent just like any media load.
  • add test case

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
    • Change has been verified in an actual browser (Chrome, Firefox, IE)
    • Unit Tests updated or fixed
    • Docs/guides updated
    • Example created (starter template on JSBin)
  • Reviewed by Two Core Contributors

@codecov
Copy link

codecov bot commented May 13, 2023

Codecov Report

Merging #8274 (05d02e8) into main (f1558c6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 05d02e8 differs from pull request most recent head 2df6378. Consider uploading reports for the commit 2df6378 to get more accurate results

@@           Coverage Diff           @@
##             main    #8274   +/-   ##
=======================================
  Coverage   82.36%   82.36%           
=======================================
  Files         112      112           
  Lines        7483     7486    +3     
  Branches     1804     1805    +1     
=======================================
+ Hits         6163     6166    +3     
  Misses       1320     1320           
Impacted Files Coverage Δ
src/js/player.js 90.29% <100.00%> (+0.01%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@amtins amtins mentioned this pull request May 14, 2023
7 tasks
@misteroneill misteroneill merged commit 35fad1d into videojs:main May 31, 2023
7 checks passed
@misteroneill misteroneill mentioned this pull request Jun 1, 2023
2 tasks
edirub pushed a commit to edirub/video.js that referenced this pull request Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants