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: Use timeupdate as well as rvfc/raf for cuechanges #7918

Merged
merged 1 commit into from
Sep 9, 2022

Conversation

mister-ben
Copy link
Contributor

@mister-ben mister-ben commented Sep 8, 2022

Description

A couple more edge cases have come up where usng rvfc or raf for more accurate cue changes isn't working., with audio in video els (there's already handling for audio in audio) and background tabs.

Specific Changes proposed

Use the timeupdate event as well as the rvfc and raf callbacks to check cues. This is a bit overkill for "usual" playback but avoids edge cases. If the more preceise callback trigger first the cue will update but the timeupdate event should catch any that were missed, notwithstanding that timeupdate was always somewhat unpredictable.

Should fix #7910 (audio in video els and Samsung being weird) and #7902 (no updates off screen).

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

Will need to be added to next.

@codecov
Copy link

codecov bot commented Sep 8, 2022

Codecov Report

Merging #7918 (325a538) into main (3c70573) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #7918      +/-   ##
==========================================
+ Coverage   80.93%   80.94%   +0.01%     
==========================================
  Files         116      116              
  Lines        7463     7467       +4     
  Branches     1813     1816       +3     
==========================================
+ Hits         6040     6044       +4     
  Misses       1423     1423              
Impacted Files Coverage Δ
src/js/tracks/text-track.js 93.12% <100.00%> (+0.17%) ⬆️

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

@gkatsev gkatsev merged commit 9b81afe into videojs:main Sep 9, 2022
misteroneill pushed a commit that referenced this pull request Nov 22, 2022
Use the timeupdate event as well as the rvfc and raf callbacks to check cues. This is a bit overkill for "usual" playback but avoids edge cases. If the more preceise callback trigger first the cue will update but the timeupdate event should catch any that were missed, notwithstanding that timeupdate was always somewhat unpredictable.

Fixes #7910 (audio in video els and Samsung being weird) and fixes #7902 (no updates off screen).
edirub pushed a commit to edirub/video.js that referenced this pull request Jun 8, 2023
Use the timeupdate event as well as the rvfc and raf callbacks to check cues. This is a bit overkill for "usual" playback but avoids edge cases. If the more preceise callback trigger first the cue will update but the timeupdate event should catch any that were missed, notwithstanding that timeupdate was always somewhat unpredictable.

Fixes videojs#7910 (audio in video els and Samsung being weird) and fixes videojs#7902 (no updates off screen).
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.

In video.js, when the source content type is "audio/mp3", the subtitle of the track is not updated.
2 participants