From 8e578d2068eb39ed77c08c36d38d27672f3a20c3 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 4 Jun 2026 06:55:39 -0400 Subject: [PATCH] fix(android): 2s transport-ack lockout so Prev/Next isn't undone by stale poll --- .../player/MinstrelForwardingPlayer.kt | 3 +++ .../minstrel/player/PlayerController.kt | 19 ++++++++++++++----- .../minstrel/player/RemotePlayerState.kt | 13 +++++++++++++ 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelForwardingPlayer.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelForwardingPlayer.kt index 60525efd..6b221739 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelForwardingPlayer.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelForwardingPlayer.kt @@ -189,6 +189,7 @@ class MinstrelForwardingPlayer( return } super.seekTo(mediaItemIndex, positionMs) + remoteState.markUserTransport(SystemClock.elapsedRealtime()) scope.launch { runCatching { active.avTransport.seekToTrack(mediaItemIndex + 1) @@ -214,6 +215,7 @@ class MinstrelForwardingPlayer( // then delegate to Sonos Next. PollLoop reconciles cursor via // Track index if they diverge. super.seekToNextMediaItem() + remoteState.markUserTransport(SystemClock.elapsedRealtime()) scope.launch { runCatching { active.avTransport.next() } .onFailure { handleSoapFailure(active, it) } @@ -234,6 +236,7 @@ class MinstrelForwardingPlayer( // Super first for immediate local cursor advance (UI feedback); // then delegate to Sonos Previous. PollLoop reconciles. super.seekToPreviousMediaItem() + remoteState.markUserTransport(SystemClock.elapsedRealtime()) scope.launch { runCatching { active.avTransport.previous() } .onFailure { handleSoapFailure(active, it) } diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt index 9a12fa26..4adc375a 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt @@ -525,11 +525,19 @@ class PlayerController @Inject constructor( val newPos = effectivePosition.coerceAtLeast(0) val newDur = effectiveDuration(upnpActive, remoteState.durationMs, controller.duration) val newBuf = controller.bufferedPosition.coerceAtLeast(0) - // Forward-only. If desiredIdx < queueIndex, the user just pressed Next - // and Sonos's poll hasn't caught up yet -- walking backward would undo - // their press (logcat 2026-06-04 showed exactly that jump-back-then- - // forward cycle). Mirrors maybeSyncLocalCursor's forward-only policy. - val trackChanged = desiredIdx > current.queueIndex && desiredIdx in queueRefs.indices + // Two guards against undoing the user's transport action while + // remoteState is mid-refresh from the next SOAP poll: + // 1. Forward-only: a backward move from desiredIdx < queueIndex + // would undo a Next press (logcat 2026-06-04). + // 2. Transport-ack lockout: even forward moves are ignored within + // TRANSPORT_ACK_WINDOW_MS of a user transport action, so a Prev + // press isn't undone by a stale remoteState reading the older + // pre-press track. + val sinceTransport = SystemClock.elapsedRealtime() - remoteState.lastUserTransportAtMs + val inTransportAckWindow = sinceTransport < TRANSPORT_ACK_WINDOW_MS + val trackChanged = !inTransportAckWindow && + desiredIdx > current.queueIndex && + desiredIdx in queueRefs.indices val somethingChanged = trackChanged || current.isPlaying != effectiveIsPlaying || current.positionMs != newPos || @@ -667,6 +675,7 @@ class PlayerController @Inject constructor( const val MINSTREL_SOURCE_KEY: String = "minstrel_source" private const val MS_PER_SECOND = 1_000L private const val MAX_INTERPOLATION_DRIFT_MS = 5_000L + private const val TRANSPORT_ACK_WINDOW_MS = 2_000L } } diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/RemotePlayerState.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/RemotePlayerState.kt index fdbd5fcc..1e44947c 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/RemotePlayerState.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/RemotePlayerState.kt @@ -27,6 +27,18 @@ class RemotePlayerState @Inject constructor() { @Volatile var lastError: Throwable? = null; private set @Volatile var trackNumber: Int = 0; private set + // SystemClock.elapsedRealtime() at the last user-initiated transport + // (next/prev/seekToIndex). PlayerController.tickPositionPoll uses this to + // suppress track-change adjustments during the window where remoteState + // hasn't yet refreshed from the next SOAP poll -- without it the polling + // tick would walk the cursor back to Sonos's pre-press track and undo + // user input. Set from MinstrelForwardingPlayer's transport overrides. + @Volatile var lastUserTransportAtMs: Long = 0L; private set + + fun markUserTransport(nowMs: Long) { + lastUserTransportAtMs = nowMs + } + @Volatile private var consecutivePollFailures: Int = 0 fun applyPositionInfo(positionMs: Long, durationMs: Long, trackUri: String, trackNumber: Int) { @@ -64,6 +76,7 @@ class RemotePlayerState @Inject constructor() { lastError = null consecutivePollFailures = 0 trackNumber = 0 + lastUserTransportAtMs = 0L } private companion object {