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

Add codec string parsing for h264, h265, and mp4a #4996

Merged

Conversation

uvjustin
Copy link
Contributor

This PR will...

Add codec string parsing for avc, hevc, and mp4a.

Why is this Pull Request needed?

When testing the new HEVC playback on using hls.js on Chrome 107 for Windows, I noticed that some playlists were failing. These playlists have been working fine with hls.js on MS Edge on the same machine for quite some time, and they continue to work there. They also previously worked fine using hls.js on Chrome 104 for Windows also on the same machine (with the PlatformHEVCDecoderSupport flag set). Additionally, the playlists work fine on MacOS and on Android Webview 107 using hls.js.
After poking around a bit it seems that this regression seems to be due to a change in Chrome's MSE (of which I know very little) - it looks like the default hvc1.1.c.L120.90 codec string hls.js uses for HEVC is getting rejected.

To reproduce, try playing https://devstreaming-cdn.apple.com/videos/streaming/examples/bipbop_adv_example_hevc/v10/prog_index.m3u8 (the specific variant playlist and not the master playlist) with hls.js in Chrome 107 on Windows. It should return a Buffer add codec error for video/mp4;codecs=hvc1.1.c.L120.90:Failed to execute 'addSourceBuffer' on 'MediaSource': The type provided ('video/mp4;codecs=hvc1.1.c.L120.90') is unsupported. error. (I reproduced this on two Windows 10 machines both with Intel graphics.)

This PR adds some simple codec string parsing for AVC, HEVC, and mp4a. The code is essentially translated from code we have been using for over 2 years at https://github.com/home-assistant/core/blob/dev/homeassistant/components/stream/fmp4utils.py (thanks to @hunterjm). I haven't tested all the strings that get generated by this PR, but it does seem to resolve the issue I noted above.

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

Resolves issues:

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

@robwalch robwalch self-requested a review October 29, 2022 02:36
@StaZhu
Copy link
Contributor

StaZhu commented Oct 29, 2022

hvc1.1.c.L120.90 is not a correct codec type. The letter c which identity the so called general_profile_compatibility_flag causes chrome internally think it is referring to hevc main still-picture profile not main profile.

On Windows, we never support hevc main still-pciture, because dxva doesnt support such profile, the reason why its not got rejected on Mac or Android is because macOS/Android do support hevc main still-picture.

So if you are referring to hevc main profile, then you should use hvc1.1.6.L120.90 instead as the default codec value.

@uvjustin
Copy link
Contributor Author

hvc1.1.c.L120.90 is not a correct codec type. The letter c which identity the so called general_profile_compatibility_flag causes chrome internally think it is referring to hevc main still-picture profile not main profile.

On Windows, we never support hevc main still-pciture, because dxva doesnt support such profile, the reason why its not got rejected on Mac or Android is because macOS/Android do support hevc main still-picture.

So if you are referring to hevc main profile, then you should use hvc1.1.6.L120.90 instead as the default codec value.

Thanks for the detailed information @StaZhu . I'm guessing Edge for Windows and version 104 of Chrome somehow bypassed the profile check.
Your information suggests that the default profile here:

return 'hvc1.1.c.L120.90';
should be changed to hvc1.1.6.L120.90, but that should probably be in a separate PR.

@StaZhu
Copy link
Contributor

StaZhu commented Oct 29, 2022

hvc1.1.c.L120.90 is not a correct codec type. The letter c which identity the so called general_profile_compatibility_flag causes chrome internally think it is referring to hevc main still-picture profile not main profile.
On Windows, we never support hevc main still-pciture, because dxva doesnt support such profile, the reason why its not got rejected on Mac or Android is because macOS/Android do support hevc main still-picture.
So if you are referring to hevc main profile, then you should use hvc1.1.6.L120.90 instead as the default codec value.

Thanks for the detailed information @StaZhu . I'm guessing Edge for Windows and version 104 of Chrome somehow bypassed the profile check. Your information suggests that the default profile here:

return 'hvc1.1.c.L120.90';

should be changed to hvc1.1.6.L120.90, but that should probably be in a separate PR.

Edge do “support” hevc main still-picture, you can open edge://gpu and check what profile is supported by the browser.

As for chrome 104, I don't think it will return “supported” since we never change the logic after chrome 104, but anyway i agree that you should submit another commit to do the fix.

@uvjustin
Copy link
Contributor Author

As for chrome 104, I don't think it will return “supported” since we never change the logic after chrome 104, but anyway i agree that you should submit another commit to do the fix.

I was pretty sure I had it working on 104, but I don't regularly use Chrome so I uninstalled that version and am unable to find an old version to test on. It's a moot point anyway.

I've just added the changes to the default to this PR since it is a minimal change and keeping them together facilitates some changes to the comments.

@robwalch robwalch added this to the 1.5.0 milestone Nov 15, 2022
@robwalch robwalch mentioned this pull request Nov 15, 2022
3 tasks
@itsjamie
Copy link
Collaborator

@uvjustin can you please add the Home Assistant Apache 2.0 license to the file header and include a description of which part is derived in the header?

@robwalch
Copy link
Collaborator

robwalch commented May 8, 2023

As part os this issue we'll have to make sure that variants with CODECS using hvc1.1.c... rather than hvc1.1.6... are checked for playback support correctly - either by using the media parsed codec or less preferably by replacing the CODECS attribute still-picture profile value with main profile codec string. Media parsed strings should be used as-is and not replaced.

#4996 (comment)
#5418 (comment)

@robwalch robwalch changed the base branch from master to feature/mp4-codec-parsing May 30, 2023 23:46
@robwalch robwalch merged commit 934df77 into video-dev:feature/mp4-codec-parsing May 30, 2023
@robwalch
Copy link
Collaborator

robwalch commented May 31, 2023

Merged into https://github.com/video-dev/hls.js/compare/feature/mp4-codec-parsing to resolve conflicts with #5024, and for further changes.

robwalch pushed a commit that referenced this pull request May 31, 2023
* Add codec string parsing for h264, h265, and mp4a

* Update default HEVC codec string

* Update comments
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.

hls HEVC fmp4 not play in chrome but plays in edge
4 participants