Bug 1729071 - AVIF with multiple ColourInformationBox (colr) entries …

…per item are rejected. r=aosmond The logic to allow parsing of the multiple colr boxes is in the parent update of mp4parse-rust to 72eb355. This change adds a confirmatory test in gecko and updates telemetry to add new new "both" case for colr tracking. Differential Revision: https://phabricator.services.mozilla.com/D125744
Brli · Sep 16, 2021 · c5728b6 · c5728b6
1 parent 44c5255
commit c5728b6
Show file tree

Hide file tree

Showing 8 changed files with 49 additions and 25 deletions.
diff --git a/image/decoders/nsAVIFDecoder.cpp b/image/decoders/nsAVIFDecoder.cpp
@@ -391,13 +391,13 @@ void AVIFDecodedData::SetCicpValues(
     if (aAv1ColourPrimaries != CICP::ColourPrimaries::CP_UNSPECIFIED) {
       cp = aAv1ColourPrimaries;
       MOZ_LOG(sAVIFLog, LogLevel::Info,
-              ("No colour_primaries value specified in colr box, "
+              ("Unspecified colour_primaries value specified in colr box, "
                "using AV1 sequence header (%hhu)",
                cp));
     } else {
       cp = CICP::ColourPrimaries::CP_BT709;
       MOZ_LOG(sAVIFLog, LogLevel::Warning,
-              ("No colour_primaries value specified in colr box "
+              ("Unspecified colour_primaries value specified in colr box "
                "or AV1 sequence header, using fallback value (%hhu)",
                cp));
     }
@@ -413,13 +413,13 @@ void AVIFDecodedData::SetCicpValues(
         CICP::TransferCharacteristics::TC_UNSPECIFIED) {
       tc = aAv1TransferCharacteristics;
       MOZ_LOG(sAVIFLog, LogLevel::Info,
-              ("No transfer_characteristics value specified in "
+              ("Unspecified transfer_characteristics value specified in "
                "colr box, using AV1 sequence header (%hhu)",
                tc));
     } else {
       tc = CICP::TransferCharacteristics::TC_SRGB;
       MOZ_LOG(sAVIFLog, LogLevel::Warning,
-              ("No transfer_characteristics value specified in "
+              ("Unspecified transfer_characteristics value specified in "
                "colr box or AV1 sequence header, using fallback value (%hhu)",
                tc));
     }
@@ -434,13 +434,13 @@ void AVIFDecodedData::SetCicpValues(
     if (aAv1MatrixCoefficients != CICP::MatrixCoefficients::MC_UNSPECIFIED) {
       mc = aAv1MatrixCoefficients;
       MOZ_LOG(sAVIFLog, LogLevel::Info,
-              ("No matrix_coefficients value specified in "
+              ("Unspecified matrix_coefficients value specified in "
                "colr box, using AV1 sequence header (%hhu)",
                mc));
     } else {
       mc = CICP::MatrixCoefficients::MC_BT601;
       MOZ_LOG(sAVIFLog, LogLevel::Warning,
-              ("No matrix_coefficients value specified in "
+              ("Unspecified matrix_coefficients value specified in "
                "colr box or AV1 sequence header, using fallback value (%hhu)",
                mc));
     }
@@ -1229,6 +1229,28 @@ nsAVIFDecoder::DecodeResult nsAVIFDecoder::Decode(
   }
   const Mp4parseAvifImage& parsedImg = *parsedImagePtr;
 
+  if (parsedImg.icc_colour_information.data) {
+    const auto& icc = parsedImg.icc_colour_information;
+    MOZ_LOG(
+        sAVIFLog, LogLevel::Debug,
+        ("[this=%p] colr type ICC: %zu bytes %p", this, icc.length, icc.data));
+  }
+
+  if (parsedImg.nclx_colour_information) {
+    const auto& nclx = *parsedImg.nclx_colour_information;
+    MOZ_LOG(
+        sAVIFLog, LogLevel::Debug,
+        ("[this=%p] colr type CICP: cp/tc/mc/full-range %u/%u/%u/%s", this,
+         nclx.colour_primaries, nclx.transfer_characteristics,
+         nclx.matrix_coefficients, nclx.full_range_flag ? "true" : "false"));
+  }
+
+  if (!parsedImg.icc_colour_information.data &&
+      !parsedImg.nclx_colour_information) {
+    MOZ_LOG(sAVIFLog, LogLevel::Debug,
+            ("[this=%p] colr box not present", this));
+  }
+
   if (parsedImg.alpha_image.coded_data.data) {
     PostHasTransparency();
   }
@@ -1280,23 +1302,6 @@ nsAVIFDecoder::DecodeResult nsAVIFDecoder::Decode(
            StaticPrefs::image_avif_use_dav1d() ? "Dav1d" : "AOM",
            IsDecodeSuccess(r) ? "succeeds" : "fails"));
 
-  if (parsedImg.icc_colour_information.data) {
-    auto& icc = parsedImg.icc_colour_information;
-    MOZ_LOG(
-        sAVIFLog, LogLevel::Debug,
-        ("[this=%p] colr type ICC: %zu bytes %p", this, icc.length, icc.data));
-  } else if (parsedImg.nclx_colour_information) {
-    auto& nclx = *parsedImg.nclx_colour_information;
-    MOZ_LOG(
-        sAVIFLog, LogLevel::Debug,
-        ("[this=%p] colr type CICP: cp/tc/mc/full-range %u/%u/%u/%s", this,
-         nclx.colour_primaries, nclx.transfer_characteristics,
-         nclx.matrix_coefficients, nclx.full_range_flag ? "true" : "false"));
-  } else {
-    MOZ_LOG(sAVIFLog, LogLevel::Debug,
-            ("[this=%p] colr box not present", this));
-  }
-
   if (!IsDecodeSuccess(r)) {
     return r;
   }
@@ -1367,7 +1372,10 @@ nsAVIFDecoder::DecodeResult nsAVIFDecoder::Decode(
   IntSize rgbSize = Size();
   MOZ_ASSERT(rgbSize == decodedData.mPicSize);
 
-  if (parsedImg.nclx_colour_information) {
+  if (parsedImg.nclx_colour_information &&
+      parsedImg.icc_colour_information.data) {
+    AccumulateCategorical(LABELS_AVIF_COLR::both);
+  } else if (parsedImg.nclx_colour_information) {
     AccumulateCategorical(LABELS_AVIF_COLR::nclx);
   } else if (parsedImg.icc_colour_information.data) {
     AccumulateCategorical(LABELS_AVIF_COLR::icc);

diff --git a/image/test/gtest/Common.cpp b/image/test/gtest/Common.cpp
@@ -448,6 +448,13 @@ ImageTestCase NonzeroReservedAVIFTestCase() {
   return testCase;
 }
 
+ImageTestCase MultipleColrAVIFTestCase() {
+  auto testCase = ImageTestCase("valid-avif-colr-nclx-and-prof.avif",
+                                "image/avif", IntSize(1, 1));
+  testCase.mColor = BGRAColor(0x00, 0x00, 0x00, 0xFF);
+  return testCase;
+}
+
 ImageTestCase Transparent10bit420AVIFTestCase() {
   auto testCase =
       ImageTestCase("transparent-green-50pct-10bit-yuv420.avif", "image/avif",

diff --git a/image/test/gtest/Common.h b/image/test/gtest/Common.h
@@ -478,6 +478,7 @@ ImageTestCase GreenWebPTestCase();
 ImageTestCase GreenAVIFTestCase();
 
 ImageTestCase NonzeroReservedAVIFTestCase();
+ImageTestCase MultipleColrAVIFTestCase();
 ImageTestCase Transparent10bit420AVIFTestCase();
 ImageTestCase Transparent10bit422AVIFTestCase();
 ImageTestCase Transparent10bit444AVIFTestCase();

diff --git a/image/test/gtest/TestDecoders.cpp b/image/test/gtest/TestDecoders.cpp
@@ -712,6 +712,10 @@ TEST_F(ImageDecoders, AVIFSingleChunkNonzeroReserved) {
   CheckDecoderSingleChunk(NonzeroReservedAVIFTestCase());
 }
 
+TEST_F(ImageDecoders, AVIFSingleChunkMultipleColr) {
+  CheckDecoderSingleChunk(MultipleColrAVIFTestCase());
+}
+
 TEST_F(ImageDecoders, AVIFSingleChunkTransparent10bit420) {
   CheckDecoderSingleChunk(Transparent10bit420AVIFTestCase());
 }

diff --git a/image/test/gtest/moz.build b/image/test/gtest/moz.build
@@ -139,6 +139,7 @@ TEST_HARNESS_FILES.gtest += [
     "transparent.jxl",
     "transparent.png",
     "transparent.webp",
+    "valid-avif-colr-nclx-and-prof.avif",
 ]
 
 include("/ipc/chromium/chromium-config.mozbuild")

diff --git a/image/test/gtest/valid-avif-colr-nclx-and-prof.avif b/image/test/gtest/valid-avif-colr-nclx-and-prof.avif
diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json
@@ -1813,7 +1813,7 @@
     "expires_in_version": "never",
     "releaseChannelCollection": "opt-out",
     "kind": "categorical",
-    "labels": ["nclx", "icc", "absent"],
+    "labels": ["nclx", "icc", "absent", "both"],
     "description": "AVIF colour information type",
     "bug_numbers": [1696045]
   },

diff --git a/toolkit/components/telemetry/geckoview/streaming/metrics.yaml b/toolkit/components/telemetry/geckoview/streaming/metrics.yaml
@@ -971,13 +971,16 @@ avif:
       - nclx
       - icc
       - absent
+      - both
     gecko_datapoint: AVIF_COLR
     description: >
       AVIF colour information type.
     bugs:
       - https://bugzilla.mozilla.org/show_bug.cgi?id=1696045
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1729071
     data_reviews:
       - https://bugzilla.mozilla.org/show_bug.cgi?id=1696045#c5
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1729071#c15
     notification_emails:
       - cchang@mozilla.com
       - jbauman@mozilla.com