fix(android): remove pollLoop natural-advance -- races with selectUpnp and skip
android / Build + lint + test (push) Successful in 4m1s
android / Build + lint + test (push) Successful in 4m1s
Polling alone cannot distinguish Sonos auto-advancing via SetNextAVTransportURI from URI changes we made ourselves via syncCurrentItemToRemote. This produced two races: (1) activation race -- first poll returns stale URI from prior session, second returns new URI, false-positive fires and double-advances the cursor; (2) user-skip race -- skip's syncCurrentItemToRemote changes the URI, next poll sees the change and fires again. Remove the detection block and previousTrackUri capture from pollOnce entirely. pollLoop is now a pure state-tracker (position + transport state) plus the one-shot initial pre-queue gate. GENA event subscriptions to AVTransport LastChange are the correct fix; deferred to its own slice (see parity-map). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
+18
-38
@@ -53,9 +53,9 @@ class MinstrelForwardingPlayer(
|
|||||||
holder.active.collect { active -> onActiveChanged(active) }
|
holder.active.collect { active -> onActiveChanged(active) }
|
||||||
}
|
}
|
||||||
// No Player.Listener -- the seekToNextMediaItem/seekToPreviousMediaItem
|
// No Player.Listener -- the seekToNextMediaItem/seekToPreviousMediaItem
|
||||||
// overrides and pollLoop natural-advance branch each call preQueueNext
|
// overrides call preQueueNext explicitly. A listener-based path raced
|
||||||
// explicitly. A listener-based path raced with the override's launched
|
// with the override's launched syncCurrentItemToRemote, producing
|
||||||
// syncCurrentItemToRemote, producing non-deterministic SOAP order.
|
// non-deterministic SOAP order.
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun isRemote(): Boolean = holder.active.value != null
|
private fun isRemote(): Boolean = holder.active.value != null
|
||||||
@@ -169,11 +169,9 @@ class MinstrelForwardingPlayer(
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Speculative pre-queue of the next track so Sonos can advance gap-free.
|
* Speculative pre-queue of the next track so Sonos can advance gap-free.
|
||||||
* Failures (token mint OR SetNextAVTransportURI rejection) silently fall
|
* Failures (token mint OR SetNextAVTransportURI rejection) are logged and
|
||||||
* back to the poll-driven advance path -- the pollLoop detects STOPPED +
|
* swallowed. The only consequence is a sub-second audible gap at the track
|
||||||
* 0:00 RelTime and calls syncCurrentItemToRemote on whatever the new
|
* boundary, not a broken route.
|
||||||
* currentMediaItem is. The only consequence of failure here is a sub-second
|
|
||||||
* audible gap at track boundary, not a broken route.
|
|
||||||
*/
|
*/
|
||||||
private suspend fun preQueueNext(active: ActiveUpnp) {
|
private suspend fun preQueueNext(active: ActiveUpnp) {
|
||||||
val nextMediaId = handlerEvaluate { nextItemMediaId() } ?: return
|
val nextMediaId = handlerEvaluate { nextItemMediaId() } ?: return
|
||||||
@@ -223,6 +221,15 @@ class MinstrelForwardingPlayer(
|
|||||||
}
|
}
|
||||||
|
|
||||||
private suspend fun pollLoop(active: ActiveUpnp) {
|
private suspend fun pollLoop(active: ActiveUpnp) {
|
||||||
|
// pollLoop is a state-tracker for position + transport state. We do NOT
|
||||||
|
// attempt to mirror Sonos's auto-advance into the local queue cursor --
|
||||||
|
// that would require GENA event subscriptions to AVTransport's LastChange
|
||||||
|
// event (see the parity-map). Without that, polling alone cannot
|
||||||
|
// distinguish "Sonos finished track N and auto-played track N+1 via
|
||||||
|
// SetNextAVTransportURI" from "we just changed Sonos's URI ourselves
|
||||||
|
// via syncCurrentItemToRemote". The local cursor stays in sync via user
|
||||||
|
// transport actions (pause/seek/skip flow through the override which
|
||||||
|
// re-runs syncCurrentItemToRemote).
|
||||||
var initialPreQueueDone = false
|
var initialPreQueueDone = false
|
||||||
while (scope.isActive && holder.active.value?.routeId == active.routeId) {
|
while (scope.isActive && holder.active.value?.routeId == active.routeId) {
|
||||||
val outcome = runCatching { pollOnce(active) }
|
val outcome = runCatching { pollOnce(active) }
|
||||||
@@ -255,16 +262,11 @@ class MinstrelForwardingPlayer(
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* One poll tick: read position + transport state from Sonos, apply to
|
* One poll tick: read position + transport state from Sonos and apply to
|
||||||
* [remoteState], and detect natural-advance (URI change driven by a
|
* [remoteState]. Extracted from [pollLoop] to keep that method within
|
||||||
* previously queued SetNextAVTransportURI). Extracted from [pollLoop]
|
* detekt's LongMethod=60 limit.
|
||||||
* to keep that method within detekt's LongMethod=60 limit.
|
|
||||||
*/
|
*/
|
||||||
private suspend fun pollOnce(active: ActiveUpnp) {
|
private suspend fun pollOnce(active: ActiveUpnp) {
|
||||||
// Capture before applyPositionInfo so we can detect a URI
|
|
||||||
// change that Sonos made via the pre-queued SetNextAVTransportURI
|
|
||||||
// (natural auto-advance without any user skip action).
|
|
||||||
val previousTrackUri = remoteState.currentTrackUri
|
|
||||||
val info = active.avTransport.getPositionInfo()
|
val info = active.avTransport.getPositionInfo()
|
||||||
remoteState.applyPositionInfo(
|
remoteState.applyPositionInfo(
|
||||||
positionMs = info.relTimeMs,
|
positionMs = info.relTimeMs,
|
||||||
@@ -278,28 +280,6 @@ class MinstrelForwardingPlayer(
|
|||||||
TransportState.STOPPED -> remoteState.applyTransportStopped()
|
TransportState.STOPPED -> remoteState.applyTransportStopped()
|
||||||
TransportState.TRANSITIONING, TransportState.UNKNOWN -> Unit
|
TransportState.TRANSITIONING, TransportState.UNKNOWN -> Unit
|
||||||
}
|
}
|
||||||
// Detect remote-side natural advance: Sonos auto-advanced
|
|
||||||
// through SetNextAVTransportURI so the URI changed without
|
|
||||||
// any local user action. Advance the local queue cursor and
|
|
||||||
// call preQueueNext explicitly to set up the next-next URI on
|
|
||||||
// Sonos. Both guards must be non-blank: previousTrackUri="" on
|
|
||||||
// the very first poll and we must not treat that initial
|
|
||||||
// empty-to-URI transition as an advance.
|
|
||||||
if (info.trackUri.isNotBlank() &&
|
|
||||||
previousTrackUri.isNotBlank() &&
|
|
||||||
info.trackUri != previousTrackUri
|
|
||||||
) {
|
|
||||||
Timber.w(
|
|
||||||
"UPnP natural-advance detected: %s -> %s",
|
|
||||||
previousTrackUri,
|
|
||||||
info.trackUri,
|
|
||||||
)
|
|
||||||
// Run on the application looper. ExoPlayer is paused
|
|
||||||
// while UPnP is active (playWhenReady=false invariant) so
|
|
||||||
// the cursor advances without producing local audio.
|
|
||||||
handler.post { delegate.seekToNextMediaItem() }
|
|
||||||
preQueueNext(active)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Run [block] on the application looper and bring its result back. */
|
/** Run [block] on the application looper and bring its result back. */
|
||||||
|
|||||||
Reference in New Issue
Block a user