SYM-22: Support MP3 audio in legacy web embeds#266
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2285ed17a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ? props.parentNode?.stream?.sources[0]?.uri | ||
| : videoMedia?.sources[0]?.uri; | ||
| ? props.parentNode?.stream?.sources?.find((streamSource) => streamSource?.uri)?.uri | ||
| : videoMedia?.sources?.find((videoSource) => videoSource?.uri)?.uri; |
There was a problem hiding this comment.
After this change, hlsMedia/youtubeMedia can match a video because a later entry in sources contains .m3u8 or youtu, but the player URL is still chosen as the first non-empty source URI. In a mixed-source item (for example, MP3 first and HLS second), this silently drops the intended YouTube/HLS preference and can route playback to the wrong format. Use the URI that matched the preferred type (or compute preferred source directly) instead of always taking the first URI.
Useful? React with 👍 / 👎.
nlewis84
left a comment
nlewis84
left a comment
There was a problem hiding this comment.
@redreceipt I spent 25 minutes trying to test this unsuccessfully. I trust your code changes and I'm sure you are going to validate in prod and follow up with another PR if for some reason this doesn't work.
redreceipt
deleted the
sym-22-support-mp3-audio-in-legacy-web-embeds-copy
branch
2 participants
🐛 Issue
Closes SYM-22
Closes APO-7576
Closes APO-7468
✏️ Solution
Legacy embed playback only selected YouTube or HLS video sources, so content items with only an MP3 source rendered no player at all.
This change keeps the existing YouTube/HLS priority but falls back to the first available media source when those formats are absent. That allows MP3-backed legacy embeds to render through the existing player path without changing the current preferred-source behavior.
🔬 To Test
cd packages/web-shared && npx -y yarn@1.22.22 lint.cd web-embeds && npx -y yarn@1.22.22 lint.cd web-embeds && CI=1 npx -y yarn@1.22.22 test --watch=false --runInBand --testPathPattern=VideoPlayer.test.jsand confirm the MP3-onlyVideoPlayertest passes.cd web-embeds && npx -y yarn@1.22.22 build.cd micro-service && npx -y yarn@1.22.22 build.📸 Screenshots