Archive/screentinker
1
0
Fork 0
mirror of https://github.com/screentinker/screentinker.git synced 2026-08-13 13:53:12 -06:00
Code Issues Actions 5 Packages Projects Releases Wiki Activity

Drop an image decode that finished after the screen moved on

Browse source 
A remote image is decoded on a background thread and mounted on the main thread, and it was mounted
unconditionally — nothing checked it was still wanted.

ImageLoader allows 10s connect plus 30s read, against a slot that is typically 10s, so a slow or
briefly unreachable host finished long after the playlist had advanced and painted itself over
whatever was playing. When 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.
Nothing scheduled the next item, so the playlist stopped permanently. The routine refresh could not
rescue it: the playlist signature was unchanged, so the update returned early, and content was still
on screen so nothing looked wrong from the server's side.

The failure branch had the same shape more mildly — onImageError posts next(), cutting short
whatever had since started playing.

Every path that takes the screen now bumps a generation, and a decode applies only if the value it
captured is still current. PipOverlay.loadImageInto has always carried this token; the fullscreen
path was the one place a background result was applied with no staleness check.

4 tests over the guard, kept as pure arithmetic so they need no Android runtime, including that only
the latest of several queued decodes wins and that the error branch is gated too. 134 Android JVM
tests green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL
This commit is contained in:
Claude 2026-07-30 21:01:37 -05:00
parent 88e4a9eb49
commit 6e9a1f9711
2 changed files with 87 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
android/app/src/main/java/com/remotedisplay/player/player/MediaPlayerManager.kt
View file

@ -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

59
android/app/src/test/java/com/remotedisplay/player/player/StaleDecodeTest.kt Normal file
View file

@ -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))
}
}