Remember tune=iq option across frames (#2997)

Author: juliobbv

src/codec_aom.c

@@ -69,6 +69,7 @@ struct avifCodecInternal
     aom_img_fmt_t aomFormat;
     uint32_t currentLayer;
     int qualityFirstLayer;
+    avifBool previousLayerUsedTuneIq;
 #endif
 };
 
@@ -403,9 +404,10 @@ 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 isFirstLayer)
 {
-    avifBool ret = AVIF_FALSE;
+    avifBool useTuneIq = AVIF_FALSE;
+    avifBool isAnyTuneDefined = 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
@@ -423,16 +425,27 @@ 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)) {
+            // Check whether any tuning mode has been defined, excluding the { "tune", NULL } case
+            isAnyTuneDefined = entry->value != NULL;
+
+            if (aomOptionParseEnum(entry->value, tuneIqEnum, &val)) {
+                useTuneIq = (val == AOM_TUNE_IQ);
+            } else {
+                useTuneIq = AVIF_FALSE;
+            }
         }
     }
 
-    // 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 && !isFirstLayer && codec->internal->previousLayerUsedTuneIq) {
+        // Handle the case where the encoder was called with avifEncoderSetCodecSpecificOption("tune", "iq")
+        // for a previous layer (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.
+        useTuneIq = AVIF_TRUE;
+    }
+
+    return useTuneIq;
 }
 
 #if !defined(HAVE_AOM_CODEC_SET_OPTION)
@@ -739,9 +752,16 @@ static avifResult aomCodecEncodeImage(avifCodec * codec,
     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 = useLibavifDefaultTuneMetric ? libavifDefaultTuneMetric == AOM_TUNE_IQ
+                                                           : avifImageUsesTuneIq(codec, alpha, codec->internal->currentLayer == 0);
     const int quantizer = aomQualityToQuantizer(quality, useTuneIq);
 
+    if (!alpha && encoder->extraLayerCount > 0) {
+        // We need to know whether the current layer uses tune=iq for the next layer, as libaom persists
+        // tuning modes across frames
+        codec->internal->previousLayerUsedTuneIq = 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