cladera / videojs-offset

VideoJS plugin for play a segment of a video.
MIT License
64 stars 43 forks source link

Fails on replay: video doesn't stop at specified end time. #34

Closed roypardi closed 5 years ago

roypardi commented 6 years ago

This works on first play: video starts and stops at the specified times (there is a slight flash of the very first video frame but that is minor) and, with restart_beginning=true, the video goes back to the specified start time.

If the "replay" button is pressed, the video will play from the specified start time but then keep playing past the specified end time and play the rest of the video.

With restart_beginning=false, after playing the specified section and stopping, if the "replay" button is pressed, the video starts playing from the specified end time.

This seems like a bug: the video should be constrained to play only the range specified.

Replay button

screen shot 2018-02-26 at 5 15 59 pm

My set-up code is below. Let me know if I can provide more info or test anything out on my end.

function showVideo(vidData) {

    $.featherlight('#video-lightbox', {
        afterContent: function (event) {
            $('.featherlight-inner').append(createPlayerDOM(vidData));
        },
        afterOpen: function (event) {
            player = videojs('#video-player', {
                "controls": true, "autoplay": false,
                width: 640, height: 480,
                poster: vidData.poster,
                autoplay: vidData.autoPlay,
                "preload": "meta_data"
            }, function () {

            });

            if (vidData.start != undefined){
                player.offset({
                    start: vidData.start,
                    end: vidData.end,
                    restart_beginning: vidData.restart_beginning
                  });
            }

            if (vidData.autoPlay == true) {
                player.requestFullscreen();
            }
        },
        beforeClose: function (event) {
            player.pause();
            player.dispose();
            player = null;
        },
        afterClose: function (event) {
            // nada yet
        }
    });
}
matthieu-fesselier commented 6 years ago

I had the same issue. I solved this by removing the line: this.off('timeupdate', onPlayerTimeUpdate); (line 25, src/plugin.js) It seems to work

cladera commented 6 years ago

@roypardi thank you for reporting the issue. I'll take a look asp.

roypardi commented 6 years ago

@matthieu-fesselier –

Thanks for the tip! In brief testing just now your suggestion works if restart_beginning=true but fails if restart_beginning=false. In the latter case it seems to play just a last 1/2 second or so of the video rather then going back to start. So a rewind issue?

boris-petrov commented 5 years ago

@cladera - is there any progress on this? :) We're stuck on 2.0.0-beta.0 because of this (as 2.0.0-beta.1 and 2.0.0-beta.2 are broken).

tilttran commented 5 years ago

@roypardi Did you fix the issue? now I have them same issue like u? Can u help me? Thanks...

roypardi commented 5 years ago

@tilttran – No. I looked through the code but didn't see an obvious issue and didn't have the time to dive into it further. It is something simple – but requires a bit more understanding of the videojs API than I have right now.

tilttran commented 5 years ago

@roypardi I found the problem, it's in file plugin, player.one('play', function () {} => player.on. Hope this help.

Sighter commented 5 years ago

Hey ho,

I am attaching you a workaround for the issue. The idea is to rebind the timeupdate handler whenever the end of the video is detected. This is done in the same fashion like it is normally done in the code, by waiting for the play event once. @matthieu-fesselier suggested to just leave the handler. This leads to weird stuttering of the video (at least for me). The following code is part of the videojs.offset.js file. U have to replace the following function.

/**
   * Checks whether the clip should be ended.
   *
   * @function onPlayerTimeUpdate
   *
   */

  var onPlayerTimeUpdate = function onPlayerTimeUpdate() {
    var curr = this.currentTime();

    if (curr < 0) {
      this.currentTime(0);
      this.play();
    }

    if (this._offsetEnd > 0 && curr > this._offsetEnd - this._offsetStart) {
      this.off('timeupdate', onPlayerTimeUpdate);
      this.pause();
      this.trigger('ended');

      if (!this._restartBeginning) {
        this.currentTime(this._offsetEnd - this._offsetStart);
      } else {
        // rebind timeupdate handler on play
        this.one('play', function () {
          this.on('timeupdate', onPlayerTimeUpdate);
        });
        this.trigger('loadstart');
        this.currentTime(0);
      }
    }
  };

Note: The handler is only rebound as long _restartBeginning is true. Its probably also needed when its false.

Cheers Sascha

boris-petrov commented 5 years ago

@Sighter - do you mind opening a PR for that? :)

cladera commented 5 years ago

Thank you @Sighter

Yeah, I think your hack will work regardless the _restartBeginning flag is enabled or not so I'd rebind the event anyways.

As @boris-petrov suggested, feel free to open a PR.

Regards

Sighter commented 5 years ago

@cladera Thx for your answer. If you tell me, why exactly it is a hack, then I can try to make a clean solution :) I will make a pr in the next days.

cladera commented 5 years ago

Yeah sorry... :) I meant fix, solution, workaround... I think the solution is clean enough.

Sighter commented 5 years ago

Allrighty ! :+1:

BTW.: Nice Bird-Set!

boris-petrov commented 5 years ago

@cladera - could you take a look at this please and merge some of the proposed fixes?