diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerController.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerController.kt index 806897d6..9d0dddec 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerController.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerController.kt @@ -139,11 +139,13 @@ 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 + // Last-synced queue identity. Used by observeQueueChangesForSonosResync + // to detect when the user has mutated the queue (full replacement, + // playNext insert, or radio-append) while UPnP is active and apply the + // minimum-incremental set of Sonos SOAP operations to bring its native + // queue back in sync. Stored as the full id list (not a join-key) so we + // can run the longest-common-prefix / common-suffix diff. + private var lastSyncedQueueIds: List? = null init { // Two-arg addCallback registers with no discovery flag — @@ -179,14 +181,14 @@ class OutputPickerController @Inject constructor( */ private suspend fun observeQueueChangesForSonosResync() { playerController.uiState.collect { state -> - val newKey = state.queue.joinToString("|") { it.id } - if (newKey == lastSyncedQueueKey) return@collect - lastSyncedQueueKey = newKey + val newIds = state.queue.map { it.id } + val oldIds = lastSyncedQueueIds + if (newIds == oldIds) return@collect + lastSyncedQueueIds = newIds // 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). + // addMediaItem / removeMediaItems etc. which don't hit the + // markPending hook). val routeId = activeUpnpHolder.target.value ?: activeUpnpHolder.active.value?.routeId ?: return@collect @@ -195,20 +197,26 @@ class OutputPickerController @Inject constructor( 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, oldIds.orEmpty(), state.queue, state.queueIndex) } - scope.launch { resyncSonosQueue(routeId, state.queue, state.queueIndex) } } } + /** + * Bring Sonos's native queue back in sync with the local queue after a + * mutation. Tries an incremental SOAP diff first (RemoveTrackRangeFromQueue + * + AddURIToQueue at the insertion point) so playback continues without + * interruption -- that's what playNext / radio-append want. Falls back to + * the full removeAllTracks + reload path when the diff implies the current + * Sonos track was deleted (e.g. user switched playlists), which is what + * the user-reported "Sonos queue does not update" bug needed. + */ private suspend fun resyncSonosQueue( routeId: String, - queue: List, - currentIndex: Int, + oldIds: List, + newQueue: List, + newCurrentIndex: Int, ) = selectUpnpMutex.withLock { val upnpRoute = upnpDiscovery.routes.value.firstOrNull { it.id == routeId } val transport = upnpDiscovery.transportFor(routeId) @@ -221,11 +229,32 @@ class OutputPickerController @Inject constructor( selectedUpnpRouteIdInternal.value = null return@withLock } + val handledIncrementally = runCatching { + tryIncrementalResync(transport, oldIds, newQueue) + }.getOrElse { e -> + Timber.w(e, "Sonos incremental resync errored; falling back to full reload") + false + } + if (handledIncrementally) { + // Active was never cleared on the incremental path; clear any + // target that markPendingResyncIfRemote set (it didn't, for + // incremental cases that don't go through setMediaItems, but + // belt-and-suspenders). + activeUpnpHolder.setTarget(null) + return@withLock + } + // Full rebuild: ensure active is cleared so transport calls drop + // (markPendingResyncIfRemote may already have done this on the + // setMediaItems path). + if (activeUpnpHolder.active.value != null) { + activeUpnpHolder.set(null) + activeUpnpHolder.setTarget(routeId) + } val outputRoute = OutputRoute.fromUpnpRoute(upnpRoute) val rendering = renderingClientFor(routeId) - Timber.w("Sonos resync: reloading %d tracks on %s", queue.size, outputRoute.name) + Timber.w("Sonos resync: full reload of %d tracks on %s", newQueue.size, outputRoute.name) runCatching { - loadQueueOnSonos(transport, outputRoute, queue, currentIndex) + loadQueueOnSonos(transport, outputRoute, newQueue, newCurrentIndex) activeUpnpHolder.set( ActiveUpnp( routeId = routeId, @@ -236,12 +265,105 @@ class OutputPickerController @Inject constructor( ) activeUpnpHolder.setTarget(null) }.onFailure { e -> - Timber.w(e, "Sonos resync failed -- dropping to local") + Timber.w(e, "Sonos resync (full) failed -- dropping to local") activeUpnpHolder.setTarget(null) selectedUpnpRouteIdInternal.value = null } } + /** + * Diff-based incremental Sonos queue sync. Returns true when the new + * queue can be produced from the old one with a remove-then-insert at + * the same middle slice -- the common-prefix and common-suffix portions + * stay untouched, and the current Sonos track must lie in the preserved + * prefix (otherwise the diff would orphan playback). Returns false to + * signal the caller to fall back to a full reload. + */ + private suspend fun tryIncrementalResync( + transport: AVTransportClient, + oldIds: List, + newQueue: List, + ): Boolean { + val newIds = newQueue.map { it.id } + if (oldIds == newIds) return true + val prefixLen = commonPrefixLength(oldIds, newIds) + val suffixLen = commonSuffixLength( + oldIds.subList(prefixLen, oldIds.size), + newIds.subList(prefixLen, newIds.size), + ) + val removedCount = oldIds.size - prefixLen - suffixLen + val addedCount = newIds.size - prefixLen - suffixLen + // Sonos's current track number is 1-based; compare against the + // preserved-prefix range as 0-based. If the current track is in + // the removed slice, incremental can't preserve playback -- caller + // falls back to full rebuild. + val currentSonosIdx0 = remoteState.trackNumber - 1 + val canApply = currentSonosIdx0 in 0 until prefixLen + if (canApply) { + applyQueueDiff(transport, newQueue, prefixLen, removedCount, addedCount) + } else { + Timber.w( + "Sonos incremental: current track %d not in preserved prefix [0,%d); full rebuild", + currentSonosIdx0, + prefixLen, + ) + } + return canApply + } + + private suspend fun applyQueueDiff( + transport: AVTransportClient, + newQueue: List, + prefixLen: Int, + removedCount: Int, + addedCount: Int, + ) { + if (removedCount > 0) { + Timber.w( + "Sonos incremental: RemoveTrackRangeFromQueue start=%d count=%d", + prefixLen + 1, + removedCount, + ) + transport.removeTrackRangeFromQueue( + startingIndex = prefixLen + 1, + numberOfTracks = removedCount, + ) + } + if (addedCount == 0) return + Timber.w( + "Sonos incremental: AddURIToQueue x%d starting at position %d", + addedCount, + prefixLen + 1, + ) + for (i in 0 until addedCount) { + val ref = newQueue[prefixLen + i] + val token = streamTokens.mint(ref.id) + transport.addURIToQueue( + uri = token.url, + mime = token.mime, + title = token.title, + enqueuedURIPosition = prefixLen + i + 1, + ) + if (i > 0) delay(EXTEND_THROTTLE_MS) + } + } + + private fun commonPrefixLength(a: List, b: List): Int { + val limit = minOf(a.size, b.size) + for (i in 0 until limit) { + if (a[i] != b[i]) return i + } + return limit + } + + private fun commonSuffixLength(a: List, b: List): Int { + val limit = minOf(a.size, b.size) + for (i in 0 until limit) { + if (a[a.size - 1 - i] != b[b.size - 1 - i]) return i + } + return limit + } + /** * Called when the active UPnP route drops unexpectedly (poll-failure * threshold or SOAP exception). Captures the last remote position + diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/AVTransportClient.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/AVTransportClient.kt index 11dbbe58..0d27a4f2 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/AVTransportClient.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/AVTransportClient.kt @@ -62,6 +62,31 @@ class AVTransportClient( ) } + /** + * Remove a contiguous range of tracks from the renderer's native queue. + * Sonos-specific extension to AVTransport; `UpdateID=0` skips the queue- + * version check so this works without first calling GetQueue to learn + * the current update id. + * + * [startingIndex] is 1-based per Sonos convention; [numberOfTracks] is + * the count to remove. Used by OutputPickerController's incremental + * queue resync path (radio-append: remove tail, then AddURIToQueue + * the new items). + */ + suspend fun removeTrackRangeFromQueue(startingIndex: Int, numberOfTracks: Int) { + soap.call( + controlUrl = controlUrl, + serviceType = SERVICE_TYPE, + action = "RemoveTrackRangeFromQueue", + args = mapOf( + "InstanceID" to "0", + "UpdateID" to "0", + "StartingIndex" to startingIndex.toString(), + "NumberOfTracks" to numberOfTracks.toString(), + ), + ) + } + /** * Append a track to the renderer's queue. Sonos returns the assigned * track number + new total queue length in the response, but we don't