Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changing source does not clear tracks #3000

Closed
nickygerritsen opened this issue Jan 13, 2016 · 7 comments
Closed

Changing source does not clear tracks #3000

nickygerritsen opened this issue Jan 13, 2016 · 7 comments

Comments

@nickygerritsen
Copy link
Contributor

It seems that if I dynamically change the source of a video.js instance, it does not clear the text tracks.

If on an HLS-supported browser, this can easily be seen by doing the following javascript on videojs.com:

var hls = "http://media.streamonecloud.net/hls/account=6xINlI/item=HZ8kWupWhRqR/HZ8kWupWhRqR.m3u8";
var player = videojs.getPlayers()["preview-player"];
player.src(hls);
setTimeout(function() {
    player.src(hls);
    player.play();
}, 1000);

If I manually call player.textTracks().removeTrack_ for all tracks it seems to work, but that seems like a private method.

This seems to be the case for both native and emulated text tracks as far as I could find.

@gkatsev
Copy link
Member

gkatsev commented Jan 13, 2016

Happy issue 3000.
This is just for the m3u8/native text tracks?
Also, we don't clear out the current text tracks because the browser doesn't do that. Though, it's also partly because natively, html5 doesn't quite understand having multi-video videos.

@nickygerritsen
Copy link
Contributor Author

I haven't tried with non-m3u8 text tracks yet. I did try with non-native and that does the same.

What do you mean with that html5 doesn't quite understand multi-video videos?

Anyway, if we need to clear tracks manually that's not a problem, but currently the function is private. Maybe we should add a player.clearTextTracks()?

p.s. @ 3000 issues: party time! 🎂

@gkatsev
Copy link
Member

gkatsev commented Jan 13, 2016

By multi-video videos, I mean a "video"/player that plays more than a single video throughout the entire lifetime of the video/player.

If it's just hls, specifically contrib-hls, then it's probably something that we need to do there assuming that safari handles this cleanup natively as well.

As for remove track, we have a player.removeRemoteTextTrack method which is used for removing tracks that are added via the <track> element or via player.addRemoteTextTrack. player.textTracks().removeTrack_ is private because the native implementation of tracks in html don't have it because you shouldn't be able to remove in-band tracks because you don't know if the vendor is still going to be writing to it, so, it may break if a user removes it too early.

@nickygerritsen
Copy link
Contributor Author

I was using Safari on Mac, so I didn't use contrib-hls at all. So I guess it is just in the html5 tech.

Anyway, we use the same video element for multiple videos a lot and we didn't see any problems yet. We tested in IE9+, Safari, Firefox, Chrome and mobile browsers. It seems to work fine until now. Only that the tracks are not removed as described.

@gkatsev
Copy link
Member

gkatsev commented Jan 13, 2016

Ah, I see. It's possibly a bug then that we don't clear out the native tracks properly from our emulated text track list.

@nickygerritsen
Copy link
Contributor Author

Ah need any help? I can try to make a PR if you provide some suggestions :)

@gkatsev
Copy link
Member

gkatsev commented Jan 13, 2016

Always :)
So, the way it works now is that we're always using the emulated text track list for player.textTracks(). This is to provide an easier transition between techs without breaking everything.
In html5, we've started listening to add and remove track events and in proxyNativeTextTracks_.
I would've hoped that safari would trigger removetrack on that track but seems like it may not be the case. In which case, we would want to go through the list of native tracks (perhaps, particularly those that have the inbandmetadatatrack option set and clear them out from the list if they're no longer there. This should probably be done on loadstart or something like that.
This is also tangentially related to this PR: #2957
Hope that helps :)

nickygerritsen pushed a commit to StreamOneNL/video.js that referenced this issue May 4, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants