fix: video default audio codec fix (attempt 3) #22
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
This causes it to get picked up in the places that it is actually needed to handle issues in audio quality switching.
nsprenkle
requested review from
Copilot,
jansenk,
jristau1984,
michaelroytman and
sundarthapa2u
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the default audio codec configuration for HLS video playback by moving the defaultAudioCodec setting from the HLS video player constructor to the caller. Instead of setting a default value inside the HLSVideo.Player constructor when not provided, the default value "mp4a.40.5" is now explicitly passed as a configuration parameter when creating the HLS video player instance.
- Removed conditional logic for setting
defaultAudioCodecinside the HLS video player constructor - Added explicit
defaultAudioCodec: "mp4a.40.5"parameter when instantiatingHTML5HLSVideo.Player
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| xmodule/js/src/video/02_html5_hls_video.js | Removed conditional logic that sets a default audio codec if not provided |
| xmodule/js/src/video/03_video_player.js | Added explicit defaultAudioCodec parameter with default value when creating HLS video player instance |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
michaelroytman
approved these changes
Oct 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.
Description
I found that setting a default audio codec (
defaultAudioCodec = "mp4a.40.5") helps reduce issues in HLS streams switching bitrates mid-stream.After several attempt to set this config (#19 and #21) I found that I needed to set the config much higher up the initialization chain to actually set that value all the places that are used in the level switching process.
Third time's the charm?
Deadline
ASAP but also I probably won't be able to monitor rollout 🤷