-
Notifications
You must be signed in to change notification settings - Fork 136
feat: add disableAudioTrack API #1715
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
43efc64 to
401158e
Compare
We previously never added the `disableAudioTrack` API for two reasons: 1. It's often much less needed than a `disableTextTrack` API and than a `disableVideoTrack` API (second one for "audio-only") 2. It was unclear what behavior the RxPlayer should have if both audio and video were disabled Since @Florent-Bouisset work on the new `onAudioTracksNotPlayable` / `onVideoTracksNotPlayable` options however, we do have a concrete way to deal with the absence of both audio and video: the `noPlayableTrack` `error` event. So no that there's no real barrier to add that API, I'm adding it. --- This does not seem to really be needed right now by the different applications we work with, I just wanted to add that API now because it facilitates the writing of edge-cases-tests: I especially wanted to add a test for a pretty rare situation where there's no audio selected yet we're fallbacking from all video tracks due to DRM constraints. I'm doing this now because now that advanced DRM integration tests are merged, we can begin to add tests for all those advanced edge cases scenarios to ensure we're keeping them under control.
401158e to
b430e37
Compare
|
✅ Automated performance checks have passed on commit DetailsPerformance tests 1st run outputNo significative change in performance for tests:
|
peaBerberian
added a commit
that referenced
this pull request
Jul 21, 2025
… 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
added a commit
that referenced
this pull request
Jul 21, 2025
… 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
added a commit
that referenced
this pull request
Jul 21, 2025
… 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.
… all the others' one (#1716) * Allow application to reset an audio/video track when fallbacking from 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. * fix: let a time window to switch track when receiving noPlayableTrack before sending an error * simplify code * remove debug setup * fix: don't throw the trackUpdate event on initial track selection * coden review * code review 2 * re-add the comment --------- Co-authored-by: Florent Bouisset <[email protected]>
|
✅ Automated performance checks have passed on commit DetailsPerformance tests 1st run outputNo significative change in performance for tests:
|
Florent-Bouisset
approved these changes
Jul 30, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We previously never added the
disableAudioTrackAPI for two reasons:It's often much less needed than a
disableTextTrackAPI and than adisableVideoTrackAPI (second one for "audio-only")It was unclear what behavior the RxPlayer should have if both audio and video were disabled
Since @Florent-Bouisset work on the new
onAudioTracksNotPlayable/onVideoTracksNotPlayableoptions however, we do have a concrete way to deal with the absence of both audio and video: thenoPlayableTrackevent andNO_AUDIO_VIDEO_TRACKSerror.So now that there's no real barrier to add that API, I'm adding it.
This does not seem to really be needed right now by the different applications we work with, I just wanted to add that API now because it facilitates the writing of edge-cases-tests: I especially wanted to add a test for a pretty rare situation where there's no audio/video selected yet we're fallbacking from all of the other types' (video/audio) tracks due to DRM constraints.
I'm doing this now because now that advanced DRM integration tests are merged, we can begin to add tests for all those advanced edge cases scenarios to ensure we're keeping them under control.