From 4145273bc4ad88cd44578312ba9311118054c3b6 Mon Sep 17 00:00:00 2001 From: hschlueter Date: Mon, 17 Jan 2022 12:23:05 +0000 Subject: [PATCH] Revise TransformationRequest MIME type validation. PiperOrigin-RevId: 422333929 --- .../transformer/TransformationRequest.java | 23 +++++++--- .../TransformationRequestBuilderTest.java | 43 +++++++++++++++++++ .../transformer/TransformerBuilderTest.java | 4 -- 3 files changed, 59 insertions(+), 11 deletions(-) create mode 100644 library/transformer/src/test/java/com/google/android/exoplayer2/transformer/TransformationRequestBuilderTest.java diff --git a/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/TransformationRequest.java b/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/TransformationRequest.java index a870365754..f623365fce 100644 --- a/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/TransformationRequest.java +++ b/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/TransformationRequest.java @@ -16,6 +16,8 @@ package com.google.android.exoplayer2.transformer; +import static com.google.android.exoplayer2.util.Assertions.checkArgument; + import android.graphics.Matrix; import androidx.annotation.Nullable; import com.google.android.exoplayer2.C; @@ -128,6 +130,7 @@ public final class TransformationRequest { * * @param outputHeight The output height in pixels. * @return This builder. + * @throws IllegalArgumentException If the {@code outputHeight} is not supported. */ public Builder setResolution(int outputHeight) { // TODO(b/209781577): Define outputHeight in the javadoc as height can be ambiguous for videos @@ -135,9 +138,9 @@ public final class TransformationRequest { // TODO(b/201293185): Restructure to input a Presentation class. // TODO(b/201293185): Check encoder codec capabilities in order to allow arbitrary // resolutions and reasonable fallbacks. - if (outputHeight != C.LENGTH_UNSET && !SUPPORTED_OUTPUT_HEIGHTS.contains(outputHeight)) { - throw new IllegalArgumentException("Unsupported outputHeight: " + outputHeight); - } + checkArgument( + outputHeight == C.LENGTH_UNSET || SUPPORTED_OUTPUT_HEIGHTS.contains(outputHeight), + "Unsupported outputHeight: " + outputHeight); this.outputHeight = outputHeight; return this; } @@ -155,10 +158,13 @@ public final class TransformationRequest { * * @param videoMimeType The MIME type of the video samples in the output. * @return This builder. + * @throws IllegalArgumentException If the {@code videoMimeType} is non-null but not a video + * {@link MimeTypes MIME type}. */ public Builder setVideoMimeType(@Nullable String videoMimeType) { - // TODO(b/209469847): Validate videoMimeType here once deprecated - // Transformer.Builder#setOuputMimeType(String) has been removed. + checkArgument( + videoMimeType == null || MimeTypes.isVideo(videoMimeType), + "Not a video MIME type: " + videoMimeType); this.videoMimeType = videoMimeType; return this; } @@ -175,10 +181,13 @@ public final class TransformationRequest { * * @param audioMimeType The MIME type of the audio samples in the output. * @return This builder. + * @throws IllegalArgumentException If the {@code audioMimeType} is non-null but not an audio + * {@link MimeTypes MIME type}. */ public Builder setAudioMimeType(@Nullable String audioMimeType) { - // TODO(b/209469847): Validate audioMimeType here once deprecated - // Transformer.Builder#setOuputMimeType(String) has been removed. + checkArgument( + audioMimeType == null || MimeTypes.isAudio(audioMimeType), + "Not an audio MIME type: " + audioMimeType); this.audioMimeType = audioMimeType; return this; } diff --git a/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/TransformationRequestBuilderTest.java b/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/TransformationRequestBuilderTest.java new file mode 100644 index 0000000000..b55125c98a --- /dev/null +++ b/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/TransformationRequestBuilderTest.java @@ -0,0 +1,43 @@ +/* + * Copyright 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.android.exoplayer2.transformer; + +import static org.junit.Assert.assertThrows; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import com.google.android.exoplayer2.util.MimeTypes; +import org.junit.Test; +import org.junit.runner.RunWith; + +/** Unit test for {@link TransformationRequest.Builder}. */ +@RunWith(AndroidJUnit4.class) +public class TransformationRequestBuilderTest { + + @Test + public void setAudioMimeType_withVideoMimeType_throws() { + assertThrows( + IllegalArgumentException.class, + () -> new TransformationRequest.Builder().setAudioMimeType(MimeTypes.VIDEO_H264)); + } + + @Test + public void setVideoMimeType_withAudioMimeType_throws() { + assertThrows( + IllegalArgumentException.class, + () -> new TransformationRequest.Builder().setVideoMimeType(MimeTypes.AUDIO_AAC)); + } +} diff --git a/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/TransformerBuilderTest.java b/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/TransformerBuilderTest.java index bff87ff35e..e96f136fdd 100644 --- a/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/TransformerBuilderTest.java +++ b/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/TransformerBuilderTest.java @@ -52,8 +52,6 @@ public class TransformerBuilderTest { () -> new Transformer.Builder(context).setRemoveAudio(true).setRemoveVideo(true).build()); } - // TODO(b/209469847): Move this test to TransformationRequestBuilderTest once deprecated - // Transformer.Builder#setOuputMimeType(String) has been removed. @Test public void build_withUnsupportedAudioMimeType_throws() { Context context = ApplicationProvider.getApplicationContext(); @@ -68,8 +66,6 @@ public class TransformerBuilderTest { .build()); } - // TODO(b/209469847): Move this test to TransformationRequestBuilderTest once deprecated - // Transformer.Builder#setOuputMimeType(String) has been removed. @Test public void build_withUnsupportedVideoMimeType_throws() { Context context = ApplicationProvider.getApplicationContext();