Add beforesourcechange and sourcechange events

[misteroneill]

Description

This adds two new events which are triggered by the Player#src() method.

• "beforesourcechange" fires before a playable source is set on the player.
• "sourcechange" fires after the playable source has been set on the player (including after any tech-readiness and autoplay/preload behavior is triggered).

These are non-standard events, but I'd like to open discussion on adding them. They have a lot of practical application as we've seen at Brightcove and other companies where core contributors work. This may be ultimately more appropriate in a separate project - in order to keep the core more pure, but modifying the Player prototype seems out of the scope of plugins to me. An alternative might be coming up with some kind of flag that notes an event as non-standard.

Discuss!

Requirements Checklist

• Feature implemented
  • Unit Tests updated
  • Docs/guides updated
  • Example created (starter template on JSBin)
• Reviewed by Two Core Contributors

[gkatsev]

You forgot to mention #2382

[gkatsev]

I was going to ask if this should be triggered on line 1888 (and I do think that it probably should) but given how recursive src is, we probably don't want to move this up.

[misteroneill]

My thought was that we'd only call it when we are changing to a playable source because that's what people will care about in most cases, I think.

[gkatsev]

@nickygerritsen would love to get your input on this given you opened #2382.

[misteroneill]

Also, I'd love to get some @videojs/core-committers thoughts on the notion of firing non-standard events like this. And whether we should namespace them to make it crystal clear they're not standard.

[gkatsev]

Oh, cool, totally forgot that you can pass a dataobject as a second param to trigger.

[dmlap]

Would you mind elaborating on some use-cases? I think it would be easier to evaluate if we were talking about specific examples where the native events aren't sufficient.

[heff]

Don't forget about #1194! Some good discussion between @dmlap and I around this.

I'd be interested to know specifically how sourcechange would differ from loadstart in this case, and what would be the added benefits. If they are different enough, we should make sure there's a strict order of events between sourcechange and loadstart. It could get really confusing if they fire in different orders, and by the description it sounds like there's some edge cases where that could happen (without looking too deeply into it).

If they turn out to be not different enough, we could also consider just adding a beforeloadstart, which would be more of piggybacking on native events than creating new ones.

[misteroneill]

The problem with loadstart is that it is not reliable.

Our existing analytics code attempted to shoehorn it and ended up with bugs and unintelligible event spaghetti. Different browsers behave differently - for example, in IE on Windows 7, play fires before loadstart in some cases. In Windows 8.1/10, the order can be play, playing, and then loadstart. This is particularly noticeable when used in conjunction with something like brightcove/videojs-playlist, which, essentially, does this:

player.src(...);
player.play();

These issues mean we cannot reliably detect when a new video has started without doing something like polling currentSrc, which is a much uglier solution in my mind than adding a non-native event.

I would be agreeable to the idea of making it beforeloadstart and removing the sourcechange, though. We don't have a need for sourcechange at the moment, though @nickygerritsen apparently did.

[gkatsev]

Maybe for videojs-playlist, we should have play be called on a setTimeout(0)?

Also, related to #1194, @incompl was thinking that maybe it's time for videojs to make it's own set of events (based on the native events but maybe with different names) that have some guarantees. Like, a version of loadstart that will always be the first event you will see for a given source, or a playing event can never happen before a firstcanplay.

[misteroneill]

I'm obviously a vote in favor of something like that since that's basically what I'm proposing here. 😄 And when considering it, also consider the idea of namespacing non-native/standard events. Could be something like 'vjs:custom-event'.

[dmlap]

@misteroneill are the browser behaviors you're seeing not compliant with the standard? If that's the case, I think it's reasonable to normalize their behavior in video.js (and maybe that gets rid of the need for new events).

[misteroneill]

As we discussed in person, the standard doesn't seem to give us any recommendation about ordering of events like loadstart, play, and playing.

I've been spending the morning observing behaviors in Chrome with different combinations of preload, autoplay, playlists, and video tech.

HTML5, thankfully, is fairly consistent. It follows the loadstart -> play -> playing pattern for all combinations except preload: "none", where it's play -> loadstart -> playing in most cases.

Flash is less consistent. In fact, with autoplay: true and playlists, we get patterns like:

play
playing
play
playing
play
playing
loadstart
play
playing

Also, I want to be clear that I don't want to merge this just to solve some problems we're having that, in some cases, are caused by external projects (SWF, playlists, etc). I'm open to all sorts of ideas - like moving it into a separate project that modifies video.js with experimental features or something similar.

[misteroneill]

Thought I'd note the logic we're now using internally to detect source changes: on loadstart, canplay, and playing we look for a new source and only once all 3 events have triggered, we can consider the source changed. The list previously included play, but IE9 sometimes fires play before currentSrc() returns a new value.

Maybe this will help someone else struggling with similar issues.

[gkatsev]

Closing again for now. We have https://github.com/brightcove/videojs-per-source-behaviors and we will potentially do some more work around this soon.