videojs / video.js

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

Backward seeking issue in Chrome 55 #3839

Closed bitactive closed 7 years ago

bitactive commented 7 years ago

Description

Chrome 55 introduced bug in VideoJS that results in wrong progressbar position after seeking backward. It can be reproduced on latest stable 5.12.6 and master 5.14.1

Steps to reproduce

Open latest Chrome 55 on Windows

  1. Go to http://videojs.com/
  2. Click play on the video player
  3. Quickly seek to position near end (for example 0:40)
  4. After video start playing do another seek to beginning (for example 0:10)

Results

In most cases progressbar does not move to seeked position. Sometimes (depends on connection speed) progressbar move to seeked position, start buffering and go back just after start playing.

Expected

Progressbar move to seeked position.

Actual

Video does seek to selected position, but progres bar does not reflect this status.

Additional Information

Chrome 54 works fine. Bug appear in Chrome 55 on Windows (tested on Win7 & 10). Other html5 players are working fine.

Bug exists on latest stable 5.12.6 and master 5.14.1

RDGHZ commented 7 years ago

I got a temporary solution in version 5.12.6, replacing:

  SeekBar.prototype.handleMouseDown = function handleMouseDown(event) {
    _Slider.prototype.handleMouseDown.call(this, event);

    this.player_.scrubbing(true);

    this.videoWasPlaying = !this.player_.paused();
    this.player_.pause();
  };

For This:

SeekBar.prototype.handleMouseDown = function handleMouseDown(event) {
    _Slider.prototype.handleMouseDown.call(this, event);

    this.player_.scrubbing(true);

  };

Please tell me if this works for you.

(Translated by Google)

bitactive commented 7 years ago

Yes, i can confirm that this patch fixes the issue.

gkatsev commented 7 years ago

Interesting, I can confirm this. Unfortunately, just removing the pause isn't a good solution. The reason is that if you're seeking via scrubbing (if you press and hold the mouse on the progress bar and then move it around), removing the pause causes the video to continue to play if you wait for a second.

I think a potential fix would be to just wrap the two lines that were removed in a setTimeout:

SeekBar.prototype.handleMouseDown = function handleMouseDown(event) {
    _Slider.prototype.handleMouseDown.call(this, event);

    this.player_.scrubbing(true);

    this.setTimeout(function() {
        this.videoWasPlaying = !this.player_.paused();
         this.player_.pause();
    });
};

This seems to work but not sure whether there are any implications for this.

bitactive commented 7 years ago

Indeed, RodrigoMaster7 solution results in tons of requests made by browser during scrubbing. EDIT: It is original version behaviour, but all these requests are cancelled, so it looks like it is working fine.

As far as i am testing gkatsev solution does not work well (it works randomly on larger videos).

gkatsev commented 7 years ago

oops, I forgot a part. Missing a , 0 in the setTimeout. It should be

SeekBar.prototype.handleMouseDown = function handleMouseDown(event) {
    _Slider.prototype.handleMouseDown.call(this, event);

    this.player_.scrubbing(true);

    this.setTimeout(function() {
        this.videoWasPlaying = !this.player_.paused();
         this.player_.pause();
    }, 0);
};
bitactive commented 7 years ago

gkatsev: your last code (with missing ,0) results in about 20% chance that bug will still appear.

EDIT: Only RodrigoMaster7 solution works each time i try on different streams.

gkatsev commented 7 years ago

thanks for checking. I didn't do any thoroughly extensive testing on this yet, it was just a quick thought on what could potentially help. Potentially, if the 0 is changed to a higher value like 50, it could probably be more reliable.

It's also possible that there's a better solution altogether.

bitactive commented 7 years ago

Setting timeout value too high results in stream pause after start playing. Setting too low doesn't fix bug. setTimeout does not look like good solution but balancing between fixing bug and breaking playback.

gkatsev commented 7 years ago

I think I have a potential solution for this that's reliable.

gkatsev commented 7 years ago

Please give it a shot: https://github.com/videojs/video.js/pull/3886

RDGHZ commented 7 years ago

Has this problem not been solved yet?

gkatsev commented 7 years ago

This issue has been solved twice. 5.15.1 has a fix, though, apparently, not complete. And 5.16.0 seems to fix the rest of the issues. It's currently in prerelease but should be stable.

Also, I want to reiterate that this is actually a browser bug that were working around/fixing.