Allow application to reset an audio/video track when fallbacking from all the others' one #1716
Conversation
peaBerberian
added
the
work-in-progress
peaBerberian
force-pushed
the
fix/no-playable-track-timeing
branch
from
036d1aa to
1618249
Compare
… all the others' one Based on #1715 This commit shows an issue that can arise in the very rare scenario where: 1. An application decides to load a content with no audio track (could be also with no video track). 2. We fallback for some reason from all video tracks (or audio tracks if it's video that has been disabled in `1.`) - in the test here this is due to the impossibility to fetch their licenses. 3. We should here receive a `noPlayableTrack` event indicating that no video track is playable, allowing us to re-set our audio choice to still be able to play something **BEFORE** the `NO_AUDIO_VIDEO_TRACKS` error that arises when no video nor audio is selected. However we don't here, we receive directly a `NO_AUDIO_VIDEO_TRACKS` error and stop playback.
peaBerberian
force-pushed
the
fix/no-playable-track-timeing
branch
from
1618249 to
3e44237
Compare
|
✅ Automated performance checks have passed on commit DetailsPerformance tests 1st run outputNo significative change in performance for tests:
|
… before sending an error
|
✅ Automated performance checks have passed on commit DetailsPerformance tests 1st run outputNo significative change in performance for tests:
|
| { tracks: undefined }, | ||
| ); | ||
| noPlayableTrackToTrigger.push({ | ||
| typeInfo.dispatcher?.updateTrack(storedSettings); |
There was a problem hiding this comment.
As we're just after a trigger, shouldn't we:
- Check if it is disposed
- check
hasStoredSettingsChanged(app could change the track on atrackUpdatefor the reason"no-playable-representation"to choose its own fallback instead).
There was a problem hiding this comment.
We should still check that the previous trackUpdate did not lead to a track change no?
There was a problem hiding this comment.
I have added that check
|
✅ Automated performance checks have passed on commit DetailsPerformance tests 1st run outputNo significative change in performance for tests:
|
|
✅ Automated performance checks have passed on commit DetailsPerformance tests 1st run outputNo significative change in performance for tests:
|
| return; | ||
| } | ||
| if (trackType === "text") { | ||
| // we don't care for text fallback |
There was a problem hiding this comment.
Now we care if it is the initial one no?
And shouldn't we care, by setting it to null, in the unlikely case of encrypted subtitles
There was a problem hiding this comment.
handleMissingOrUnplayableTrack is never called with trackType "text" for initial selection, because we don't select text track by default, excepted if it has the "forceSubtitles" property.
But It's not done within this function.
Should I change anything ?
There was a problem hiding this comment.
I feel like this isn't something that's very clear from the scope of this function. I don't see why we would handle text as other types beside the NO_AUDIO_VIDEO thing from the current context. No? What do you think?
There was a problem hiding this comment.
Yes you are right, it was not clear from this function's perspective, I have made an update
|
✅ Automated performance checks have passed on commit DetailsPerformance tests 1st run outputNo significative change in performance for tests:
|
|
✅ Automated performance checks have passed on commit DetailsPerformance tests 1st run outputNo significative change in performance for tests:
|
| globals: false, | ||
| reporters: "dot", | ||
| include: [ | ||
| // integration tests |
There was a problem hiding this comment.
No need to remove that line
|
✅ Automated performance checks have passed on commit DetailsPerformance tests 1st run outputNo significative change in performance for tests:
|
peaBerberian
removed
the
work-in-progress
Based on #1715
This commit shows an issue that can arise in the very rare scenario where:
An application decides to load a content with no audio track (could be also with no video track).
We fallback for some reason from all video tracks (or audio tracks if it's video that has been disabled in
1.) - in the test here this is due to the impossibility to fetch their licenses.We should here receive a
noPlayableTrackevent indicating that no video track is playable, allowing us to re-set our audio choice to still be able to play something BEFORE theNO_AUDIO_VIDEO_TRACKSerror that arises when no video nor audio is selected.However we don't here, we receive directly a
NO_AUDIO_VIDEO_TRACKSerror and stop playback.