Address review comments

tests/gtest/avif_fuzztest_dec_incr.cc

@@ -101,7 +101,10 @@ void DecodeIncr(const std::string& arbitrary_bytes, bool is_persistent,
     const uint32_t max_cell_height = reference->height;
     const avifResult result = DecodeIncrementally(
         encoded_data, decoder.get(), is_persistent, give_size_hint,
-        use_nth_image_api, *reference, max_cell_height);
+        use_nth_image_api, *reference, max_cell_height,
+        /*enable_fine_incremental_check=*/false,
+        /*expect_whole_file_read=*/true,
+        /*expect_parse_success_from_partial_file=*/false);
     // The result does not matter, as long as we do not crash.
     (void)result;
   }

tests/gtest/avif_fuzztest_enc_dec_incr.cc

@@ -61,7 +61,9 @@ void EncodeDecodeGridValid(ImagePtr image, EncoderPtr encoder,
 
   const avifResult decode_result = DecodeNonIncrementallyAndIncrementally(
       encoded_data, decoder.get(), is_encoded_data_persistent,
-      give_size_hint_to_decoder, /*use_nth_image_api=*/true, cell_height);
+      give_size_hint_to_decoder, /*use_nth_image_api=*/true, cell_height,
+      /*enable_fine_incremental_check=*/false, /*expect_whole_file_read=*/true,
+      /*expect_parse_success_from_partial_file=*/false);
   ASSERT_EQ(decode_result, AVIF_RESULT_OK) << avifResultToString(decode_result);
 }
 

tests/gtest/avif_fuzztest_enc_dec_incr_experimental.cc

@@ -93,7 +93,8 @@ void EncodeDecodeGridValid(ImagePtr image, EncoderPtr encoder,
   const avifResult decode_result = DecodeNonIncrementallyAndIncrementally(
       encoded_data, decoder.get(), is_encoded_data_persistent,
       give_size_hint_to_decoder, /*use_nth_image_api=*/true, cell_height,
-      /*enable_fine_incremental_check=*/false, expect_whole_file_read);
+      /*enable_fine_incremental_check=*/false, expect_whole_file_read,
+      /*expect_parse_success_from_partial_file=*/false);
   ASSERT_EQ(decode_result, AVIF_RESULT_OK) << avifResultToString(decode_result);
 }
 

tests/gtest/avifincrtest_helpers.cc

