From 8548e3519e3d6ef214d5c18d640a8bf6efe19c83 Mon Sep 17 00:00:00 2001 From: huangdarwin Date: Fri, 7 Oct 2022 13:57:06 +0000 Subject: [PATCH] HDR: Disable tone mapping on unsupported pixel build ID. Also, update tests to allow AnyOf error codes, and no longer check exception messages, which caused quite a bit of churn. PiperOrigin-RevId: 479570861 --- .../androidx/media3/common/util/Util.java | 4 +- .../transformer/mh/SetHdrEditingTest.java | 25 +++--------- .../mh/SetHdrToSdrToneMapTest.java | 39 ++++--------------- .../VideoTranscodingSamplePipeline.java | 11 +++++- 4 files changed, 24 insertions(+), 55 deletions(-) diff --git a/libraries/common/src/main/java/androidx/media3/common/util/Util.java b/libraries/common/src/main/java/androidx/media3/common/util/Util.java index 018e1a475a..138a2d9c9c 100644 --- a/libraries/common/src/main/java/androidx/media3/common/util/Util.java +++ b/libraries/common/src/main/java/androidx/media3/common/util/Util.java @@ -125,8 +125,8 @@ import org.checkerframework.checker.nullness.qual.PolyNull; public final class Util { /** - * Like {@link android.os.Build.VERSION#SDK_INT}, but in a place where it can be conveniently - * overridden for local testing. + * Like {@link Build.VERSION#SDK_INT}, but in a place where it can be conveniently overridden for + * local testing. */ @UnstableApi public static final int SDK_INT = Build.VERSION.SDK_INT; diff --git a/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/SetHdrEditingTest.java b/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/SetHdrEditingTest.java index a62ba5acd1..39a50131b1 100644 --- a/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/SetHdrEditingTest.java +++ b/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/SetHdrEditingTest.java @@ -77,10 +77,6 @@ public class SetHdrEditingTest { assertThat(exception).hasCauseThat().isInstanceOf(IllegalArgumentException.class); assertThat(exception.errorCode) .isEqualTo(TransformationException.ERROR_CODE_HDR_EDITING_UNSUPPORTED); - assertThat(exception) - .hasCauseThat() - .hasMessageThat() - .isEqualTo("HDR editing and tone mapping not supported under API 31."); } } @@ -155,22 +151,11 @@ public class SetHdrEditingTest { } catch (TransformationException exception) { Log.i(TAG, checkNotNull(exception.getCause()).toString()); assertThat(exception).hasCauseThat().isInstanceOf(IllegalArgumentException.class); - if (Util.SDK_INT < 31) { - assertThat(exception.errorCode) - .isEqualTo(TransformationException.ERROR_CODE_HDR_EDITING_UNSUPPORTED); - assertThat(exception) - .hasCauseThat() - .hasMessageThat() - .isEqualTo("HDR editing and tone mapping not supported under API 31."); - } else { - assertThat(exception.errorCode) - .isEqualTo(TransformationException.ERROR_CODE_DECODING_FORMAT_UNSUPPORTED); - assertThat(exception) - .hasCauseThat() - .hasMessageThat() - .isEqualTo("Tone-mapping requested but not supported by the decoder."); - assertThat(isFallbackListenerInvoked.get()).isFalse(); - } + assertThat(exception.errorCode) + .isAnyOf( + TransformationException.ERROR_CODE_HDR_EDITING_UNSUPPORTED, + TransformationException.ERROR_CODE_DECODING_FORMAT_UNSUPPORTED); + assertThat(isFallbackListenerInvoked.get()).isFalse(); return; } } diff --git a/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/SetHdrToSdrToneMapTest.java b/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/SetHdrToSdrToneMapTest.java index a1a40d2802..08f84d858d 100644 --- a/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/SetHdrToSdrToneMapTest.java +++ b/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/SetHdrToSdrToneMapTest.java @@ -25,7 +25,6 @@ import android.net.Uri; import androidx.media3.common.C; import androidx.media3.common.MediaItem; import androidx.media3.common.util.Log; -import androidx.media3.common.util.Util; import androidx.media3.transformer.TransformationException; import androidx.media3.transformer.TransformationRequest; import androidx.media3.transformer.TransformationTestResult; @@ -76,21 +75,10 @@ public class SetHdrToSdrToneMapTest { } catch (TransformationException exception) { Log.i(TAG, checkNotNull(exception.getCause()).toString()); assertThat(exception).hasCauseThat().isInstanceOf(IllegalArgumentException.class); - if (Util.SDK_INT < 31) { - assertThat(exception.errorCode) - .isEqualTo(TransformationException.ERROR_CODE_HDR_EDITING_UNSUPPORTED); - assertThat(exception) - .hasCauseThat() - .hasMessageThat() - .isEqualTo("HDR editing and tone mapping not supported under API 31."); - } else { - assertThat(exception.errorCode) - .isEqualTo(TransformationException.ERROR_CODE_DECODING_FORMAT_UNSUPPORTED); - assertThat(exception) - .hasCauseThat() - .hasMessageThat() - .isEqualTo("Tone-mapping requested but not supported by the decoder."); - } + assertThat(exception.errorCode) + .isAnyOf( + TransformationException.ERROR_CODE_HDR_EDITING_UNSUPPORTED, + TransformationException.ERROR_CODE_DECODING_FORMAT_UNSUPPORTED); return; } } @@ -133,21 +121,10 @@ public class SetHdrToSdrToneMapTest { } catch (TransformationException exception) { Log.i(TAG, checkNotNull(exception.getCause()).toString()); assertThat(exception).hasCauseThat().isInstanceOf(IllegalArgumentException.class); - if (Util.SDK_INT < 31) { - assertThat(exception.errorCode) - .isEqualTo(TransformationException.ERROR_CODE_HDR_EDITING_UNSUPPORTED); - assertThat(exception) - .hasCauseThat() - .hasMessageThat() - .isEqualTo("HDR editing and tone mapping not supported under API 31."); - } else { - assertThat(exception.errorCode) - .isEqualTo(TransformationException.ERROR_CODE_DECODING_FORMAT_UNSUPPORTED); - assertThat(exception) - .hasCauseThat() - .hasMessageThat() - .isEqualTo("Tone-mapping requested but not supported by the decoder."); - } + assertThat(exception.errorCode) + .isAnyOf( + TransformationException.ERROR_CODE_HDR_EDITING_UNSUPPORTED, + TransformationException.ERROR_CODE_DECODING_FORMAT_UNSUPPORTED); return; } } diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/VideoTranscodingSamplePipeline.java b/libraries/transformer/src/main/java/androidx/media3/transformer/VideoTranscodingSamplePipeline.java index 0c10c8c9a5..2c786483dc 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/VideoTranscodingSamplePipeline.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/VideoTranscodingSamplePipeline.java @@ -22,6 +22,7 @@ import static androidx.media3.common.util.Util.SDK_INT; import android.content.Context; import android.media.MediaCodec; +import android.os.Build; import android.view.Surface; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; @@ -76,9 +77,10 @@ import org.checkerframework.dataflow.qual.Pure; Transformer.AsyncErrorListener asyncErrorListener, DebugViewProvider debugViewProvider) throws TransformationException { - if (SDK_INT < 31 && ColorInfo.isTransferHdr(inputFormat.colorInfo)) { + if (ColorInfo.isTransferHdr(inputFormat.colorInfo) + && (SDK_INT < 31 || deviceNeedsNoToneMappingWorkaround())) { throw TransformationException.createForCodec( - new IllegalArgumentException("HDR editing and tone mapping not supported under API 31."), + new IllegalArgumentException("HDR editing and tone mapping not supported."), /* isVideo= */ true, /* isDecoder= */ false, inputFormat, @@ -290,6 +292,11 @@ import org.checkerframework.dataflow.qual.Pure; .build(); } + private static boolean deviceNeedsNoToneMappingWorkaround() { + // Pixel build ID does not support tone mapping. See http://b/249297370#comment8. + return Build.ID.startsWith("TP1A.220905.004"); + } + /** * Feeds at most one decoder output frame to the next step of the pipeline. *