Combine the flash tech and HLS tech's currentTime methods

[heff]

As part of scoping out the source handler interface I'm trying to reduce the need to override existing tech methods.

The HLS tech's currentTime and Flash's currentTime are pretty similar, and it's not clear what the need for the differences is. Does anyone know the background? Could features in the HLS version be added to the Flash version?

HLS

videojs.Hls.prototype.currentTime = function() {
  if (this.lastSeekedTime_) {
    return this.lastSeekedTime_;
  }
  // currentTime is zero while the tech is initializing
  if (!this.el() || !this.el().vjs_getProperty) {
    return 0;
  }
  return this.el().vjs_getProperty('currentTime');
};

Flash

vjs.Flash.prototype['currentTime'] = function(time){
  // when seeking make the reported time keep up with the requested time
  // by reading the time we're seeking to
  if (this.seeking()) {
    return this.lastSeekTarget_ || 0;
  }
  return this.el_.vjs_getProperty('currentTime');
};

[gkatsev]

It's possible that we don't need to override the currentTime function in HLS. We definitely need to override the setCurrentTime function.

Also, you accidentally linked to currentTime for HLS and setCurrentTime for flash.

[heff]

Bah! Thanks. Updated it. Yeah there's nothing that jumps out to me there that has to be different. The lastSeekTarget_ vs lastSeekedTime_ is interesting. Unless someone else (@dmlap) knows otherwise I'll probably move forward assuming we can combine these.

SetCurrentTime was going to be my next question so I guess I don't need to open a different issue for it. Could we just listen for the seeking event from the tech, get the currentTime from the tech (which is updated to the seek target), and use that to update the buffer? That at least appears to be the native workflow when using MSE with a raw video element.

[gkatsev]

Listening to seeking may work.

[heff]

Cool, I'll try to validate that. I'm guessing the swf will need to be updated to handle (or just ignore) seeks in the appendBytes mode.

[dmlap]

There was a reason for overriding currentTime() at some point but that may have disappeared in video.js updates since then. I can not recall what it might have been right now.

[dmlap]

I believe it was important that we had access to the seek target so that we could update the NetStream with the new base time offset once seeking had completed. The seek-related events we tried only had access to the current playback time, not the time that the seek was headed to.

[heff]

The flash tech should be reporting the seek target for currentTime when the seeking even is fired.
http://jsbin.com/vapoha/1

I don't know if this was before or after you were testing it for HLS, but looks like it was updated in January. videojs/video.js@dd8c25e

The video element appears to do the same, at least with MSE.

[dmlap]

Fixed in the video.js 5.0 / development branch.