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 the gainMap member of avifImage a pointer. #1801

Merged
merged 2 commits into from
Nov 23, 2023

Conversation

maryla-uc
Copy link
Collaborator

Allows adding members in the future without breaking the ABI.

Allows adding members in the future without breaking the ABI.
Copy link
Collaborator

@vrabaud vrabaud left a comment

Choose a reason for hiding this comment

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

Just wondering if there should be function that checks for gainMap and gainMap->image and returns NULL or the image to avoid doing the double condition every time. Then again, that would be more to maintain so no.

@@ -57,13 +57,13 @@ avifResult PrintMetadataCommand::Run() {
<< decoder->diag.error << ")\n";
return result;
}
if (!decoder->gainMapPresent) {
if (!decoder->gainMapPresent || decoder->image->gainMap == nullptr) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

gainMapPresent coule be true and decoder->image->gainMap nullptr?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In theory no. Changed to an assert.

@@ -1208,8 +1208,10 @@ static avifResult avifValidateGrid(uint32_t gridCols,
const avifImage * bottomRightCell = cellImages[cellCount - 1];
#if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP)
if (validateGainMap) {
firstCell = firstCell->gainMap.image;
bottomRightCell = bottomRightCell->gainMap.image;
assert(firstCell->gainMap && firstCell->gainMap->image);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just wondering if that can never happen and whether AVIF_CHECKRES should be used

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 caller should only call the function with validateGainMap if there is a gain map. I think defensive programming is not useful in this instance.

@maryla-uc maryla-uc merged commit f4948db into AOMediaCodec:main Nov 23, 2023
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