Skip to content

Correctly update encoder settings, clear csOptions after avifEncoderAddImage() #1058

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.

Sign up for GitHub

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

Merged
wantehchang merged 5 commits into from
Aug 26, 2022
Merged

Correctly update encoder settings, clear csOptions after avifEncoderAddImage() #1058

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9a6758c
Correctly update encoder settings, clear csOptions after avifEncoderA…
tongyuantongyu Aug 21, 2022
4ed2425
review changes
tongyuantongyu Aug 22, 2022
bcc47ea
review changes 2
tongyuantongyu Aug 23, 2022
9fec472
only allow quantizer and tileRows/Cols to change
tongyuantongyu Aug 25, 2022
f2db7ed
review changes 3
tongyuantongyu Aug 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions include/avif/avif.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1058,8 +1058,8 @@ struct avifCodecSpecificOptions;
// image in less bytes. AVIF_SPEED_DEFAULT means "Leave the AV1 codec to its default speed settings"./
// If avifEncoder uses rav1e, the speed value is directly passed through (0-10). If libaom is used,
// a combination of settings are tweaked to simulate this speed range.
// * AV1 encoder settings and codec specific options set by avifEncoderSetCodecSpecificOption()
// will be applied / updated to AV1 encoder before each call to avifEncoderAddImage().
// * Some encoder settings can be changed after encoding starts. Changes will take effect in the next
// call to avifEncoderAddImage().
typedef struct avifEncoder
{
// Defaults to AVIF_CODEC_CHOICE_AUTO: Preference determined by order in availableCodecs table (avif.c)
Expand All @@ -1068,15 +1068,15 @@ typedef struct avifEncoder
// settings (see Notes above)
int keyframeInterval; // How many frames between automatic forced keyframes; 0 to disable (default).
uint64_t timescale; // timescale of the media (Hz)
// AV1 encoder settings.
int maxThreads;
int speed;
// changeable encoder settings.
int minQuantizer;
int maxQuantizer;
int minQuantizerAlpha;
int maxQuantizerAlpha;
int tileRowsLog2;
int tileColsLog2;
int speed;

// stats from the most recent write
avifIOStats ioStats;
Expand Down Expand Up @@ -1128,8 +1128,10 @@ AVIF_API avifResult avifEncoderAddImageGrid(avifEncoder * encoder,
avifAddImageFlags addImageFlags);
AVIF_API avifResult avifEncoderFinish(avifEncoder * encoder, avifRWData * output);

// Codec-specific, optional "advanced" tuning settings, in the form of string key/value pairs.
// key must be non-NULL, but passing a NULL value will delete that key, if it exists.
// Codec-specific, optional "advanced" tuning settings, in the form of string key/value pairs,
// to be consumed by the codec at the next avifEncoderAddImage() call.
// See the codec documentation to know if a setting is persistent or applied only to the next frame.
// key must be non-NULL, but passing a NULL value will delete the pending key, if it exists.
// Setting an incorrect or unknown option for the current codec will cause errors of type
// AVIF_RESULT_INVALID_CODEC_SPECIFIC_OPTION from avifEncoderWrite() or avifEncoderAddImage().
AVIF_API void avifEncoderSetCodecSpecificOption(avifEncoder * encoder, const char * key, const char * value);
Expand Down
16 changes: 15 additions & 1 deletion include/avif/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ typedef struct avifCodecSpecificOption
} avifCodecSpecificOption;
AVIF_ARRAY_DECLARE(avifCodecSpecificOptions, avifCodecSpecificOption, entries);
avifCodecSpecificOptions * avifCodecSpecificOptionsCreate(void);
void avifCodecSpecificOptionsClear(avifCodecSpecificOptions * csOptions);
void avifCodecSpecificOptionsDestroy(avifCodecSpecificOptions * csOptions);
void avifCodecSpecificOptionsSet(avifCodecSpecificOptions * csOptions, const char * key, const char * value); // if(value==NULL), key is deleted

Expand All @@ -253,6 +254,19 @@ void avifCodecSpecificOptionsSet(avifCodecSpecificOptions * csOptions, const cha
struct avifCodec;
struct avifCodecInternal;

typedef enum avifEncoderChange
{
AVIF_ENCODER_CHANGE_MIN_QUANTIZER = (1 << 0),
AVIF_ENCODER_CHANGE_MAX_QUANTIZER = (1 << 1),
AVIF_ENCODER_CHANGE_MIN_QUANTIZER_ALPHA = (1 << 2),
AVIF_ENCODER_CHANGE_MAX_QUANTIZER_ALPHA = (1 << 3),
AVIF_ENCODER_CHANGE_TILE_ROWS_LOG2 = (1 << 4),
AVIF_ENCODER_CHANGE_TILE_COLS_LOG2 = (1 << 5),

AVIF_ENCODER_CHANGE_CODEC_SPECIFIC = (1 << 31)
} avifEncoderChange;
typedef uint32_t avifEncoderChanges;

typedef avifBool (*avifCodecGetNextImageFunc)(struct avifCodec * codec,
struct avifDecoder * decoder,
const avifDecodeSample * sample,
Expand All @@ -267,7 +281,7 @@ typedef avifResult (*avifCodecEncodeImageFunc)(struct avifCodec * codec,
avifEncoder * encoder,
const avifImage * image,
avifBool alpha,
avifBool updateConfig,
avifEncoderChanges encoderChanges,
avifAddImageFlags addImageFlags,
avifCodecEncodeOutput * output);
typedef avifBool (*avifCodecEncodeFinishFunc)(struct avifCodec * codec, avifCodecEncodeOutput * output);
Expand Down
27 changes: 18 additions & 9 deletions src/avif.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -878,17 +878,24 @@ avifCodecSpecificOptions * avifCodecSpecificOptionsCreate(void)
return NULL;
}

void avifCodecSpecificOptionsDestroy(avifCodecSpecificOptions * csOptions)
void avifCodecSpecificOptionsClear(avifCodecSpecificOptions * csOptions)
{
if (!csOptions) {
return;
}

for (uint32_t i = 0; i < csOptions->count; ++i) {
avifCodecSpecificOption * entry = &csOptions->entries[i];
avifFree(entry->key);
avifFree(entry->value);
}

csOptions->count = 0;
Comment thread
wantehchang marked this conversation as resolved.
}

void avifCodecSpecificOptionsDestroy(avifCodecSpecificOptions * csOptions)
{
if (!csOptions) {
return;
}

avifCodecSpecificOptionsClear(csOptions);
avifArrayDestroy(csOptions);
avifFree(csOptions);
}
Expand Down Expand Up @@ -916,10 +923,12 @@ void avifCodecSpecificOptionsSet(avifCodecSpecificOptions * csOptions, const cha
}
}

// Add a new key
avifCodecSpecificOption * entry = (avifCodecSpecificOption *)avifArrayPushPtr(csOptions);
entry->key = avifStrdup(key);
entry->value = avifStrdup(value);
if (value) {
// Add a new key
avifCodecSpecificOption * entry = (avifCodecSpecificOption *)avifArrayPushPtr(csOptions);
entry->key = avifStrdup(key);
entry->value = avifStrdup(value);
}
}

// ---------------------------------------------------------------------------
Expand Down
188 changes: 113 additions & 75 deletions src/codec_aom.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,11 +527,17 @@ static avifResult aomCodecEncodeImage(avifCodec * codec,
avifEncoder * encoder,
const avifImage * image,
avifBool alpha,
avifBool updateConfig,
avifEncoderChanges encoderChanges,
avifAddImageFlags addImageFlags,
avifCodecEncodeOutput * output)
{
if (!codec->internal->encoderInitialized || updateConfig) {
struct aom_codec_enc_cfg * cfg = &codec->internal->cfg;
aom_codec_iface_t * encoderInterface = NULL;
unsigned int aomUsage = AOM_USAGE_GOOD_QUALITY;
int aomCpuUsed = -1;
avifBool lossless = AVIF_FALSE;

if (!codec->internal->encoderInitialized) {
// Map encoder speed to AOM usage + CpuUsed:
// Speed 0: GoodQuality CpuUsed 0
// Speed 1: GoodQuality CpuUsed 1
Expand All @@ -544,15 +550,13 @@ static avifResult aomCodecEncodeImage(avifCodec * codec,
// Speed 8: RealTime CpuUsed 8
// Speed 9: RealTime CpuUsed 9
// Speed 10: RealTime CpuUsed 9
unsigned int aomUsage = AOM_USAGE_GOOD_QUALITY;
// Use the new AOM_USAGE_ALL_INTRA (added in https://crbug.com/aomedia/2959) for still
// image encoding if it is available.
#if defined(AOM_USAGE_ALL_INTRA)
if (addImageFlags & AVIF_ADD_IMAGE_FLAG_SINGLE) {
aomUsage = AOM_USAGE_ALL_INTRA;
}
#endif
int aomCpuUsed = -1;
if (encoder->speed != AVIF_SPEED_DEFAULT) {
aomCpuUsed = AVIF_CLAMP(encoder->speed, 0, 9);
if (aomCpuUsed >= 7) {
Expand Down Expand Up @@ -589,23 +593,18 @@ static avifResult aomCodecEncodeImage(avifCodec * codec,
}
}

struct aom_codec_enc_cfg * cfg = &codec->internal->cfg;

aom_codec_iface_t * encoderInterface = NULL;
if (!codec->internal->encoderInitialized) {
codec->internal->aomFormat = avifImageCalcAOMFmt(image, alpha);
if (codec->internal->aomFormat == AOM_IMG_FMT_NONE) {
return AVIF_RESULT_UNKNOWN_ERROR;
}
codec->internal->aomFormat = avifImageCalcAOMFmt(image, alpha);
if (codec->internal->aomFormat == AOM_IMG_FMT_NONE) {
return AVIF_RESULT_UNKNOWN_ERROR;
}

avifGetPixelFormatInfo(image->yuvFormat, &codec->internal->formatInfo);
avifGetPixelFormatInfo(image->yuvFormat, &codec->internal->formatInfo);

encoderInterface = aom_codec_av1_cx();
aom_codec_err_t err = aom_codec_enc_config_default(encoderInterface, cfg, aomUsage);
if (err != AOM_CODEC_OK) {
avifDiagnosticsPrintf(codec->diag, "aom_codec_enc_config_default() failed: %s", aom_codec_err_to_string(err));
return AVIF_RESULT_UNKNOWN_ERROR;
}
encoderInterface = aom_codec_av1_cx();
aom_codec_err_t err = aom_codec_enc_config_default(encoderInterface, cfg, aomUsage);
if (err != AOM_CODEC_OK) {
avifDiagnosticsPrintf(codec->diag, "aom_codec_enc_config_default() failed: %s", aom_codec_err_to_string(err));
return AVIF_RESULT_UNKNOWN_ERROR;
}
Copy link
Copy Markdown
Collaborator

@wantehchang wantehchang Aug 25, 2022

Choose a reason for hiding this comment

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

Add the following code after this line:

        // Set our own default cfg.rc_end_usage value, which may differ from libaom's default.
        switch (aomUsage) {
            case AOM_USAGE_GOOD_QUALITY:
                // libaom's default is AOM_VBR. Change the default to AOM_Q since we don't need to
                // hit a certain target bit rate. It's easier to control the worst quality in Q
                // mode.
                cfg.rc_end_usage = AOM_Q;
                break;
            case AOM_USAGE_REALTIME:
                // For real-time mode we need to use CBR rate control mode. AOM_Q doesn't fit the
                // rate control requirements for real-time mode. CBR does.
                cfg.rc_end_usage = AOM_CBR;
                break;
#if defined(AOM_USAGE_ALL_INTRA)
            case AOM_USAGE_ALL_INTRA:
                cfg.rc_end_usage = AOM_Q;
                break;
#endif
        }

This code changes some default values from libaom's defaults to ours, so it should immediately follow a successful call to aom_codec_enc_config_default().

Copy link
Copy Markdown
Contributor Author

@tongyuantongyu tongyuantongyu Aug 26, 2022

Choose a reason for hiding this comment

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

Done


// Set our own default cfg->rc_end_usage value, which may differ from libaom's default.
Expand Down Expand Up @@ -666,8 +665,7 @@ static avifResult aomCodecEncodeImage(avifCodec * codec,
cfg->g_profile = seqProfile;
cfg->g_bit_depth = image->depth;
cfg->g_input_bit_depth = image->depth;
cfg->g_w = image->width;
cfg->g_h = image->height;

if (addImageFlags & AVIF_ADD_IMAGE_FLAG_SINGLE) {
// Set the maximum number of frames to encode to 1. This instructs
// libaom to set still_picture and reduced_still_picture_header to
Expand All @@ -690,16 +688,6 @@ static avifResult aomCodecEncodeImage(avifCodec * codec,
cfg->g_threads = encoder->maxThreads;
}

int minQuantizer = AVIF_CLAMP(encoder->minQuantizer, 0, 63);
int maxQuantizer = AVIF_CLAMP(encoder->maxQuantizer, 0, 63);
if (alpha) {
minQuantizer = AVIF_CLAMP(encoder->minQuantizerAlpha, 0, 63);
maxQuantizer = AVIF_CLAMP(encoder->maxQuantizerAlpha, 0, 63);
}
avifBool lossless = ((minQuantizer == AVIF_QUANTIZER_LOSSLESS) && (maxQuantizer == AVIF_QUANTIZER_LOSSLESS));
cfg->rc_min_quantizer = minQuantizer;
cfg->rc_max_quantizer = maxQuantizer;

codec->internal->monochromeEnabled = AVIF_FALSE;
if (aomVersion > aomVersion_2_0_0) {
// There exists a bug in libaom's chroma_check() function where it will attempt to
Expand All @@ -715,76 +703,126 @@ static avifResult aomCodecEncodeImage(avifCodec * codec,
cfg->monochrome = 1;
}
}
}

if (!avifProcessAOMOptionsPreInit(codec, alpha, cfg)) {
return AVIF_RESULT_INVALID_CODEC_SPECIFIC_OPTION;
avifBool dimensionsChanged = AVIF_FALSE;
if (!codec->internal->encoderInitialized) {
cfg->g_w = image->width;
cfg->g_h = image->height;
Copy link
Copy Markdown
Contributor Author

@tongyuantongyu tongyuantongyu Aug 25, 2022

Choose a reason for hiding this comment

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

Seems just changing g_w and g_h here only works when sending smaller frames, due to aom writing first frame's dimension to stream header.

To support larger frames, seems we need user to provide the expect dimension before starting encoding, and set g_forced_max_frame_width and g_forced_max_frame_height. I'm going to implement this in another PR.

Copy link
Copy Markdown
Collaborator

@wantehchang wantehchang Aug 25, 2022

Choose a reason for hiding this comment

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

That's correct. An AV1 bitstream begins with a sequence header OBU, which advertises the maximum frame width and maximum frame height. Then each frame has a frame header OBU that may optionally specify a frame width and height smaller than the maximum. So if we are encoding multiple frames with increasing frame widths and heights, we will need a way to specify the maximum frame width and maximum frame height before encoding the first frame.

} else if ((cfg->g_w != image->width) || (cfg->g_h != image->height)) {
// We are not ready for dimension change for now.
return AVIF_RESULT_NOT_IMPLEMENTED;
}

if (!codec->internal->encoderInitialized || encoderChanges) {
int minQuantizer = AVIF_CLAMP(encoder->minQuantizer, 0, 63);
int maxQuantizer = AVIF_CLAMP(encoder->maxQuantizer, 0, 63);
if (alpha) {
minQuantizer = AVIF_CLAMP(encoder->minQuantizerAlpha, 0, 63);
maxQuantizer = AVIF_CLAMP(encoder->maxQuantizerAlpha, 0, 63);
}
lossless = ((minQuantizer == AVIF_QUANTIZER_LOSSLESS) && (maxQuantizer == AVIF_QUANTIZER_LOSSLESS));
cfg->rc_min_quantizer = minQuantizer;
cfg->rc_max_quantizer = maxQuantizer;
}
Copy link
Copy Markdown
Collaborator

@wantehchang wantehchang Aug 26, 2022

Choose a reason for hiding this comment

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

Since we do not allow "end-usage" to change, we only need to call avifProcessAOMOptionsPreInit() before the aom_codec_enc_init() call. Please move lines 708-710 right before line 736, inside the if (!codec->internal->encoderInitialized) block.

Copy link
Copy Markdown
Contributor Author

@tongyuantongyu tongyuantongyu Aug 26, 2022

Choose a reason for hiding this comment

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

Done


if (!codec->internal->encoderInitialized) {
aom_codec_flags_t encoderFlags = 0;
if (image->depth > 8) {
encoderFlags |= AOM_CODEC_USE_HIGHBITDEPTH;
}
if (!codec->internal->encoderInitialized) {
aom_codec_flags_t encoderFlags = 0;
if (image->depth > 8) {
encoderFlags |= AOM_CODEC_USE_HIGHBITDEPTH;
}

if (aom_codec_enc_init(&codec->internal->encoder, encoderInterface, cfg, encoderFlags) != AOM_CODEC_OK) {
avifDiagnosticsPrintf(codec->diag,
"aom_codec_enc_init() failed: %s: %s",
aom_codec_error(&codec->internal->encoder),
aom_codec_error_detail(&codec->internal->encoder));
return AVIF_RESULT_UNKNOWN_ERROR;
}
codec->internal->encoderInitialized = AVIF_TRUE;
} else {
if (aom_codec_enc_config_set(&codec->internal->encoder, cfg) != AOM_CODEC_OK) {
avifDiagnosticsPrintf(codec->diag,
"aom_codec_enc_config_set() failed: %s: %s",
aom_codec_error(&codec->internal->encoder),
aom_codec_error_detail(&codec->internal->encoder));
return AVIF_RESULT_UNKNOWN_ERROR;
}
if (!avifProcessAOMOptionsPreInit(codec, alpha, cfg)) {
return AVIF_RESULT_INVALID_CODEC_SPECIFIC_OPTION;
}

if (lossless) {
aom_codec_control(&codec->internal->encoder, AV1E_SET_LOSSLESS, 1);
if (aom_codec_enc_init(&codec->internal->encoder, encoderInterface, cfg, encoderFlags) != AOM_CODEC_OK) {
avifDiagnosticsPrintf(codec->diag,
"aom_codec_enc_init() failed: %s: %s",
aom_codec_error(&codec->internal->encoder),
aom_codec_error_detail(&codec->internal->encoder));
return AVIF_RESULT_UNKNOWN_ERROR;
}

if (encoder->maxThreads > 1) {
aom_codec_control(&codec->internal->encoder, AV1E_SET_ROW_MT, 1);
}
if (encoder->tileRowsLog2 != 0) {
int tileRowsLog2 = AVIF_CLAMP(encoder->tileRowsLog2, 0, 6);
aom_codec_control(&codec->internal->encoder, AV1E_SET_TILE_ROWS, tileRowsLog2);
}
if (encoder->tileColsLog2 != 0) {
int tileColsLog2 = AVIF_CLAMP(encoder->tileColsLog2, 0, 6);
aom_codec_control(&codec->internal->encoder, AV1E_SET_TILE_COLUMNS, tileColsLog2);
}
if (aomCpuUsed != -1) {
if (aom_codec_control(&codec->internal->encoder, AOME_SET_CPUUSED, aomCpuUsed) != AOM_CODEC_OK) {
return AVIF_RESULT_UNKNOWN_ERROR;
}
}
} else if ((encoderChanges & ~AVIF_ENCODER_CHANGE_CODEC_SPECIFIC) || dimensionsChanged) {
// Codec specific options does not change cfg, so no need to update it.
aom_codec_err_t err = aom_codec_enc_config_set(&codec->internal->encoder, cfg);
if (err != AOM_CODEC_OK) {
avifDiagnosticsPrintf(codec->diag,
"aom_codec_enc_config_set() failed: %s: %s",
aom_codec_error(&codec->internal->encoder),
aom_codec_error_detail(&codec->internal->encoder));
return AVIF_RESULT_UNKNOWN_ERROR;
}
}

if (!codec->internal->encoderInitialized || (encoderChanges & AVIF_ENCODER_CHANGE_CODEC_SPECIFIC)) {
if (!avifProcessAOMOptionsPostInit(codec, alpha)) {
return AVIF_RESULT_INVALID_CODEC_SPECIFIC_OPTION;
}
#if defined(AOM_USAGE_ALL_INTRA)
if (aomUsage == AOM_USAGE_ALL_INTRA && !codec->internal->endUsageSet && !codec->internal->cqLevelSet) {
// The default rc_end_usage in all intra mode is AOM_Q, which requires cq-level to
// function. A libavif user may not know this internal detail and therefore may only
// set the min and max quantizers in the avifEncoder struct. If this is the case, set
// cq-level to a reasonable value for the user, otherwise the default cq-level
// (currently 10) will be unknowingly used.
assert(cfg->rc_end_usage == AOM_Q);
unsigned int cqLevel = (cfg->rc_min_quantizer + cfg->rc_max_quantizer) / 2;
aom_codec_control(&codec->internal->encoder, AOME_SET_CQ_LEVEL, cqLevel);
}
Copy link
Copy Markdown
Collaborator

@wantehchang wantehchang Aug 26, 2022

Choose a reason for hiding this comment

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

Lines 774-785 should only be done when initializing the encoder or when cfg->rc_min_quantizer or cfg->rc_max_quantizer changed. If it takes more than five lines to implement this, just add a TODO comment.

Copy link
Copy Markdown
Contributor Author

@tongyuantongyu tongyuantongyu Aug 26, 2022

Choose a reason for hiding this comment

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

Done


avifBool quantizerUpdated = AVIF_FALSE;
if (!codec->internal->encoderInitialized) {
quantizerUpdated = AVIF_TRUE;
if (lossless) {
aom_codec_control(&codec->internal->encoder, AV1E_SET_LOSSLESS, lossless);
}
int tileRowsLog2 = AVIF_CLAMP(encoder->tileRowsLog2, 0, 6);
if (tileRowsLog2 > 0) {
aom_codec_control(&codec->internal->encoder, AV1E_SET_TILE_ROWS, tileRowsLog2);
}
int tileColsLog2 = AVIF_CLAMP(encoder->tileColsLog2, 0, 6);
if (tileColsLog2 > 0) {
aom_codec_control(&codec->internal->encoder, AV1E_SET_TILE_COLUMNS, tileColsLog2);
}
#endif
if (!codec->internal->tuningSet) {
if (aom_codec_control(&codec->internal->encoder, AOME_SET_TUNING, AOM_TUNE_SSIM) != AOM_CODEC_OK) {
return AVIF_RESULT_UNKNOWN_ERROR;
}
}
codec->internal->encoderInitialized = AVIF_TRUE;
} else if (encoderChanges) {
if (alpha) {
if (encoderChanges & (AVIF_ENCODER_CHANGE_MIN_QUANTIZER_ALPHA | AVIF_ENCODER_CHANGE_MAX_QUANTIZER_ALPHA)) {
quantizerUpdated = AVIF_TRUE;
aom_codec_control(&codec->internal->encoder, AV1E_SET_LOSSLESS, lossless);
}
} else {
if (encoderChanges & (AVIF_ENCODER_CHANGE_MIN_QUANTIZER | AVIF_ENCODER_CHANGE_MAX_QUANTIZER)) {
quantizerUpdated = AVIF_TRUE;
aom_codec_control(&codec->internal->encoder, AV1E_SET_LOSSLESS, lossless);
}
}
Copy link
Copy Markdown
Collaborator

@wantehchang wantehchang Aug 26, 2022

Choose a reason for hiding this comment

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

This conditional expression is very complicated. I suggest we aim for clarify and rewrite it as follows:

        if (!alpha) {
            if ((updatedConfig & AVIF_ENCODER_CONFIG_MIN_QUANTIZER) || (updatedConfig & AVIF_ENCODER_CONFIG_MAX_QUANTIZER)) {
                aom_codec_control(&codec->internal->encoder, AV1E_SET_LOSSLESS, lossless);
            }
        } else {
            if ((updatedConfig & AVIF_ENCODER_CONFIG_MIN_QUANTIZER_ALPHA) || (updatedConfig & AVIF_ENCODER_CONFIG_MAX_QUANTIZER_ALPHA)) {
                aom_codec_control(&codec->internal->encoder, AV1E_SET_LOSSLESS, lossless);
            }
        }

Optional: We can simplify the conditionals even further:

        if (!alpha) {
            if (updatedConfig & (AVIF_ENCODER_CONFIG_MIN_QUANTIZER | AVIF_ENCODER_CONFIG_MAX_QUANTIZER)) {
                aom_codec_control(&codec->internal->encoder, AV1E_SET_LOSSLESS, lossless);
            }
        } else {
            if (updatedConfig & (AVIF_ENCODER_CONFIG_MIN_QUANTIZER_ALPHA | AVIF_ENCODER_CONFIG_MAX_QUANTIZER_ALPHA)) {
                aom_codec_control(&codec->internal->encoder, AV1E_SET_LOSSLESS, lossless);
            }
        }

Copy link
Copy Markdown
Contributor Author

@tongyuantongyu tongyuantongyu Aug 26, 2022

Choose a reason for hiding this comment

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

Done

if (encoderChanges & AVIF_ENCODER_CHANGE_TILE_ROWS_LOG2) {
aom_codec_control(&codec->internal->encoder, AV1E_SET_TILE_ROWS, AVIF_CLAMP(encoder->tileRowsLog2, 0, 6));
}
if (encoderChanges & AVIF_ENCODER_CHANGE_TILE_COLS_LOG2) {
aom_codec_control(&codec->internal->encoder, AV1E_SET_TILE_COLUMNS, AVIF_CLAMP(encoder->tileColsLog2, 0, 6));
}
}

#if defined(AOM_USAGE_ALL_INTRA)
if (aomUsage == AOM_USAGE_ALL_INTRA && !codec->internal->endUsageSet && !codec->internal->cqLevelSet && quantizerUpdated) {
// The default rc_end_usage in all intra mode is AOM_Q, which requires cq-level to
// function. A libavif user may not know this internal detail and therefore may only
// set the min and max quantizers in the avifEncoder struct. If this is the case, set
// cq-level to a reasonable value for the user, otherwise the default cq-level
// (currently 10) will be unknowingly used.
assert(cfg->rc_end_usage == AOM_Q);
unsigned int cqLevel = (cfg->rc_min_quantizer + cfg->rc_max_quantizer) / 2;
aom_codec_control(&codec->internal->encoder, AOME_SET_CQ_LEVEL, cqLevel);
}
#endif

aom_image_t aomImage;
// We prefer to simply set the aomImage.planes[] pointers to the plane buffers in 'image'. When
// doing this, we set aomImage.w equal to aomImage.d_w and aomImage.h equal to aomImage.d_h and
Expand Down
Loading