From adc50515e93e6fdcf303d168e8388050503c46ef 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 --- RELEASENOTES.md | 3 ++ .../source/DefaultMediaSourceFactory.java | 13 ++--- .../dash/DefaultMediaSourceFactoryTest.java | 54 +++++++++++++++++++ .../hls/DefaultMediaSourceFactoryTest.java | 54 +++++++++++++++++++ .../exoplayer_smoothstreaming/build.gradle | 1 + .../DefaultMediaSourceFactoryTest.java | 54 +++++++++++++++++++ 6 files changed, 173 insertions(+), 6 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 97aed03d91..78f6a3e7a8 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -17,6 +17,9 @@ * Use `SingleThreadExecutor` for releasing `AudioTrack` instances to avoid OutOfMemory errors when releasing multiple players at the same time ([#10057](https://github.com/google/ExoPlayer/issues/10057)). + * Fix implementation of `setDataSourceFactory` in + `DefaultMediaSourceFactory`, which was non-functional in some cases + ([#116](https://github.com/androidx/media/issues/116)). * Extractors: * Add support for AVI ([#2092](https://github.com/google/ExoPlayer/issues/2092)). diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/DefaultMediaSourceFactory.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/DefaultMediaSourceFactory.java index f0a8cb1164..6a55a3a13e 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/DefaultMediaSourceFactory.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/DefaultMediaSourceFactory.java @@ -282,6 +282,7 @@ public final class DefaultMediaSourceFactory implements MediaSourceFactory { */ public DefaultMediaSourceFactory setDataSourceFactory(DataSource.Factory dataSourceFactory) { this.dataSourceFactory = dataSourceFactory; + delegateFactoryLoader.setDataSourceFactory(dataSourceFactory); return this; } @@ -594,6 +595,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(); } } @@ -627,6 +629,7 @@ public final class DefaultMediaSourceFactory implements MediaSourceFactory { } @Nullable Supplier mediaSourceFactorySupplier = null; + DataSource.Factory dataSourceFactory = checkNotNull(this.dataSourceFactory); try { Class clazz; switch (contentType) { @@ -634,19 +637,19 @@ public final class DefaultMediaSourceFactory implements MediaSourceFactory { clazz = Class.forName("androidx.media3.exoplayer.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("androidx.media3.exoplayer.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("androidx.media3.exoplayer.hls.HlsMediaSource$Factory") .asSubclass(MediaSource.Factory.class); - mediaSourceFactorySupplier = () -> newInstance(clazz, checkNotNull(dataSourceFactory)); + mediaSourceFactorySupplier = () -> newInstance(clazz, dataSourceFactory); break; case C.CONTENT_TYPE_RTSP: clazz = @@ -656,9 +659,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/libraries/exoplayer_dash/src/test/java/androidx/media3/exoplayer/dash/DefaultMediaSourceFactoryTest.java b/libraries/exoplayer_dash/src/test/java/androidx/media3/exoplayer/dash/DefaultMediaSourceFactoryTest.java index b6d4ac102f..77c1dde311 100644 --- a/libraries/exoplayer_dash/src/test/java/androidx/media3/exoplayer/dash/DefaultMediaSourceFactoryTest.java +++ b/libraries/exoplayer_dash/src/test/java/androidx/media3/exoplayer/dash/DefaultMediaSourceFactoryTest.java @@ -15,16 +15,21 @@ */ package androidx.media3.exoplayer.dash; +import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation; import static com.google.common.truth.Truth.assertThat; import android.content.Context; import androidx.media3.common.C; import androidx.media3.common.MediaItem; import androidx.media3.common.MimeTypes; +import androidx.media3.exoplayer.analytics.PlayerId; import androidx.media3.exoplayer.source.DefaultMediaSourceFactory; import androidx.media3.exoplayer.source.MediaSource; +import androidx.media3.test.utils.FakeDataSource; +import androidx.media3.test.utils.robolectric.RobolectricUtil; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; +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/libraries/exoplayer_hls/src/test/java/androidx/media3/exoplayer/hls/DefaultMediaSourceFactoryTest.java b/libraries/exoplayer_hls/src/test/java/androidx/media3/exoplayer/hls/DefaultMediaSourceFactoryTest.java index 2a2ff66b28..8062cff051 100644 --- a/libraries/exoplayer_hls/src/test/java/androidx/media3/exoplayer/hls/DefaultMediaSourceFactoryTest.java +++ b/libraries/exoplayer_hls/src/test/java/androidx/media3/exoplayer/hls/DefaultMediaSourceFactoryTest.java @@ -15,16 +15,21 @@ */ package androidx.media3.exoplayer.hls; +import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation; import static com.google.common.truth.Truth.assertThat; import android.content.Context; import androidx.media3.common.C; import androidx.media3.common.MediaItem; import androidx.media3.common.MimeTypes; +import androidx.media3.exoplayer.analytics.PlayerId; import androidx.media3.exoplayer.source.DefaultMediaSourceFactory; import androidx.media3.exoplayer.source.MediaSource; +import androidx.media3.test.utils.FakeDataSource; +import androidx.media3.test.utils.robolectric.RobolectricUtil; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; +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/libraries/exoplayer_smoothstreaming/build.gradle b/libraries/exoplayer_smoothstreaming/build.gradle index a379d25558..4b145ec6b3 100644 --- a/libraries/exoplayer_smoothstreaming/build.gradle +++ b/libraries/exoplayer_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 + 'test-utils-robolectric') testImplementation project(modulePrefix + 'test-utils') testImplementation 'org.robolectric:robolectric:' + robolectricVersion } diff --git a/libraries/exoplayer_smoothstreaming/src/test/java/androidx/media3/exoplayer/smoothstreaming/DefaultMediaSourceFactoryTest.java b/libraries/exoplayer_smoothstreaming/src/test/java/androidx/media3/exoplayer/smoothstreaming/DefaultMediaSourceFactoryTest.java index f5a205fcbe..4036fb9472 100644 --- a/libraries/exoplayer_smoothstreaming/src/test/java/androidx/media3/exoplayer/smoothstreaming/DefaultMediaSourceFactoryTest.java +++ b/libraries/exoplayer_smoothstreaming/src/test/java/androidx/media3/exoplayer/smoothstreaming/DefaultMediaSourceFactoryTest.java @@ -15,16 +15,21 @@ */ package androidx.media3.exoplayer.smoothstreaming; +import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation; import static com.google.common.truth.Truth.assertThat; import android.content.Context; import androidx.media3.common.C; import androidx.media3.common.MediaItem; import androidx.media3.common.MimeTypes; +import androidx.media3.exoplayer.analytics.PlayerId; import androidx.media3.exoplayer.source.DefaultMediaSourceFactory; import androidx.media3.exoplayer.source.MediaSource; +import androidx.media3.test.utils.FakeDataSource; +import androidx.media3.test.utils.robolectric.RobolectricUtil; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; +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; + } + }); + } }