Fix dead skip-unavailable-track logic

app/src/androidTest/kotlin/org/grakovne/lissen/playback/LissenMediaSourceFactoryTest.kt

@@ -1,58 +1,109 @@
 package org.grakovne.lissen.playback
 
+import androidx.core.os.BundleCompat
 import androidx.core.os.bundleOf
 import androidx.media3.common.MediaItem
 import androidx.media3.common.MediaMetadata
-import androidx.media3.exoplayer.source.DefaultMediaSourceFactory
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import io.mockk.mockk
 import org.grakovne.lissen.content.ExternalCoverProvider
 import org.grakovne.lissen.playback.service.FileClip
+import org.grakovne.lissen.playback.service.LissenDataSourceFactory
 import org.grakovne.lissen.playback.service.LissenMediaSourceFactory
 import org.grakovne.lissen.playback.service.PlaybackService.Companion.CHAPTER_START_MS
 import org.grakovne.lissen.playback.service.PlaybackService.Companion.FILE_SEGMENTS
+import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNotNull
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
 
 @RunWith(AndroidJUnit4::class)
+// The factory is given a DataSource.Factory rather than a DefaultMediaSourceFactory so that these
+// tests assert on the MediaSource the factory returns, which is what BasePlayer.getCurrentMediaItem
+// reads via the timeline window. Mocking the inner factory would only prove what we passed into it.
+//
+// The extras assertions are not redundant with the enclosing metadata assertions: in Media3 1.11.0,
+// MediaMetadata.equals and RequestMetadata.equals only check whether extras is null, never its
+// contents. FILE_SEGMENTS and CHAPTER_START_MS have to be checked by hand.
 class LissenMediaSourceFactoryTest {
-  private lateinit var mediaSourceFactory: DefaultMediaSourceFactory
   private lateinit var lissenMediaSourceFactory: LissenMediaSourceFactory
 
+  private lateinit var lissenDataSourceFactory: LissenDataSourceFactory
+
   @Before
   fun setUp() {
-    mediaSourceFactory = mockk(relaxed = true)
-    lissenMediaSourceFactory = LissenMediaSourceFactory(mediaSourceFactory)
+    lissenDataSourceFactory = mockk(relaxed = true)
+    lissenMediaSourceFactory = LissenMediaSourceFactory(lissenDataSourceFactory)
   }
 
   @Test
   fun no_exception_thrown_if_no_files() {
-    val mediaSource =
-      lissenMediaSourceFactory.createMediaSource(
-        MediaItem
-          .Builder()
-          .setMediaId(LissenMediaSourceFactory.MediaId("book-id", 5).toString())
-          .setRequestMetadata(
-            MediaItem.RequestMetadata
-              .Builder()
-              .setExtras(bundleOf(FILE_SEGMENTS to arrayListOf<FileClip>()))
-              .build(),
-          ).setMediaMetadata(
-            MediaMetadata
-              .Builder()
-              .setAlbumTitle("title")
-              .setTitle("chapter")
-              .setArtist("book")
-              .setIsBrowsable(false)
-              .setIsPlayable(true)
-              .setArtworkUri(ExternalCoverProvider.bookCoverUri("book-id"))
-              .setMediaType(MediaMetadata.MEDIA_TYPE_AUDIO_BOOK_CHAPTER)
-              .setExtras(bundleOf(CHAPTER_START_MS to (500 * 1000).toLong()))
-              .build(),
-          ).build(),
-      )
+    val mediaSource = lissenMediaSourceFactory.createMediaSource(chapterMediaItem(arrayListOf()))
     assertNotNull(mediaSource)
   }
+
+  @Test
+  fun media_id_and_request_metadata_preserved_for_single_segment_chapter() {
+    val mediaItem = chapterMediaItem(arrayListOf(FileClip("file-1", 0.0, 30.0)))
+    val reportedItem = lissenMediaSourceFactory.createMediaSource(mediaItem).mediaItem
+
+    assertEquals(mediaItem.mediaId, reportedItem.mediaId)
+    assertEquals(mediaItem.requestMetadata, reportedItem.requestMetadata)
+    assertEquals(mediaItem.mediaMetadata, reportedItem.mediaMetadata)
+    assertEquals(500_000L, reportedItem.mediaMetadata.extras?.getLong(CHAPTER_START_MS))
+    val reportedSegments =
+      reportedItem.requestMetadata.extras?.let {
+        BundleCompat.getParcelableArrayList(it, FILE_SEGMENTS, FileClip::class.java)
+      }
+    assertNotNull(reportedSegments)
+    assertEquals(1, reportedSegments!!.size)
+  }
+
+  @Test
+  fun media_id_and_request_metadata_preserved_for_multi_segment_chapter() {
+    val mediaItem =
+      chapterMediaItem(
+        arrayListOf(
+          FileClip("file-1", 0.0, 30.0),
+          FileClip("file-2", 30.0, 60.0),
+        ),
+      )
+
+    val reportedItem = lissenMediaSourceFactory.createMediaSource(mediaItem).mediaItem
+
+    assertEquals(mediaItem.mediaId, reportedItem.mediaId)
+    assertEquals(mediaItem.requestMetadata, reportedItem.requestMetadata)
+    assertEquals(mediaItem.mediaMetadata, reportedItem.mediaMetadata)
+    assertEquals(500_000L, reportedItem.mediaMetadata.extras?.getLong(CHAPTER_START_MS))
+    val reportedSegments =
+      reportedItem.requestMetadata.extras?.let {
+        BundleCompat.getParcelableArrayList(it, FILE_SEGMENTS, FileClip::class.java)
+      }
+    assertNotNull(reportedSegments)
+    assertEquals(2, reportedSegments!!.size)
+  }
+
+  private fun chapterMediaItem(segments: ArrayList<FileClip>): MediaItem =
+    MediaItem
+      .Builder()
+      .setMediaId(LissenMediaSourceFactory.MediaId("book-id", 5).toString())
+      .setRequestMetadata(
+        MediaItem.RequestMetadata
+          .Builder()
+          .setExtras(bundleOf(FILE_SEGMENTS to segments))
+          .build(),
+      ).setMediaMetadata(
+        MediaMetadata
+          .Builder()
+          .setAlbumTitle("title")
+          .setTitle("chapter")
+          .setArtist("book")
+          .setIsBrowsable(false)
+          .setIsPlayable(true)
+          .setArtworkUri(ExternalCoverProvider.bookCoverUri("book-id"))
+          .setMediaType(MediaMetadata.MEDIA_TYPE_AUDIO_BOOK_CHAPTER)
+          .setExtras(bundleOf(CHAPTER_START_MS to (500 * 1000).toLong()))
+          .build(),
+      ).build()
 }