@@ -105,13 +105,16 @@ uint32_t GetMinDecodedRowCount(uint32_t height, uint32_t cell_height,
   available_byte_count -= 500;
   byte_count -= 500;
   // Extra planes (alpha, gain map), if any, are assumed to be located before
-  // the other planes and to represent at most 50% of the payload.
+  // the color planes. It's assumed that each extra planes is at most
+  // total_size / (1 + num_extra_planes).
   const int num_extra_planes = (has_alpha ? 1 : 0) + (has_gain_map ? 1 : 0);
-  if (available_byte_count <= ((byte_count / 2) * num_extra_planes)) {
+  const size_t max_size_of_extra_planes = static_cast<size_t>(
+      (byte_count / (num_extra_planes + 1)) * num_extra_planes);
+  if (available_byte_count <= max_size_of_extra_planes) {
     return 0;
   }
-  available_byte_count -= (byte_count / 2) * num_extra_planes;
-  byte_count -= (byte_count / 2) * num_extra_planes;
+  available_byte_count -= max_size_of_extra_planes;
+  byte_count -= max_size_of_extra_planes;
   // Linearly map the input availability ratio to the decoded row ratio.
   const uint32_t min_decoded_cell_row_count = static_cast<uint32_t>(
       (height / cell_height) * available_byte_count / byte_count);
@@ -294,7 +297,8 @@ avifResult DecodeIncrementally(const avifRWData& encoded_avif,
                                bool give_size_hint, bool use_nth_image_api,
                                const avifImage& reference, uint32_t cell_height,
                                bool enable_fine_incremental_check,
-                               bool expect_whole_file_read) {
+                               bool expect_whole_file_read,
+                               bool expect_parse_success_from_partial_file) {
   // AVIF cells are at least 64 pixels tall.
   if (cell_height != reference.height) {
     AVIF_CHECKERR(cell_height >= 64u, AVIF_RESULT_INVALID_ARGUMENT);
@@ -331,12 +335,14 @@ avifResult DecodeIncrementally(const avifRWData& encoded_avif,
     data.available.size = std::min(data.available.size + step, data.full_size);
     parse_result = avifDecoderParse(decoder);
   }
-  if (data.available.size == data.full_size) {
+  if (data.available.size == data.full_size &&
+      expect_parse_success_from_partial_file) {
     // Can happen if the data is in 'idat', or if some metadata is at the end of
     // the file. But ideally this should be avoided.
     printf(
-        "WARNING: had to provide the whole file for avifDecoderParse() to "
+        "ERROR: had to provide the whole file for avifDecoderParse() to "
         "succeed\n");
+    AVIF_CHECKERR(false, AVIF_RESULT_INVALID_ARGUMENT);
   }
   AVIF_CHECKRES(parse_result);
 
@@ -350,13 +356,13 @@ avifResult DecodeIncrementally(const avifRWData& encoded_avif,
       std::cerr << (use_nth_image_api ? "avifDecoderNthImage(0)"
                                       : "avifDecoderNextImage()")
                 << " returned WAITING_ON_IO instead of OK";
-      return AVIF_RESULT_INVALID_ARGUMENT;
+      AVIF_CHECKERR(false, AVIF_RESULT_INVALID_ARGUMENT);
     }
     const uint32_t decoded_row_count = avifDecoderDecodedRowCount(decoder);
     if (decoded_row_count < previously_decoded_row_count) {
       printf("ERROR: decoded row count decreased from %d to %d\n",
              previously_decoded_row_count, decoded_row_count);
-      return AVIF_RESULT_INVALID_ARGUMENT;
+      AVIF_CHECKERR(false, AVIF_RESULT_INVALID_ARGUMENT);
     }
     bool has_gain_map = false;
 #if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP)
@@ -366,10 +372,6 @@ avifResult DecodeIncrementally(const avifRWData& encoded_avif,
         reference.height, cell_height, reference.alphaPlane != nullptr,
         has_gain_map, data.available.size, data.full_size,
         enable_fine_incremental_check);
-    if (decoded_row_count != previously_decoded_row_count) {
-      printf("bytes %zu decoded %d lines (expected min %d)\n",
-             data.available.size, decoded_row_count, min_decoded_row_count);
-    }
     if (decoded_row_count < min_decoded_row_count) {
       printf(
           "ERROR: expected to have decoded at least %d rows with %zu available "
@@ -399,7 +401,8 @@ avifResult DecodeIncrementally(const avifRWData& encoded_avif,
 avifResult DecodeNonIncrementallyAndIncrementally(
     const avifRWData& encoded_avif, avifDecoder* decoder, bool is_persistent,
     bool give_size_hint, bool use_nth_image_api, uint32_t cell_height,
-    bool enable_fine_incremental_check, bool expect_whole_file_read) {
+    bool enable_fine_incremental_check, bool expect_whole_file_read,
+    bool expect_parse_success_from_partial_file) {
   ImagePtr reference(avifImageCreateEmpty());
   if (reference == nullptr) return AVIF_RESULT_INVALID_ARGUMENT;
   decoder->allowIncremental = AVIF_FALSE;
@@ -411,7 +414,7 @@ avifResult DecodeNonIncrementallyAndIncrementally(
   const avifResult result = DecodeIncrementally(
       encoded_avif, decoder, is_persistent, give_size_hint, use_nth_image_api,
       *reference, cell_height, enable_fine_incremental_check,
-      expect_whole_file_read);
+      expect_whole_file_read, expect_parse_success_from_partial_file);
   return result;
 }
 

tests/gtest/avifincrtest_helpers.h

@@ -52,20 +52,26 @@ void EncodeRectAsIncremental(const avifImage& image, uint32_t width,
 // incremental granularity. enable_fine_incremental_check checks that sample
 // rows are gradually output when feeding more and more input bytes to the
 // decoder.
-avifResult DecodeIncrementally(const avifRWData& encoded_avif,
-                               avifDecoder* decoder, bool is_persistent,
-                               bool give_size_hint, bool use_nth_image_api,
-                               const avifImage& reference, uint32_t cell_height,
-                               bool enable_fine_incremental_check = false,
-                               bool expect_whole_file_read = true);
+// If 'expect_parse_success_from_partial_file' is true, avifDecoderParse
+// should succeed before the whole file is available. Returns an error
+// if avifDecoderParse fails until all the bytes are available.
+// `expect_parse_success_from_partial_file` should be set to false if the file
+// may be using 'idat' or may have some metadata at the end of the file.
+avifResult DecodeIncrementally(
+    const avifRWData& encoded_avif, avifDecoder* decoder, bool is_persistent,
+    bool give_size_hint, bool use_nth_image_api, const avifImage& reference,
+    uint32_t cell_height, bool enable_fine_incremental_check = false,
+    bool expect_whole_file_read = true,
+    bool expect_parse_success_from_partial_file = true);
 
 // Calls DecodeIncrementally() with the reference being a regular decoding of
 // encoded_avif.
 avifResult DecodeNonIncrementallyAndIncrementally(
     const avifRWData& encoded_avif, avifDecoder* decoder, bool is_persistent,
     bool give_size_hint, bool use_nth_image_api, uint32_t cell_height,
     bool enable_fine_incremental_check = false,
-    bool expect_whole_file_read = true);
+    bool expect_whole_file_read = true,
+    bool expect_parse_success_from_partial_file = true);
 
 }  // namespace testutil
 }  // namespace avif