From 6f649e76d8c2c6d9c1433b9ecccb546d8d9f16d2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 6 May 2026 19:47:08 +0000 Subject: [PATCH] refactor(nests): extract MoqLiteBroadcastHandle + HotSwappablePublisherSource (Audit-10) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `MoqLiteNestsSpeaker.kt` mixed three top-level concerns at 387 lines: - The `MoqLiteNestsSpeaker` class itself — the speaker entry point that builds a publisher + broadcaster pair on `startBroadcasting`. - `MoqLiteBroadcastHandle` — internal `BroadcastHandle` implementation tying the broadcaster, audio publisher, and catalog publisher together with a fixed-order shutdown. - `HotSwappablePublisherSource` — internal interface that lets `ReconnectingNestsSpeaker.runHotSwapIteration` retarget a long-lived broadcaster onto fresh moq-lite session publishers without restarting the AudioRecord / Opus encoder pipeline. The handle and the interface are independently reachable from `ReconnectingNestsSpeaker` and have no behavioural coupling to `MoqLiteNestsSpeaker` beyond a `parent` reference (handle) or an `as?` cast (interface). Move each to its own file: - `MoqLiteBroadcastHandle.kt` (109 lines). - `HotSwappablePublisherSource.kt` (62 lines). `MoqLiteNestsSpeaker.kt` is now 276 lines focused on the speaker class. Same package, same `internal` visibility — no call-site changes needed elsewhere. Tests + spotless green. https://claude.ai/code/session_014JfZJHSTvyYYWJbC9VbB47 --- .../HotSwappablePublisherSource.kt | 72 ++++++++++++ .../nestsclient/MoqLiteBroadcastHandle.kt | 104 +++++++++++++++++ .../nestsclient/MoqLiteNestsSpeaker.kt | 110 ------------------ 3 files changed, 176 insertions(+), 110 deletions(-) create mode 100644 nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/HotSwappablePublisherSource.kt create mode 100644 nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteBroadcastHandle.kt diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/HotSwappablePublisherSource.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/HotSwappablePublisherSource.kt new file mode 100644 index 000000000..1a895d238 --- /dev/null +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/HotSwappablePublisherSource.kt @@ -0,0 +1,72 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.nestsclient + +import com.vitorpamplona.nestsclient.moq.lite.MoqLitePublisherHandle + +/** + * Internal hot-swap seam: speakers that expose this interface let the + * reconnect wrapper retarget a long-lived + * [com.vitorpamplona.nestsclient.audio.NestMoqLiteBroadcaster] onto a + * freshly-opened moq-lite session's publisher without restarting the + * AudioRecord / Opus encoder pipeline. Implemented by + * [MoqLiteNestsSpeaker]; not implemented by the IETF reference + * [DefaultNestsSpeaker], which falls back to the close-then-restart path + * inside [com.vitorpamplona.nestsclient.connectReconnectingNestsSpeaker]. + * + * The wrapper uses an `as?` cast to detect support so this interface + * can stay package-internal — protocol consumers never see it. + */ +internal interface HotSwappablePublisherSource { + /** + * Open a fresh [MoqLitePublisherHandle] on the underlying moq-lite + * session. Caller owns the returned handle's lifetime (typically + * via [com.vitorpamplona.nestsclient.audio.NestMoqLiteBroadcaster.swapPublisher]'s + * close-the-old contract). + * + * @param startSequence first group sequence the new publisher will + * assign. Used by the hot-swap path to seed the new session's + * audio track with the previous session's + * [MoqLitePublisherHandle.nextSequence] so kixelated/hang's + * `Container.Consumer.#run` doesn't drop every post-recycle + * group as `sequence < #active`. Pass `0L` for fresh (non- + * continuation) publishers — the catalog track is one such + * case, since its `#active` semantics are different from audio. + */ + suspend fun openPublisherForHotSwap( + track: String, + startSequence: Long = 0L, + ): MoqLitePublisherHandle + + /** + * Surface a broadcast-pipeline terminal failure (e.g. sustained + * `publisher.send` errors past + * [com.vitorpamplona.nestsclient.audio.NestMoqLiteBroadcaster.MAX_CONSECUTIVE_SEND_ERRORS]) + * by flipping the speaker's state to [NestsSpeakerState.Failed]. + * Called by the hot-swap pump when the long-lived broadcaster's + * `onTerminalFailure` fires; lets the reconnect orchestrator + * observe the terminal state and recycle the session, matching + * the legacy + * [MoqLiteNestsSpeaker.startBroadcasting] path's failure + * propagation. + */ + fun reportBroadcastTerminalFailure() +} diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteBroadcastHandle.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteBroadcastHandle.kt new file mode 100644 index 000000000..bd8635ac6 --- /dev/null +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteBroadcastHandle.kt @@ -0,0 +1,104 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.nestsclient + +import com.vitorpamplona.nestsclient.audio.NestMoqLiteBroadcaster +import com.vitorpamplona.nestsclient.moq.lite.MoqLitePublisherHandle + +/** + * [BroadcastHandle] returned by [MoqLiteNestsSpeaker.startBroadcasting] + * for the non-reconnecting code path. Wraps: + * + * - the long-lived audio [broadcaster] (mic + encoder + send loop), + * - the audio-track [publisher] on the moq-lite session, + * - the [catalogPublisher] companion track that emits the + * `catalog.json` manifest on every new SUBSCRIBE. + * + * Mute is forwarded to the broadcaster (which also reports the user- + * facing state back via [parent.reportMuteState]). [close] tears down + * all three components in a fixed order — broadcaster → audio + * publisher → catalog publisher — so a `CancellationException` + * mid-shutdown still releases every resource before re-throwing. + * + * Reconnecting / hot-swap callers go through `ReissuingBroadcastHandle` + * in `ReconnectingNestsSpeaker` instead, which manages the audio + + * catalog publishers across session swaps. + */ +internal class MoqLiteBroadcastHandle( + private val broadcaster: NestMoqLiteBroadcaster, + private val publisher: MoqLitePublisherHandle, + private val catalogPublisher: MoqLitePublisherHandle, + private val parent: MoqLiteNestsSpeaker, +) : BroadcastHandle { + @Volatile private var muted: Boolean = false + + @Volatile private var closed: Boolean = false + + override val isMuted: Boolean get() = muted + + override suspend fun setMuted(muted: Boolean) { + if (closed) return + this.muted = muted + broadcaster.setMuted(muted) + parent.reportMuteState(muted) + } + + override suspend fun close() { + if (closed) return + closed = true + // Stop the broadcaster first so the audio capture + encoder + // don't keep producing into a closing publisher. + try { + broadcaster.stop() + } catch (ce: kotlinx.coroutines.CancellationException) { + // Even on cancel, run the rest of cleanup before rethrowing + // — broadcaster.stop already cancels its own job, so the + // mic + encoder + publisher are owed their close paths. + runCatching { catalogPublisher.close() } + runCatching { publisher.close() } + parent.broadcastClosed(this) + throw ce + } catch (_: Throwable) { + // Best-effort; fall through to the defensive publisher.close. + } + // broadcaster.stop() already calls publisher.close(); call again + // defensively to make this method idempotent against partial + // failures on the broadcaster.stop path. + try { + publisher.close() + } catch (ce: kotlinx.coroutines.CancellationException) { + runCatching { catalogPublisher.close() } + parent.broadcastClosed(this) + throw ce + } catch (_: Throwable) { + // Best-effort. + } + try { + catalogPublisher.close() + } catch (ce: kotlinx.coroutines.CancellationException) { + parent.broadcastClosed(this) + throw ce + } catch (_: Throwable) { + // Best-effort. + } + parent.broadcastClosed(this) + } +} diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteNestsSpeaker.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteNestsSpeaker.kt index de09a3424..0d02d1051 100644 --- a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteNestsSpeaker.kt +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteNestsSpeaker.kt @@ -274,113 +274,3 @@ class MoqLiteNestsSpeaker internal constructor( } } } - -internal class MoqLiteBroadcastHandle( - private val broadcaster: NestMoqLiteBroadcaster, - private val publisher: MoqLitePublisherHandle, - private val catalogPublisher: MoqLitePublisherHandle, - private val parent: MoqLiteNestsSpeaker, -) : BroadcastHandle { - @Volatile private var muted: Boolean = false - - @Volatile private var closed: Boolean = false - - override val isMuted: Boolean get() = muted - - override suspend fun setMuted(muted: Boolean) { - if (closed) return - this.muted = muted - broadcaster.setMuted(muted) - parent.reportMuteState(muted) - } - - override suspend fun close() { - if (closed) return - closed = true - // Stop the broadcaster first so the audio capture + encoder - // don't keep producing into a closing publisher. - try { - broadcaster.stop() - } catch (ce: kotlinx.coroutines.CancellationException) { - // Even on cancel, run the rest of cleanup before rethrowing - // — broadcaster.stop already cancels its own job, so the - // mic + encoder + publisher are owed their close paths. - runCatching { catalogPublisher.close() } - runCatching { publisher.close() } - parent.broadcastClosed(this) - throw ce - } catch (_: Throwable) { - // Best-effort; fall through to the defensive publisher.close. - } - // broadcaster.stop() already calls publisher.close(); call again - // defensively to make this method idempotent against partial - // failures on the broadcaster.stop path. - try { - publisher.close() - } catch (ce: kotlinx.coroutines.CancellationException) { - runCatching { catalogPublisher.close() } - parent.broadcastClosed(this) - throw ce - } catch (_: Throwable) { - // Best-effort. - } - try { - catalogPublisher.close() - } catch (ce: kotlinx.coroutines.CancellationException) { - parent.broadcastClosed(this) - throw ce - } catch (_: Throwable) { - // Best-effort. - } - parent.broadcastClosed(this) - } -} - -/** - * Internal hot-swap seam: speakers that expose this interface let the - * reconnect wrapper retarget a long-lived - * [com.vitorpamplona.nestsclient.audio.NestMoqLiteBroadcaster] onto a - * freshly-opened moq-lite session's publisher without restarting the - * AudioRecord / Opus encoder pipeline. Implemented by - * [MoqLiteNestsSpeaker]; not implemented by the IETF reference - * [DefaultNestsSpeaker], which falls back to the close-then-restart path - * inside [com.vitorpamplona.nestsclient.connectReconnectingNestsSpeaker]. - * - * The wrapper uses an `as?` cast to detect support so this interface - * can stay package-internal — protocol consumers never see it. - */ -internal interface HotSwappablePublisherSource { - /** - * Open a fresh [MoqLitePublisherHandle] on the underlying moq-lite - * session. Caller owns the returned handle's lifetime (typically - * via [com.vitorpamplona.nestsclient.audio.NestMoqLiteBroadcaster.swapPublisher]'s - * close-the-old contract). - * - * @param startSequence first group sequence the new publisher will - * assign. Used by the hot-swap path to seed the new session's - * audio track with the previous session's - * [MoqLitePublisherHandle.nextSequence] so kixelated/hang's - * `Container.Consumer.#run` doesn't drop every post-recycle - * group as `sequence < #active`. Pass `0L` for fresh (non- - * continuation) publishers — the catalog track is one such - * case, since its `#active` semantics are different from audio. - */ - suspend fun openPublisherForHotSwap( - track: String, - startSequence: Long = 0L, - ): MoqLitePublisherHandle - - /** - * Surface a broadcast-pipeline terminal failure (e.g. sustained - * `publisher.send` errors past - * [com.vitorpamplona.nestsclient.audio.NestMoqLiteBroadcaster.MAX_CONSECUTIVE_SEND_ERRORS]) - * by flipping the speaker's state to [NestsSpeakerState.Failed]. - * Called by the hot-swap pump when the long-lived broadcaster's - * `onTerminalFailure` fires; lets the reconnect orchestrator - * observe the terminal state and recycle the session, matching - * the legacy - * [MoqLiteNestsSpeaker.startBroadcasting] path's failure - * propagation. - */ - fun reportBroadcastTerminalFailure() -}