Fix current time range reset (#581)

Author: StaehliJ

pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/tracker/TimeRangeTracker.kt

@@ -86,13 +86,15 @@ internal class TimeRangeTracker(
 
         listTrackers.forEach {
             pillarboxPlayer.removeListener(it)
+            it.clear()
         }
         listTrackers.clear()
     }
 }
 
 private sealed interface PlayerTimeRangeTracker<T : TimeRange> : PillarboxPlayer.Listener {
     fun createMessages(player: PillarboxExoPlayer): List<PlayerMessage>
+    fun clear() {}
 }
 
 private class ChapterCreditsTracker<T : TimeRange>(
@@ -118,15 +120,13 @@ private class ChapterCreditsTracker<T : TimeRange>(
         newPosition: Player.PositionInfo,
         @DiscontinuityReason reason: Int,
     ) {
-        if (
-            (reason == Player.DISCONTINUITY_REASON_SEEK || reason == Player.DISCONTINUITY_REASON_SEEK_ADJUSTMENT) &&
-            oldPosition.mediaItemIndex == newPosition.mediaItemIndex
-        ) {
-            val currentPosition = oldPosition.positionMs
+        if (oldPosition.mediaItemIndex != newPosition.mediaItemIndex) {
+            clear()
+        } else {
+            val currentPosition = newPosition.positionMs
             val currentTimeRange = currentTimeRange
                 ?.takeIf { timeRange -> currentPosition in timeRange }
                 ?: timeRanges.firstOrNullAtPosition(currentPosition)
-
             this.currentTimeRange = currentTimeRange
         }
     }
@@ -164,6 +164,10 @@ private class ChapterCreditsTracker<T : TimeRange>(
         return playerMessages
     }
 
+    override fun clear() {
+        currentTimeRange = null
+    }
+
     private companion object {
         private const val TYPE_ENTER = 1
         private const val TYPE_EXIT = 2

pillarbox-player/src/test/java/ch/srgssr/pillarbox/player/tracker/ChapterTrackerTest.kt

@@ -23,6 +23,7 @@ import ch.srgssr.pillarbox.player.asset.timeRange.Chapter
 import ch.srgssr.pillarbox.player.source.PillarboxMediaSourceFactory
 import io.mockk.clearAllMocks
 import io.mockk.spyk
+import io.mockk.verify
 import io.mockk.verifyOrder
 import org.junit.runner.RunWith
 import kotlin.test.AfterTest
@@ -67,15 +68,13 @@ class ChapterTrackerTest {
         player.addMediaItem(ChapterAssetLoader.MEDIA_ITEM)
         TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_ENDED)
 
-        val expectedChapters = listOf(ChapterAssetLoader.CHAPTER_1, ChapterAssetLoader.CHAPTER_2)
-        val receivedChapters = mutableListOf<Chapter>()
-        verifyOrder {
-            listener.onChapterChanged(capture(receivedChapters))
-            listener.onChapterChanged(null)
-            listener.onChapterChanged(capture(receivedChapters))
-            listener.onChapterChanged(null)
+        val expectedChapters = listOf(ChapterAssetLoader.CHAPTER_1, null, ChapterAssetLoader.CHAPTER_2, null)
+        val receivedChapters = mutableListOf<Chapter?>()
+        verify {
+            listener.onChapterChanged(captureNullable(receivedChapters))
         }
-        assertEquals(expectedChapters, receivedChapters.reversed())
+        assertEquals(expectedChapters.size, receivedChapters.size)
+        assertEquals(expectedChapters, receivedChapters)
     }
 
     @Test
@@ -97,6 +96,38 @@ class ChapterTrackerTest {
         }
         assertEquals(expectedChapters, receivedChapters.reversed())
     }
+
+    @Test
+    fun `chapter transition after seek back`() {
+        player.addMediaItem(ChapterAssetLoader.MEDIA_ITEM_WITH_CHAPTER)
+        TestPlayerRunHelper.playUntilPosition(player, 0, ChapterAssetLoader.CHAPTER_3.start + 1_000L)
+        player.seekBack()
+        TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_ENDED)
+
+        val expectedChapters = listOf(ChapterAssetLoader.CHAPTER_3, null, ChapterAssetLoader.CHAPTER_3, null, ChapterAssetLoader.CHAPTER_4, null)
+        val receivedChapters = mutableListOf<Chapter?>()
+
+        verify {
+            listener.onChapterChanged(captureNullable(receivedChapters))
+        }
+        assertEquals(expectedChapters, receivedChapters)
+    }
+
+    @Test
+    fun `chapter transition skip next`() {
+        player.addMediaItems(listOf(ChapterAssetLoader.MEDIA_ITEM_WITH_CHAPTER, ChapterAssetLoader.NO_CHAPTER_MEDIA_ITEM))
+        TestPlayerRunHelper.playUntilPosition(player, 0, ChapterAssetLoader.CHAPTER_3.start + 1_000L)
+        player.seekToNext()
+        TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_ENDED)
+
+        val expectedChapters = listOf(ChapterAssetLoader.CHAPTER_3, null)
+        val receivedChapters = mutableListOf<Chapter?>()
+
+        verify {
+            listener.onChapterChanged(captureNullable(receivedChapters))
+        }
+        assertEquals(expectedChapters, receivedChapters)
+    }
 }
 
 private class ChapterAssetLoader(context: Context) : AssetLoader(DefaultMediaSourceFactory(context)) {
@@ -107,20 +138,49 @@ private class ChapterAssetLoader(context: Context) : AssetLoader(DefaultMediaSou
 
     override suspend fun loadAsset(mediaItem: MediaItem): Asset {
         val itemBuilder = mediaItem.buildUpon()
-        return Asset(
-            mediaSource = mediaSourceFactory.createMediaSource(itemBuilder.build()),
-            mediaMetadata = mediaItem.mediaMetadata,
-            chapters = listOf(CHAPTER_1, CHAPTER_2)
-        )
+        val mediaSource = mediaSourceFactory.createMediaSource(itemBuilder.build())
+        return when (mediaItem.mediaId) {
+            ID_START_WITH_CHAPTER -> {
+                Asset(
+                    mediaSource = mediaSource,
+                    mediaMetadata = mediaItem.mediaMetadata,
+                    chapters = listOf(CHAPTER_1, CHAPTER_2)
+                )
+            }
+
+            ID_WITH_CHAPTER -> {
+                Asset(
+                    mediaSource = mediaSource,
+                    mediaMetadata = mediaItem.mediaMetadata,
+                    chapters = listOf(CHAPTER_3, CHAPTER_4)
+                )
+            }
+
+            else -> {
+                Asset(
+                    mediaSource = mediaSource,
+                    mediaMetadata = mediaItem.mediaMetadata,
+                    chapters = emptyList()
+                )
+            }
+        }
     }
 
     companion object {
         private const val URL = "https://rts-vod-amd.akamaized.net/ww/13317145/f1d49f18-f302-37ce-866c-1c1c9b76a824/master.m3u8"
-        val MEDIA_ITEM = MediaItem.fromUri(URL)
+        const val ID_START_WITH_CHAPTER = "ID_START_WITH_CHAPTER"
+        const val ID_WITH_CHAPTER = "ID_WITH_CHAPTER"
+
+        val MEDIA_ITEM = MediaItem.Builder().setMediaId(ID_START_WITH_CHAPTER).setUri(URL).build()
+        val MEDIA_ITEM_WITH_CHAPTER = MediaItem.Builder().setMediaId(ID_WITH_CHAPTER).setUri(URL).build()
+        val NO_CHAPTER_MEDIA_ITEM = MediaItem.Builder().setMediaId("NoChapter").setUri(URL).build()
 
         const val NEAR_END_POSITION_MS = 15_000L // the video has 17 sec duration
 
-        val CHAPTER_1 = Chapter(id = "Chapter1", 0, 5, MediaMetadata.EMPTY)
-        val CHAPTER_2 = Chapter(id = "Chapter1", 5, NEAR_END_POSITION_MS, MediaMetadata.EMPTY)
+        val CHAPTER_1 = Chapter(id = "Chapter1", 0, 5_000L, MediaMetadata.EMPTY)
+        val CHAPTER_2 = Chapter(id = "Chapter2", 5_000L, NEAR_END_POSITION_MS, MediaMetadata.EMPTY)
+
+        val CHAPTER_3 = Chapter(id = "Chapter3", 2_000L, 5_000L, MediaMetadata.EMPTY)
+        val CHAPTER_4 = Chapter(id = "Chapter4", 10_000L, NEAR_END_POSITION_MS, MediaMetadata.EMPTY)
     }
 }