Skip to content
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

Fix parsing of multiple video codecs when converting from AVC1 to AVCOTI #6533

Conversation

jhonatangcavalcanti
Copy link
Contributor

This PR will...

Fix the parse of multiple value codecs (CODECS="avc1.64001E,avc1.64001f,mp4a.40.2") on convertAVC1ToAVCOTI function.

Before this MR, the codec value was converted from "avc1.64001E,avc1.64001f" to "avc1.fa01fa01" leading to the "no level with compatible codecs found in manifest" error.

Now, it is keeping the "avc1.64001E,avc1.64001f" value.

Why is this Pull Request needed?

It fixes media with multiple video codecs broken on 1.5.x hls.js versions.
Ex.:
#EXT-X-STREAM-INF:BANDWIDTH=1199000,RESOLUTION=854x480,CODECS="avc1.64001E,avc1.64001f,mp4a.40.2",AUDIO="audio",SUBTITLES="subs"

Are there any points in the code the reviewer needs to double check?

Resolves issues:

Related to #5378

Checklist

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • API or design changes are documented in API.md

Copy link
Collaborator

@robwalch robwalch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for identifying this issue and contributing a fix.

src/utils/codecs.ts Outdated Show resolved Hide resolved
src/utils/codecs.ts Outdated Show resolved Hide resolved
src/utils/codecs.ts Show resolved Hide resolved
@robwalch robwalch added this to the 1.5.13 milestone Jul 2, 2024
@jhonatangcavalcanti jhonatangcavalcanti force-pushed the feature/fix-manifest-incompatible-codecs-error branch from dda848d to 274d91e Compare July 3, 2024 00:59
@jhonatangcavalcanti
Copy link
Contributor Author

Hello, @robwalch. Thanks for the fast review.

I made the changes you asked for, can you check again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants