Refactor avifAOMOptionsContainExplicitTuning() (#3060)
diff --git a/src/codec_aom.c b/src/codec_aom.c index 97431fb..f9d72b2 100644 --- a/src/codec_aom.c +++ b/src/codec_aom.c
@@ -385,15 +385,39 @@ (!strncmp(key, shortPrefix, shortPrefixLen) && !strcmp(key + shortPrefixLen, name)); } -static avifBool avifAOMOptionsContainExplicitTuning(const avifCodec * codec, avifBool alpha) +#if !defined(AOM_HAVE_TUNE_IQ) +// Define the tune IQ value here if libaom doesn't define it. The enum value is guaranteed to never change +// in libaom, so this definition won't ever get out of sync. +#define AOM_TUNE_IQ 10 +#endif +// Tune IQ string -> enum mapping +static const struct aomOptionEnumList tuneIqEnum[] = { { "iq", AOM_TUNE_IQ }, { NULL, 0 } }; + +// Returns true if codec-specific options for AOM contain a tune metric setting. Returns false otherwise. +// Sets *useTuneIq to true if codec-specific options for AOM contain AOM_TUNE_IQ. Otherwise sets *useTuneIq to false. +static avifBool avifAOMOptionsContainExplicitTuning(const avifCodec * codec, avifBool alpha, avifBool * useTuneIq) { + *useTuneIq = AVIF_FALSE; + avifBool isAnyTuneDefined = AVIF_FALSE; + + // If there are multiple "tune" options specified, honor the last one. + // For consistent behavior, handle both cases where tune was either specified as a string (e.g. tune=iq), + // or as an enum value (e.g. tune=10). for (uint32_t i = 0; i < codec->csOptions->count; ++i) { const avifCodecSpecificOption * entry = &codec->csOptions->entries[i]; if (avifKeyEqualsName(entry->key, "tune", alpha)) { - return AVIF_TRUE; + isAnyTuneDefined = AVIF_TRUE; + + int val; + if (aomOptionParseEnum(entry->value, tuneIqEnum, &val)) { + assert(val == AOM_TUNE_IQ); + *useTuneIq = AVIF_TRUE; + } else { + *useTuneIq = AVIF_FALSE; + } } } - return AVIF_FALSE; + return isAnyTuneDefined; } static avifBool avifProcessAOMOptionsPreInit(avifCodec * codec, avifBool alpha, struct aom_codec_enc_cfg * cfg) @@ -412,50 +436,6 @@ return AVIF_TRUE; } -static avifBool avifImageUsesTuneIq(const avifCodec * codec, avifBool alpha) -{ -#if !defined(AOM_HAVE_TUNE_IQ) -// Define the tune IQ value here if libaom doesn't define it. The enum value is guaranteed to never change -// in libaom, so this definition won't ever get out of sync. -#define AOM_TUNE_IQ 10 -#endif - - avifBool useTuneIq = AVIF_FALSE; - avifBool isAnyTuneDefined = AVIF_FALSE; - - // Tune IQ string -> enum mapping - static const struct aomOptionEnumList tuneIqEnum[] = { { "iq", AOM_TUNE_IQ }, // Image Quality (IQ) mode - { NULL, 0 } }; - - for (uint32_t i = 0; i < codec->csOptions->count; ++i) { - const avifCodecSpecificOption * entry = &codec->csOptions->entries[i]; - int val; - // If there are multiple "tune" options specified, honor the last one. - // For consistent behavior, handle both cases where tune IQ was either specified as a string (tune=iq), - // or as an enum value (tune=10). - if (avifKeyEqualsName(entry->key, "tune", alpha)) { - isAnyTuneDefined = AVIF_TRUE; - - if (aomOptionParseEnum(entry->value, tuneIqEnum, &val)) { - assert(val == AOM_TUNE_IQ); - useTuneIq = AVIF_TRUE; - } else { - useTuneIq = AVIF_FALSE; - } - } - } - - if (isAnyTuneDefined) { - return useTuneIq; - } - - // Handle the case where the encoder was called with avifEncoderSetCodecSpecificOption("tune", "iq") - // for a previous frame and not called (or called with NULL) for this frame, because the tune - // option persists across frames in libaom. - // In this case, return what the previous frame used. - return codec->internal->previousFrameUsedTuneIq; -} - #if !defined(HAVE_AOM_CODEC_SET_OPTION) typedef enum { @@ -747,10 +727,21 @@ avifBool useLibavifDefaultTuneMetric = AVIF_FALSE; // If true, override libaom's default tune option. aom_tune_metric libavifDefaultTuneMetric = AOM_TUNE_PSNR; // Meaningless unless useLibavifDefaultTuneMetric. - // libavif only needs to set the default tune metric for the first frame, - // because libaom will persist that setting until explicitly changed. - if (!codec->internal->encoderInitialized) { - if (quality != AVIF_QUALITY_LOSSLESS && !avifAOMOptionsContainExplicitTuning(codec, alpha)) { + // True if libavif knows that tune=iq is used, either by default by libavif, or explicitly set by the user. + // False otherwise (including if libaom uses tune=iq by default, which is not the case as of v3.13.1 and earlier versions). + avifBool useTuneIq; + + if (avifAOMOptionsContainExplicitTuning(codec, alpha, &useTuneIq)) { + // avifAOMOptionsContainExplicitTuning() has set useTuneIq. + } else if (!codec->internal->encoderInitialized) { + // libavif only needs to set the default tune metric for the first frame, + // because libaom will persist that setting until explicitly changed. + + if (quality == AVIF_QUALITY_LOSSLESS) { + // AOM_TUNE_IQ is not libaom's default tune option as of v3.13.1. + // Even if it was, it does not matter for lossless. + useTuneIq = AVIF_FALSE; + } else { useLibavifDefaultTuneMetric = AVIF_TRUE; if (alpha) { // Minimize ringing for alpha. @@ -772,20 +763,19 @@ } #endif } + useTuneIq = (libavifDefaultTuneMetric == AOM_TUNE_IQ); } + } else { + // The tune option persists across frames in libaom until explicitly set to another value. + useTuneIq = codec->internal->previousFrameUsedTuneIq; } + // Remember the current tune option for the next frame. + codec->internal->previousFrameUsedTuneIq = useTuneIq; struct aom_codec_enc_cfg * cfg = &codec->internal->cfg; avifBool quantizerUpdated = AVIF_FALSE; - // True if libavif knows that tune=iq is used, either by default by libavif, or explicitly set by the user. - // False otherwise (including if libaom uses tune=iq by default, which is not the case as of v3.13.1 and earlier versions). - const avifBool useTuneIq = useLibavifDefaultTuneMetric ? libavifDefaultTuneMetric == AOM_TUNE_IQ : avifImageUsesTuneIq(codec, alpha); const int quantizer = aomQualityToQuantizer(quality, useTuneIq); - // libavif needs to know whether the current frame uses tune=iq for the next frame, as libaom persists - // tuning modes across frames - codec->internal->previousFrameUsedTuneIq = useTuneIq; - // For encoder->scalingMode.horizontal and encoder->scalingMode.vertical to take effect in AOM // encoder, config should be applied for each frame, so we don't care about changes on these // two fields.