Skip to content

Commit

Permalink
MF-464: Explictly remove cache keys that come from InvalidResponseCod…
Browse files Browse the repository at this point in the history
…eException

This involves some changes to cache management since it used to live on a lower
scope.
  • Loading branch information
stoyicker committed Sep 18, 2024
1 parent 78bf5fe commit 74d6a05
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -164,6 +193,7 @@ internal object ExoPlayerPlaybackEngineModule {
djSessionManager: DjSessionManager,
undeterminedPlaybackSessionResolver: UndeterminedPlaybackSessionResolver,
outputDeviceManager: OutputDeviceManager,
playerCache: PlayerCache,
) = ExoPlayerPlaybackEngine(
coroutineScope,
extendedExoPlayerFactory,
Expand All @@ -181,6 +211,7 @@ internal object ExoPlayerPlaybackEngineModule {
djSessionManager,
undeterminedPlaybackSessionResolver,
outputDeviceManager,
playerCache,
)

@Provides
Expand All @@ -190,3 +221,5 @@ internal object ExoPlayerPlaybackEngineModule {
delegate: ExoPlayerPlaybackEngine,
): PlaybackEngine = SingleHandlerPlaybackEngine(handler, delegate)
}

private const val CACHE_DIR = "exoplayer-cache"
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,13 @@ 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
* state for ExtendedExoPlayer.
*/
internal class ExtendedExoPlayer(
private val delegate: ExoPlayer,
private val playerCache: PlayerCache,
private val loadControl: LoadControl,
private val mediaSourcerer: MediaSourcerer,
private val extendedExoPlayerState: ExtendedExoPlayerState,
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -117,6 +119,7 @@ internal class ExoPlayerPlaybackEngineTest {
private val djSessionManager = mock<DjSessionManager>()
private val undeterminedPlaybackSessionResolver = mock<UndeterminedPlaybackSessionResolver>()
private val outputDeviceManager = mock<OutputDeviceManager>()
private val playerCache = mock<PlayerCache.Internal>()
private lateinit var playbackEngine: ExoPlayerPlaybackEngine

private val forwardingMediaProduct =
Expand Down Expand Up @@ -146,6 +149,7 @@ internal class ExoPlayerPlaybackEngineTest {
djSessionManager,
undeterminedPlaybackSessionResolver,
outputDeviceManager,
playerCache,
)
}

Expand Down Expand Up @@ -472,13 +476,17 @@ internal class ExoPlayerPlaybackEngineTest {
}

@Test
fun releaseReleasesExtendedExoPlayerAndQuitsTheLooper() {
fun releaseReleasesExtendedExoPlayerAndQuitsTheLooperAndReleasesTheCache() {
val looper = mock<Looper>()
val cache = mock<Cache>()
whenever(playerCache.cache) doReturn cache

playbackEngine.release()

verify(initialExtendedExoPlayer).release()
verifyNoMoreInteractions(initialExtendedExoPlayer, looper)
verify(playerCache).cache
verify(cache).release()
verifyNoMoreInteractions(initialExtendedExoPlayer, looper, cache)
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,12 @@ import org.mockito.kotlin.whenever
internal class ExtendedExoPlayerTest {

private val delegate = mock<ExoPlayer>()
private val playerCache = mock<PlayerCache.Internal>()
private val loadControl = mock<LoadControl>()
private val mediaSourcerer = mock<MediaSourcerer>()
private val extendedExoPlayerState = mock<ExtendedExoPlayerState>()
private val extendedExoPlayer by lazy {
ExtendedExoPlayer(
delegate,
playerCache,
loadControl,
mediaSourcerer,
extendedExoPlayerState,
Expand Down Expand Up @@ -155,7 +153,6 @@ internal class ExtendedExoPlayerTest {
}
val extendedExoPlayer = ExtendedExoPlayer(
delegate,
playerCache,
loadControl,
mediaSourcerer,
extendedExoPlayerState,
Expand All @@ -170,13 +167,9 @@ internal class ExtendedExoPlayerTest {
}

@Test
fun releaseForwardsToDelegateAndReleasesCache() {
val playerCache = mock<PlayerCache.Internal> {
on { cache } doReturn mock()
}
fun releaseForwardsToDelegate() {
val extendedExoPlayer = ExtendedExoPlayer(
delegate,
playerCache,
loadControl,
mediaSourcerer,
extendedExoPlayerState,
Expand All @@ -185,7 +178,6 @@ internal class ExtendedExoPlayerTest {
extendedExoPlayer.release()

verify(delegate).release()
verify(playerCache.cache).release()
verify(mediaSourcerer).release()
verify(extendedExoPlayerState).playbackInfoListener = null
}
Expand Down

0 comments on commit 74d6a05

Please sign in to comment.