Add mediaCannotPlay and onTrackPlaybackFailure #356
Conversation
|
Don't know if we should pass the activeTrackIndex or something to onTrackPlaybackFailure. |
| @@ -166,6 +153,10 @@ export class PlayerContextProvider extends Component { | |||
| this.videoHostOccupiedCallbacks = new Map(); | |||
| this.videoHostVacatedCallbacks = new Map(); | |||
|
|
|||
| // bind internal methods | |||
| this.setMediaElementSources = this.setMediaElementSources.bind(this); | |||
There was a problem hiding this comment.
I think setMediaElementSources doesn't need to be bound, unless I missed something.
There was a problem hiding this comment.
setMediaElementSources method uses this, wouldn't that require it to be bound?
There was a problem hiding this comment.
It's not passed anywhere as a callback so it's bound at call time when we call this.setMediaElementSources(...).
There was a problem hiding this comment.
Ahh, yes! Just removed it.
| if (source.type) { | ||
| sourceElement.type = source.type; | ||
| } | ||
| sourceElement.addEventListener('error', this.onTrackPlaybackFailure); |
There was a problem hiding this comment.
I think we should remove the event listeners on the source elements on component unmount, in case someone is holding onto a reference to the media element.
There was a problem hiding this comment.
Wouldn't it be garbage collected as you said in the issue?
Except in old browsers.
There was a problem hiding this comment.
However, doesn't hurt to remove them, check out the last commit.
There was a problem hiding this comment.
Normally yes, but if someone retained an outside ref to the media element then I want to make sure we don't have callbacks belonging to an unmounted component getting called.
| mediaCannotPlay: true | ||
| }); | ||
| if (this.props.onTrackPlaybackFailure) { | ||
| this.props.onTrackPlaybackFailure(event); |
There was a problem hiding this comment.
I think this should be:
this.props.onTrackPlaybackFailure(
this.props.playlist[this.state.activeTrackIndex],
this.state.activeTrackIndex,
event
);
|
Released in v2.0.0-alpha.23. |
Solves #352.
I documented
mediaCannotPlay, but not onTrackPlaybackFailure, since the markdown file for playerContextProvider is empty and the Readme options look outdated.