refactor(player): split Sonos queue loading out of the picker — #2728
android / Build + lint + test (push) Successful in 3m48s
android / Build + lint + test (push) Successful in 3m48s
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.
This commit is contained in:
+11
-288
@@ -10,11 +10,9 @@ import com.fabledsword.minstrel.models.TrackRef
|
|||||||
import com.fabledsword.minstrel.player.PlayerController
|
import com.fabledsword.minstrel.player.PlayerController
|
||||||
import com.fabledsword.minstrel.player.PlayerFactory
|
import com.fabledsword.minstrel.player.PlayerFactory
|
||||||
import com.fabledsword.minstrel.player.RemotePlayerState
|
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.AVTransportClient
|
||||||
import com.fabledsword.minstrel.player.output.upnp.RenderingControlClient
|
import com.fabledsword.minstrel.player.output.upnp.RenderingControlClient
|
||||||
import com.fabledsword.minstrel.player.output.upnp.SoapClient
|
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.TransportState
|
||||||
import com.fabledsword.minstrel.player.output.upnp.UpnpDiscoveryController
|
import com.fabledsword.minstrel.player.output.upnp.UpnpDiscoveryController
|
||||||
import com.fabledsword.minstrel.player.output.upnp.bareUdn
|
import com.fabledsword.minstrel.player.output.upnp.bareUdn
|
||||||
@@ -61,7 +59,7 @@ data class RouteSnapshot(
|
|||||||
* - [OutputRoute.Protocol.SYSTEM] — MediaRouter.selectRoute (built-in,
|
* - [OutputRoute.Protocol.SYSTEM] — MediaRouter.selectRoute (built-in,
|
||||||
* wired, Bluetooth)
|
* wired, Bluetooth)
|
||||||
* - [OutputRoute.Protocol.UPNP] — mint a signed stream token via
|
* - [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
|
* AVTransport.SetAVTransportURI + Play, pause local playback so
|
||||||
* audio yields to the network speaker
|
* audio yields to the network speaker
|
||||||
* - [OutputRoute.Protocol.CAST] / [OutputRoute.Protocol.SONOS] —
|
* - [OutputRoute.Protocol.CAST] / [OutputRoute.Protocol.SONOS] —
|
||||||
@@ -79,7 +77,7 @@ class OutputPickerController @Inject constructor(
|
|||||||
private val upnpDiscovery: UpnpDiscoveryController,
|
private val upnpDiscovery: UpnpDiscoveryController,
|
||||||
private val playerController: PlayerController,
|
private val playerController: PlayerController,
|
||||||
private val playerFactory: PlayerFactory,
|
private val playerFactory: PlayerFactory,
|
||||||
private val streamTokens: StreamTokenProvider,
|
private val sonosQueue: SonosQueueLoader,
|
||||||
private val activeUpnpHolder: ActiveUpnpHolder,
|
private val activeUpnpHolder: ActiveUpnpHolder,
|
||||||
private val remoteState: RemotePlayerState,
|
private val remoteState: RemotePlayerState,
|
||||||
private val okHttp: OkHttpClient,
|
private val okHttp: OkHttpClient,
|
||||||
@@ -256,7 +254,7 @@ class OutputPickerController @Inject constructor(
|
|||||||
* setMediaItems override clears holder.active + sets target so the
|
* setMediaItems override clears holder.active + sets target so the
|
||||||
* imminent play() call drops (drops via isLoadingUpnp() = true). Then
|
* imminent play() call drops (drops via isLoadingUpnp() = true). Then
|
||||||
* this collector observes the uiState.queue change and re-runs
|
* 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
|
* Discrimination: selectUpnp's initial-load path doesn't change
|
||||||
* uiState.queue (the queue was already populated before route
|
* 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
|
* Rebuild the renderer's queue when playback stopped because the renderer
|
||||||
* ran off the end of a queue shorter than ours.
|
* ran off the end of a queue shorter than ours.
|
||||||
*
|
*
|
||||||
* [extendQueueOnSonos] tolerates individual AddURIToQueue failures and
|
* [SonosQueueLoader] tolerates individual AddURIToQueue failures and
|
||||||
* gives up entirely after [EXTEND_ABORT_AFTER_FAILURES] consecutive ones
|
* gives up appending after a few consecutive ones
|
||||||
* -- Sonos rate-limits burst adds. Until this existed that left a short
|
* -- 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
|
* 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
|
* 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
|
* 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
|
* 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.
|
* current track and plays, so recovery lands where the listener was.
|
||||||
*/
|
*/
|
||||||
private suspend fun observeQueueRepairRequests() {
|
private suspend fun observeQueueRepairRequests() {
|
||||||
@@ -343,7 +341,7 @@ class OutputPickerController @Inject constructor(
|
|||||||
queue.size, outputRoute.name, currentIndex,
|
queue.size, outputRoute.name, currentIndex,
|
||||||
)
|
)
|
||||||
runCatching {
|
runCatching {
|
||||||
loadQueueOnSonos(transport, outputRoute, queue, currentIndex)
|
sonosQueue.load(transport, outputRoute, queue, currentIndex)
|
||||||
}.onFailure { e ->
|
}.onFailure { e ->
|
||||||
// Leave the route active: the renderer is reachable enough to have
|
// Leave the route active: the renderer is reachable enough to have
|
||||||
// told us its queue length, so dropping to local would be a harsher
|
// told us its queue length, so dropping to local would be a harsher
|
||||||
@@ -379,7 +377,7 @@ class OutputPickerController @Inject constructor(
|
|||||||
return@withLock
|
return@withLock
|
||||||
}
|
}
|
||||||
val handledIncrementally = runCatching {
|
val handledIncrementally = runCatching {
|
||||||
tryIncrementalResync(transport, oldIds, newQueue)
|
sonosQueue.tryIncrementalResync(transport, oldIds, newQueue)
|
||||||
}.getOrElse { e ->
|
}.getOrElse { e ->
|
||||||
Timber.w(e, "Sonos incremental resync errored; falling back to full reload")
|
Timber.w(e, "Sonos incremental resync errored; falling back to full reload")
|
||||||
false
|
false
|
||||||
@@ -403,7 +401,7 @@ class OutputPickerController @Inject constructor(
|
|||||||
val rendering = renderingClientFor(routeId)
|
val rendering = renderingClientFor(routeId)
|
||||||
Timber.w("Sonos resync: full reload of %d tracks on %s", newQueue.size, outputRoute.name)
|
Timber.w("Sonos resync: full reload of %d tracks on %s", newQueue.size, outputRoute.name)
|
||||||
runCatching {
|
runCatching {
|
||||||
loadQueueOnSonos(transport, outputRoute, newQueue, newCurrentIndex)
|
sonosQueue.load(transport, outputRoute, newQueue, newCurrentIndex)
|
||||||
activeUpnpHolder.set(
|
activeUpnpHolder.set(
|
||||||
ActiveUpnp(
|
ActiveUpnp(
|
||||||
routeId = routeId,
|
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<String>,
|
|
||||||
newQueue: List<TrackRef>,
|
|
||||||
): 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<TrackRef>,
|
|
||||||
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<String>, b: List<String>): 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<String>, b: List<String>): 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
|
* 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.
|
* 1. Pause local so the user doesn't keep hearing local audio.
|
||||||
* 2. Set target early so ForwardingPlayer drops transport taps
|
* 2. Set target early so ForwardingPlayer drops transport taps
|
||||||
* while the 17-second queue load is in progress.
|
* 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.
|
* are never routed to a half-loaded Sonos queue.
|
||||||
*/
|
*/
|
||||||
private suspend fun selectUpnp(route: OutputRoute) = selectUpnpMutex.withLock {
|
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.
|
// taps don't hit Sonos's stale state from a prior session.
|
||||||
activeUpnpHolder.setTarget(effectiveRoute.id)
|
activeUpnpHolder.setTarget(effectiveRoute.id)
|
||||||
runCatching {
|
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.
|
// Wire active LAST -- SOAP path is now safe to use.
|
||||||
activeUpnpHolder.set(
|
activeUpnpHolder.set(
|
||||||
ActiveUpnp(
|
ActiveUpnp(
|
||||||
@@ -763,179 +669,6 @@ class OutputPickerController @Inject constructor(
|
|||||||
return if (i >= 0) segments.getOrNull(i + 1) else null
|
return if (i >= 0) segments.getOrNull(i + 1) else null
|
||||||
}
|
}
|
||||||
|
|
||||||
private suspend fun loadQueueOnSonos(
|
|
||||||
transport: AVTransportClient,
|
|
||||||
route: OutputRoute,
|
|
||||||
queue: List<TrackRef>,
|
|
||||||
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<TrackRef>,
|
|
||||||
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<TrackRef>,
|
|
||||||
) {
|
|
||||||
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<TrackRef>,
|
|
||||||
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? {
|
private fun renderingClientFor(routeId: String): RenderingControlClient? {
|
||||||
val rcUrl = upnpDiscovery.routes.value
|
val rcUrl = upnpDiscovery.routes.value
|
||||||
@@ -967,16 +700,6 @@ class OutputPickerController @Inject constructor(
|
|||||||
}
|
}
|
||||||
|
|
||||||
private companion object {
|
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
|
// 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
|
// revert to the phone speaker, so a stale Sonos selection can't make
|
||||||
// a later "tap play" do nothing.
|
// a later "tap play" do nothing.
|
||||||
|
|||||||
@@ -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<TrackRef>,
|
||||||
|
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<TrackRef>,
|
||||||
|
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<TrackRef>,
|
||||||
|
) {
|
||||||
|
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<TrackRef>,
|
||||||
|
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<String>,
|
||||||
|
newQueue: List<TrackRef>,
|
||||||
|
): 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<TrackRef>,
|
||||||
|
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<String>, b: List<String>): 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<String>, b: List<String>): 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
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user