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 failing assert if there are several 'dimg' boxes for the same item. #1847

Merged
merged 1 commit into from
Dec 4, 2023

Conversation

maryla-uc
Copy link
Collaborator

This is not allowed by the HEIF specification.

Also reject images where the base image and the gain map are the same image. This is not expecitly forbidden by existing draft specs but makes little sense and is not supported by the current code.

Fixes b/314637273

This is not allowed by the HEIF specification.

Also reject images where the base image and the gain map are the same image.
This is not expecitly forbidden by existing draft specs but makes little sense
and is not supported by the current code.

Fixes b/314637273
Comment on lines +4457 to +4459
// Even with numDimgItemIDs == 2, one of the ids could be 0 if there are duplicate entries in the 'dimg' box.
if (numDimgItemIDs != 2 || dimgItemIDs[0] == 0 || dimgItemIDs[1] == 0) {
avifDiagnosticsPrintf(data->diag, "box[dimg] for 'tmap' item %d must have exactly 2 entries with distinct ids", item->id);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would the dimgItemIDs[otherItem->dimgIdx] == 0 assertion fail if that were to happen? Should it be removed or replaced by a proper error instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, the assert can fail only if there are mutiple dimg boxes for the same item, which we now reject earlier.

@maryla-uc maryla-uc merged commit c2fb27e into AOMediaCodec:main Dec 4, 2023
17 checks passed
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