Skip to content

HEVC: initialize Format.codecs (#8393)#8401

Closed
equeim wants to merge 1 commit intogoogle:dev-v2from
equeim:hevc-codecs
Closed

HEVC: initialize Format.codecs (#8393)#8401
equeim wants to merge 1 commit intogoogle:dev-v2from
equeim:hevc-codecs

Conversation

@equeim
Copy link
Contributor

@equeim equeim commented Dec 27, 2020

This will allow ExoPlayer to check if video track's profile and level are supported by decoder when playing progressive media sources.

final int generalProfileSpace = bitArray.readBits(2);
final boolean generalTierFlag = bitArray.readBit();
final int generalProfileIdc = bitArray.readBits(5);
final int generalProfileCompatibilityFlags = bitArray.readBits(32);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@equeim I think the order of bits in the resulting int is reversed compared to what's needed to build the codecs string. Please could you confirm whether that's consistent with your understanding or if I'm missing something?

I will take care of making the change if it's needed when merging this, so please just let me know whether it is correct. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure, actually. MPEG-DASH for DVB specification says that bits must be reversed, but MPEG-4 Part 15 (where codecs string for HEVC is defined officially) doesn't mention it. Although I don't have access to the latest version of MPEG-4 Part 15 yet so I can't check whether it was added later.
It seems that almost all HLS/DASH manifests out there have it reversed though, and that was google/shaka-packager does too (https://github.com/google/shaka-packager/blob/d90cf9a0fd87833f7d9210ffe65d175dcf994c42/packager/media/codecs/hevc_decoder_configuration_record.cc#L133).

This will allow ExoPlayer to check if video track's profile
and level are supported by decoder when playing progressive media sources.
@equeim equeim changed the title Initialize Format.codecs from HEVC SPS NAL unit (#8393) HEVC: initialize Format.codecs (#8393) Jan 5, 2021
@equeim
Copy link
Contributor Author

equeim commented Jan 5, 2021

Updated PR to build codec string from HEVCDecoderConfigurationRecord instead of SPS in case of MP4/Matroska.

@andrewlewis
Copy link
Collaborator

Updated PR to build codec string from HEVCDecoderConfigurationRecord instead of SPS in case of MP4/Matroska.

Are there any particular benefits to doing it this way round? I was going to parse in one place for both HevcConfig and H265Reader but they can't share code if they don't both use ParsableNalUnitBitArray.

@equeim
Copy link
Contributor Author

equeim commented Jan 7, 2021

Not really, aside from the fact that specification mentions HEVCDecoderConfigurationRecord specifically. I just though that code in HevcConfig would be clearer that way.

More interesting, in Apple sample HLS stream with HEVC and MP4 segments (https://devstreaming-cdn.apple.com/videos/streaming/examples/bipbop_adv_example_hevc/master.m3u8), I discovered that information in HEVCDecoderConfigurationRecord and in SPS are not the same. In particular, generalProfileCompatibilityFlags in HEVCDecoderConfigurationRecord seems to have its bit order inverted compared to SPS. I'm not sure how it conforms to MP4 specification (probably doesn't), and I haven't seen other MP4 files like this (and ffmpeg generates MP4 with SPS and HEVCDecoderConfigurationRecord equivalent).

@andrewlewis
Copy link
Collaborator

Thanks for the info. I'll test with that stream to make sure the compatibility flags look right in the output string.

icbaker pushed a commit that referenced this pull request Jan 8, 2021
Imported from GitHub PR #8401

This will allow ExoPlayer to check if video track's profile and level are supported by decoder when playing progressive media sources.
Merge e582fb9 into 1347d57

Issue: #8393
COPYBARA_INTEGRATE_REVIEW=#8401 from equeim:hevc-codecs e582fb9
PiperOrigin-RevId: 350738065
@ojw28
Copy link
Contributor

ojw28 commented Jan 8, 2021

This was merged in ff8c864, although for some reason our tooling didn't update the pull request to reflect this. Closing manually since it's now merged. Thanks!

@ojw28 ojw28 closed this Jan 8, 2021
@andrewlewis
Copy link
Collaborator

Just in case you are trying out ff8c864 on the development branch: I introduced a bug in how the buffer limit is set. I plan to submit a fix for that soon. Sorry for the inconvenience.

icbaker pushed a commit that referenced this pull request Jan 11, 2021
*** Original commit ***

Merge #8401: Initialize Format.codecs from HEVC SPS NAL unit (#8393)

Imported from GitHub PR #8401

This will allow ExoPlayer to check if video track's profile and level are supported by decoder when playing progressive media sources.
Merge e582fb9 into 1347d57

Issue: #8393

***

PiperOrigin-RevId: 350871621
icbaker pushed a commit that referenced this pull request Jan 11, 2021
*** Original commit ***

Rollback of ff8c864

*** Original commit ***

Merge #8401: Initialize Format.codecs from HEVC SPS NAL unit (#8393)

Imported from GitHub PR #8401

This will allow ExoPlayer to check if video track's profile and level are supported by decoder when playing progressive media so...

***

PiperOrigin-RevId: 351139861
@google google locked and limited conversation to collaborators Mar 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants