-
Notifications
You must be signed in to change notification settings - Fork 208
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
Add avifIsAlpha() to internal.h #2057
Conversation
src/write.c
Outdated
@@ -2272,15 +2280,16 @@ static avifResult avifRWStreamWriteProperties(avifItemPropertyDedup * const dedu | |||
hasPixi = AVIF_FALSE; | |||
} | |||
#endif | |||
const avifBool isAlpha = avifIsAlpha(item->itemCategory); | |||
uint8_t depth = (uint8_t)itemMetadata->depth; |
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.
const
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.
It will be removed in the final PR but done.
src/write.c
Outdated
const int quantizer = (item->itemCategory == AVIF_ITEM_ALPHA) ? encoder->data->quantizerAlpha | ||
|
||
const avifBool isAlpha = avifIsAlpha(item->itemCategory); | ||
int quantizer = isAlpha ? encoder->data->quantizerAlpha |
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.
const
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.
It will be removed in the final PR but done.
@@ -1192,7 +1192,12 @@ static avifBool avifEncoderDataShouldForceKeyframeForAlpha(const avifEncoderData | |||
|
|||
static avifResult avifGetErrorForItemCategory(avifItemCategory itemCategory) | |||
{ | |||
return (itemCategory == AVIF_ITEM_ALPHA) ? AVIF_RESULT_ENCODE_ALPHA_FAILED : AVIF_RESULT_ENCODE_COLOR_FAILED; | |||
#if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP) | |||
if (itemCategory == AVIF_ITEM_GAIN_MAP) { |
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.
that's a bug fix then?
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.
Yes, a tiny one
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.
Please mention this bug fix in the commit message.
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.
LGTM. The title of this pull request "Move avifIsAlpha() to internal.h" implies avifIsAlpha already exists. But it is a new function added in this PR. How about changing the title to "Add avifIsAlpha() to internal.h"?
@@ -1192,7 +1192,12 @@ static avifBool avifEncoderDataShouldForceKeyframeForAlpha(const avifEncoderData | |||
|
|||
static avifResult avifGetErrorForItemCategory(avifItemCategory itemCategory) | |||
{ | |||
return (itemCategory == AVIF_ITEM_ALPHA) ? AVIF_RESULT_ENCODE_ALPHA_FAILED : AVIF_RESULT_ENCODE_COLOR_FAILED; | |||
#if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP) | |||
if (itemCategory == AVIF_ITEM_GAIN_MAP) { |
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.
Please mention this bug fix in the commit message.
No description provided.