From 6d265476e4dedce6d7659f6faac4af380eb7e4cb Mon Sep 17 00:00:00 2001 From: Colin Kho Date: Fri, 8 Nov 2024 10:21:26 -0800 Subject: [PATCH 1/7] Added Forwarding classes for Extractor and ExtractorOutput --- .../media3/extractor/ForwardingExtractor.java | 55 +++++++++++++++++++ .../extractor/ForwardingExtractorOutput.java | 44 +++++++++++++++ 2 files changed, 99 insertions(+) create mode 100644 libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractor.java create mode 100644 libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractorOutput.java diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractor.java b/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractor.java new file mode 100644 index 0000000000..26dd392c28 --- /dev/null +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractor.java @@ -0,0 +1,55 @@ +/* + * Copyright 2020 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 androidx.media3.extractor; + +import androidx.media3.common.util.UnstableApi; +import java.io.IOException; + +/** An overridable {@link Extractor} implementation forwarding all methods to an underlying Extractor. */ +@UnstableApi +public class ForwardingExtractor implements Extractor { + private final Extractor delegate; + + public ForwardingExtractor(Extractor delegate) { + this.delegate = delegate; + } + + @Override + public boolean sniff(ExtractorInput input) throws IOException { + return delegate.sniff(input); + } + + @Override + public void init(ExtractorOutput output) { + delegate.init(output); + } + + @Override + public @ReadResult int read(ExtractorInput input, PositionHolder seekPosition) + throws IOException { + return delegate.read(input, seekPosition); + } + + @Override + public void seek(long position, long timeUs) { + delegate.seek(position, timeUs); + } + + @Override + public void release() { + delegate.release(); + } +} diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractorOutput.java b/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractorOutput.java new file mode 100644 index 0000000000..6a23de48eb --- /dev/null +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractorOutput.java @@ -0,0 +1,44 @@ +/* + * Copyright 2020 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 androidx.media3.extractor; + +import androidx.media3.common.C; +import androidx.media3.common.util.UnstableApi; + +/** An overridable {@link ExtractorOutput} implementation forwarding all methods to another input. */ +@UnstableApi +public class ForwardingExtractorOutput implements ExtractorOutput { + private final ExtractorOutput output; + + public ForwardingExtractorOutput(ExtractorOutput output) { + this.output = output; + } + + @Override + public TrackOutput track(int id, @C.TrackType int type) { + return output.track(id, type); + } + + @Override + public void endTracks() { + output.endTracks(); + } + + @Override + public void seekMap(SeekMap seekMap) { + output.seekMap(seekMap); + } +} From 01422d83227cc39d9bbadbca1f9194aabe73332e Mon Sep 17 00:00:00 2001 From: Ian Baker Date: Thu, 28 Nov 2024 11:45:03 +0000 Subject: [PATCH 2/7] Reformat with google-java-format --- .../java/androidx/media3/extractor/ForwardingExtractor.java | 5 ++++- .../androidx/media3/extractor/ForwardingExtractorOutput.java | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractor.java b/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractor.java index 26dd392c28..0d57712fa9 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractor.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractor.java @@ -18,7 +18,10 @@ package androidx.media3.extractor; import androidx.media3.common.util.UnstableApi; import java.io.IOException; -/** An overridable {@link Extractor} implementation forwarding all methods to an underlying Extractor. */ +/** + * An overridable {@link Extractor} implementation forwarding all methods to an underlying + * Extractor. + */ @UnstableApi public class ForwardingExtractor implements Extractor { private final Extractor delegate; diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractorOutput.java b/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractorOutput.java index 6a23de48eb..6ad825dc08 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractorOutput.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractorOutput.java @@ -18,7 +18,9 @@ package androidx.media3.extractor; import androidx.media3.common.C; import androidx.media3.common.util.UnstableApi; -/** An overridable {@link ExtractorOutput} implementation forwarding all methods to another input. */ +/** + * An overridable {@link ExtractorOutput} implementation forwarding all methods to another input. + */ @UnstableApi public class ForwardingExtractorOutput implements ExtractorOutput { private final ExtractorOutput output; From 2683331299f973e0bc095e33d2fbc8e4ea3137f9 Mon Sep 17 00:00:00 2001 From: Ian Baker Date: Thu, 28 Nov 2024 14:00:36 +0000 Subject: [PATCH 3/7] Add tests and missing overrides to ForwardingExtractor --- .../media3/extractor/ForwardingExtractor.java | 11 +++++ .../ForwardingExtractorOutputTest.java | 23 ++++++++++ .../extractor/ForwardingExtractorTest.java | 22 ++++++++++ .../androidx/media3/test/utils/TestUtil.java | 42 +++++++++++++++++++ 4 files changed, 98 insertions(+) create mode 100644 libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingExtractorOutputTest.java create mode 100644 libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingExtractorTest.java diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractor.java b/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractor.java index 0d57712fa9..b8816cec06 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractor.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractor.java @@ -17,6 +17,7 @@ package androidx.media3.extractor; import androidx.media3.common.util.UnstableApi; import java.io.IOException; +import java.util.List; /** * An overridable {@link Extractor} implementation forwarding all methods to an underlying @@ -35,6 +36,11 @@ public class ForwardingExtractor implements Extractor { return delegate.sniff(input); } + @Override + public List getSniffFailureDetails() { + return delegate.getSniffFailureDetails(); + } + @Override public void init(ExtractorOutput output) { delegate.init(output); @@ -55,4 +61,9 @@ public class ForwardingExtractor implements Extractor { public void release() { delegate.release(); } + + @Override + public Extractor getUnderlyingImplementation() { + return delegate.getUnderlyingImplementation(); + } } diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingExtractorOutputTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingExtractorOutputTest.java new file mode 100644 index 0000000000..34bce79e8c --- /dev/null +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingExtractorOutputTest.java @@ -0,0 +1,23 @@ +package androidx.media3.extractor; + +import static androidx.media3.test.utils.TestUtil.assertForwardingClassForwardsAllMethods; +import static androidx.media3.test.utils.TestUtil.assertForwardingClassOverridesAllMethods; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(AndroidJUnit4.class) +public class ForwardingExtractorOutputTest { + + @Test + public void overridesAllMethods() throws Exception { + assertForwardingClassOverridesAllMethods( + ExtractorOutput.class, ForwardingExtractorOutput.class); + } + + @Test + public void forwardsAllMethods() throws Exception { + assertForwardingClassForwardsAllMethods(ExtractorOutput.class, ForwardingExtractorOutput::new); + } +} diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingExtractorTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingExtractorTest.java new file mode 100644 index 0000000000..38546bdc59 --- /dev/null +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingExtractorTest.java @@ -0,0 +1,22 @@ +package androidx.media3.extractor; + +import static androidx.media3.test.utils.TestUtil.assertForwardingClassForwardsAllMethods; +import static androidx.media3.test.utils.TestUtil.assertForwardingClassOverridesAllMethods; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(AndroidJUnit4.class) +public class ForwardingExtractorTest { + + @Test + public void overridesAllMethods() throws Exception { + assertForwardingClassOverridesAllMethods(Extractor.class, ForwardingExtractor.class); + } + + @Test + public void forwardsAllMethods() throws Exception { + assertForwardingClassForwardsAllMethods(Extractor.class, ForwardingExtractor::new); + } +} diff --git a/libraries/test_utils/src/main/java/androidx/media3/test/utils/TestUtil.java b/libraries/test_utils/src/main/java/androidx/media3/test/utils/TestUtil.java index 9608b662bd..3e5a49ff30 100644 --- a/libraries/test_utils/src/main/java/androidx/media3/test/utils/TestUtil.java +++ b/libraries/test_utils/src/main/java/androidx/media3/test/utils/TestUtil.java @@ -18,6 +18,8 @@ package androidx.media3.test.utils; import static androidx.media3.common.util.Assertions.checkNotNull; import static androidx.media3.common.util.Assertions.checkState; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import android.content.Context; import android.database.sqlite.SQLiteDatabase; @@ -36,6 +38,7 @@ import androidx.media3.common.MediaMetadata; import androidx.media3.common.StreamKey; import androidx.media3.common.Timeline; import androidx.media3.common.TrackGroup; +import androidx.media3.common.util.NullableType; import androidx.media3.common.util.UnstableApi; import androidx.media3.common.util.Util; import androidx.media3.database.DatabaseProvider; @@ -51,6 +54,7 @@ import androidx.media3.extractor.ExtractorInput; import androidx.media3.extractor.PositionHolder; import androidx.media3.extractor.SeekMap; import androidx.media3.extractor.metadata.MetadataInputBuffer; +import com.google.common.base.Function; import com.google.common.collect.BoundType; import com.google.common.collect.ImmutableList; import com.google.common.collect.Range; @@ -63,6 +67,8 @@ import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.lang.reflect.Array; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.nio.ByteBuffer; @@ -618,6 +624,42 @@ public class TestUtil { return list; } + /** + * Use reflection to assert that every method declared on {@code superType} is overridden by + * {@code forwardingType}. + */ + public static void assertForwardingClassOverridesAllMethods( + Class superType, Class forwardingType) throws NoSuchMethodException { + for (Method method : TestUtil.getPublicMethods(superType)) { + assertThat( + forwardingType + .getDeclaredMethod(method.getName(), method.getParameterTypes()) + .getDeclaringClass()) + .isEqualTo(forwardingType); + } + } + + /** + * Use reflection to assert calling every method declared on {@code superType} on an instance of + * {@code forwardingType} results in the call being forwarded to the {@code superType} delegate. + */ + public static void assertForwardingClassForwardsAllMethods( + Class superType, Function forwardingInstanceFactory) + throws InvocationTargetException, IllegalAccessException { + for (Method method : getPublicMethods(superType)) { + T delegate = mock(superType); + F forwardingInstance = forwardingInstanceFactory.apply(delegate); + @NullableType Object[] parameters = new Object[method.getParameterCount()]; + for (int i = 0; i < method.getParameterCount(); i++) { + // Get a default value of the right type by creating a single-element array. This gives us + // null for object types, and the 'default' value for primitives. + parameters[i] = Array.get(Array.newInstance(method.getParameterTypes()[i], 1), 0); + } + method.invoke(forwardingInstance, parameters); + method.invoke(verify(delegate), parameters); + } + } + /** Returns a {@link MediaItem} that has all fields set to non-default values. */ public static MediaItem buildFullyCustomizedMediaItem() { return new MediaItem.Builder() From c5af8ba360ee45f5227daab881e96887b75bb023 Mon Sep 17 00:00:00 2001 From: Ian Baker Date: Thu, 28 Nov 2024 14:04:38 +0000 Subject: [PATCH 4/7] Add & fix copyright headers --- .../media3/extractor/ForwardingExtractor.java | 2 +- .../extractor/ForwardingExtractorOutput.java | 2 +- .../extractor/ForwardingExtractorOutputTest.java | 15 +++++++++++++++ .../media3/extractor/ForwardingExtractorTest.java | 15 +++++++++++++++ 4 files changed, 32 insertions(+), 2 deletions(-) diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractor.java b/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractor.java index b8816cec06..9e4a40a5ef 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractor.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractor.java @@ -1,5 +1,5 @@ /* - * Copyright 2020 The Android Open Source Project + * Copyright 2024 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. diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractorOutput.java b/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractorOutput.java index 6ad825dc08..f18f4bf2f1 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractorOutput.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractorOutput.java @@ -1,5 +1,5 @@ /* - * Copyright 2020 The Android Open Source Project + * Copyright 2024 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. diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingExtractorOutputTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingExtractorOutputTest.java index 34bce79e8c..16b6a71139 100644 --- a/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingExtractorOutputTest.java +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingExtractorOutputTest.java @@ -1,3 +1,18 @@ +/* + * Copyright 2024 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 androidx.media3.extractor; import static androidx.media3.test.utils.TestUtil.assertForwardingClassForwardsAllMethods; diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingExtractorTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingExtractorTest.java index 38546bdc59..b64cd7c7ec 100644 --- a/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingExtractorTest.java +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/ForwardingExtractorTest.java @@ -1,3 +1,18 @@ +/* + * Copyright 2024 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 androidx.media3.extractor; import static androidx.media3.test.utils.TestUtil.assertForwardingClassForwardsAllMethods; From 037a6f3219d225a39514f1118dd92306fac9ca54 Mon Sep 17 00:00:00 2001 From: Ian Baker Date: Thu, 28 Nov 2024 14:47:35 +0000 Subject: [PATCH 5/7] Fix nullness errors --- .../java/androidx/media3/test/utils/TestUtil.java | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/libraries/test_utils/src/main/java/androidx/media3/test/utils/TestUtil.java b/libraries/test_utils/src/main/java/androidx/media3/test/utils/TestUtil.java index 3e5a49ff30..8214887d4c 100644 --- a/libraries/test_utils/src/main/java/androidx/media3/test/utils/TestUtil.java +++ b/libraries/test_utils/src/main/java/androidx/media3/test/utils/TestUtil.java @@ -84,6 +84,7 @@ import java.util.Random; import java.util.Set; import java.util.UUID; import java.util.concurrent.ExecutionException; +import org.checkerframework.checker.nullness.qual.NonNull; /** Utility methods for tests. */ @UnstableApi @@ -643,9 +644,16 @@ public class TestUtil { * Use reflection to assert calling every method declared on {@code superType} on an instance of * {@code forwardingType} results in the call being forwarded to the {@code superType} delegate. */ - public static void assertForwardingClassForwardsAllMethods( - Class superType, Function forwardingInstanceFactory) - throws InvocationTargetException, IllegalAccessException { + // The nullness checker is deliberately over-conservative and doesn't permit passing a null + // parameter to method.invoke(), even if the real method does accept null. Regardless, we expect + // the null to be passed straight to our mocked delegate, so it's OK to pass null even for + // non-null parameters. See also + // https://github.com/typetools/checker-framework/blob/c26bb695ebc572fac1e9cd2e331fc5b9d3953ec0/checker/jdk/nullness/src/java/lang/reflect/Method.java#L109 + @SuppressWarnings("nullness:argument.type.incompatible") + public static + void assertForwardingClassForwardsAllMethods( + Class superType, Function forwardingInstanceFactory) + throws InvocationTargetException, IllegalAccessException { for (Method method : getPublicMethods(superType)) { T delegate = mock(superType); F forwardingInstance = forwardingInstanceFactory.apply(delegate); From 21e627da1c3d503eed80fd61f79df9fe0e80148a Mon Sep 17 00:00:00 2001 From: Ian Baker Date: Thu, 28 Nov 2024 16:54:00 +0000 Subject: [PATCH 6/7] Fix review comments --- .../src/main/java/androidx/media3/test/utils/TestUtil.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libraries/test_utils/src/main/java/androidx/media3/test/utils/TestUtil.java b/libraries/test_utils/src/main/java/androidx/media3/test/utils/TestUtil.java index 8214887d4c..1d5d45a5f1 100644 --- a/libraries/test_utils/src/main/java/androidx/media3/test/utils/TestUtil.java +++ b/libraries/test_utils/src/main/java/androidx/media3/test/utils/TestUtil.java @@ -664,6 +664,9 @@ public class TestUtil { parameters[i] = Array.get(Array.newInstance(method.getParameterTypes()[i], 1), 0); } method.invoke(forwardingInstance, parameters); + + // Reflective version of verify(delegate).method(parameters), to assert the expected method + // was invoked on the delegate instance. method.invoke(verify(delegate), parameters); } } From 7f62f7473778befdc086fa09347fe6ae72f170cc Mon Sep 17 00:00:00 2001 From: Ian Baker Date: Fri, 29 Nov 2024 17:10:29 +0000 Subject: [PATCH 7/7] Fix review comments --- .../java/androidx/media3/extractor/ForwardingExtractor.java | 4 ++-- .../androidx/media3/extractor/ForwardingExtractorOutput.java | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractor.java b/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractor.java index 9e4a40a5ef..70d3c465a6 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractor.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractor.java @@ -20,8 +20,8 @@ import java.io.IOException; import java.util.List; /** - * An overridable {@link Extractor} implementation forwarding all methods to an underlying - * Extractor. + * An overridable {@link Extractor} implementation which forwards all methods to another {@link + * Extractor}. */ @UnstableApi public class ForwardingExtractor implements Extractor { diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractorOutput.java b/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractorOutput.java index f18f4bf2f1..46ae0e7653 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractorOutput.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/ForwardingExtractorOutput.java @@ -19,7 +19,8 @@ import androidx.media3.common.C; import androidx.media3.common.util.UnstableApi; /** - * An overridable {@link ExtractorOutput} implementation forwarding all methods to another input. + * An overridable {@link ExtractorOutput} implementation which forwards all methods to another + * {@link ExtractorOutput}. */ @UnstableApi public class ForwardingExtractorOutput implements ExtractorOutput {