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 position stops updating after seek on HLS streams #165

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kontrollanten
Copy link
Contributor

Authored by @andreasgangso some years ago.

As far as I remember the first commit solved some general issue where an error was thrown due to "tech.seeking is not defined".

The seeking/scrubbing issue is described in #103.

@coveralls
Copy link

coveralls commented Aug 26, 2023

Coverage Status

coverage: 18.166% (-0.2%) from 18.386%
when pulling 50447c0 on kontrollanten:fix-hls-seek
into 718c832 on silvermine:master.

@kontrollanten
Copy link
Contributor Author

Comment from izkmdz:

Unfortunately, this fix for #103 is causing the seek bar to not be as responsive as before. Also, the delay does not allow for quick mouse clicks/taps to be registered on the seek bar.

@RavWar
Copy link

RavWar commented Sep 11, 2023

I think i was able to fix the issue by adding the check for this.videojsPlayer.scrubbing() in pause method, like this:

   pause: function() {
      // Do not pause while scrubbing or else chromecast will stop timeupdate events
      if (!this.paused() && this._remotePlayer.canPause && !this.videojsPlayer.scrubbing()) {
         this._remotePlayerController.playOrPause();
      }
   }

@kontrollanten
Copy link
Contributor Author

Interesting. Is it necessary to check for !this.paused() and then call this._remotePlayerController.playOrPause()? Sounds like it should be enough to remove the this.paused check and then just call this._remotePlayerController.pause()?

@RavWar
Copy link

RavWar commented Sep 20, 2023

I just left this part as it was before, didn't check, but i think playOrPause is used because there is no pause method
https://developers.google.com/cast/docs/reference/web_sender/cast.framework.RemotePlayerController#playOrPause

@RavWar
Copy link

RavWar commented Dec 9, 2023

In the latest video.js version this fix is not working, scrubbing(true) is not being set on single clicks on seek bar anymore. Not sure how to fix it differently, i've just patched scrubbing(true) back into video.js in SeekBar handleMouseDown function

fixes an issue where scrubbing would cause CURRENT_TIME_CHANGED
to stop firing
90% working seek/scrub functionality with HLS streams. The issue still
remains upon "quick scrubs", when scrubbing for a half a second or so.
@kontrollanten
Copy link
Contributor Author

In the latest video.js version this fix is not working, scrubbing(true) is not being set on single clicks on seek bar anymore. Not sure how to fix it differently, i've just patched scrubbing(true) back into video.js in SeekBar handleMouseDown function

The latest PR should work with latest video.js as well. It's not 100% working and the solution is not perfect though. I've been sitting in days and trying to figure out why chromecast stops sending time_update events upon scrubbing HLS, but can't find the core issue.

Known issues:

  • When doing quick scrubs (just scrubbing half a second or so) it still gets stuck.
  • The UI is still not snappy, since we need some kind of delay. This affects non HLS streams as well.

@izkmdz Should we put the functionality in this PR under a specific tech option since it's not perfect? For HLS users it's still a great step forward compared to how it works today.

Proposal:

const videojsOptions = {
   fluid: true,
   techOrder: [ 'chromecast', 'html5' ],
   plugins: {
      chromecast: {
        enableExperimentalHlsSeekSupport: true,
      },
   },
};

@kontrollanten
Copy link
Contributor Author

@RavWar @ayrtonbrut Can you this PR and see if it solves your issues?

I've created a demo with HLS in #166 which you can use if you'd like to.

@RavWar
Copy link

RavWar commented Apr 4, 2024

With the new PR changes original issue is fixed for me, couldn't thoroughly test if it causes any other problems though

@kontrollanten
Copy link
Contributor Author

With the new PR changes original issue is fixed for me, couldn't thoroughly test if it causes any other problems though

Great, thanks for testing.

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.

3 participants