Don't crash when spritesheet can't be downloaded. (#893)

StaehliJ · MGaetan89 · web-flow · commit e8413492ab74 · 2025-02-19T14:33:15.000Z
Co-authored-by: Gaëtan Muller &lt;m.gaetan89@gmail.com&gt;
diff --git a/pillarbox-core-business/src/main/java/ch/srgssr/pillarbox/core/business/source/SRGAssetLoader.kt b/pillarbox-core-business/src/main/java/ch/srgssr/pillarbox/core/business/source/SRGAssetLoader.kt
@@ -37,6 +37,7 @@ import ch.srgssr.pillarbox.player.tracker.MediaItemTracker
 import ch.srgssr.pillarbox.player.tracker.MutableMediaItemTrackerData
 import kotlinx.serialization.SerializationException
 import java.io.IOException
+import kotlin.coroutines.CoroutineContext
 
 /**
  * Creates an [SRGAssetLoader] instance using a DSL-style configuration.
@@ -100,6 +101,7 @@ class SRGAssetLoader internal constructor(
     private val customMediaMetadata: (suspend MediaMetadata.Builder.(MediaMetadata, Chapter, MediaComposition) -> Unit)?,
     private val resourceSelector: ResourceSelector,
     private val spriteSheetLoader: SpriteSheetLoader,
+    private val spriteSheetLoaderCoroutineContext: CoroutineContext,
 ) : AssetLoader(
     mediaSourceFactory = DefaultMediaSourceFactory(AkamaiTokenDataSource.Factory(akamaiTokenProvider, dataSourceFactory))
 ) {
@@ -153,7 +155,7 @@ class SRGAssetLoader internal constructor(
             .build()
         val contentMediaSource = mediaSourceFactory.createMediaSource(loadingMediaItem)
         val mediaSource = chapter.spriteSheet?.let {
-            MergingMediaSource(contentMediaSource, SpriteSheetMediaSource(it, loadingMediaItem, spriteSheetLoader))
+            MergingMediaSource(contentMediaSource, SpriteSheetMediaSource(it, loadingMediaItem, spriteSheetLoader, spriteSheetLoaderCoroutineContext))
         } ?: contentMediaSource
         return Asset(
             mediaSource = mediaSource,
diff --git a/pillarbox-core-business/src/main/java/ch/srgssr/pillarbox/core/business/source/SRGAssetLoaderConfig.kt b/pillarbox-core-business/src/main/java/ch/srgssr/pillarbox/core/business/source/SRGAssetLoaderConfig.kt
@@ -28,6 +28,7 @@ import ch.srgssr.pillarbox.player.tracker.MediaItemTracker
 import ch.srgssr.pillarbox.player.tracker.MutableMediaItemTrackerData
 import kotlinx.coroutines.Dispatchers
 import okhttp3.OkHttpClient
+import kotlin.coroutines.CoroutineContext
 
 /**
  * Configuration class for [SRGAssetLoader].
@@ -53,7 +54,8 @@ class SRGAssetLoaderConfig internal constructor(context: Context) {
     private var commanderActTrackerFactory: MediaItemTracker.Factory<CommandersActTracker.Data> =
         CommandersActTracker.Factory(SRGAnalytics.commandersAct, Dispatchers.Default)
     private var comscoreTrackerFactory: MediaItemTracker.Factory<ComScoreTracker.Data> = ComScoreTracker.Factory()
-    private var spriteSheetLoader: SpriteSheetLoader = SpriteSheetLoader.Default()
+    private var spriteSheetLoader: SpriteSheetLoader = SpriteSheetLoader.Default
+    private var spriteSheetLoaderCoroutineContext: CoroutineContext = Dispatchers.IO
 
     @VisibleForTesting
     internal fun commanderActTrackerFactory(commanderActTrackerFactory: MediaItemTracker.Factory<CommandersActTracker.Data>) {
@@ -165,6 +167,12 @@ class SRGAssetLoaderConfig internal constructor(context: Context) {
         this.spriteSheetLoader = spriteSheetLoader
     }
 
+    @VisibleForTesting
+    internal fun spriteSheetLoader(spriteSheetLoader: SpriteSheetLoader, coroutineContext: CoroutineContext) {
+        spriteSheetLoader(spriteSheetLoader)
+        this.spriteSheetLoaderCoroutineContext = coroutineContext
+    }
+
     internal fun create(): SRGAssetLoader {
         return SRGAssetLoader(
             akamaiTokenProvider = akamaiTokenProvider,
@@ -175,7 +183,8 @@ class SRGAssetLoaderConfig internal constructor(context: Context) {
             comscoreTrackerFactory = comscoreTrackerFactory,
             mediaCompositionService = mediaCompositionService,
             resourceSelector = ResourceSelector(),
-            spriteSheetLoader = spriteSheetLoader
+            spriteSheetLoader = spriteSheetLoader,
+            spriteSheetLoaderCoroutineContext = spriteSheetLoaderCoroutineContext
         )
     }
 }
diff --git a/pillarbox-core-business/src/main/java/ch/srgssr/pillarbox/core/business/source/SpriteSheetLoader.kt b/pillarbox-core-business/src/main/java/ch/srgssr/pillarbox/core/business/source/SpriteSheetLoader.kt
@@ -7,10 +7,6 @@ package ch.srgssr.pillarbox.core.business.source
 import android.graphics.Bitmap
 import android.graphics.BitmapFactory
 import ch.srgssr.pillarbox.core.business.integrationlayer.data.SpriteSheet
-import kotlinx.coroutines.CoroutineDispatcher
-import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.MainScope
-import kotlinx.coroutines.launch
 import java.net.URL
 
 /**
@@ -24,20 +20,18 @@ fun interface SpriteSheetLoader {
      * Load sprite sheet
      *
      * @param spriteSheet The [SpriteSheet] to load the [Bitmap] from.
-     * @param onComplete The callback to call when the [Bitmap] has been loaded. Passing `null` means that the [Bitmap] could not be loaded.
+     * @return The [Result] of the loading operation.
      */
-    fun loadSpriteSheet(spriteSheet: SpriteSheet, onComplete: (Bitmap?) -> Unit)
+    suspend fun loadSpriteSheet(spriteSheet: SpriteSheet): Result<Bitmap>
 
     /**
      * Default
-     *
-     * @param dispatcher The [CoroutineDispatcher] to use for loading the sprite sheet. Should not be on the main thread.
-     */
-    class Default(private val dispatcher: CoroutineDispatcher = Dispatchers.IO) : SpriteSheetLoader {
-        override fun loadSpriteSheet(spriteSheet: SpriteSheet, onComplete: (Bitmap?) -> Unit) {
-            MainScope().launch(dispatcher) {
+     **/
+    object Default : SpriteSheetLoader {
+        override suspend fun loadSpriteSheet(spriteSheet: SpriteSheet): Result<Bitmap> {
+            return runCatching {
                 URL(spriteSheet.url).openStream().use {
-                    onComplete(BitmapFactory.decodeStream(it))
+                    BitmapFactory.decodeStream(it)
                 }
             }
         }
diff --git a/pillarbox-core-business/src/main/java/ch/srgssr/pillarbox/core/business/source/SpriteSheetMediaPeriod.kt b/pillarbox-core-business/src/main/java/ch/srgssr/pillarbox/core/business/source/SpriteSheetMediaPeriod.kt
@@ -5,6 +5,7 @@
 package ch.srgssr.pillarbox.core.business.source
 
 import android.graphics.Bitmap
+import androidx.annotation.VisibleForTesting
 import androidx.media3.common.C
 import androidx.media3.common.Format
 import androidx.media3.common.MimeTypes
@@ -18,9 +19,12 @@ import androidx.media3.exoplayer.source.SampleStream
 import androidx.media3.exoplayer.source.TrackGroupArray
 import androidx.media3.exoplayer.trackselection.ExoTrackSelection
 import ch.srgssr.pillarbox.core.business.integrationlayer.data.SpriteSheet
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Job
+import kotlinx.coroutines.launch
 import java.io.ByteArrayOutputStream
-import java.io.IOException
 import java.util.concurrent.atomic.AtomicBoolean
+import kotlin.coroutines.CoroutineContext
 import kotlin.math.max
 import kotlin.time.Duration.Companion.milliseconds
 
@@ -30,8 +34,10 @@ import kotlin.time.Duration.Companion.milliseconds
 internal class SpriteSheetMediaPeriod(
     private val spriteSheet: SpriteSheet,
     private val spriteSheetLoader: SpriteSheetLoader,
+    private val coroutineContext: CoroutineContext,
 ) : MediaPeriod {
-    private var bitmap: Bitmap? = null
+    @VisibleForTesting
+    internal var bitmap: Bitmap? = null
     private val isLoading = AtomicBoolean(true)
     private val format = Format.Builder()
         .setId("SpriteSheet")
@@ -43,20 +49,25 @@ internal class SpriteSheetMediaPeriod(
         .build()
     private val tracks = TrackGroupArray(TrackGroup("sprite-sheet-srg", format))
     private var positionUs = 0L
-
+    private var currentLoadingJob: Job? = null
     override fun prepare(callback: MediaPeriod.Callback, positionUs: Long) {
-        callback.onPrepared(this)
         this.positionUs = positionUs
+        currentLoadingJob?.cancel()
         isLoading.set(true)
-        spriteSheetLoader.loadSpriteSheet(spriteSheet) { bitmap ->
-            this.bitmap = bitmap
+        bitmap = null
+        callback.onPrepared(this)
+        currentLoadingJob = CoroutineScope(coroutineContext).launch {
+            val result = spriteSheetLoader.loadSpriteSheet(spriteSheet)
+            bitmap = result.getOrNull()
             isLoading.set(false)
         }
     }
 
     fun releasePeriod() {
-        bitmap?.recycle()
+        currentLoadingJob?.cancel()
+        currentLoadingJob = null
         bitmap = null
+        isLoading.set(false)
     }
 
     override fun selectTracks(
@@ -127,9 +138,7 @@ internal class SpriteSheetMediaPeriod(
         }
 
         override fun maybeThrowError() {
-            if (bitmap == null && !isLoading.get()) {
-                throw IOException("Can't decode ${spriteSheet.url}")
-            }
+            // Nothing if no image, there will be no image
         }
 
         @Suppress("ReturnCount")
diff --git a/pillarbox-core-business/src/main/java/ch/srgssr/pillarbox/core/business/source/SpriteSheetMediaSource.kt b/pillarbox-core-business/src/main/java/ch/srgssr/pillarbox/core/business/source/SpriteSheetMediaSource.kt
@@ -13,6 +13,7 @@ import androidx.media3.exoplayer.source.MediaSource
 import androidx.media3.exoplayer.source.SinglePeriodTimeline
 import androidx.media3.exoplayer.upstream.Allocator
 import ch.srgssr.pillarbox.core.business.integrationlayer.data.SpriteSheet
+import kotlin.coroutines.CoroutineContext
 import kotlin.time.Duration.Companion.milliseconds
 
 /**
@@ -21,11 +22,13 @@ import kotlin.time.Duration.Companion.milliseconds
  * @param spriteSheet The [SpriteSheet] to build thumbnails.
  * @param mediaItem The [MediaItem].
  * @param spriteSheetLoader The [SpriteSheetLoader] to use to load a [Bitmap] from a [SpriteSheet].
+ * @param coroutineContext The [CoroutineContext] to use for loading the [Bitmap].
  */
 internal class SpriteSheetMediaSource(
     private val spriteSheet: SpriteSheet,
     private val mediaItem: MediaItem,
     private val spriteSheetLoader: SpriteSheetLoader,
+    private val coroutineContext: CoroutineContext,
 ) : BaseMediaSource() {
 
     override fun getMediaItem(): MediaItem {
@@ -35,7 +38,7 @@ internal class SpriteSheetMediaSource(
     override fun maybeThrowSourceInfoRefreshError() = Unit
 
     override fun createPeriod(id: MediaSource.MediaPeriodId, allocator: Allocator, startPositionUs: Long): MediaPeriod {
-        return SpriteSheetMediaPeriod(spriteSheet, spriteSheetLoader)
+        return SpriteSheetMediaPeriod(spriteSheet, spriteSheetLoader, coroutineContext)
     }
 
     override fun releasePeriod(mediaPeriod: MediaPeriod) {
diff --git a/pillarbox-core-business/src/test/java/ch/srgssr/pillarbox/core/business/source/DefaultSpriteSheetLoaderTest.kt b/pillarbox-core-business/src/test/java/ch/srgssr/pillarbox/core/business/source/DefaultSpriteSheetLoaderTest.kt
@@ -0,0 +1,69 @@
+/*
+ * Copyright (c) SRG SSR. All rights reserved.
+ * License information is available from the LICENSE file.
+ */
+package ch.srgssr.pillarbox.core.business.source
+
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import ch.srgssr.pillarbox.core.business.integrationlayer.data.SpriteSheet
+import junit.framework.TestCase.assertTrue
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.runTest
+import org.junit.runner.RunWith
+import kotlin.test.Test
+import kotlin.test.assertNotNull
+
+@OptIn(ExperimentalCoroutinesApi::class)
+@RunWith(AndroidJUnit4::class)
+class DefaultSpriteSheetLoaderTest {
+
+    @Test
+    fun `malformed image url`() = runTest {
+        val spriteSheet = SpriteSheet(
+            urn = "urn:123",
+            rows = 10,
+            columns = 10,
+            thumbnailHeight = 10,
+            thumbnailWidth = 10,
+            interval = 10,
+            url = "not_valid url"
+        )
+
+        val result = SpriteSheetLoader.Default.loadSpriteSheet(spriteSheet)
+        assertTrue(result.isFailure)
+    }
+
+    @Test
+    fun `image url returns http 404`() = runTest {
+        val spriteSheet = SpriteSheet(
+            urn = "urn:123",
+            rows = 10,
+            columns = 10,
+            thumbnailHeight = 10,
+            thumbnailWidth = 10,
+            interval = 10,
+            url = "https://www.server.com/noimage.png"
+        )
+
+        val result = SpriteSheetLoader.Default.loadSpriteSheet(spriteSheet)
+        assertTrue(result.isFailure)
+    }
+
+    @Suppress("MaxLineLength")
+    @Test
+    fun `valid image url`() = runTest {
+        val spriteSheet = SpriteSheet(
+            urn = "urn:123",
+            rows = 10,
+            columns = 10,
+            thumbnailHeight = 10,
+            thumbnailWidth = 10,
+            interval = 10,
+            url = "https://il.srgssr.ch/spritesheet/urn/srf/video/881be9c2-65ec-4fa9-ba4a-926d15d046ef/sprite-881be9c2-65ec-4fa9-ba4a-926d15d046ef.jpeg"
+        )
+
+        val result = SpriteSheetLoader.Default.loadSpriteSheet(spriteSheet)
+        assertTrue(result.isSuccess)
+        assertNotNull(result.getOrNull())
+    }
+}
diff --git a/pillarbox-core-business/src/test/java/ch/srgssr/pillarbox/core/business/source/SpriteSheetMediaPeriodTest.kt b/pillarbox-core-business/src/test/java/ch/srgssr/pillarbox/core/business/source/SpriteSheetMediaPeriodTest.kt
@@ -0,0 +1,108 @@
+/*
+ * Copyright (c) SRG SSR. All rights reserved.
+ * License information is available from the LICENSE file.
+ */
+package ch.srgssr.pillarbox.core.business.source
+
+import android.os.Looper
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import ch.srgssr.pillarbox.core.business.integrationlayer.data.SpriteSheet
+import io.mockk.clearAllMocks
+import io.mockk.mockk
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.TestDispatcher
+import kotlinx.coroutines.test.UnconfinedTestDispatcher
+import kotlinx.coroutines.test.advanceUntilIdle
+import kotlinx.coroutines.test.resetMain
+import kotlinx.coroutines.test.runTest
+import kotlinx.coroutines.test.setMain
+import org.junit.runner.RunWith
+import org.robolectric.Shadows.shadowOf
+import kotlin.test.AfterTest
+import kotlin.test.BeforeTest
+import kotlin.test.Test
+import kotlin.test.assertFalse
+import kotlin.test.assertNotNull
+import kotlin.test.assertNull
+
+@OptIn(ExperimentalCoroutinesApi::class)
+@RunWith(AndroidJUnit4::class)
+class SpriteSheetMediaPeriodTest {
+    private val spriteSheetLoader = SpriteSheetLoader.Default
+    private lateinit var testDispatcher: TestDispatcher
+
+    @BeforeTest
+    fun init() {
+        testDispatcher = UnconfinedTestDispatcher()
+        Dispatchers.setMain(testDispatcher)
+    }
+
+    @AfterTest
+    fun release() {
+        Dispatchers.resetMain()
+        shadowOf(Looper.getMainLooper()).idle()
+        clearAllMocks()
+    }
+
+    @Test
+    fun `prepare doesn't crash with malformed url`() = runTest(testDispatcher) {
+        val spriteSheet = SpriteSheet(
+            urn = "urn:123",
+            rows = 10,
+            columns = 10,
+            thumbnailHeight = 10,
+            thumbnailWidth = 10,
+            interval = 10,
+            url = "not_valid url"
+        )
+
+        val mediaPeriod = SpriteSheetMediaPeriod(spriteSheet = spriteSheet, spriteSheetLoader = spriteSheetLoader, testDispatcher)
+        mediaPeriod.prepare(mockk(relaxed = true), 1L)
+        advanceUntilIdle()
+
+        assertFalse(mediaPeriod.isLoading)
+        assertNull(mediaPeriod.bitmap)
+    }
+
+    @Test
+    fun `prepare doesn't crash with image url producing 404 error`() = runTest(testDispatcher) {
+        val spriteSheet = SpriteSheet(
+            urn = "urn:123",
+            rows = 10,
+            columns = 10,
+            thumbnailHeight = 10,
+            thumbnailWidth = 10,
+            interval = 10,
+            url = "https://www.server.com/noimage.png"
+        )
+
+        val mediaPeriod = SpriteSheetMediaPeriod(spriteSheet = spriteSheet, spriteSheetLoader = spriteSheetLoader, testDispatcher)
+        mediaPeriod.prepare(mockk(relaxed = true), 1L)
+        advanceUntilIdle()
+
+        assertFalse(mediaPeriod.isLoading)
+        assertNull(mediaPeriod.bitmap)
+    }
+
+    @Suppress("MaxLineLength")
+    @Test
+    fun `prepare loads image`() = runTest(testDispatcher) {
+        val spriteSheet = SpriteSheet(
+            urn = "urn:123",
+            rows = 10,
+            columns = 10,
+            thumbnailHeight = 10,
+            thumbnailWidth = 10,
+            interval = 10,
+            url = "https://il.srgssr.ch/spritesheet/urn/srf/video/881be9c2-65ec-4fa9-ba4a-926d15d046ef/sprite-881be9c2-65ec-4fa9-ba4a-926d15d046ef.jpeg"
+        )
+
+        val mediaPeriod = SpriteSheetMediaPeriod(spriteSheet = spriteSheet, spriteSheetLoader = spriteSheetLoader, testDispatcher)
+        mediaPeriod.prepare(mockk(relaxed = true), 1L)
+        advanceUntilIdle()
+
+        assertFalse(mediaPeriod.isLoading)
+        assertNotNull(mediaPeriod.bitmap)
+    }
+}
diff --git a/pillarbox-demo/src/main/java/ch/srgssr/pillarbox/demo/ui/showcases/layouts/thumbnail/ThumbnailViewModel.kt b/pillarbox-demo/src/main/java/ch/srgssr/pillarbox/demo/ui/showcases/layouts/thumbnail/ThumbnailViewModel.kt
