From 687a50e9b313e5db37a0b32f5f40559252ab6031 Mon Sep 17 00:00:00 2001
From: huangdarwin <huangdarwin@google.com>
Date: Fri, 29 Jul 2022 19:33:44 +0000
Subject: [PATCH] HDR: Have @C.ColorTransfer check consider NO_VALUE as SDR.

Format.NO_VALUE is a placeholder value for an invalid @C.ColorTransfer, used
for example when the decoder doesn't support this transfer function.

When encountering this invalid value, interpret this as COLOR_TRANSFER_SDR.

Confirmed locally that an exception is thrown when transcoding on p4head, and no exception is thrown when transcoding with this CL.

PiperOrigin-RevId: 464135080
---
 .../media3/common/util/MediaFormatUtil.java      |  2 ++
 .../mh/SetHdrEditingTransformationTest.java      | 10 ++++++++--
 .../media3/transformer/DefaultCodec.java         | 16 ++++++++++------
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/libraries/common/src/main/java/androidx/media3/common/util/MediaFormatUtil.java b/libraries/common/src/main/java/androidx/media3/common/util/MediaFormatUtil.java
index 3e526c7c28..b1d56ddb01 100644
--- a/libraries/common/src/main/java/androidx/media3/common/util/MediaFormatUtil.java
+++ b/libraries/common/src/main/java/androidx/media3/common/util/MediaFormatUtil.java
@@ -213,6 +213,8 @@ public final class MediaFormatUtil {
     @Nullable
     byte[] hdrStaticInfo =
         hdrStaticInfoByteBuffer != null ? getArray(hdrStaticInfoByteBuffer) : null;
+    // Some devices may produce invalid values from MediaFormat#getInteger.
+    // See b/239435670 for more information.
     if (!isValidColorSpace(colorSpace)) {
       colorSpace = Format.NO_VALUE;
     }
diff --git a/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/SetHdrEditingTransformationTest.java b/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/SetHdrEditingTransformationTest.java
index e8d33dc35f..68b9fdcf3c 100644
--- a/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/SetHdrEditingTransformationTest.java
+++ b/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/SetHdrEditingTransformationTest.java
@@ -16,6 +16,7 @@
 package androidx.media3.transformer.mh;
 
 import static androidx.media3.transformer.AndroidTestUtil.MP4_ASSET_1080P_1_SECOND_HDR10_VIDEO_SDR_CONTAINER;
+import static androidx.media3.transformer.AndroidTestUtil.recordTestSkipped;
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertThrows;
 
@@ -36,9 +37,14 @@ import org.junit.runner.RunWith;
 @RunWith(AndroidJUnit4.class)
 public class SetHdrEditingTransformationTest {
   @Test
-  public void videoDecoderUnexpectedColorInfo_completesWithError() {
+  public void videoDecoderUnexpectedColorInfo_completesWithError() throws Exception {
     Context context = ApplicationProvider.getApplicationContext();
-    if (Util.SDK_INT < 24) {
+    if (Util.SDK_INT < 29) {
+      recordTestSkipped(
+          context,
+          "SetHdrEditingTransformationTest",
+          /* reason= */ "Skipping on this API version due to lack of support for"
+              + " MediaFormat#getInteger(String, int).");
       return;
     }
 
diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/DefaultCodec.java b/libraries/transformer/src/main/java/androidx/media3/transformer/DefaultCodec.java
index 71a809dfc5..c8331510a9 100644
--- a/libraries/transformer/src/main/java/androidx/media3/transformer/DefaultCodec.java
+++ b/libraries/transformer/src/main/java/androidx/media3/transformer/DefaultCodec.java
@@ -320,7 +320,7 @@ public final class DefaultCodec implements Codec {
     if (outputBufferIndex < 0) {
       if (outputBufferIndex == MediaCodec.INFO_OUTPUT_FORMAT_CHANGED) {
         outputFormat = convertToFormat(mediaCodec.getOutputFormat());
-        if (!isColorTransferEqual(configuredOutputColor, outputFormat.colorInfo)) {
+        if (!areColorTransfersEqual(configuredOutputColor, outputFormat.colorInfo)) {
           // TODO(b/237674316): These exceptions throw when the container ColorInfo doesn't match
           //  the video ColorInfo. Instead of throwing when seeing unexpected ColorInfos, consider
           //  reconfiguring downstream components (ex. FrameProcessor and encoder) when different
@@ -374,12 +374,16 @@ public final class DefaultCodec implements Codec {
             : TransformationException.ERROR_CODE_ENCODING_FAILED);
   }
 
-  private static boolean isColorTransferEqual(
+  private static boolean areColorTransfersEqual(
       @Nullable ColorInfo colorInfo1, @Nullable ColorInfo colorInfo2) {
-    @C.ColorTransfer
-    int transfer1 = (colorInfo1 != null) ? colorInfo1.colorTransfer : C.COLOR_TRANSFER_SDR;
-    @C.ColorTransfer
-    int transfer2 = (colorInfo2 != null) ? colorInfo2.colorTransfer : C.COLOR_TRANSFER_SDR;
+    @C.ColorTransfer int transfer1 = C.COLOR_TRANSFER_SDR;
+    if (colorInfo1 != null && colorInfo1.colorTransfer != Format.NO_VALUE) {
+      transfer1 = colorInfo1.colorTransfer;
+    }
+    @C.ColorTransfer int transfer2 = C.COLOR_TRANSFER_SDR;
+    if (colorInfo2 != null && colorInfo2.colorTransfer != Format.NO_VALUE) {
+      transfer2 = colorInfo2.colorTransfer;
+    }
     return transfer1 == transfer2;
   }