fix(android): re-prime Sonos queue when user plays a different playlist
android / Build + lint + test (push) Successful in 3m56s
android / Build + lint + test (push) Successful in 3m56s
Before: tapping a different playlist while Sonos was the active route updated the player view but Sonos kept the old queue and played those tracks (or whatever was last there). PlayerController.setQueue replaced the local ExoPlayer queue and called play(), which forwarded SOAP Play to Sonos -- but Sonos's native queue (loaded once at route selection via removeAllTracks + AddURIToQueue + SetAVTransportURI) was never touched on subsequent setQueue calls. Now: MinstrelForwardingPlayer.setMediaItems (all 3 overloads) clears holder.active + sets target synchronously so the immediately-following play() drops via isLoadingUpnp(). OutputPickerController observes uiState.queue identity changes; when target or active is non-null and the queue key shifted, it re-runs loadQueueOnSonos under the existing selectUpnpMutex and restores active when done. Sonos resync failures drop cleanly to local (selectedUpnpRouteIdInternal nulled). Doesn't touch addMediaItem / radio-append paths -- those leave Sonos's queue stale and need a separate AddURIToQueue extension hook; out of scope for this fix.
This commit is contained in:
@@ -8,6 +8,7 @@ import androidx.lifecycle.DefaultLifecycleObserver
|
||||
import androidx.lifecycle.LifecycleOwner
|
||||
import androidx.lifecycle.ProcessLifecycleOwner
|
||||
import androidx.media3.common.ForwardingPlayer
|
||||
import androidx.media3.common.MediaItem
|
||||
import androidx.media3.common.Player
|
||||
import com.fabledsword.minstrel.player.output.ActiveUpnp
|
||||
import com.fabledsword.minstrel.player.output.ActiveUpnpHolder
|
||||
@@ -124,6 +125,43 @@ class MinstrelForwardingPlayer(
|
||||
private fun isLoadingUpnp(): Boolean =
|
||||
holder.target.value != null && holder.active.value == null
|
||||
|
||||
// ─── setMediaItems intercepts ──────────────────────────────────────
|
||||
// When PlayerController.setQueue replaces the queue while Sonos is the
|
||||
// active route, the wrapped delegate's queue gets the new items but
|
||||
// Sonos's native queue still holds the OLD tracks -- and the play()
|
||||
// that PlayerController fires immediately after setMediaItems would
|
||||
// resume the old Sonos queue (user reported on-device: "player view
|
||||
// updates but Sonos queue does not"). We clear active + set target
|
||||
// synchronously here so the next play() in the same IPC sequence
|
||||
// drops via isLoadingUpnp() = true; the OutputPickerController
|
||||
// observes the uiState.queue change and runs the resync (re-clears
|
||||
// Sonos's native queue + AddURIToQueue the new tracks + Play).
|
||||
|
||||
override fun setMediaItems(mediaItems: List<MediaItem>) {
|
||||
super.setMediaItems(mediaItems)
|
||||
markPendingResyncIfRemote()
|
||||
}
|
||||
|
||||
override fun setMediaItems(mediaItems: List<MediaItem>, resetPosition: Boolean) {
|
||||
super.setMediaItems(mediaItems, resetPosition)
|
||||
markPendingResyncIfRemote()
|
||||
}
|
||||
|
||||
override fun setMediaItems(mediaItems: List<MediaItem>, startIndex: Int, startPositionMs: Long) {
|
||||
super.setMediaItems(mediaItems, startIndex, startPositionMs)
|
||||
markPendingResyncIfRemote()
|
||||
}
|
||||
|
||||
private fun markPendingResyncIfRemote() {
|
||||
val wasActive = holder.active.value ?: return
|
||||
Timber.w(
|
||||
"setMediaItems while UPnP active (%s) -- marking pending resync",
|
||||
wasActive.routeName,
|
||||
)
|
||||
holder.set(null)
|
||||
holder.setTarget(wasActive.routeId)
|
||||
}
|
||||
|
||||
override fun play() {
|
||||
if (isLoadingUpnp()) {
|
||||
Timber.w("ForwardingPlayer.play() dropped -- UPnP loading")
|
||||
|
||||
+86
@@ -139,6 +139,12 @@ class OutputPickerController @Inject constructor(
|
||||
) = refresh()
|
||||
}
|
||||
|
||||
// Last-synced queue identity (track ids joined). Used by the
|
||||
// observeQueueChangesForSonosResync loop below to detect when the user
|
||||
// has played a different playlist while UPnP is active and re-prime
|
||||
// Sonos's native queue accordingly.
|
||||
private var lastSyncedQueueKey: String? = null
|
||||
|
||||
init {
|
||||
// Two-arg addCallback registers with no discovery flag —
|
||||
// androidx.mediarouter 1.7.0's default passive behavior:
|
||||
@@ -154,6 +160,86 @@ class OutputPickerController @Inject constructor(
|
||||
scope.launch {
|
||||
playerFactory.dropEvents.collect { handleRemoteDrop() }
|
||||
}
|
||||
scope.launch { observeQueueChangesForSonosResync() }
|
||||
}
|
||||
|
||||
/**
|
||||
* When the user plays a different playlist while Sonos is active,
|
||||
* PlayerController.setQueue replaces the local queue but Sonos's
|
||||
* native queue still holds the OLD tracks. MinstrelForwardingPlayer's
|
||||
* setMediaItems override clears holder.active + sets target so the
|
||||
* imminent play() call drops (drops via isLoadingUpnp() = true). Then
|
||||
* this collector observes the uiState.queue change and re-runs
|
||||
* loadQueueOnSonos to push the new tracks to Sonos.
|
||||
*
|
||||
* Discrimination: selectUpnp's initial-load path doesn't change
|
||||
* uiState.queue (the queue was already populated before route
|
||||
* selection), so this collector doesn't fire during that window. Only
|
||||
* a fresh setQueue from PlayerController bumps the joined-ids key.
|
||||
*/
|
||||
private suspend fun observeQueueChangesForSonosResync() {
|
||||
playerController.uiState.collect { state ->
|
||||
val newKey = state.queue.joinToString("|") { it.id }
|
||||
if (newKey == lastSyncedQueueKey) return@collect
|
||||
lastSyncedQueueKey = newKey
|
||||
// Route can be in target (setMediaItems-induced clearing already
|
||||
// ran in ForwardingPlayer) OR in active (queue changed via
|
||||
// addMediaItem / replaceMediaItems etc. which don't hit the
|
||||
// markPending hook, OR the dispatcher race where the collector
|
||||
// observed uiState before markPendingResyncIfRemote ran).
|
||||
val routeId = activeUpnpHolder.target.value
|
||||
?: activeUpnpHolder.active.value?.routeId
|
||||
?: return@collect
|
||||
if (state.queue.isEmpty()) {
|
||||
Timber.w("Sonos resync skipped: empty queue (clearing target)")
|
||||
activeUpnpHolder.setTarget(null)
|
||||
return@collect
|
||||
}
|
||||
// Clear active + set target if it wasn't already cleared, so any
|
||||
// transport calls during the resync drop via isLoadingUpnp.
|
||||
if (activeUpnpHolder.active.value != null) {
|
||||
activeUpnpHolder.set(null)
|
||||
activeUpnpHolder.setTarget(routeId)
|
||||
}
|
||||
scope.launch { resyncSonosQueue(routeId, state.queue, state.queueIndex) }
|
||||
}
|
||||
}
|
||||
|
||||
private suspend fun resyncSonosQueue(
|
||||
routeId: String,
|
||||
queue: List<TrackRef>,
|
||||
currentIndex: Int,
|
||||
) = selectUpnpMutex.withLock {
|
||||
val upnpRoute = upnpDiscovery.routes.value.firstOrNull { it.id == routeId }
|
||||
val transport = upnpDiscovery.transportFor(routeId)
|
||||
if (upnpRoute == null || transport == null) {
|
||||
Timber.w(
|
||||
"Sonos resync: route or transport gone for %s, dropping to local",
|
||||
routeId,
|
||||
)
|
||||
activeUpnpHolder.setTarget(null)
|
||||
selectedUpnpRouteIdInternal.value = null
|
||||
return@withLock
|
||||
}
|
||||
val outputRoute = OutputRoute.fromUpnpRoute(upnpRoute)
|
||||
val rendering = renderingClientFor(routeId)
|
||||
Timber.w("Sonos resync: reloading %d tracks on %s", queue.size, outputRoute.name)
|
||||
runCatching {
|
||||
loadQueueOnSonos(transport, outputRoute, queue, currentIndex)
|
||||
activeUpnpHolder.set(
|
||||
ActiveUpnp(
|
||||
routeId = routeId,
|
||||
routeName = outputRoute.name,
|
||||
avTransport = transport,
|
||||
rendering = rendering,
|
||||
),
|
||||
)
|
||||
activeUpnpHolder.setTarget(null)
|
||||
}.onFailure { e ->
|
||||
Timber.w(e, "Sonos resync failed -- dropping to local")
|
||||
activeUpnpHolder.setTarget(null)
|
||||
selectedUpnpRouteIdInternal.value = null
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user