videojs / video.js

Video.js - open source HTML5 video player
https://videojs.com
Other
38.11k stars 7.46k forks source link

random order of audio tracks in safari #5768

Open 0caspy opened 5 years ago

0caspy commented 5 years ago

Description

When using HLS variant playlist with audio renditions, in Safari we have audio tracks menu with randomised (every time) order of tracks.

Steps to reproduce

Explain in detail the exact steps necessary to reproduce the issue.

  1. Create HLS playlist with multiple audio renditions.
  2. Open player in Safari, start playback
  3. Click AudioTrack button

Results

Expected

Audio tracks sorted in order they appear in playlist.

Actual

Audio tracks sorted random. Each time you reload player, you will get new random order.

Error output

no errors

Additional Information

Please include any additional information necessary here. Including the following:

versions

videojs

videojs 7.5.0

browsers

Safari 12.0.3 (also several previous versions of safari)

OSes

macOS 10.14.3

plugins

no plugins

welcome[bot] commented 5 years ago

👋 Thanks for opening your first issue here! 👋

If you're reporting a 🐞 bug, please make sure you include steps to reproduce it. We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can. To help make it easier for us to investigate your issue, please follow the contributing guidelines.

0caspy commented 5 years ago

Also, in 'console.log(player)' we can see that 'id:' of track has the same order as in playlist. but track selection menu has another order. the same order as this array is presented in web inspector.

tracks_: Array (8) 0 AudioTrack {id: "0", kind: "main", label: "Russian", language: "ru", enabled: false, …} 1 AudioTrack {id: "4", kind: "alternative", label: "Arabic", language: "ar", enabled: false, …} 2 AudioTrack {id: "2", kind: "alternative", label: "French", language: "fr", enabled: false, …} 3 AudioTrack {id: "3", kind: "alternative", label: "Spanish", language: "es", enabled: false, …} 4 AudioTrack {id: "6", kind: "alternative", label: "Chinese", language: "zh", enabled: false, …} 5 AudioTrack {id: "7", kind: "alternative", label: "German", language: "de", enabled: false, …} 6 AudioTrack {id: "1", kind: "alternative", label: "English", language: "en", enabled: true, …} 7 AudioTrack {id: "5", kind: "alternative", label: "Japanese", language: "ja", enabled: false, …}

gkatsev commented 5 years ago

I can reproduce it with bipbop: https://videojs.github.io/http-streaming/, I'm not really sure why it's loaded like that.

0caspy commented 5 years ago

seems like due to native HLS for Safari. if there is a way to resort MenuItem's in order of 'id' value - this can solve a problem.

gkatsev commented 5 years ago

well, looking at the audiotracklist of the video element in safari, it seems to be ordered correctly. So, we'll want to investigate where the discrepancy is.

0caspy commented 5 years ago

If you mean clear html5 video element in Safari -- it always sort langs alphabetically, not in the way they appear in m3u8.

bc-paul commented 5 years ago

The order of the AudioTrackList is based on the sequence of addtrack events.

proxyNativeTracksForType_

The random order is the result of Safari randomly firing the addtrack events when compared to how the tracks are present in the manifest.

Codepens: videoEl video.js

I'm not sure if there's an HLS spec that requires a specific order but, best practice seems to be to order them alphabetically which leads me to think the expected behavior here should be to order the audio tracks alphabetically.

@0caspy I see that as well. Safari always sorts alphabetically and not by how they appear in the manifest.

0caspy commented 5 years ago

Greetings! is there are any updates on this issue? unfortunately i'm too lame in js to fix this :(

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

0caspy commented 5 years ago

hi! would not like to lost this ticket due timeout. very ugly bug for safari

gkatsev commented 5 years ago

While, it would be nice to make it consistent, we don't consider this a huge bug. When dealing with text tracks, it's always best to search for the track you want rather than assuming a specific index is the one you want.

0caspy commented 5 years ago

sure, not a huge bug. just only unexpectedly/surprisingly that tracks (audio) sorted different way each time you reload player :)

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

0caspy commented 5 years ago

hi! still hope for resolution

gkatsev commented 5 years ago

This makes sense to fix but unfortunately, it's unlikely that we'll get to it any time soon. If you're interested in helping out, we can point you at the places where changes would potentially go.

innomanik commented 3 years ago

@gkatsev, Please point me, I will see if I can try to take a look at this issue.

gkatsev commented 3 years ago

Might be a bit tricky because there's some abstraction in the code. But if you're willing to try, that would be great! In the Html5 tech, we have a proxyNativeTracks method, which runs proxyNativeTracksForType_ for the three types: audio, video, and text. https://github.com/videojs/video.js/blob/a8a5e02cbaab766698b72e5078dc0396fb2a1c24/src/js/tech/html5.js#L350-L354

Inside proxyNativeTracksForType_ we have an addtrack listener, which when triggered, we add the new track to techTracks https://github.com/videojs/video.js/blob/a8a5e02cbaab766698b72e5078dc0396fb2a1c24/src/js/tech/html5.js#L300-L302

Internally, in techTracks, we store things in an array techTracks.tracks_. Potentially, we can call sort on that array at some point. Not sure whether it's good to call it on every addtrack event, since if you have a lot of audio tracks, it could get really slow. Maybe we have a debounced operation to sort? That way, if we get a lot of addtrack events we only sort once it's settled.