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

Make Opus sample entry lowercase #4526

Merged
merged 1 commit into from
Oct 25, 2022

Conversation

stolen
Copy link
Contributor

@stolen stolen commented Jan 26, 2022

This commit violates RFC6381 + MP4RA ISO code list for compatibility with browsers and other players.

This PR will make hls.js accept opus codec

Chrome and Firefox accept opus codec only as lowercase despite ISO code has a capital O:

No player plays an M3U8 with CODECS="Opus", but some players do play an M3U8 with CODECS="opus".
codecs="opus" is also playable in DASH.

Why is this Pull Request needed?

Opus is modern open audio codec already used in Internet communications and it has a potential use in video streaming.
Browsers (I checked Firefox and Chrome) violate some specs and accept only lowercase codec for Opus:

> MediaSource.isTypeSupported('audio/mp4; codecs="opus"')
true
> MediaSource.isTypeSupported('audio/mp4; codecs="Opus"')
false

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

no

Resolves issues:

#4527

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

@stale stale bot added the Stale label Apr 16, 2022
@stale stale bot closed this Apr 19, 2022
@robwalch robwalch added Bug and removed Stale labels Jul 14, 2022
@robwalch robwalch reopened this Jul 14, 2022
@robwalch robwalch requested a review from gkatsev July 14, 2022 23:25
@robwalch robwalch added this to Top priorities in Release Planning and Backlog via automation Jul 14, 2022
@robwalch robwalch moved this from Top priorities to Backlog in Release Planning and Backlog Jul 14, 2022
@robwalch robwalch added this to the 1.2.5 milestone Oct 6, 2022
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.

This commit violates RFC6381 + MP4RA ISO code list for compatibility with browsers and other players.

Chrome and Firefox accept `opus` codec only as lowercase despite ISO code has a capital O:
```
> MediaSource.isTypeSupported('audio/mp4; codecs="opus"')
true
> MediaSource.isTypeSupported('audio/mp4; codecs="Opus"')
false
```
No player plays an M3U8 with `CODECS="Opus"`, but some players do play an M3U8 with `CODECS="opus"`.
`codecs="opus"` is also playable in DASH.
@stolen
Copy link
Contributor Author

stolen commented Oct 12, 2022

@robwalch I don't understand what "Not a Contribution" means for me.
Should I add anything in the commit message?

@gkatsev
Copy link
Member

gkatsev commented Oct 12, 2022

@stolen you can ignore that it's an apache licensing related thing.

@stolen stolen requested review from robwalch and removed request for gkatsev October 12, 2022 17:28
@video-dev video-dev deleted a comment from stale bot Oct 12, 2022
@video-dev video-dev deleted a comment from stale bot Oct 12, 2022
@robwalch robwalch merged commit 22e195d into video-dev:master Oct 25, 2022
robwalch pushed a commit that referenced this pull request Oct 25, 2022
This commit violates RFC6381 + MP4RA ISO code list for compatibility with browsers and other players.

Chrome and Firefox accept `opus` codec only as lowercase despite ISO code has a capital O:
```
> MediaSource.isTypeSupported('audio/mp4; codecs="opus"')
true
> MediaSource.isTypeSupported('audio/mp4; codecs="Opus"')
false
```
No player plays an M3U8 with `CODECS="Opus"`, but some players do play an M3U8 with `CODECS="opus"`.
`codecs="opus"` is also playable in DASH.
littlespex pushed a commit to cbsinteractive/hls.js that referenced this pull request Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants