Remember tune=iq option across frames (AOMediaCodec#2997)

Author: juliobbv

src/codec_aom.c

@@ -69,6 +69,7 @@ struct avifCodecInternal
     aom_img_fmt_t aomFormat;
     uint32_t currentLayer;
     int qualityFirstLayer;
+    avifBool previousFrameUsedTuneIq;
 #endif
 };
 
@@ -403,16 +404,21 @@ static avifBool avifProcessAOMOptionsPreInit(avifCodec * codec, avifBool alpha,
     return AVIF_TRUE;
 }
 
-static avifBool avifImageUsesTuneIq(const avifCodec * codec, avifBool alpha)
+static avifBool avifImageUsesTuneIq(const avifCodec * codec, avifBool alpha, avifBool useLibavifDefaultTuneMetric, aom_tune_metric libavifDefaultTuneMetric)
 {
-    avifBool ret = AVIF_FALSE;
-
 #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
 
+    if (useLibavifDefaultTuneMetric) {
+        return libavifDefaultTuneMetric == AOM_TUNE_IQ;
+    }
+
+    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 } };
@@ -423,16 +429,24 @@ static avifBool avifImageUsesTuneIq(const avifCodec * codec, avifBool alpha)
         // 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) && aomOptionParseEnum(entry->value, tuneIqEnum, &val)) {
-            ret = (val == AOM_TUNE_IQ);
+        if (avifKeyEqualsName(entry->key, "tune", alpha)) {
+            isAnyTuneDefined = AVIF_TRUE;
+
+            if (aomOptionParseEnum(entry->value, tuneIqEnum, &val)) {
+                useTuneIq = (val == AOM_TUNE_IQ);
+            }
         }
     }
 
-    // In practice this function should also return true if avifEncoderSetCodecSpecificOption("tune", "iq")
-    // was called for a previous frame and not called (or called with NULL) for this frame, because the tune
-    // option persists across frames in libaom. However AOM_TUNE_IQ is only supported with still images in
-    // libavif and libaom as of today, so there is no need to remember this option across frames.
-    return ret;
+    if (!isAnyTuneDefined && codec->internal->previousFrameUsedTuneIq) {
+        // 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, we know libaom will also use tune=iq for this frame.
+        return AVIF_TRUE;
+    }
+
+    return useTuneIq;
 }
 
 #if !defined(HAVE_AOM_CODEC_SET_OPTION)
@@ -725,23 +739,33 @@ static avifResult aomCodecEncodeImage(avifCodec * codec,
 
     avifBool useLibavifDefaultTuneMetric = AVIF_FALSE;        // If true, override libaom's default tune option.
     aom_tune_metric libavifDefaultTuneMetric = AOM_TUNE_PSNR; // Meaningless unless useLibavifDefaultTuneMetric.
-    if (quality != AVIF_QUALITY_LOSSLESS && !avifAOMOptionsContainExplicitTuning(codec, alpha)) {
-        useLibavifDefaultTuneMetric = AVIF_TRUE;
-        if (alpha) {
-            // Minimize ringing for alpha.
-            libavifDefaultTuneMetric = AOM_TUNE_PSNR;
-        } else {
-            libavifDefaultTuneMetric = AOM_TUNE_SSIM;
+
+    // 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)) {
+            useLibavifDefaultTuneMetric = AVIF_TRUE;
+            if (alpha) {
+                // Minimize ringing for alpha.
+                libavifDefaultTuneMetric = AOM_TUNE_PSNR;
+            } else {
+                libavifDefaultTuneMetric = AOM_TUNE_SSIM;
+            }
         }
+        AVIF_ASSERT_OR_RETURN(!codec->internal->previousFrameUsedTuneIq);
     }
 
     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 avifBool useTuneIq = avifImageUsesTuneIq(codec, alpha, useLibavifDefaultTuneMetric, libavifDefaultTuneMetric);
     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.

tests/gtest/avifprogressivetest.cc

@@ -87,6 +87,37 @@ TEST_F(ProgressiveTest, QualityChange) {
   TestDecode(kImageSize, kImageSize);
 }
 
+// NOTE: This test requires libaom v3.12.0 or later, as this was the first
+// version where tune IQ was available
+TEST_F(ProgressiveTest, TuneIq) {
+  encoder_->extraLayerCount = 1;
+  // Tune IQ requires all-intra mode, which libavif determines when the first
+  // layer is encoded at a very low quality (e.g. quality 10)
+  encoder_->quality = 10;
+  encoder_->codecChoice = AVIF_CODEC_CHOICE_AOM;
+
+  ASSERT_EQ(avifEncoderSetCodecSpecificOption(encoder_.get(), "tune", "iq"),
+            AVIF_RESULT_OK);
+  avifResult result = avifEncoderAddImage(encoder_.get(), image_.get(), 1,
+                                          AVIF_ADD_IMAGE_FLAG_NONE);
+
+  if (result == AVIF_RESULT_INVALID_CODEC_SPECIFIC_OPTION) {
+    // The aom version that libavif was built with likely does not support
+    // AOM_TUNE_IQ.
+    return;
+  }
+
+  ASSERT_EQ(result, AVIF_RESULT_OK);
+  encoder_->quality = 50;
+  ASSERT_EQ(avifEncoderAddImage(encoder_.get(), image_.get(), 1,
+                                AVIF_ADD_IMAGE_FLAG_NONE),
+            AVIF_RESULT_OK);
+
+  ASSERT_EQ(avifEncoderFinish(encoder_.get(), &encoded_avif_), AVIF_RESULT_OK);
+
+  TestDecode(kImageSize, kImageSize);
+}
+
 // NOTE: This test requires libaom v3.6.0 or later, otherwise the following
 // assertion in libaom fails:
 //   av1/encoder/mcomp.c:1717: av1_full_pixel_search: Assertion