From 74d6a05f7fdc0bf7fa7ca107e073c44a77157cdb Mon Sep 17 00:00:00 2001 From: Jorge Antonio Diaz-Benito Soriano Date: Wed, 18 Sep 2024 13:18:55 +0200 Subject: [PATCH] MF-464: Explictly remove cache keys that come from InvalidResponseCodeException This involves some changes to cache management since it used to live on a lower scope. --- .../playbackengine/ExoPlayerPlaybackEngine.kt | 14 ++++++++ .../di/ExoPlayerPlaybackEngineModule.kt | 33 +++++++++++++++++++ .../player/ExtendedExoPlayer.kt | 5 --- .../player/di/ExtendedExoPlayerModule.kt | 3 -- .../player/di/MediaSourcererModule.kt | 30 ----------------- .../ExoPlayerPlaybackEngineTest.kt | 12 +++++-- .../player/ExtendedExoPlayerTest.kt | 10 +----- 7 files changed, 58 insertions(+), 49 deletions(-) diff --git a/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/ExoPlayerPlaybackEngine.kt b/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/ExoPlayerPlaybackEngine.kt index b4079fa0..02109cf7 100644 --- a/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/ExoPlayerPlaybackEngine.kt +++ b/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/ExoPlayerPlaybackEngine.kt @@ -9,6 +9,7 @@ import androidx.media3.common.PlaybackException import androidx.media3.common.Player import androidx.media3.common.Timeline import androidx.media3.common.VideoSize +import androidx.media3.datasource.HttpDataSource.InvalidResponseCodeException import androidx.media3.exoplayer.DecoderReuseEvaluation import androidx.media3.exoplayer.analytics.AnalyticsListener import androidx.media3.exoplayer.analytics.AnalyticsListener.EventTime @@ -53,6 +54,7 @@ import com.tidal.sdk.player.playbackengine.model.PlaybackState import com.tidal.sdk.player.playbackengine.outputdevice.OutputDevice import com.tidal.sdk.player.playbackengine.outputdevice.OutputDeviceManager import com.tidal.sdk.player.playbackengine.player.ExtendedExoPlayerFactory +import com.tidal.sdk.player.playbackengine.player.PlayerCache import com.tidal.sdk.player.playbackengine.quality.AudioQualityRepository import com.tidal.sdk.player.playbackengine.util.SynchronousSurfaceHolder import com.tidal.sdk.player.playbackengine.view.AspectRatioAdjustingSurfaceView @@ -91,6 +93,7 @@ internal class ExoPlayerPlaybackEngine( private val djSessionManager: DjSessionManager, private val undeterminedPlaybackSessionResolver: UndeterminedPlaybackSessionResolver, private val outputDeviceManager: OutputDeviceManager, + private val playerCache: PlayerCache, ) : PlaybackEngine, StreamingPrivilegesListener, PlaybackInfoListener, @@ -356,6 +359,9 @@ internal class ExoPlayerPlaybackEngine( override fun release() { extendedExoPlayer.release() + if (playerCache is PlayerCache.Internal) { + playerCache.cache.release() + } coroutineScope.launch { eventSink.emit(Event.Release) cancel() @@ -802,6 +808,14 @@ internal class ExoPlayerPlaybackEngine( var errorMessage = "${error.errorCodeName}: ${crawler?.message}" while (crawler?.cause != null) { + @Suppress("MagicNumber") + if ((crawler as? InvalidResponseCodeException)?.responseCode == 416) { + with(playerCache.cache) { + keys.forEach { + removeResource(it) + } + } + } crawler = crawler.cause errorMessage += " -> ${crawler?.message}" if (crawler is PlaybackInfoFetchException) { diff --git a/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/di/ExoPlayerPlaybackEngineModule.kt b/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/di/ExoPlayerPlaybackEngineModule.kt index 56773c68..c4c59d53 100644 --- a/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/di/ExoPlayerPlaybackEngineModule.kt +++ b/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/di/ExoPlayerPlaybackEngineModule.kt @@ -6,6 +6,10 @@ import android.net.ConnectivityManager import android.os.Handler import android.os.HandlerThread import android.os.Looper +import androidx.media3.database.DatabaseProvider +import androidx.media3.database.StandaloneDatabaseProvider +import androidx.media3.datasource.cache.LeastRecentlyUsedCacheEvictor +import androidx.media3.datasource.cache.SimpleCache import androidx.media3.exoplayer.drm.ExoMediaDrm import androidx.media3.exoplayer.drm.FrameworkMediaDrm import com.tidal.sdk.player.commonandroid.TrueTimeWrapper @@ -25,7 +29,9 @@ import com.tidal.sdk.player.playbackengine.mediasource.streamingsession.Versione import com.tidal.sdk.player.playbackengine.model.Event import com.tidal.sdk.player.playbackengine.network.NetworkTransportHelper import com.tidal.sdk.player.playbackengine.outputdevice.OutputDeviceManager +import com.tidal.sdk.player.playbackengine.player.CacheProvider import com.tidal.sdk.player.playbackengine.player.ExtendedExoPlayerFactory +import com.tidal.sdk.player.playbackengine.player.PlayerCache import com.tidal.sdk.player.playbackengine.quality.AudioQualityRepository import com.tidal.sdk.player.playbackengine.util.SynchronousSurfaceHolder import com.tidal.sdk.player.playbackengine.volume.LoudnessNormalizer @@ -34,6 +40,7 @@ import com.tidal.sdk.player.streamingprivileges.StreamingPrivileges import dagger.Module import dagger.Provides import dagger.Reusable +import java.io.File import javax.inject.Named import javax.inject.Singleton import kotlinx.coroutines.CoroutineScope @@ -145,6 +152,28 @@ internal object ExoPlayerPlaybackEngineModule { @Singleton fun audioModeRepository() = AudioModeRepository() + @Provides + @Singleton + fun provideDatabaseProvider(context: Context): DatabaseProvider = + StandaloneDatabaseProvider(context) + + @Provides + @Singleton + fun cache( + appSpecificCacheDir: File, + databaseProvider: DatabaseProvider, + cacheProvider: CacheProvider, + ): PlayerCache { + return when (cacheProvider) { + is CacheProvider.External -> PlayerCache.Provided(cacheProvider.cache) + is CacheProvider.Internal -> { + val cacheDir = File(appSpecificCacheDir, CACHE_DIR) + val cacheEvictor = LeastRecentlyUsedCacheEvictor(cacheProvider.cacheSizeBytes.value) + PlayerCache.Internal(SimpleCache(cacheDir, cacheEvictor, databaseProvider)) + } + } + } + @Provides @Singleton fun exoPlayerPlaybackEngine( @@ -164,6 +193,7 @@ internal object ExoPlayerPlaybackEngineModule { djSessionManager: DjSessionManager, undeterminedPlaybackSessionResolver: UndeterminedPlaybackSessionResolver, outputDeviceManager: OutputDeviceManager, + playerCache: PlayerCache, ) = ExoPlayerPlaybackEngine( coroutineScope, extendedExoPlayerFactory, @@ -181,6 +211,7 @@ internal object ExoPlayerPlaybackEngineModule { djSessionManager, undeterminedPlaybackSessionResolver, outputDeviceManager, + playerCache, ) @Provides @@ -190,3 +221,5 @@ internal object ExoPlayerPlaybackEngineModule { delegate: ExoPlayerPlaybackEngine, ): PlaybackEngine = SingleHandlerPlaybackEngine(handler, delegate) } + +private const val CACHE_DIR = "exoplayer-cache" diff --git a/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/player/ExtendedExoPlayer.kt b/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/player/ExtendedExoPlayer.kt index 9ee55372..6f668ff6 100644 --- a/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/player/ExtendedExoPlayer.kt +++ b/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/player/ExtendedExoPlayer.kt @@ -16,7 +16,6 @@ import kotlin.properties.Delegates * A proxy to an [ExoPlayer] instance which defines requirements for additional functionality to * satisfy library capabilities. * @param delegate An [ExoPlayer] instance that all calls needing one will utilize. - * @param playerCache A [PlayerCache] instance that will be used to cache assets. * @param loadControl A [LoadControl] instance that controls the buffering of media. * @param mediaSourcerer A [MediaSourcerer] instance that holds the MediaSource of what we play. * @param extendedExoPlayerState A [ExtendedExoPlayerState] instance that holds the some shared @@ -24,7 +23,6 @@ import kotlin.properties.Delegates */ internal class ExtendedExoPlayer( private val delegate: ExoPlayer, - private val playerCache: PlayerCache, private val loadControl: LoadControl, private val mediaSourcerer: MediaSourcerer, private val extendedExoPlayerState: ExtendedExoPlayerState, @@ -80,9 +78,6 @@ internal class ExtendedExoPlayer( override fun release() { delegate.release() - if (playerCache is PlayerCache.Internal) { - playerCache.cache.release() - } mediaSourcerer.release() extendedExoPlayerState.playbackInfoListener = null analyticsListener = null diff --git a/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/player/di/ExtendedExoPlayerModule.kt b/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/player/di/ExtendedExoPlayerModule.kt index 2985b986..ceb2f526 100644 --- a/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/player/di/ExtendedExoPlayerModule.kt +++ b/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/player/di/ExtendedExoPlayerModule.kt @@ -19,7 +19,6 @@ import com.tidal.sdk.player.playbackengine.model.BufferConfiguration import com.tidal.sdk.player.playbackengine.player.ExtendedExoPlayer import com.tidal.sdk.player.playbackengine.player.ExtendedExoPlayerState import com.tidal.sdk.player.playbackengine.player.ExtendedExoPlayerStateUpdateRunnable -import com.tidal.sdk.player.playbackengine.player.PlayerCache import dagger.Module import dagger.Provides import dagger.Reusable @@ -96,13 +95,11 @@ internal object ExtendedExoPlayerModule { @ExtendedExoPlayerComponent.Scoped fun extendedExoPlayer( exoPlayer: ExoPlayer, - playerCache: PlayerCache, loadControl: LoadControl, mediaSourcerer: MediaSourcerer, extendedExoPlayerState: ExtendedExoPlayerState, ) = ExtendedExoPlayer( exoPlayer, - playerCache, loadControl, mediaSourcerer, extendedExoPlayerState, diff --git a/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/player/di/MediaSourcererModule.kt b/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/player/di/MediaSourcererModule.kt index 771f2298..222fa692 100644 --- a/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/player/di/MediaSourcererModule.kt +++ b/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/player/di/MediaSourcererModule.kt @@ -1,15 +1,10 @@ package com.tidal.sdk.player.playbackengine.player.di -import android.content.Context import androidx.media3.common.C -import androidx.media3.database.DatabaseProvider -import androidx.media3.database.StandaloneDatabaseProvider import androidx.media3.datasource.FileDataSource import androidx.media3.datasource.cache.CacheDataSink import androidx.media3.datasource.cache.CacheDataSource import androidx.media3.datasource.cache.CacheKeyFactory -import androidx.media3.datasource.cache.LeastRecentlyUsedCacheEvictor -import androidx.media3.datasource.cache.SimpleCache import androidx.media3.datasource.okhttp.OkHttpDataSource import androidx.media3.exoplayer.ExoPlayer import androidx.media3.exoplayer.dash.DashMediaSource @@ -68,7 +63,6 @@ import com.tidal.sdk.player.playbackengine.offline.OfflineStorageProvider import com.tidal.sdk.player.playbackengine.offline.StorageDataSource import com.tidal.sdk.player.playbackengine.offline.cache.OfflineCacheProvider import com.tidal.sdk.player.playbackengine.playbackprivilege.PlaybackPrivilegeProvider -import com.tidal.sdk.player.playbackengine.player.CacheProvider import com.tidal.sdk.player.playbackengine.player.ExtendedExoPlayerState import com.tidal.sdk.player.playbackengine.player.PlayerCache import com.tidal.sdk.player.playbackengine.quality.AudioQualityRepository @@ -77,41 +71,17 @@ import com.tidal.sdk.player.streamingapi.StreamingApi import dagger.Module import dagger.Provides import dagger.Reusable -import java.io.File import javax.inject.Named import kotlin.time.toJavaDuration import kotlinx.coroutines.CoroutineDispatcher import okhttp3.OkHttpClient -private const val CACHE_DIR = "exoplayer-cache" private const val MINIMUM_LOADABLE_RETRY_COUNT = 10 @Module @Suppress("TooManyFunctions") internal object MediaSourcererModule { - @Provides - @ExtendedExoPlayerComponent.Scoped - fun provideDatabaseProvider(context: Context): DatabaseProvider = - StandaloneDatabaseProvider(context) - - @Provides - @ExtendedExoPlayerComponent.Scoped - fun cache( - appSpecificCacheDir: File, - databaseProvider: DatabaseProvider, - cacheProvider: CacheProvider, - ): PlayerCache { - return when (cacheProvider) { - is CacheProvider.External -> PlayerCache.Provided(cacheProvider.cache) - is CacheProvider.Internal -> { - val cacheDir = File(appSpecificCacheDir, CACHE_DIR) - val cacheEvictor = LeastRecentlyUsedCacheEvictor(cacheProvider.cacheSizeBytes.value) - PlayerCache.Internal(SimpleCache(cacheDir, cacheEvictor, databaseProvider)) - } - } - } - @Provides @Reusable @ExtendedExoPlayerComponent.Local diff --git a/player/playback-engine/src/test/kotlin/com/tidal/sdk/player/playbackengine/ExoPlayerPlaybackEngineTest.kt b/player/playback-engine/src/test/kotlin/com/tidal/sdk/player/playbackengine/ExoPlayerPlaybackEngineTest.kt index d215218f..ba640e41 100644 --- a/player/playback-engine/src/test/kotlin/com/tidal/sdk/player/playbackengine/ExoPlayerPlaybackEngineTest.kt +++ b/player/playback-engine/src/test/kotlin/com/tidal/sdk/player/playbackengine/ExoPlayerPlaybackEngineTest.kt @@ -9,6 +9,7 @@ import androidx.media3.common.MediaItem import androidx.media3.common.Player import androidx.media3.common.Timeline import androidx.media3.common.VideoSize +import androidx.media3.datasource.cache.Cache import androidx.media3.exoplayer.DecoderReuseEvaluation import androidx.media3.exoplayer.analytics.AnalyticsListener.EventTime import androidx.media3.exoplayer.hls.HlsManifest @@ -51,6 +52,7 @@ import com.tidal.sdk.player.playbackengine.model.PlaybackState import com.tidal.sdk.player.playbackengine.outputdevice.OutputDeviceManager import com.tidal.sdk.player.playbackengine.player.ExtendedExoPlayer import com.tidal.sdk.player.playbackengine.player.ExtendedExoPlayerFactory +import com.tidal.sdk.player.playbackengine.player.PlayerCache import com.tidal.sdk.player.playbackengine.quality.AudioQualityRepository import com.tidal.sdk.player.playbackengine.util.SynchronousSurfaceHolder import com.tidal.sdk.player.playbackengine.view.AspectRatioAdjustingSurfaceView @@ -117,6 +119,7 @@ internal class ExoPlayerPlaybackEngineTest { private val djSessionManager = mock() private val undeterminedPlaybackSessionResolver = mock() private val outputDeviceManager = mock() + private val playerCache = mock() private lateinit var playbackEngine: ExoPlayerPlaybackEngine private val forwardingMediaProduct = @@ -146,6 +149,7 @@ internal class ExoPlayerPlaybackEngineTest { djSessionManager, undeterminedPlaybackSessionResolver, outputDeviceManager, + playerCache, ) } @@ -472,13 +476,17 @@ internal class ExoPlayerPlaybackEngineTest { } @Test - fun releaseReleasesExtendedExoPlayerAndQuitsTheLooper() { + fun releaseReleasesExtendedExoPlayerAndQuitsTheLooperAndReleasesTheCache() { val looper = mock() + val cache = mock() + whenever(playerCache.cache) doReturn cache playbackEngine.release() verify(initialExtendedExoPlayer).release() - verifyNoMoreInteractions(initialExtendedExoPlayer, looper) + verify(playerCache).cache + verify(cache).release() + verifyNoMoreInteractions(initialExtendedExoPlayer, looper, cache) } @Test diff --git a/player/playback-engine/src/test/kotlin/com/tidal/sdk/player/playbackengine/player/ExtendedExoPlayerTest.kt b/player/playback-engine/src/test/kotlin/com/tidal/sdk/player/playbackengine/player/ExtendedExoPlayerTest.kt index 77501239..f64454c7 100644 --- a/player/playback-engine/src/test/kotlin/com/tidal/sdk/player/playbackengine/player/ExtendedExoPlayerTest.kt +++ b/player/playback-engine/src/test/kotlin/com/tidal/sdk/player/playbackengine/player/ExtendedExoPlayerTest.kt @@ -33,14 +33,12 @@ import org.mockito.kotlin.whenever internal class ExtendedExoPlayerTest { private val delegate = mock() - private val playerCache = mock() private val loadControl = mock() private val mediaSourcerer = mock() private val extendedExoPlayerState = mock() private val extendedExoPlayer by lazy { ExtendedExoPlayer( delegate, - playerCache, loadControl, mediaSourcerer, extendedExoPlayerState, @@ -155,7 +153,6 @@ internal class ExtendedExoPlayerTest { } val extendedExoPlayer = ExtendedExoPlayer( delegate, - playerCache, loadControl, mediaSourcerer, extendedExoPlayerState, @@ -170,13 +167,9 @@ internal class ExtendedExoPlayerTest { } @Test - fun releaseForwardsToDelegateAndReleasesCache() { - val playerCache = mock { - on { cache } doReturn mock() - } + fun releaseForwardsToDelegate() { val extendedExoPlayer = ExtendedExoPlayer( delegate, - playerCache, loadControl, mediaSourcerer, extendedExoPlayerState, @@ -185,7 +178,6 @@ internal class ExtendedExoPlayerTest { extendedExoPlayer.release() verify(delegate).release() - verify(playerCache.cache).release() verify(mediaSourcerer).release() verify(extendedExoPlayerState).playbackInfoListener = null }