Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MF-464: Explictly remove cache keys that come from InvalidResponseCodeException #105

Merged
merged 2 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,8 +54,10 @@ 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.util.clear
import com.tidal.sdk.player.playbackengine.view.AspectRatioAdjustingSurfaceView
import com.tidal.sdk.player.playbackengine.volume.VolumeHelper
import com.tidal.sdk.player.streamingapi.playbackinfo.model.PlaybackInfo
Expand Down Expand Up @@ -91,6 +94,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 +360,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 @@ -795,13 +802,17 @@ internal class ExoPlayerPlaybackEngine(
startStall(Stall.Reason.UNEXPECTED, positionInSeconds, trueTimeWrapper.currentTimeMillis)
}

@Suppress("LongMethod", "ComplexMethod")
@Suppress("LongMethod", "ComplexMethod", "NestedBlockDepth")
override fun onPlayerError(eventTime: EventTime, error: PlaybackException) {
var crawler: Throwable? = error
var playbackInfoFetchException: PlaybackInfoFetchException? = null

var errorMessage = "${error.errorCodeName}: ${crawler?.message}"
while (crawler?.cause != null) {
@Suppress("MagicNumber")
if ((crawler as? InvalidResponseCodeException)?.responseCode == 416) {
playerCache.clear()
}
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
@@ -0,0 +1,11 @@
package com.tidal.sdk.player.playbackengine.util

import com.tidal.sdk.player.playbackengine.player.PlayerCache

internal fun PlayerCache.clear() {
with(cache) {
keys.forEach {
removeResource(it)
}
}
}
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
Loading