From 6be0d6ea25a850b816b23105aa900683d30dfe8d Mon Sep 17 00:00:00 2001 From: tonihei Date: Wed, 13 Jul 2022 15:27:55 +0000 Subject: [PATCH] Fix setDataSourceFactory handling in DefaultMediaSourceFactory The call doesn't currently reset the already loaded suppliers and factories. Also fix the supplier loading code to use a local copy of the current dataSourceFactory to avoid leaking an updated instance to a later invocation. Issue: androidx/media#116 #minor-release PiperOrigin-RevId: 460721541 --- .../source/DefaultMediaSourceFactory.java | 13 ++--- .../dash/DefaultMediaSourceFactoryTest.java | 54 +++++++++++++++++++ .../hls/DefaultMediaSourceFactoryTest.java | 54 +++++++++++++++++++ library/smoothstreaming/build.gradle | 1 + .../DefaultMediaSourceFactoryTest.java | 54 +++++++++++++++++++ 5 files changed, 170 insertions(+), 6 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/DefaultMediaSourceFactory.java b/library/core/src/main/java/com/google/android/exoplayer2/source/DefaultMediaSourceFactory.java index ee9d1e08a2..bd6c35af3a 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/DefaultMediaSourceFactory.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/DefaultMediaSourceFactory.java @@ -274,6 +274,7 @@ public final class DefaultMediaSourceFactory implements MediaSourceFactory { */ public DefaultMediaSourceFactory setDataSourceFactory(DataSource.Factory dataSourceFactory) { this.dataSourceFactory = dataSourceFactory; + delegateFactoryLoader.setDataSourceFactory(dataSourceFactory); return this; } @@ -576,6 +577,7 @@ public final class DefaultMediaSourceFactory implements MediaSourceFactory { this.dataSourceFactory = dataSourceFactory; // TODO(b/233577470): Call MediaSource.Factory.setDataSourceFactory on each value when it // exists on the interface. + mediaSourceFactorySuppliers.clear(); mediaSourceFactories.clear(); } } @@ -609,6 +611,7 @@ public final class DefaultMediaSourceFactory implements MediaSourceFactory { } @Nullable Supplier mediaSourceFactorySupplier = null; + DataSource.Factory dataSourceFactory = checkNotNull(this.dataSourceFactory); try { Class clazz; switch (contentType) { @@ -616,20 +619,20 @@ public final class DefaultMediaSourceFactory implements MediaSourceFactory { clazz = Class.forName("com.google.android.exoplayer2.source.dash.DashMediaSource$Factory") .asSubclass(MediaSource.Factory.class); - mediaSourceFactorySupplier = () -> newInstance(clazz, checkNotNull(dataSourceFactory)); + mediaSourceFactorySupplier = () -> newInstance(clazz, dataSourceFactory); break; case C.CONTENT_TYPE_SS: clazz = Class.forName( "com.google.android.exoplayer2.source.smoothstreaming.SsMediaSource$Factory") .asSubclass(MediaSource.Factory.class); - mediaSourceFactorySupplier = () -> newInstance(clazz, checkNotNull(dataSourceFactory)); + mediaSourceFactorySupplier = () -> newInstance(clazz, dataSourceFactory); break; case C.CONTENT_TYPE_HLS: clazz = Class.forName("com.google.android.exoplayer2.source.hls.HlsMediaSource$Factory") .asSubclass(MediaSource.Factory.class); - mediaSourceFactorySupplier = () -> newInstance(clazz, checkNotNull(dataSourceFactory)); + mediaSourceFactorySupplier = () -> newInstance(clazz, dataSourceFactory); break; case C.CONTENT_TYPE_RTSP: clazz = @@ -639,9 +642,7 @@ public final class DefaultMediaSourceFactory implements MediaSourceFactory { break; case C.CONTENT_TYPE_OTHER: mediaSourceFactorySupplier = - () -> - new ProgressiveMediaSource.Factory( - checkNotNull(dataSourceFactory), extractorsFactory); + () -> new ProgressiveMediaSource.Factory(dataSourceFactory, extractorsFactory); break; default: // Do nothing. diff --git a/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/DefaultMediaSourceFactoryTest.java b/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/DefaultMediaSourceFactoryTest.java index 7dfccefe77..31f50f963b 100644 --- a/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/DefaultMediaSourceFactoryTest.java +++ b/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/DefaultMediaSourceFactoryTest.java @@ -15,6 +15,7 @@ */ package com.google.android.exoplayer2.source.dash; +import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation; import static com.google.common.truth.Truth.assertThat; import android.content.Context; @@ -22,9 +23,13 @@ import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.MediaItem; +import com.google.android.exoplayer2.analytics.PlayerId; +import com.google.android.exoplayer2.robolectric.RobolectricUtil; import com.google.android.exoplayer2.source.DefaultMediaSourceFactory; import com.google.android.exoplayer2.source.MediaSource; +import com.google.android.exoplayer2.testutil.FakeDataSource; import com.google.android.exoplayer2.util.MimeTypes; +import java.io.IOException; import org.junit.Test; import org.junit.runner.RunWith; @@ -82,4 +87,53 @@ public class DefaultMediaSourceFactoryTest { assertThat(supportedTypes).asList().containsExactly(C.CONTENT_TYPE_OTHER, C.CONTENT_TYPE_DASH); } + + @Test + public void createMediaSource_withSetDataSourceFactory_usesDataSourceFactory() throws Exception { + FakeDataSource fakeDataSource = new FakeDataSource(); + DefaultMediaSourceFactory defaultMediaSourceFactory = + new DefaultMediaSourceFactory((Context) ApplicationProvider.getApplicationContext()) + .setDataSourceFactory(() -> fakeDataSource); + + prepareDashUrlAndWaitForPrepareError(defaultMediaSourceFactory); + + assertThat(fakeDataSource.getAndClearOpenedDataSpecs()).asList().isNotEmpty(); + } + + @Test + public void + createMediaSource_usingDefaultDataSourceFactoryAndSetDataSourceFactory_usesUpdatesDataSourceFactory() + throws Exception { + FakeDataSource fakeDataSource = new FakeDataSource(); + DefaultMediaSourceFactory defaultMediaSourceFactory = + new DefaultMediaSourceFactory((Context) ApplicationProvider.getApplicationContext()); + + // Use default DataSource.Factory first. + prepareDashUrlAndWaitForPrepareError(defaultMediaSourceFactory); + defaultMediaSourceFactory.setDataSourceFactory(() -> fakeDataSource); + prepareDashUrlAndWaitForPrepareError(defaultMediaSourceFactory); + + assertThat(fakeDataSource.getAndClearOpenedDataSpecs()).asList().isNotEmpty(); + } + + private static void prepareDashUrlAndWaitForPrepareError( + DefaultMediaSourceFactory defaultMediaSourceFactory) throws Exception { + MediaSource mediaSource = + defaultMediaSourceFactory.createMediaSource(MediaItem.fromUri(URI_MEDIA + "/file.mpd")); + getInstrumentation() + .runOnMainSync( + () -> + mediaSource.prepareSource( + (source, timeline) -> {}, /* mediaTransferListener= */ null, PlayerId.UNSET)); + // We don't expect this to prepare successfully. + RobolectricUtil.runMainLooperUntil( + () -> { + try { + mediaSource.maybeThrowSourceInfoRefreshError(); + return false; + } catch (IOException e) { + return true; + } + }); + } } diff --git a/library/hls/src/test/java/com/google/android/exoplayer2/source/hls/DefaultMediaSourceFactoryTest.java b/library/hls/src/test/java/com/google/android/exoplayer2/source/hls/DefaultMediaSourceFactoryTest.java index 84e99e8bb3..884b0738f1 100644 --- a/library/hls/src/test/java/com/google/android/exoplayer2/source/hls/DefaultMediaSourceFactoryTest.java +++ b/library/hls/src/test/java/com/google/android/exoplayer2/source/hls/DefaultMediaSourceFactoryTest.java @@ -15,6 +15,7 @@ */ package com.google.android.exoplayer2.source.hls; +import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation; import static com.google.common.truth.Truth.assertThat; import android.content.Context; @@ -22,9 +23,13 @@ import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.MediaItem; +import com.google.android.exoplayer2.analytics.PlayerId; +import com.google.android.exoplayer2.robolectric.RobolectricUtil; import com.google.android.exoplayer2.source.DefaultMediaSourceFactory; import com.google.android.exoplayer2.source.MediaSource; +import com.google.android.exoplayer2.testutil.FakeDataSource; import com.google.android.exoplayer2.util.MimeTypes; +import java.io.IOException; import org.junit.Test; import org.junit.runner.RunWith; @@ -82,4 +87,53 @@ public class DefaultMediaSourceFactoryTest { assertThat(supportedTypes).asList().containsExactly(C.CONTENT_TYPE_OTHER, C.CONTENT_TYPE_HLS); } + + @Test + public void createMediaSource_withSetDataSourceFactory_usesDataSourceFactory() throws Exception { + FakeDataSource fakeDataSource = new FakeDataSource(); + DefaultMediaSourceFactory defaultMediaSourceFactory = + new DefaultMediaSourceFactory((Context) ApplicationProvider.getApplicationContext()) + .setDataSourceFactory(() -> fakeDataSource); + + prepareHlsUrlAndWaitForPrepareError(defaultMediaSourceFactory); + + assertThat(fakeDataSource.getAndClearOpenedDataSpecs()).asList().isNotEmpty(); + } + + @Test + public void + createMediaSource_usingDefaultDataSourceFactoryAndSetDataSourceFactory_usesUpdatesDataSourceFactory() + throws Exception { + FakeDataSource fakeDataSource = new FakeDataSource(); + DefaultMediaSourceFactory defaultMediaSourceFactory = + new DefaultMediaSourceFactory((Context) ApplicationProvider.getApplicationContext()); + + // Use default DataSource.Factory first. + prepareHlsUrlAndWaitForPrepareError(defaultMediaSourceFactory); + defaultMediaSourceFactory.setDataSourceFactory(() -> fakeDataSource); + prepareHlsUrlAndWaitForPrepareError(defaultMediaSourceFactory); + + assertThat(fakeDataSource.getAndClearOpenedDataSpecs()).asList().isNotEmpty(); + } + + private static void prepareHlsUrlAndWaitForPrepareError( + DefaultMediaSourceFactory defaultMediaSourceFactory) throws Exception { + MediaSource mediaSource = + defaultMediaSourceFactory.createMediaSource(MediaItem.fromUri(URI_MEDIA + "/file.m3u8")); + getInstrumentation() + .runOnMainSync( + () -> + mediaSource.prepareSource( + (source, timeline) -> {}, /* mediaTransferListener= */ null, PlayerId.UNSET)); + // We don't expect this to prepare successfully. + RobolectricUtil.runMainLooperUntil( + () -> { + try { + mediaSource.maybeThrowSourceInfoRefreshError(); + return false; + } catch (IOException e) { + return true; + } + }); + } } diff --git a/library/smoothstreaming/build.gradle b/library/smoothstreaming/build.gradle index 116b43e7cb..1f51d4e9cc 100644 --- a/library/smoothstreaming/build.gradle +++ b/library/smoothstreaming/build.gradle @@ -29,6 +29,7 @@ dependencies { compileOnly 'org.checkerframework:checker-compat-qual:' + checkerframeworkCompatVersion compileOnly 'org.jetbrains.kotlin:kotlin-annotations-jvm:' + kotlinAnnotationsVersion implementation 'androidx.annotation:annotation:' + androidxAnnotationVersion + testImplementation project(modulePrefix + 'robolectricutils') testImplementation project(modulePrefix + 'testutils') testImplementation 'org.robolectric:robolectric:' + robolectricVersion } diff --git a/library/smoothstreaming/src/test/java/com/google/android/exoplayer2/source/smoothstreaming/DefaultMediaSourceFactoryTest.java b/library/smoothstreaming/src/test/java/com/google/android/exoplayer2/source/smoothstreaming/DefaultMediaSourceFactoryTest.java index 3b91e91d19..39c19f241b 100644 --- a/library/smoothstreaming/src/test/java/com/google/android/exoplayer2/source/smoothstreaming/DefaultMediaSourceFactoryTest.java +++ b/library/smoothstreaming/src/test/java/com/google/android/exoplayer2/source/smoothstreaming/DefaultMediaSourceFactoryTest.java @@ -15,6 +15,7 @@ */ package com.google.android.exoplayer2.source.smoothstreaming; +import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation; import static com.google.common.truth.Truth.assertThat; import android.content.Context; @@ -22,9 +23,13 @@ import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.MediaItem; +import com.google.android.exoplayer2.analytics.PlayerId; +import com.google.android.exoplayer2.robolectric.RobolectricUtil; import com.google.android.exoplayer2.source.DefaultMediaSourceFactory; import com.google.android.exoplayer2.source.MediaSource; +import com.google.android.exoplayer2.testutil.FakeDataSource; import com.google.android.exoplayer2.util.MimeTypes; +import java.io.IOException; import org.junit.Test; import org.junit.runner.RunWith; @@ -93,4 +98,53 @@ public class DefaultMediaSourceFactoryTest { assertThat(supportedTypes).asList().containsExactly(C.CONTENT_TYPE_OTHER, C.CONTENT_TYPE_SS); } + + @Test + public void createMediaSource_withSetDataSourceFactory_usesDataSourceFactory() throws Exception { + FakeDataSource fakeDataSource = new FakeDataSource(); + DefaultMediaSourceFactory defaultMediaSourceFactory = + new DefaultMediaSourceFactory((Context) ApplicationProvider.getApplicationContext()) + .setDataSourceFactory(() -> fakeDataSource); + + prepareSsUrlAndWaitForPrepareError(defaultMediaSourceFactory); + + assertThat(fakeDataSource.getAndClearOpenedDataSpecs()).asList().isNotEmpty(); + } + + @Test + public void + createMediaSource_usingDefaultDataSourceFactoryAndSetDataSourceFactory_usesUpdatesDataSourceFactory() + throws Exception { + FakeDataSource fakeDataSource = new FakeDataSource(); + DefaultMediaSourceFactory defaultMediaSourceFactory = + new DefaultMediaSourceFactory((Context) ApplicationProvider.getApplicationContext()); + + // Use default DataSource.Factory first. + prepareSsUrlAndWaitForPrepareError(defaultMediaSourceFactory); + defaultMediaSourceFactory.setDataSourceFactory(() -> fakeDataSource); + prepareSsUrlAndWaitForPrepareError(defaultMediaSourceFactory); + + assertThat(fakeDataSource.getAndClearOpenedDataSpecs()).asList().isNotEmpty(); + } + + private static void prepareSsUrlAndWaitForPrepareError( + DefaultMediaSourceFactory defaultMediaSourceFactory) throws Exception { + MediaSource mediaSource = + defaultMediaSourceFactory.createMediaSource(MediaItem.fromUri(URI_MEDIA + "/file.ism")); + getInstrumentation() + .runOnMainSync( + () -> + mediaSource.prepareSource( + (source, timeline) -> {}, /* mediaTransferListener= */ null, PlayerId.UNSET)); + // We don't expect this to prepare successfully. + RobolectricUtil.runMainLooperUntil( + () -> { + try { + mediaSource.maybeThrowSourceInfoRefreshError(); + return false; + } catch (IOException e) { + return true; + } + }); + } }