-
Notifications
You must be signed in to change notification settings - Fork 213
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
Conversation
Allows adding members in the future without breaking the ABI.
There was a problem hiding this 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) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
Allows adding members in the future without breaking the ABI.