app/src/main/kotlin/org/grakovne/lissen/playback/MediaModule.kt

@@ -92,16 +92,14 @@ object MediaModule {
         ).setRenderersFactory(renderersFactory)
         .setMediaSourceFactory(
           LissenMediaSourceFactory(
-            mediaSourceFactory =
-              DefaultMediaSourceFactory(
-                LissenDataSourceFactory(
-                  baseContext = context,
-                  mediaCache = mediaCache,
-                  requestHeadersProvider = requestHeadersProvider,
-                  session = sessionPreferences,
-                  connection = connectionPreferences,
-                  mediaProvider = mediaProvider,
-                ),
+            dataSourceFactory =
+              LissenDataSourceFactory(
+                baseContext = context,
+                mediaCache = mediaCache,
+                requestHeadersProvider = requestHeadersProvider,
+                session = sessionPreferences,
+                connection = connectionPreferences,
+                mediaProvider = mediaProvider,
               ),
           ),
         ).build()

app/src/main/kotlin/org/grakovne/lissen/playback/service/LissenMediaSourceFactory.kt

@@ -3,8 +3,8 @@ package org.grakovne.lissen.playback.service
 import android.os.Parcelable
 import androidx.core.os.BundleCompat
 import androidx.media3.common.MediaItem
-import androidx.media3.common.MediaMetadata
 import androidx.media3.common.util.UnstableApi
+import androidx.media3.datasource.DataSource
 import androidx.media3.exoplayer.drm.DrmSessionManagerProvider
 import androidx.media3.exoplayer.source.ClippingMediaSource
 import androidx.media3.exoplayer.source.ConcatenatingMediaSource2
@@ -24,8 +24,10 @@ data class FileClip(
 
 @UnstableApi
 class LissenMediaSourceFactory(
-  private val mediaSourceFactory: DefaultMediaSourceFactory,
+  dataSourceFactory: DataSource.Factory,
 ) : MediaSource.Factory {
+  private val mediaSourceFactory = DefaultMediaSourceFactory(dataSourceFactory)
+
   data class MediaId(
     val bookId: String,
     val chapterId: Int,
@@ -59,25 +61,33 @@ class LissenMediaSourceFactory(
 
   override fun getSupportedTypes(): IntArray = mediaSourceFactory.supportedTypes
 
+  /**
+   * The player reports the MediaItem of the MediaSource we return here, not the one it was given:
+   * both getCurrentMediaItem() and getMediaItemAt() read Timeline.Window.mediaItem, and that comes
+   * from the created source. So every source we build must carry the chapter item's identity
+   * (mediaId, requestMetadata with FILE_SEGMENTS, mediaMetadata with CHAPTER_START_MS) or it becomes
+   * invisible to PlaybackNavigationService and PlaybackSynchronizationService. Do not drop it.
+   */
   override fun createMediaSource(mediaItem: MediaItem): MediaSource {
     fun FileClip.toMediaSource(
       bookId: String,
-      metadata: MediaMetadata? = null,
-    ): MediaSource =
-      mediaSourceFactory
-        .createMediaSource(
-          MediaItem
-            .Builder()
-            .setUri(toLissenUri(bookId, fileId))
-            .apply { metadata?.let { setMediaMetadata(it) } }
-            .build(),
-        ).let {
+      template: MediaItem? = null,
+    ): MediaSource {
+      val innerItem =
+        (template?.buildUpon() ?: MediaItem.Builder())
+          .setUri(toLissenUri(bookId, fileId))
+          .build()
+
+      return mediaSourceFactory
+        .createMediaSource(innerItem)
+        .let {
           ClippingMediaSource
             .Builder(it)
             .setStartPositionUs((clipStart * 1_000_000).toLong())
             .setEndPositionUs((clipEnd * 1_000_000).toLong())
             .build()
         }
+    }
 
     return MediaId.fromString(mediaItem.mediaId)?.let { (bookId, chapterId) ->
       mediaItem.requestMetadata.extras?.let { extras ->
@@ -88,7 +98,7 @@ class LissenMediaSourceFactory(
             }
 
             1 -> {
-              segments.first().toMediaSource(bookId, mediaItem.mediaMetadata)
+              segments.first().toMediaSource(bookId, mediaItem)
             }
 
             else -> {
@@ -98,12 +108,8 @@ class LissenMediaSourceFactory(
                   segments.forEach {
                     add(it.toMediaSource(bookId), ((it.clipEnd - it.clipStart) * 1000).toLong())
                   }
-                }.setMediaItem(
-                  MediaItem
-                    .Builder()
-                    .setMediaMetadata(mediaItem.mediaMetadata)
-                    .build(),
-                ).build()
+                }.setMediaItem(mediaItem)
+                .build()
             }
           }
         }

app/src/main/kotlin/org/grakovne/lissen/playback/service/PlaybackNavigationService.kt

@@ -7,7 +7,6 @@ import androidx.media3.common.util.UnstableApi
 import androidx.media3.exoplayer.ExoPlayer
 import org.grakovne.lissen.common.RunningComponent
 import org.grakovne.lissen.content.LissenMediaProvider
-import org.grakovne.lissen.domain.DetailedItem
 import org.grakovne.lissen.persistence.preferences.PlaybackPreferences
 import javax.inject.Inject
 import javax.inject.Singleton
@@ -36,22 +35,19 @@ class PlaybackNavigationService
             }
           }
 
+          /**
+           * Skips chapters whose files are missing, which happens in offline mode when a book is
+           * only partly downloaded. Known limitation: this only covers transitions between items,
+           * so an unavailable first chapter is not skipped and still fails on load. Setting the
+           * playlist raises onTimelineChanged, not a discontinuity, so we never see it here.
+           */
           override fun onPositionDiscontinuity(
             oldPosition: Player.PositionInfo,
             newPosition: Player.PositionInfo,
             reason: Int,
           ) {
             val previousIndex = oldPosition.mediaItemIndex
             val currentIndex = newPosition.mediaItemIndex
-            val currentItem =
-              exoPlayer
-                .currentMediaItem
-                ?.localConfiguration
-                ?.tag as? DetailedItem
-
-            if (null == currentItem) {
-              return
-            }
 
             if (isTrackAvailable(exoPlayer.currentMediaItemIndex)) {
               return

app/src/main/kotlin/org/grakovne/lissen/playback/service/PlaybackService.kt

@@ -260,8 +260,7 @@ class PlaybackService : MediaLibraryService() {
                 .setMediaType(MediaMetadata.MEDIA_TYPE_AUDIO_BOOK_CHAPTER)
                 .setExtras(Bundle().apply { putLong(CHAPTER_START_MS, (chapter.start * 1000).toLong()) })
                 .build(),
-            ).setTag(book)
-            .build()
+            ).build()
         }
       return MediaItemsWithStartPosition(chapterMediaItems, chapterIndex, (chapterOffset * 1000).toLong())
     }