From e87516bbe4f6ee167f0cd5fbbd85c53a81685036 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 17 Aug 2026 22:35:53 -0400 Subject: [PATCH] =?UTF-8?q?refactor(player):=20split=20Sonos=20queue=20loa?= =?UTF-8?q?ding=20out=20of=20the=20picker=20=E2=80=94=20#2728?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit detekt flagged OutputPickerController as LargeClass once the verify path landed. Extracting rather than suppressing: how the renderer's queue is shaped is a different concern from which route is selected, and it had grown big enough to hide a bug — every write in here is a SOAP call that can fail on its own, and nothing ever read the result back. SonosQueueLoader now owns load / extend / verify / append and the incremental diff. The picker keeps route selection and asks it for queue work. No behaviour change. --- .../player/output/OutputPickerController.kt | 299 +---------------- .../player/output/SonosQueueLoader.kt | 316 ++++++++++++++++++ 2 files changed, 327 insertions(+), 288 deletions(-) create mode 100644 android/app/src/main/java/com/fabledsword/minstrel/player/output/SonosQueueLoader.kt 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 57b2860c..42a423b2 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 @@ -10,11 +10,9 @@ import com.fabledsword.minstrel.models.TrackRef import com.fabledsword.minstrel.player.PlayerController import com.fabledsword.minstrel.player.PlayerFactory import com.fabledsword.minstrel.player.RemotePlayerState -import com.fabledsword.minstrel.player.StreamTokenProvider import com.fabledsword.minstrel.player.output.upnp.AVTransportClient import com.fabledsword.minstrel.player.output.upnp.RenderingControlClient import com.fabledsword.minstrel.player.output.upnp.SoapClient -import com.fabledsword.minstrel.player.output.upnp.SoapFaultException import com.fabledsword.minstrel.player.output.upnp.TransportState import com.fabledsword.minstrel.player.output.upnp.UpnpDiscoveryController import com.fabledsword.minstrel.player.output.upnp.bareUdn @@ -61,7 +59,7 @@ data class RouteSnapshot( * - [OutputRoute.Protocol.SYSTEM] — MediaRouter.selectRoute (built-in, * wired, Bluetooth) * - [OutputRoute.Protocol.UPNP] — mint a signed stream token via - * [StreamTokenProvider.mint], drive the discovered renderer with + * [SonosQueueLoader], drive the discovered renderer with * AVTransport.SetAVTransportURI + Play, pause local playback so * audio yields to the network speaker * - [OutputRoute.Protocol.CAST] / [OutputRoute.Protocol.SONOS] — @@ -79,7 +77,7 @@ class OutputPickerController @Inject constructor( private val upnpDiscovery: UpnpDiscoveryController, private val playerController: PlayerController, private val playerFactory: PlayerFactory, - private val streamTokens: StreamTokenProvider, + private val sonosQueue: SonosQueueLoader, private val activeUpnpHolder: ActiveUpnpHolder, private val remoteState: RemotePlayerState, private val okHttp: OkHttpClient, @@ -256,7 +254,7 @@ class OutputPickerController @Inject constructor( * 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. + * SonosQueueLoader.load 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 @@ -291,8 +289,8 @@ class OutputPickerController @Inject constructor( * Rebuild the renderer's queue when playback stopped because the renderer * ran off the end of a queue shorter than ours. * - * [extendQueueOnSonos] tolerates individual AddURIToQueue failures and - * gives up entirely after [EXTEND_ABORT_AFTER_FAILURES] consecutive ones + * [SonosQueueLoader] tolerates individual AddURIToQueue failures and + * gives up appending after a few consecutive ones * -- Sonos rate-limits burst adds. Until this existed that left a short * queue on the renderer and nothing to notice it: the renderer played what * it had and stopped, and the app went on believing there were forty @@ -301,7 +299,7 @@ class OutputPickerController @Inject constructor( * * A full reload, not an incremental diff: the renderer's copy is known to * be wrong, and the diff path reasons from what we *think* it holds, which - * is exactly the assumption that failed. loadQueueOnSonos re-seeks to the + * is exactly the assumption that failed. The load re-seeks to the * current track and plays, so recovery lands where the listener was. */ private suspend fun observeQueueRepairRequests() { @@ -343,7 +341,7 @@ class OutputPickerController @Inject constructor( queue.size, outputRoute.name, currentIndex, ) runCatching { - loadQueueOnSonos(transport, outputRoute, queue, currentIndex) + sonosQueue.load(transport, outputRoute, queue, currentIndex) }.onFailure { e -> // Leave the route active: the renderer is reachable enough to have // told us its queue length, so dropping to local would be a harsher @@ -379,7 +377,7 @@ class OutputPickerController @Inject constructor( return@withLock } val handledIncrementally = runCatching { - tryIncrementalResync(transport, oldIds, newQueue) + sonosQueue.tryIncrementalResync(transport, oldIds, newQueue) }.getOrElse { e -> Timber.w(e, "Sonos incremental resync errored; falling back to full reload") false @@ -403,7 +401,7 @@ class OutputPickerController @Inject constructor( val rendering = renderingClientFor(routeId) Timber.w("Sonos resync: full reload of %d tracks on %s", newQueue.size, outputRoute.name) runCatching { - loadQueueOnSonos(transport, outputRoute, newQueue, newCurrentIndex) + sonosQueue.load(transport, outputRoute, newQueue, newCurrentIndex) activeUpnpHolder.set( ActiveUpnp( routeId = routeId, @@ -420,98 +418,6 @@ class OutputPickerController @Inject constructor( } } - /** - * 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 (the poll loop's @@ -605,7 +511,7 @@ class OutputPickerController @Inject constructor( * 1. Pause local so the user doesn't keep hearing local audio. * 2. Set target early so ForwardingPlayer drops transport taps * while the 17-second queue load is in progress. - * 3. Wire active LAST (after loadQueueOnSonos) so SOAP commands + * 3. Wire active LAST (after the queue load) so SOAP commands * are never routed to a half-loaded Sonos queue. */ private suspend fun selectUpnp(route: OutputRoute) = selectUpnpMutex.withLock { @@ -646,7 +552,7 @@ class OutputPickerController @Inject constructor( // taps don't hit Sonos's stale state from a prior session. activeUpnpHolder.setTarget(effectiveRoute.id) runCatching { - loadQueueOnSonos(transport, effectiveRoute, uiState.queue, uiState.queueIndex) + sonosQueue.load(transport, effectiveRoute, uiState.queue, uiState.queueIndex) // Wire active LAST -- SOAP path is now safe to use. activeUpnpHolder.set( ActiveUpnp( @@ -763,179 +669,6 @@ class OutputPickerController @Inject constructor( return if (i >= 0) segments.getOrNull(i + 1) else null } - private suspend fun loadQueueOnSonos( - transport: AVTransportClient, - route: OutputRoute, - queue: List, - currentIndex: Int, - ) { - Timber.w("UPnP select: clear queue on %s", route.name) - transport.removeAllTracksFromQueue() - val initialEnd = (currentIndex + 1).coerceAtMost(queue.size) - val initialBatch = queue.subList(0, initialEnd) - Timber.w( - "UPnP select: add %d initial tracks (currentIndex=%d, totalQueue=%d)", - initialBatch.size, currentIndex, queue.size, - ) - initialBatch.forEachIndexed { idx, ref -> - val token = streamTokens.mint(ref.id) - transport.addURIToQueue( - uri = token.url, - mime = token.mime, - title = token.title, - enqueuedURIPosition = idx + 1, - ) - } - val coordinatorUdn = route.id.bareUdn() - val queueUri = "x-rincon-queue:$coordinatorUdn#0" - Timber.w("UPnP select: SetAVTransportURI %s", queueUri) - transport.setAVTransportURI(queueUri, "") - Timber.w("UPnP select: Seek to track %d", currentIndex + 1) - transport.seekToTrack(currentIndex + 1) - Timber.w("UPnP select: Play") - transport.play() - Timber.w("UPnP select: initial done; backgrounding remainder") - val remaining = queue.drop(initialEnd) - // Verify even when there is no tail to append: the initial batch is - // sent the same way and can be dropped the same way. - scope.launch { - if (remaining.isNotEmpty()) { - extendQueueOnSonos(transport, route, remaining, initialEnd) - } - verifyQueueLength(transport, route, queue) - } - } - - /** - * Background-append tracks after activation. Runs concurrently with Sonos - * playback. Cancels if the user disconnects from this route (active.routeId - * changes or becomes null). Correctness of the result is [verifyQueueLength]'s - * job, not this function's. - */ - private suspend fun extendQueueOnSonos( - transport: AVTransportClient, - route: OutputRoute, - tracks: List, - startPosition: Int, - ) { - Timber.w( - "UPnP extend: appending %d tracks starting at position %d", - tracks.size, startPosition + 1, - ) - val succeeded = appendTracksToQueue(transport, route, tracks, startPosition) - Timber.w("UPnP extend: done (%d / %d appended)", succeeded, tracks.size) - } - - /** - * Confirm the renderer holds as many tracks as we sent, and append the - * tail it dropped. - * - * [appendTracksToQueue] tolerates individual failures and gives up after - * [EXTEND_ABORT_AFTER_FAILURES] consecutive ones, because Sonos rate-limits - * burst adds (logcat 2026-06-04: 33 consecutive failures once offset 39 was - * reached). That is the right call in the moment — some tracks loaded beats - * none — but it used to be the end of the story, and the renderer was left - * holding a queue shorter than ours with nothing aware of it. It then - * played what it had and stopped, which looked exactly like playback dying - * for no reason. - * - * Sonos appends sequentially, so a short queue means a missing tail: taking - * `fullQueue.drop(nrTracks)` is the gap. Bounded at [VERIFY_ROUNDS] passes - * so a renderer that refuses to grow can't spin here forever. - */ - @Suppress("ReturnCount") // each bail-out is a distinct reason to stop verifying - private suspend fun verifyQueueLength( - transport: AVTransportClient, - route: OutputRoute, - fullQueue: List, - ) { - repeat(VERIFY_ROUNDS) { round -> - if (activeUpnpHolder.active.value?.routeId != route.id) return - val nrTracks = runCatching { transport.getMediaInfo().nrTracks } - .getOrElse { e -> - Timber.w(e, "UPnP verify: GetMediaInfo failed on %s", route.name) - return - } - // 0 means the renderer told us nothing usable, not that its queue - // is empty. Guessing "empty" here would re-send the whole queue to - // a renderer that is playing it perfectly well. - if (nrTracks <= 0) { - Timber.w("UPnP verify: no usable NrTracks from %s; skipping", route.name) - return - } - if (nrTracks >= fullQueue.size) { - Timber.w("UPnP verify: renderer holds %d tracks, queue intact", nrTracks) - return - } - val missing = fullQueue.drop(nrTracks) - Timber.w( - "UPnP verify: %s holds %d of %d tracks; appending %d missing (round %d)", - route.name, nrTracks, fullQueue.size, missing.size, round + 1, - ) - appendTracksToQueue(transport, route, missing, nrTracks) - } - Timber.w("UPnP verify: gave up repairing queue length on %s", route.name) - } - - /** - * Append [tracks] at [startPosition] (0-based), returning how many landed. - * Tolerates individual AddURIToQueue failures — log and continue so some - * tracks loaded is better than zero tracks loaded — and stops early after - * [EXTEND_ABORT_AFTER_FAILURES] consecutive ones. - */ - private suspend fun appendTracksToQueue( - transport: AVTransportClient, - route: OutputRoute, - tracks: List, - startPosition: Int, - ): Int { - var consecutiveFailures = 0 - var succeeded = 0 - var aborted = false - for ((i, ref) in tracks.withIndex()) { - if (aborted) break - if (activeUpnpHolder.active.value?.routeId != route.id) { - Timber.w("UPnP extend: cancelled at offset %d (route changed)", i) - aborted = true - } else { - val outcome = runCatching { - val token = streamTokens.mint(ref.id) - transport.addURIToQueue( - uri = token.url, - mime = token.mime, - title = token.title, - enqueuedURIPosition = startPosition + i + 1, - ) - } - if (outcome.isSuccess) { - consecutiveFailures = 0 - succeeded += 1 - // Throttle the burst so we don't tickle Sonos's burst-add - // rejection -- logcat 2026-06-04 showed 33 consecutive - // failures clustered at ~10ms intervals once offset 39 was - // reached, which looks like a rate-limit kicking in. The - // delay is small enough that extending 100 tracks adds - // only ~5s to background work that's already async. - delay(EXTEND_THROTTLE_MS) - } else { - consecutiveFailures += 1 - val e = outcome.exceptionOrNull() - val detail = (e as? SoapFaultException)?.let { - "code=${it.code} desc=${it.description}" - } ?: e?.message - Timber.w(e, "UPnP extend: append failed at offset %d -- %s", i, detail) - if (consecutiveFailures >= EXTEND_ABORT_AFTER_FAILURES) { - Timber.w( - "UPnP extend: aborting after %d consecutive failures", - consecutiveFailures, - ) - aborted = true - } - } - } - } - return succeeded - } private fun renderingClientFor(routeId: String): RenderingControlClient? { val rcUrl = upnpDiscovery.routes.value @@ -967,16 +700,6 @@ class OutputPickerController @Inject constructor( } private companion object { - const val EXTEND_ABORT_AFTER_FAILURES = 3 - const val EXTEND_THROTTLE_MS = 50L - - // Verify/repair passes after a queue load. Two: one to catch the - // common case (a rate-limit burst dropped a chunk), one to catch a - // repair that itself got rate-limited. Beyond that the renderer is - // refusing for a reason retrying won't fix, and the stall watchdog - // becomes the backstop. - const val VERIFY_ROUNDS = 2 - // 5 minutes of continuous non-playing on a UPnP route before we // revert to the phone speaker, so a stale Sonos selection can't make // a later "tap play" do nothing. diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/SonosQueueLoader.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/SonosQueueLoader.kt new file mode 100644 index 00000000..950b5024 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/SonosQueueLoader.kt @@ -0,0 +1,316 @@ +package com.fabledsword.minstrel.player.output + +import com.fabledsword.minstrel.di.ApplicationScope +import com.fabledsword.minstrel.models.TrackRef +import com.fabledsword.minstrel.player.RemotePlayerState +import com.fabledsword.minstrel.player.StreamTokenProvider +import com.fabledsword.minstrel.player.output.upnp.AVTransportClient +import com.fabledsword.minstrel.player.output.upnp.SoapFaultException +import com.fabledsword.minstrel.player.output.upnp.bareUdn +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch +import timber.log.Timber +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Owns the shape of a Sonos renderer's native queue: loading it, growing it, + * diffing it against local queue mutations, and — the part that was missing — + * confirming the renderer actually took what we sent. + * + * Split out of [OutputPickerController], which is about *which route is + * selected*. How many tracks the renderer is holding is a separate concern + * with its own failure modes, and it had grown large enough to hide one: + * every write here is a SOAP call that can fail individually, and until + * [verifyQueueLength] nothing ever read the result back. + */ +@Singleton +class SonosQueueLoader @Inject constructor( + @ApplicationScope private val scope: CoroutineScope, + private val streamTokens: StreamTokenProvider, + private val activeUpnpHolder: ActiveUpnpHolder, + private val remoteState: RemotePlayerState, +) { + suspend fun load( + transport: AVTransportClient, + route: OutputRoute, + queue: List, + currentIndex: Int, + ) { + Timber.w("UPnP select: clear queue on %s", route.name) + transport.removeAllTracksFromQueue() + val initialEnd = (currentIndex + 1).coerceAtMost(queue.size) + val initialBatch = queue.subList(0, initialEnd) + Timber.w( + "UPnP select: add %d initial tracks (currentIndex=%d, totalQueue=%d)", + initialBatch.size, currentIndex, queue.size, + ) + initialBatch.forEachIndexed { idx, ref -> + val token = streamTokens.mint(ref.id) + transport.addURIToQueue( + uri = token.url, + mime = token.mime, + title = token.title, + enqueuedURIPosition = idx + 1, + ) + } + val coordinatorUdn = route.id.bareUdn() + val queueUri = "x-rincon-queue:$coordinatorUdn#0" + Timber.w("UPnP select: SetAVTransportURI %s", queueUri) + transport.setAVTransportURI(queueUri, "") + Timber.w("UPnP select: Seek to track %d", currentIndex + 1) + transport.seekToTrack(currentIndex + 1) + Timber.w("UPnP select: Play") + transport.play() + Timber.w("UPnP select: initial done; backgrounding remainder") + val remaining = queue.drop(initialEnd) + // Verify even when there is no tail to append: the initial batch is + // sent the same way and can be dropped the same way. + scope.launch { + if (remaining.isNotEmpty()) { + extendQueueOnSonos(transport, route, remaining, initialEnd) + } + verifyQueueLength(transport, route, queue) + } + } + + /** + * Background-append tracks after activation. Runs concurrently with Sonos + * playback. Cancels if the user disconnects from this route (active.routeId + * changes or becomes null). Correctness of the result is [verifyQueueLength]'s + * job, not this function's. + */ + private suspend fun extendQueueOnSonos( + transport: AVTransportClient, + route: OutputRoute, + tracks: List, + startPosition: Int, + ) { + Timber.w( + "UPnP extend: appending %d tracks starting at position %d", + tracks.size, startPosition + 1, + ) + val succeeded = appendTracksToQueue(transport, route, tracks, startPosition) + Timber.w("UPnP extend: done (%d / %d appended)", succeeded, tracks.size) + } + + /** + * Confirm the renderer holds as many tracks as we sent, and append the + * tail it dropped. + * + * [appendTracksToQueue] tolerates individual failures and gives up after + * [EXTEND_ABORT_AFTER_FAILURES] consecutive ones, because Sonos rate-limits + * burst adds (logcat 2026-06-04: 33 consecutive failures once offset 39 was + * reached). That is the right call in the moment — some tracks loaded beats + * none — but it used to be the end of the story, and the renderer was left + * holding a queue shorter than ours with nothing aware of it. It then + * played what it had and stopped, which looked exactly like playback dying + * for no reason. + * + * Sonos appends sequentially, so a short queue means a missing tail: taking + * `fullQueue.drop(nrTracks)` is the gap. Bounded at [VERIFY_ROUNDS] passes + * so a renderer that refuses to grow can't spin here forever. + */ + @Suppress("ReturnCount") // each bail-out is a distinct reason to stop verifying + private suspend fun verifyQueueLength( + transport: AVTransportClient, + route: OutputRoute, + fullQueue: List, + ) { + repeat(VERIFY_ROUNDS) { round -> + if (activeUpnpHolder.active.value?.routeId != route.id) return + val nrTracks = runCatching { transport.getMediaInfo().nrTracks } + .getOrElse { e -> + Timber.w(e, "UPnP verify: GetMediaInfo failed on %s", route.name) + return + } + // 0 means the renderer told us nothing usable, not that its queue + // is empty. Guessing "empty" here would re-send the whole queue to + // a renderer that is playing it perfectly well. + if (nrTracks <= 0) { + Timber.w("UPnP verify: no usable NrTracks from %s; skipping", route.name) + return + } + if (nrTracks >= fullQueue.size) { + Timber.w("UPnP verify: renderer holds %d tracks, queue intact", nrTracks) + return + } + val missing = fullQueue.drop(nrTracks) + Timber.w( + "UPnP verify: %s holds %d of %d tracks; appending %d missing (round %d)", + route.name, nrTracks, fullQueue.size, missing.size, round + 1, + ) + appendTracksToQueue(transport, route, missing, nrTracks) + } + Timber.w("UPnP verify: gave up repairing queue length on %s", route.name) + } + + /** + * Append [tracks] at [startPosition] (0-based), returning how many landed. + * Tolerates individual AddURIToQueue failures — log and continue so some + * tracks loaded is better than zero tracks loaded — and stops early after + * [EXTEND_ABORT_AFTER_FAILURES] consecutive ones. + */ + private suspend fun appendTracksToQueue( + transport: AVTransportClient, + route: OutputRoute, + tracks: List, + startPosition: Int, + ): Int { + var consecutiveFailures = 0 + var succeeded = 0 + var aborted = false + for ((i, ref) in tracks.withIndex()) { + if (aborted) break + if (activeUpnpHolder.active.value?.routeId != route.id) { + Timber.w("UPnP extend: cancelled at offset %d (route changed)", i) + aborted = true + } else { + val outcome = runCatching { + val token = streamTokens.mint(ref.id) + transport.addURIToQueue( + uri = token.url, + mime = token.mime, + title = token.title, + enqueuedURIPosition = startPosition + i + 1, + ) + } + if (outcome.isSuccess) { + consecutiveFailures = 0 + succeeded += 1 + // Throttle the burst so we don't tickle Sonos's burst-add + // rejection -- logcat 2026-06-04 showed 33 consecutive + // failures clustered at ~10ms intervals once offset 39 was + // reached, which looks like a rate-limit kicking in. The + // delay is small enough that extending 100 tracks adds + // only ~5s to background work that's already async. + delay(EXTEND_THROTTLE_MS) + } else { + consecutiveFailures += 1 + val e = outcome.exceptionOrNull() + val detail = (e as? SoapFaultException)?.let { + "code=${it.code} desc=${it.description}" + } ?: e?.message + Timber.w(e, "UPnP extend: append failed at offset %d -- %s", i, detail) + if (consecutiveFailures >= EXTEND_ABORT_AFTER_FAILURES) { + Timber.w( + "UPnP extend: aborting after %d consecutive failures", + consecutiveFailures, + ) + aborted = true + } + } + } + } + return succeeded + } + + /** + * 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. + */ + 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 + } + + private companion object { + // Abort the append loop after this many consecutive AddURIToQueue + // failures; Sonos rate-limits burst adds and a wall of failures means + // it has stopped accepting, not that the next one might land. + const val EXTEND_ABORT_AFTER_FAILURES = 3 + const val EXTEND_THROTTLE_MS = 50L + + // Verify/repair passes after a queue load. Two: one to catch the + // common case (a rate-limit burst dropped a chunk), one to catch a + // repair that itself got rate-limited. Beyond that the renderer is + // refusing for a reason retrying won't fix, and the stall watchdog + // becomes the backstop. + const val VERIFY_ROUNDS = 2 + } +}