diff --git a/android/app/src/main/java/com/remotedisplay/player/player/MediaPlayerManager.kt b/android/app/src/main/java/com/remotedisplay/player/player/MediaPlayerManager.kt index 401f6ae..15f29ec 100644 --- a/android/app/src/main/java/com/remotedisplay/player/player/MediaPlayerManager.kt +++ b/android/app/src/main/java/com/remotedisplay/player/player/MediaPlayerManager.kt @@ -152,6 +152,7 @@ class MediaPlayerManager( // Plain image mount (visibility flip + set bitmap). Shared by the transition-done swap and the // no-transition hard cut. private fun mountImageBitmap(bitmap: Bitmap) { + mountGeneration++ stopYoutubeIfPlaying() currentType = MediaType.IMAGE currentWidgetUrl = null // surface reused - a later widget show must reload @@ -183,6 +184,7 @@ class MediaPlayerManager( fun playYoutube(embedUrl: String, durationSec: Int = 0, muted: Boolean = false) { Log.i("MediaPlayerManager", "Playing YouTube: $embedUrl (muted=$muted)") + mountGeneration++ currentType = MediaType.YOUTUBE currentWidgetUrl = null // surface reused - a later widget show must reload youtubeMuted = muted || wallMute @@ -260,6 +262,7 @@ class MediaPlayerManager( return } Log.i("MediaPlayerManager", "Showing widget: $url") + mountGeneration++ currentType = MediaType.WIDGET currentWidgetUrl = url @@ -277,6 +280,7 @@ class MediaPlayerManager( fun playVideoFromUrl(url: String, muted: Boolean = false) { Log.i("MediaPlayerManager", "Streaming video from URL: $url (muted=$muted)") + mountGeneration++ stopYoutubeIfPlaying() currentType = MediaType.VIDEO currentWidgetUrl = null // surface reused - a later widget show must reload @@ -293,13 +297,36 @@ class MediaPlayerManager( } } + /** + * Bumped by every request to put something on screen. An async decode captures it and drops its + * result if the value has moved on — the same drop-if-replaced token PipOverlay.loadImageInto + * already carries. + * + * Without it a slow remote image (ImageLoader allows 10s connect + 30s read, against a slot + * that is usually 10s) finished long after the playlist had advanced and mounted itself over + * whatever was playing. If that was a video, the mount also called exoPlayer.stop(), which + * lands in STATE_IDLE — and the advance listener only fires onVideoComplete on STATE_ENDED or a + * playback error, so no advance was ever scheduled and the playlist stopped for good. The 60s + * refresh could not rescue it either: the playlist signature was unchanged, so the update + * returned early. + */ + private var mountGeneration: Long = 0L + fun showImageFromUrl(url: String, transition: TransitionSpec? = null) { Log.i("MediaPlayerManager", "Loading remote image: $url") // Capture the outgoing frame NOW, on the main thread, before the decode thread swaps it out. val from = if (transition != null) captureCurrentFrame() else null + val myGeneration = ++mountGeneration Thread { val bitmap = ImageLoader.decodeUrl(url, ImageLoader.screenWidth(context), ImageLoader.screenHeight(context)) mainHandler.post { + // Something else has been asked for since this decode started — including the + // error branch, whose onImageError posts next() and would otherwise cut short + // whatever is now playing. + if (myGeneration != mountGeneration) { + Log.i("MediaPlayerManager", "Dropping stale image decode: $url") + return@post + } if (bitmap == null) { Log.w("MediaPlayerManager", "Skipping unloadable remote image: $url") onImageError?.invoke(); return@post @@ -352,6 +379,7 @@ class MediaPlayerManager( } private fun mountVideo(file: File, muted: Boolean = false) { + mountGeneration++ stopYoutubeIfPlaying() currentType = MediaType.VIDEO currentWidgetUrl = null // surface reused - a later widget show must reload diff --git a/android/app/src/test/java/com/remotedisplay/player/player/StaleDecodeTest.kt b/android/app/src/test/java/com/remotedisplay/player/player/StaleDecodeTest.kt new file mode 100644 index 0000000..2647640 --- /dev/null +++ b/android/app/src/test/java/com/remotedisplay/player/player/StaleDecodeTest.kt @@ -0,0 +1,59 @@ +package com.remotedisplay.player.player + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * A remote image is decoded on a background thread and then mounted on the main thread. It was + * mounted unconditionally, with no check that it was still wanted. + * + * ImageLoader allows 10s connect + 30s read, against a slot that is typically 10s — so a slow or + * briefly unreachable host finished long after the playlist had moved on, and painted itself over + * whatever was playing. If that was a video the mount also called exoPlayer.stop(), landing in + * STATE_IDLE; the advance listener only fires onVideoComplete on STATE_ENDED or a playback error, + * so nothing scheduled the next item and the playlist stopped for good. The routine refresh could + * not rescue it either — the playlist signature was unchanged, so the update returned early. + * + * The error branch had the same shape: onImageError posts next(), cutting short whatever had since + * started playing. + * + * PipOverlay.loadImageInto already carried a drop-if-replaced token; this is the same idea, checked + * here as pure arithmetic so it needs no Android runtime. + */ +class StaleDecodeTest { + + /** Mirrors the guard: a decode applies only if nothing else has taken the screen since. */ + private fun applies(captured: Long, current: Long) = captured == current + + @Test fun THE_BUG_a_decode_that_finishes_after_the_playlist_moved_on_is_dropped() { + var generation = 0L + val captured = ++generation // the slow image starts loading + generation++ // ...the playlist advances to a video + assertFalse("a stale image must not paint over the current item", applies(captured, generation)) + } + + @Test fun a_decode_that_is_still_current_is_applied() { + var generation = 0L + val captured = ++generation + assertTrue(applies(captured, generation)) + } + + @Test fun only_the_LATEST_of_several_queued_decodes_wins() { + // Two images in a row, both slow: the first must not land after the second. + var generation = 0L + val first = ++generation + val second = ++generation + assertFalse(applies(first, generation)) + assertTrue(applies(second, generation)) + } + + @Test fun the_error_branch_is_gated_too() { + // onImageError posts next(). Firing it for an image nobody is waiting for would truncate + // whatever is playing now, which is the softer half of the same defect. + var generation = 0L + val captured = ++generation + generation++ + assertFalse("a stale failure must not advance the playlist", applies(captured, generation)) + } +}