Skip to content

Conversation

@pengbins
Copy link
Contributor

This PR tries to fix #2456

The function parseH265Sei3dRefDisplayInfo tries to find SEI of type 179 and extract 3d ref DisplayInfo from it. But when the SEI type does not match 179, it fails to properly skip the unprocessed SEI payload. Instead, it continues searching for the number 179 within the payload content. This may mistakenly treat payloads of other types SEI as the target type (179), leading to incorrect parsing and eventual through an exception.

The missing else branch is added to skip payload bytes.

@icbaker icbaker self-assigned this May 22, 2025
@icbaker icbaker self-requested a review May 22, 2025 14:36
@icbaker
Copy link
Collaborator

icbaker commented May 22, 2025

Thanks for the fix!

Some questions:

  • Can we use sample_with_sei_.mp4 to add a test for this (I'll do this in a follow-up after merging this PR)?
  • Did you craft it from an existing MP4 sample asset?
    • Do you have an easy command for this?
    • And which sample file did you start with (so I can mark it as correctly branched in our VCS)?

@icbaker
Copy link
Collaborator

icbaker commented May 22, 2025

Oh can you also re-target this PR agains the main branch please?

@icbaker
Copy link
Collaborator

icbaker commented May 22, 2025

Oh can you also re-target this PR agains the main branch please?

Alternatively I'm happy to make the change directly internally and send it for review, if you're happy to get the fix but without merging this PR - let me know :)

@pengbins pengbins changed the base branch from release to main May 23, 2025 01:14
@pengbins pengbins force-pushed the fix_parseH265Sei3dRefDisplayInfo branch from 7062099 to 88fb6b3 Compare May 23, 2025 01:17
@pengbins
Copy link
Contributor Author

Oh can you also re-target this PR agains the main branch please?

done

@pengbins
Copy link
Contributor Author

pengbins commented May 23, 2025

Thanks for the fix!

Some questions:

  • Can we use sample_with_sei_.mp4 to add a test for this (I'll do this in a follow-up after merging this PR)?

Yes, it's OK.

  • Did you craft it from an existing MP4 sample asset?

    • Do you have an easy command for this?
    • And which sample file did you start with (so I can mark it as correctly branched in our VCS)?

I transcoded it from sample_twos_pcm.mp4 using our own HEVC encoder. Some custom data was added to SEI by our encoder.

@icbaker icbaker force-pushed the fix_parseH265Sei3dRefDisplayInfo branch from 88fb6b3 to eb35980 Compare May 23, 2025 10:50
@icbaker
Copy link
Collaborator

icbaker commented May 23, 2025

I'm going to send this for internal review now. You may see some more commits being added as I make changes in response to review feedback. Please refrain from pushing any more substantive changes as it will complicate the internal review - thanks!

@copybara-service copybara-service bot merged commit ad430b8 into androidx:main May 23, 2025
1 check passed
copybara-service bot pushed a commit that referenced this pull request May 23, 2025
This is a follow-up to Issue: #2457 using the test file
provided in Issue: #2456.

PiperOrigin-RevId: 762387987
@androidx androidx locked and limited conversation to collaborators Jul 22, 2025
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.

Unexpected exception loading stream when playing hevc video with SEI

2 participants