From ec1e7be7dd93e2e638894b068eb25924245654f6 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sun, 16 Aug 2026 09:55:21 +0200 Subject: [PATCH] feat(android): AAudio grants a rate or fails, so the fallback had to happen before the Hello MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Work package H6, Android half — plus the near-miss margin in core, which the Apple leg independently proved was measuring the wrong thing. AAudio never substitutes a rate: an explicit request is granted or the open fails. So a 48 kHz rung sitting below a 96 kHz session on the open ladder could only ever produce playback at 2x speed, or a resample the design forbids in as many words. The fallback therefore runs BEFORE the handshake: a probe opens a stream at the requested rate, reads back what was granted and closes it without ever starting it, so nothing routes and no audio focus is taken. The ladder is 96 -> 48 keeping the depth -> the legacy pair, so a device that will not grant 96 kHz still gets a 48/24 lossless session rather than silence. Only 96 kHz is probed; a default session opens nothing and pays nothing. The open ladder still gains a rate dimension, because a probe is one measurement at one instant and the supervisor reopens across route changes. Rate is the outermost dimension, and the last rung is AAUDIO_UNSPECIFIED for the HAL that refuses an explicit request while already running at exactly the rate we wanted — `arm` still holds it to the session's rate, so that rung can rescue a session but never mislabel one. The conceal scratch is now sized per plane: Opus keeps its 120 ms legal maximum, PCM uses the longest ladder rung. Sizing both from the Opus figure was 24x too large at 96 kHz, and sizing both from the PCM one would have been far too small for Opus. The poll tick is one frame of the running plane rather than a fixed 5 ms, so on a 2 ms session the drought-concealment arm keeps pace with playout instead of losing 3 ms in 5. In core, the near-miss margin stops being a constant. Its own doc always said "less than one protocol frame was left in hand", but it was frozen at 5 ms — which against a 2 ms lossless frame means two and a half frames, so it grew the target on a ring that was never close to starving, inverting exactly what a near-miss exists to detect. Identical on every Opus session. The Android CI lane's test filter is an allowlist and eleven classes sat outside it, including the audio HUD ones, which had never run in CI at all. All eleven pass, so nothing was hiding — but nothing was gating either. They are listed now, explicitly rather than by glob, because the unfiltered task also drags in the screenshot scenes. --- .gitea/workflows/android.yml | 24 + .../unom/punktfunk/GamepadSettingsScreen.kt | 13 + .../kotlin/io/unom/punktfunk/HostConnect.kt | 12 + .../main/kotlin/io/unom/punktfunk/Profiles.kt | 14 +- .../main/kotlin/io/unom/punktfunk/Settings.kt | 67 ++ .../io/unom/punktfunk/SettingsScreen.kt | 17 + .../kotlin/io/unom/punktfunk/StatsOverlay.kt | 44 +- .../kotlin/io/unom/punktfunk/ProfilesTest.kt | 27 + .../unom/punktfunk/StatsOverlayAudioTest.kt | 59 +- .../unom/punktfunk/screenshots/ShotScenes.kt | 9 +- .../io/unom/punktfunk/kit/NativeBridge.kt | 25 +- clients/android/native/src/audio.rs | 798 ++++++++++++++---- clients/android/native/src/session/connect.rs | 153 +++- clients/android/native/src/session/planes.rs | 26 +- crates/punktfunk-core/src/audio.rs | 53 +- 15 files changed, 1125 insertions(+), 216 deletions(-) diff --git a/.gitea/workflows/android.yml b/.gitea/workflows/android.yml index ad54d2f0..ecae5350 100644 --- a/.gitea/workflows/android.yml +++ b/.gitea/workflows/android.yml @@ -222,6 +222,19 @@ jobs: # screenshot scenes, which are a release-artifact job (android-screenshots.yml, gated to v* # tags) and have no business adding a minute to every push. The filter is what lets the # contract gate here without dragging the rest of the app suite in with it. + # + # ⚠ The filter is an ALLOWLIST, so a test class that is not named here does not run — it + # reads as coverage in the tree and gates nothing. `ProfilesTest` and `StatsOverlayAudioTest` + # sat outside it and were only noticed when the hi-res audio work added cases to both; the + # HUD ones had never run in CI at all. Adding a test class to `app/src/test` is therefore + # only half the job: add it here too, or it is decoration. + # + # That audit found NINE more in the same state (gamepad palette/rows/UI, OS icons, render + # scale, safe area, SC2 bluetooth grant, settings scope, speed test) — every one of them + # passing, so nothing was hiding, but none of them gating either. They are all listed now. + # The list is deliberately explicit rather than a package glob: the unfiltered task also + # drags in the ~20 Roborazzi screenshot scenes above, and a glob would quietly re-admit them + # the moment someone added one. - name: console parity vectors + app-module logic tests working-directory: clients/android run: >- @@ -231,6 +244,17 @@ jobs: --tests 'io.unom.punktfunk.GamepadSettingsLayoutTest' --tests 'io.unom.punktfunk.ConsoleSubScreenRowsTest' --tests 'io.unom.punktfunk.ConsoleSubScreenRoutesTest' + --tests 'io.unom.punktfunk.ProfilesTest' + --tests 'io.unom.punktfunk.StatsOverlayAudioTest' + --tests 'io.unom.punktfunk.GamepadPaletteTest' + --tests 'io.unom.punktfunk.GamepadSettingsRowsTest' + --tests 'io.unom.punktfunk.GamepadUiTest' + --tests 'io.unom.punktfunk.OsIconsTest' + --tests 'io.unom.punktfunk.RenderScaleTest' + --tests 'io.unom.punktfunk.SafeAreaTest' + --tests 'io.unom.punktfunk.Sc2BluetoothGrantTest' + --tests 'io.unom.punktfunk.SettingsScopeTest' + --tests 'io.unom.punktfunk.SpeedTestTest' --stacktrace - name: assembleDebug (cargo-ndk → jniLibs → APK) diff --git a/clients/android/app/src/main/kotlin/io/unom/punktfunk/GamepadSettingsScreen.kt b/clients/android/app/src/main/kotlin/io/unom/punktfunk/GamepadSettingsScreen.kt index ff16e42b..e57bffb4 100644 --- a/clients/android/app/src/main/kotlin/io/unom/punktfunk/GamepadSettingsScreen.kt +++ b/clients/android/app/src/main/kotlin/io/unom/punktfunk/GamepadSettingsScreen.kt @@ -917,6 +917,19 @@ internal fun buildSettingsRows( "The speaker layout requested from the host.", AUDIO_CHANNEL_OPTIONS, s.audioChannels, ) { update(s.copy(audioChannels = it)) }, + // Follows the row above rather than disappearing, the same relationship the pad rows draw + // with the forwarding switch — this list is fixed-shape and a row that vanishes under the + // cursor is worse on a pad than one that dims. The lossless plane is stereo-only at the + // default MTU (a surround frame does not fit one datagram and this plane is never + // fragmented), so on 5.1/7.1 there is genuinely nothing here to choose. + choice( + "audioFormat", GpTab.AUDIO, null, "Audio quality", + "Lossless sends bit-exact PCM instead of compressed audio — 2.3 Mbps at 48 kHz, " + + "4.6 at 96 — on top of the video. It must be enabled on the host too, and this " + + "device's output has to accept the rate; otherwise the session falls back to " + + "Standard.", + AUDIO_FORMAT_OPTIONS, s.audioFormat, enabled = s.audioChannels == 2, + ) { update(s.copy(audioFormat = it)) }, toggle( "mic", GpTab.AUDIO, null, "Microphone", "Send this device's microphone to the host's virtual mic.", diff --git a/clients/android/app/src/main/kotlin/io/unom/punktfunk/HostConnect.kt b/clients/android/app/src/main/kotlin/io/unom/punktfunk/HostConnect.kt index d9196720..86bc16a9 100644 --- a/clients/android/app/src/main/kotlin/io/unom/punktfunk/HostConnect.kt +++ b/clients/android/app/src/main/kotlin/io/unom/punktfunk/HostConnect.kt @@ -41,6 +41,13 @@ suspend fun connectToHost( val hdrEnabled = settings.hdrEnabled && displaySupportsHdr(context) // "Automatic" resolves to a concrete pad type from the connected controller's VID/PID. val gamepadPref = Gamepad.resolvePref(settings.gamepad) + // The requested audio format as the two Hello fields. Stereo only — a lossless surround frame + // does not fit one QUIC datagram at the default MTU and this plane is never fragmented, so the + // host would decline; asking anyway would only spend a probe and a decline line. (The settings + // screen hides the picker on 5.1/7.1 for the same reason, but a profile can still carry a + // lossless choice into a surround session, and this is where the two settings meet.) + val (audioRateHz, audioBits) = + if (settings.audioChannels == 2) settings.audioFormatWire() else 48_000 to 16 return withContext(Dispatchers.IO) { // Transport-level half of "Low-latency mode (experimental)" (DSCP marking on the media // sockets) — must be applied before connect, since sockets are tagged at creation. @@ -75,6 +82,11 @@ suspend fun connectToHost( hdrEnabled, multiSlice, frameParts, settings.audioChannels, + // The audio format this session asks for. Only ever a request: the host's five-condition + // gate may resolve it back to Opus, and the native side downgrades it first if AAudio on + // this device will not open the rate — a rate the wire has committed to cannot be + // rescued afterwards, so the fallback has to happen before the Hello. + audioRateHz, audioBits, // What this device can decode (H.264|HEVC always, AV1 when a real decoder exists) + // the soft codec preference (user choice, or the Automatic AV1 rule above) — the // host resolves the emitted codec from both. diff --git a/clients/android/app/src/main/kotlin/io/unom/punktfunk/Profiles.kt b/clients/android/app/src/main/kotlin/io/unom/punktfunk/Profiles.kt index ec23975f..373096ea 100644 --- a/clients/android/app/src/main/kotlin/io/unom/punktfunk/Profiles.kt +++ b/clients/android/app/src/main/kotlin/io/unom/punktfunk/Profiles.kt @@ -37,6 +37,12 @@ data class SettingsOverlay( val hdrEnabled: Boolean? = null, val compositor: Int? = null, val audioChannels: Int? = null, + /** + * The requested audio format ([AUDIO_FORMAT_OPTIONS]'s stored value). Profileable because it + * is about how a HOST is streamed — a wired desktop can afford lossless, a phone on a hotspot + * cannot — rather than about this device's hardware. + */ + val audioFormat: String? = null, val micEnabled: Boolean? = null, val echoCancel: Boolean? = null, val touchMode: TouchMode? = null, @@ -73,6 +79,7 @@ data class SettingsOverlay( hdrEnabled = hdrEnabled ?: base.hdrEnabled, compositor = compositor ?: base.compositor, audioChannels = audioChannels ?: base.audioChannels, + audioFormat = audioFormat ?: base.audioFormat, micEnabled = micEnabled ?: base.micEnabled, echoCancel = echoCancel ?: base.echoCancel, touchMode = touchMode ?: base.touchMode, @@ -110,6 +117,7 @@ data class SettingsOverlay( hdrEnabled = if (after.hdrEnabled != before.hdrEnabled) after.hdrEnabled else hdrEnabled, compositor = if (after.compositor != before.compositor) after.compositor else compositor, audioChannels = if (after.audioChannels != before.audioChannels) after.audioChannels else audioChannels, + audioFormat = if (after.audioFormat != before.audioFormat) after.audioFormat else audioFormat, micEnabled = if (after.micEnabled != before.micEnabled) after.micEnabled else micEnabled, echoCancel = if (after.echoCancel != before.echoCancel) after.echoCancel else echoCancel, touchMode = if (after.touchMode != before.touchMode) after.touchMode else touchMode, @@ -141,6 +149,7 @@ data class SettingsOverlay( "hdr_enabled" -> copy(hdrEnabled = null) "compositor" -> copy(compositor = null) "audio_channels" -> copy(audioChannels = null) + "audio_format" -> copy(audioFormat = null) "mic_enabled" -> copy(micEnabled = null) "echo_cancel" -> copy(echoCancel = null) "touch_mode" -> copy(touchMode = null) @@ -167,6 +176,7 @@ data class SettingsOverlay( if (hdrEnabled != null) add("hdr_enabled") if (compositor != null) add("compositor") if (audioChannels != null) add("audio_channels") + if (audioFormat != null) add("audio_format") if (micEnabled != null) add("mic_enabled") if (echoCancel != null) add("echo_cancel") if (touchMode != null) add("touch_mode") @@ -201,6 +211,7 @@ data class SettingsOverlay( hdrEnabled?.let { j.put("hdr_enabled", it) } compositor?.let { j.put("compositor", it) } audioChannels?.let { j.put("audio_channels", it) } + audioFormat?.let { j.put("audio_format", it) } micEnabled?.let { j.put("mic_enabled", it) } echoCancel?.let { j.put("echo_cancel", it) } touchMode?.let { j.put("touch_mode", it.name) } @@ -224,7 +235,7 @@ data class SettingsOverlay( /** Keys this build models; everything else in a stored overlay is carried through. */ private val KNOWN = setOf( "width", "height", "refresh_hz", "bitrate_kbps", "render_scale", "codec", - "hdr_enabled", "compositor", "audio_channels", "mic_enabled", "echo_cancel", + "hdr_enabled", "compositor", "audio_channels", "audio_format", "mic_enabled", "echo_cancel", "touch_mode", "mouse_mode", "invert_scroll", "gamepad", "gamepad_forwarding", "system_buttons", "guide_gesture", "stats_verbosity", @@ -241,6 +252,7 @@ data class SettingsOverlay( hdrEnabled = j.optBooleanOrNull("hdr_enabled"), compositor = j.optIntOrNull("compositor"), audioChannels = j.optIntOrNull("audio_channels"), + audioFormat = j.optStringOrNull("audio_format"), micEnabled = j.optBooleanOrNull("mic_enabled"), echoCancel = j.optBooleanOrNull("echo_cancel"), touchMode = j.optStringOrNull("touch_mode") diff --git a/clients/android/app/src/main/kotlin/io/unom/punktfunk/Settings.kt b/clients/android/app/src/main/kotlin/io/unom/punktfunk/Settings.kt index 27cb4975..864d38af 100644 --- a/clients/android/app/src/main/kotlin/io/unom/punktfunk/Settings.kt +++ b/clients/android/app/src/main/kotlin/io/unom/punktfunk/Settings.kt @@ -62,6 +62,19 @@ data class Settings( /** Requested audio channel count: 2 (stereo), 6 (5.1) or 8 (7.1). The host clamps to what it * can capture; the resolved count drives the decoder + AAudio layout. */ val audioChannels: Int = 2, + /** + * Requested audio format — the cross-client `audio_format` key: [AUDIO_FORMAT_OPUS] (the + * default, and byte-for-byte the session every build before the lossless plane ran), + * [AUDIO_FORMAT_LOSSLESS_48] or [AUDIO_FORMAT_LOSSLESS_96]. + * + * Off by default and deliberately: lossless takes 2.3–4.6 Mbps off the top of the link, + * OUTSIDE the ABR loop that manages the video budget, against the ~256 kbps Opus it replaces — + * so it has to be asked for at both ends (`PUNKTFUNK_AUDIO_HIRES` is the host's half, also off + * by default). A REQUEST, never a fact: the host runs a five-condition gate and may answer Opus + * anyway, and the native side downgrades it further if THIS device will not open the rate. + * What actually happened is in logcat's `audio: plane codec=… rate=…` line. + */ + val audioFormat: String = AUDIO_FORMAT_OPUS, /** Preferred video codec: `"auto"` (host decides), `"hevc"`, `"h264"`, or `"av1"`. A soft * preference — the host emits it when it can, else falls back. AMediaCodec decodes whichever * the host resolves (AV1 is only advertised/offered when the device has a real AV1 decoder). */ @@ -295,6 +308,7 @@ class SettingsStore(context: Context) { systemButtons = prefs.getString(K_SYSTEM_BUTTONS, "auto") ?: "auto", guideGesture = prefs.getString(K_GUIDE_GESTURE, "auto") ?: "auto", audioChannels = prefs.getInt(K_AUDIO_CH, 2), + audioFormat = prefs.getString(K_AUDIO_FORMAT, AUDIO_FORMAT_OPUS) ?: AUDIO_FORMAT_OPUS, codec = prefs.getString(K_CODEC, "auto") ?: "auto", micEnabled = prefs.getBoolean(K_MIC, false), echoCancel = prefs.getBoolean(K_ECHO_CANCEL, true), @@ -350,6 +364,7 @@ class SettingsStore(context: Context) { .putString(K_SYSTEM_BUTTONS, s.systemButtons) .putString(K_GUIDE_GESTURE, s.guideGesture) .putInt(K_AUDIO_CH, s.audioChannels) + .putString(K_AUDIO_FORMAT, s.audioFormat) .putString(K_CODEC, s.codec) .putBoolean(K_MIC, s.micEnabled) .putBoolean(K_ECHO_CANCEL, s.echoCancel) @@ -387,6 +402,7 @@ class SettingsStore(context: Context) { const val K_SYSTEM_BUTTONS = "system_buttons" const val K_GUIDE_GESTURE = "guide_gesture" const val K_AUDIO_CH = "audio_channels" + const val K_AUDIO_FORMAT = "audio_format" const val K_CODEC = "codec" const val K_MIC = "mic_enabled" const val K_ECHO_CANCEL = "echo_cancel" @@ -691,6 +707,57 @@ val AUDIO_CHANNEL_OPTIONS = listOf( 8 to "7.1 Surround", ) +/** Opus 48 kHz — the default, and byte-for-byte the session every earlier build ran. */ +const val AUDIO_FORMAT_OPUS = "opus" + +/** + * Bit-exact PCM at 48 kHz / 24-bit (~2.3 Mbps). The honest win even without a hi-res interface: + * no lossy stage at all, and no double resample on a host whose engine already runs at 48 kHz. + */ +const val AUDIO_FORMAT_LOSSLESS_48 = "lossless48" + +/** + * Bit-exact PCM at 96 kHz / 24-bit (~4.6 Mbps), and only real if the host's capture endpoint + * genuinely runs at 96 kHz — the host declines rather than upsampling to meet the request. + */ +const val AUDIO_FORMAT_LOSSLESS_96 = "lossless96" + +/** + * (stored value, label) for the requested audio format — the cross-client table, matching the + * Apple client's `AudioFormatChoice` raw values so a profile written on either is honoured on the + * other. + * + * **The ladder is 48/96 kHz only, and that is arithmetic rather than bandwidth.** Every buffer + * figure in the shared jitter policy is `ms × perMs` with `perMs` an INTEGER number of samples per + * millisecond: 48 000 → 48 and 96 000 → 96 are exact, but 44 100 → 44.1 truncates to 44 — a silent + * 2.3 % error in every target, every de-prime fuse and every reported buffer depth. 44.1 kHz and + * its multiples are deferred behind reworking that arithmetic, not behind carrying them on the + * wire (design/hi-res-audio.md §4.1). + * + * Lossless at 48 kHz / **16**-bit is deliberately absent: it spends ~1.5 Mbps to sound like the + * transparent 256 kbps Opus it replaces, and it is the one lossless request whose wire parameters + * are indistinguishable from a legacy one. 24-bit is where the plane earns its bandwidth. + */ +val AUDIO_FORMAT_OPTIONS = listOf( + AUDIO_FORMAT_OPUS to "Standard (Opus)", + AUDIO_FORMAT_LOSSLESS_48 to "Lossless 48 kHz / 24-bit", + AUDIO_FORMAT_LOSSLESS_96 to "Lossless 96 kHz / 24-bit", +) + +/** + * The `(rateHz, bits)` pair [audioFormat] asks the host for, in `nativeConnect`'s terms. + * + * `48000`/`16` is exactly a pre-lossless request, so it keeps the legacy wire byte for byte; + * anything else makes core set `CLIENT_CAP_AUDIO_HIRES` in the Hello. Deriving the bit FROM the + * format is what stops the two ever disagreeing. An unrecognized stored value — a newer build's, + * or a corrupted pref — resolves to Opus rather than blocking the connect. + */ +fun Settings.audioFormatWire(): Pair = when (audioFormat) { + AUDIO_FORMAT_LOSSLESS_48 -> 48_000 to 24 + AUDIO_FORMAT_LOSSLESS_96 -> 96_000 to 24 + else -> 48_000 to 16 +} + /** * (stored value, label) for the preferred video codec — the cross-client table (the Rust * `CODECS`), so a value another client or a profile stored is always representable here. diff --git a/clients/android/app/src/main/kotlin/io/unom/punktfunk/SettingsScreen.kt b/clients/android/app/src/main/kotlin/io/unom/punktfunk/SettingsScreen.kt index 572b28f2..7022c110 100644 --- a/clients/android/app/src/main/kotlin/io/unom/punktfunk/SettingsScreen.kt +++ b/clients/android/app/src/main/kotlin/io/unom/punktfunk/SettingsScreen.kt @@ -824,6 +824,23 @@ private fun AudioSettings(s: Settings, update: (Settings) -> Unit, onMicChange: field = "audio_channels", caption = "Requested from the host; it downmixes if it has fewer.", ) { ch -> update(s.copy(audioChannels = ch)) } + // Stereo only, and the row is hidden rather than disabled on 5.1/7.1: a lossless surround + // frame does not fit one QUIC datagram at the default MTU and this plane is never + // fragmented, so the host declines the request outright. Offering a picker whose every + // non-default row would be refused is worse than not offering it — and the surround rows + // above are the setting a user in that state actually chose. + if (s.audioChannels == 2) { + SettingDropdown( + label = "Audio format", + options = AUDIO_FORMAT_OPTIONS, + selected = s.audioFormat, + field = "audio_format", + caption = "Lossless sends uncompressed audio — 2.3 Mbps at 48 kHz, 4.6 at " + + "96 kHz, on top of the video. The host has its own switch for it and both " + + "must be on; otherwise the session stays on Opus, which is already " + + "effectively transparent.", + ) { f -> update(s.copy(audioFormat = f)) } + } ToggleRow( title = "Microphone", subtitle = "Feeds this device's microphone to the host", diff --git a/clients/android/app/src/main/kotlin/io/unom/punktfunk/StatsOverlay.kt b/clients/android/app/src/main/kotlin/io/unom/punktfunk/StatsOverlay.kt index bb28db5d..cbeec558 100644 --- a/clients/android/app/src/main/kotlin/io/unom/punktfunk/StatsOverlay.kt +++ b/clients/android/app/src/main/kotlin/io/unom/punktfunk/StatsOverlay.kt @@ -18,13 +18,13 @@ import kotlin.math.roundToInt * The live stats overlay — the unified HUD (`design/stats-unification.md`): headline is * `capture→displayed` tiled by `host+network` + `decode` + `display` when the platform delivered * OnFrameRendered render callbacks this window (`dispValid`), falling back to the v1 - * `capture→decoded` headline without the `display` term when it didn't. Reads the 35-double + * `capture→decoded` headline without the `display` term when it didn't. Reads the 38-double * layout from [NativeBridge.nativeVideoStats] (that KDoc is the authoritative index list): * `[fps, mbps, e2eP50Ms, e2eP95Ms, latValid, skew, w, h, hz, lostTotal, bitDepth, colorPrimaries, * colorTransfer, chromaFormatIdc, hostNetP50Ms, decodeP50Ms, hostP50Ms, netP50Ms, lost, skipped, * fec, frames, dispValid, displayP50Ms, e2eDispP50Ms, e2eDispP95Ms, paceP50Ms, latchP50Ms, * presentsWindow, presenterActive, feedP50Ms, codecP50Ms, skippedOverflowWindow, audioBufferMs, - * audioAvOffsetMs]`. Every read + * audioAvOffsetMs, audioCodec, audioRateHz, audioBits]`. Every read * is length-guarded, so an older native lib simply omits the lines it can't feed. * * The shown `display` and `end-to-end` numbers EXCLUDE the OS present floor (see [osFloorMs]) at @@ -46,6 +46,10 @@ import kotlin.math.roundToInt * - [StatsVerbosity.DETAILED] — also the decoder label, the video-feed descriptor (10–13), the * stage equation (14/15, split into `host + network` when the Phase-2 terms at 16/17 are nonzero), * the excluded-floor line when one was measured, and the audio plane's own latency (33/34). + * + * The RESOLVED audio format (35–37) is the one figure that is not reserved for + * [StatsVerbosity.DETAILED] — it renders from [StatsVerbosity.NORMAL] up, and only on a lossless + * session. See [audioFormatLine]. (Not on COMPACT, which is one line by definition.) * [StatsVerbosity.OFF] renders nothing. Older native layouts simply omit the lines they lack (the * counter line falls back to the cumulative `lostTotal` at index 9 on a pre-window lib). */ @@ -182,6 +186,13 @@ internal fun StatsOverlay( if (detailed) { audioLine(s)?.let { statLine(it, Color.White) } } + // NOT gated to the detailed tier, unlike the audio latency above it, and deliberately: it + // is the one thing a user who turned lossless on needs to see. The failure it guards + // against (design/hi-res-audio.md §4.3, §10) is a session that costs 2.3–4.6 Mbps and + // delivers ordinary Opus, which is indistinguishable from success without a surface naming + // what the HOST resolved. `null` on the Opus plane every ordinary session runs, so the + // common case gains no line at all. + audioFormatLine(s)?.let { statLine(it, Color(0xFFB0FFD0)) } counterLine(s, lost)?.let { statLine(it, Color(0xFFFFB0B0)) } } } @@ -215,6 +226,35 @@ private fun audioLine(s: DoubleArray): String? { return "audio buffer $bufferMs ms$avTerm" } +/** + * The RESOLVED audio format from 35–37 — `audio lossless 96 kHz / 24-bit` — or `null` on the Opus + * plane and on an older native layout. + * + * Deliberately silent for Opus rather than printing `audio opus 48 kHz`: that is what every + * session has always been, so a line stating it would be noise on the HUD of every user who never + * touched the setting. The line exists for the opposite case, and it is the only surface that can + * answer it: the format the SETTINGS screen shows is what this device REQUESTED, and the host's + * five-condition gate can decline every one of them (its own switch is off by default) — leaving a + * session that looks, sounds and measures exactly like a granted one. The native side can also + * have downgraded the request before the handshake, if this device's output would not open the + * rate. Both land here as the truth. + * + * `codec` is the wire byte: 0 = Opus on `0xC9`, 2 = lossless PCM on `0xD3` (1 is reserved for a + * FLAC that was measured and not taken). + */ +private fun audioFormatLine(s: DoubleArray): String? { + if (s.size < 38) return null + if (s[35].roundToInt() != AUDIO_CODEC_PCM_WIRE) return null + val rateHz = s[36].roundToInt() + val bits = s[37].roundToInt() + if (rateHz <= 0 || bits <= 0) return null + val khz = if (rateHz % 1000 == 0) "${rateHz / 1000} kHz" else "$rateHz Hz" + return "audio lossless $khz / $bits-bit" +} + +/** `quic::AUDIO_CODEC_PCM` — the `0xD3` lossless plane's wire byte. */ +private const val AUDIO_CODEC_PCM_WIRE = 2 + /** One monospace HUD line — the shared type ramp so every tier's rows line up. */ @Composable private fun statLine(text: String, color: Color) { diff --git a/clients/android/app/src/test/kotlin/io/unom/punktfunk/ProfilesTest.kt b/clients/android/app/src/test/kotlin/io/unom/punktfunk/ProfilesTest.kt index ec5cd3e7..e6213bd4 100644 --- a/clients/android/app/src/test/kotlin/io/unom/punktfunk/ProfilesTest.kt +++ b/clients/android/app/src/test/kotlin/io/unom/punktfunk/ProfilesTest.kt @@ -50,6 +50,7 @@ class ProfilesTest { hdrEnabled = false, compositor = 4, audioChannels = 6, + audioFormat = AUDIO_FORMAT_LOSSLESS_96, micEnabled = true, touchMode = TouchMode.POINTER, mouseMode = MouseMode.CAPTURE, @@ -67,6 +68,7 @@ class ProfilesTest { assertFalse(out.hdrEnabled) assertEquals(4, out.compositor) assertEquals(6, out.audioChannels) + assertEquals(AUDIO_FORMAT_LOSSLESS_96, out.audioFormat) assertTrue(out.micEnabled) assertEquals(TouchMode.POINTER, out.touchMode) assertEquals(MouseMode.CAPTURE, out.mouseMode) @@ -236,6 +238,31 @@ class ProfilesTest { assertEquals(base, made.first().overrides.apply(base)) } + /** + * The audio-format setting is a STRING, and the two numbers it turns into are what the `Hello` + * carries — get the mapping wrong and the session either spends 4.6 Mbps it was not asked for + * or silently declines to ask for what it was. The `48000/16` row is the load-bearing one: it + * is byte-for-byte a pre-lossless request, which is what keeps `CLIENT_CAP_AUDIO_HIRES` off + * (core derives the bit from the pair) and the default session unchanged. + */ + @Test + fun theAudioFormatSettingMapsToTheWireFieldsItClaims() { + assertEquals(48_000 to 16, base.copy(audioFormat = AUDIO_FORMAT_OPUS).audioFormatWire()) + assertEquals( + 48_000 to 24, + base.copy(audioFormat = AUDIO_FORMAT_LOSSLESS_48).audioFormatWire(), + ) + assertEquals( + 96_000 to 24, + base.copy(audioFormat = AUDIO_FORMAT_LOSSLESS_96).audioFormatWire(), + ) + // The default is the legacy request — a fresh install asks for exactly what it always did. + assertEquals(48_000 to 16, Settings().audioFormatWire()) + // A newer build's value (or a corrupted pref) falls back to Opus rather than reaching the + // host as an unrepresentable rate: a settings string must never be able to block a connect. + assertEquals(48_000 to 16, base.copy(audioFormat = "lossless192").audioFormatWire()) + } + @Test fun mintedIdsAreWellFormed() { val id = newProfileId() diff --git a/clients/android/app/src/test/kotlin/io/unom/punktfunk/StatsOverlayAudioTest.kt b/clients/android/app/src/test/kotlin/io/unom/punktfunk/StatsOverlayAudioTest.kt index 171c13fc..92c02a68 100644 --- a/clients/android/app/src/test/kotlin/io/unom/punktfunk/StatsOverlayAudioTest.kt +++ b/clients/android/app/src/test/kotlin/io/unom/punktfunk/StatsOverlayAudioTest.kt @@ -30,10 +30,19 @@ class StatsOverlayAudioTest { val compose = createAndroidComposeRule() /** - * A plausible 35-double window with the audio gauges dialled in. Everything before 33 is the - * DETAILED-renderable shape the ShotScenes fixture uses; only the last two matter here. + * A plausible 38-double window with the audio gauges dialled in. Everything before 33 is the + * DETAILED-renderable shape the ShotScenes fixture uses; only the tail matters here. The + * format triple (35–37) defaults to an ordinary Opus session, so a test that says nothing + * about it is asserting against the shape every session has always had. */ - private fun stats(bufferMs: Double, avOffsetMs: Double, size: Int = 35): DoubleArray { + private fun stats( + bufferMs: Double, + avOffsetMs: Double, + size: Int = 38, + codec: Double = 0.0, + rateHz: Double = 48_000.0, + bits: Double = 16.0, + ): DoubleArray { val full = doubleArrayOf( 238.0, 921.4, 1.3, 2.1, 1.0, 1.0, 5120.0, 1440.0, 240.0, 2.0, 10.0, 9.0, 16.0, 1.0, 0.9, 0.4, 0.6, 0.3, @@ -42,6 +51,7 @@ class StatsOverlayAudioTest { 0.2, 0.3, 236.0, 1.0, 0.1, 0.3, 0.0, bufferMs, avOffsetMs, + codec, rateHz, bits, ) return full.copyOf(size) } @@ -91,4 +101,47 @@ class StatsOverlayAudioTest { show(stats(bufferMs = 42.0, avOffsetMs = 18.0, size = 33)) compose.onNodeWithText("audio buffer", substring = true).assertDoesNotExist() } + + /** + * The RESOLVED audio format (35–37) — the surface `design/hi-res-audio.md` §10 requires, and + * the only one that can answer "did lossless actually happen". The settings screen shows what + * this device REQUESTED; the host's five-condition gate (its own switch off by default) can + * decline every one of them and the session then looks, sounds and measures exactly like a + * granted one. Codec `2` is the `0xD3` lossless plane. + */ + @Test + fun aLosslessSessionNamesTheFormatItResolved() { + show(stats(bufferMs = 42.0, avOffsetMs = 0.0, codec = 2.0, rateHz = 96_000.0, bits = 24.0)) + compose.onNodeWithText("audio lossless 96 kHz / 24-bit").assertExists() + } + + /** + * Shown from NORMAL, unlike every other audio figure: a user who paid 4.6 Mbps for this should + * not have to find the DETAILED tier to learn whether they got it. + */ + @Test + fun theFormatLineIsNotReservedForTheDetailedTier() { + show( + stats(bufferMs = 42.0, avOffsetMs = 0.0, codec = 2.0, rateHz = 48_000.0, bits = 24.0), + verbosity = StatsVerbosity.NORMAL, + ) + compose.onNodeWithText("audio lossless 48 kHz / 24-bit").assertExists() + } + + /** + * Silent for Opus — which is every session anyone who never touched the setting will ever run, + * so a line stating it would be noise on almost every HUD. Absence IS the ordinary case. + */ + @Test + fun anOpusSessionSaysNothingAboutTheFormat() { + show(stats(bufferMs = 42.0, avOffsetMs = 0.0)) + compose.onNodeWithText("audio lossless", substring = true).assertDoesNotExist() + } + + /** An older native lib emits 35 doubles; the format line must be omitted, never mis-indexed. */ + @Test + fun aPreFormatNativeLayoutOmitsTheFormatLine() { + show(stats(bufferMs = 42.0, avOffsetMs = 0.0, size = 35, codec = 2.0)) + compose.onNodeWithText("audio lossless", substring = true).assertDoesNotExist() + } } diff --git a/clients/android/app/src/test/kotlin/io/unom/punktfunk/screenshots/ShotScenes.kt b/clients/android/app/src/test/kotlin/io/unom/punktfunk/screenshots/ShotScenes.kt index dacd6090..c2a5b6eb 100644 --- a/clients/android/app/src/test/kotlin/io/unom/punktfunk/screenshots/ShotScenes.kt +++ b/clients/android/app/src/test/kotlin/io/unom/punktfunk/screenshots/ShotScenes.kt @@ -446,12 +446,12 @@ internal fun StreamScene(verbosity: StatsVerbosity = StatsVerbosity.DETAILED) { Brush.linearGradient(listOf(Color(0xFF2A1E5C), Color(0xFF0E1B3D), Color(0xFF06122B))), ), ) { - // The full 35-double unified layout — NativeBridge.nativeVideoStats' KDoc is the + // The full 38-double unified layout — NativeBridge.nativeVideoStats' KDoc is the // authoritative index list: [fps, mbps, e2eP50, e2eP95, latValid, skew, w, h, hz, // lostTotal, bitDepth, colorPrimaries, colorTransfer, chromaFormatIdc, hostNetP50, // decodeP50, hostP50, netP50, lost, skipped, fec, frames, dispValid, displayP50, // e2eDispP50, e2eDispP95, paceP50, latchP50, presents, presenterActive, feedP50, codecP50, - // skippedOverflow, audioBufferMs, audioAvOffsetMs]. + // skippedOverflow, audioBufferMs, audioAvOffsetMs, audioCodec, audioRateHz, audioBits]. // 10/9/16/1 = a 10-bit BT.2020 PQ (HDR) 4:2:0 feed so the DETAILED HUD renders its // video-feed line; the display stage is valid (dispValid 1) so the headline is the // directly-measured capture→displayed pair, less the excluded OS present floor (the 0.3 @@ -475,6 +475,11 @@ internal fun StreamScene(verbosity: StatsVerbosity = StatsVerbosity.DETAILED) { // The audio plane: a 28 ms ring placed 4 ms behind the picture — a converged sync // loop, i.e. inside the deadband it deliberately leaves alone. 28.0, 4.0, + // The resolved audio format: codec 0 = Opus at 48 kHz/16-bit, which is what an + // ordinary session runs and what these shots are of. The HUD's format line only + // renders for the lossless plane (codec 2), so this triple deliberately adds + // nothing to the capture — the scene shows the shape almost every user sees. + 0.0, 48_000.0, 16.0, ), verbosity = verbosity, decoderLabel = "c2.qti.hevc.decoder · low-latency", diff --git a/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/NativeBridge.kt b/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/NativeBridge.kt index 8847a659..e6efa300 100644 --- a/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/NativeBridge.kt +++ b/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/NativeBridge.kt @@ -56,6 +56,19 @@ object NativeBridge { * decode loop then feeds slices with `BUFFER_FLAG_PARTIAL_FRAME` as they arrive). */ framePartsOk: Boolean, audioChannels: Int, + /** Requested audio sample rate: `48000` (or `0`) for the legacy Opus plane, `96000` to ask + * for lossless PCM at 96 kHz. Paired with [audioBits]; anything other than 48000/16 sets + * `CLIENT_CAP_AUDIO_HIRES` in the Hello and asks the host for the `0xD3` plane. + * + * A request on BOTH counts. The host runs a five-condition gate (its own + * `PUNKTFUNK_AUDIO_HIRES` switch among them) and may answer Opus; and the native side + * first proves THIS device can open the rate — AAudio grants an explicit rate or fails + * the open, and there is no recovery once the wire is negotiated — downgrading the + * request if it cannot. */ + audioRateHz: Int, + /** Requested audio sample depth: `16` (or `0`) legacy, `24` for the lossless plane. See + * [audioRateHz]; 24-bit is where lossless earns its bandwidth. */ + audioBits: Int, /** `quic::CODEC_*` bitfield of codecs this device decodes ([VideoDecoders.decodableCodecBits]); * `0` falls back to H.264|HEVC. The host resolves the emitted codec from this ∩ its GPU. */ videoCodecs: Int, @@ -275,12 +288,13 @@ object NativeBridge { /** * Drain ~1 s of live decode stats for the on-stream HUD, or `null` when no decode thread runs. - * Returns 35 doubles (unified stats spec, `design/stats-unification.md`): + * Returns 38 doubles (unified stats spec, `design/stats-unification.md`): * `[fps, mbps, e2eP50Ms, e2eP95Ms, latValid, skewCorrected, width, height, refreshHz, framesLost, * bitDepth, colorPrimaries, colorTransfer, chromaFormatIdc, hostNetP50Ms, decodeP50Ms, hostP50Ms, * netP50Ms, lostWindow, skippedWindow, fecWindow, framesWindow, dispValid, displayP50Ms, * e2eDispP50Ms, e2eDispP95Ms, paceP50Ms, latchP50Ms, presentsWindow, presenterActive, - * feedP50Ms, codecP50Ms, skippedOverflowWindow, audioBufferMs, audioAvOffsetMs]` + * feedP50Ms, codecP50Ms, skippedOverflowWindow, audioBufferMs, audioAvOffsetMs, audioCodec, + * audioRateHz, audioBits]` * (the flags are 1.0/0.0; indexes 2/3 are the end-to-end capture→decoded headline; 10–13 * describe the negotiated video feed — bit depth 8/10, CICP primaries/transfer, and the HEVC * chroma_format_idc 1=4:2:0 / 3=4:4:4; 14/15 are the stage p50s tiling the headline — @@ -299,7 +313,12 @@ object NativeBridge { * `skipped` (19), i.e. the decoder falling behind rather than benign newest-wins pacing; * 33/34 are the AUDIO plane — the playback ring's live depth in ms and the A/V sync loop's * smoothed offset in ms, positive meaning audio plays BEHIND the picture. Those two are live - * gauges, not windowed samples, and the offset reads 0 until the loop has a video reference). + * gauges, not windowed samples, and the offset reads 0 until the loop has a video reference; + * 35–37 are the audio FORMAT the host RESOLVED at the handshake — `audioCodec` 0 = Opus on + * `0xC9`, 2 = lossless PCM on `0xD3` — plus the resolved rate in Hz and depth in bits. Static + * for the session, and separate from 33/34 because they answer a different question: not "how + * late is the audio" but "is this the format the user asked for", which nothing else can tell + * apart — a declined lossless session looks exactly like a granted one from the outside). * Poll ~1 Hz; each call resets the measurement window. */ external fun nativeVideoStats(handle: Long): DoubleArray? diff --git a/clients/android/native/src/audio.rs b/clients/android/native/src/audio.rs index 6f449839..99bfcab5 100644 --- a/clients/android/native/src/audio.rs +++ b/clients/android/native/src/audio.rs @@ -1,8 +1,17 @@ -//! Android audio playback (android-only): pull Opus packets from the connector, decode to +//! Android audio playback (android-only): pull audio packets from the connector, decode to //! interleaved f32 (stereo or 5.1/7.1 surround), and feed AAudio via its realtime data callback -//! through a jitter ring. Mirrors [`crate::decode`]: one thread we own (the Opus decode producer) +//! through a jitter ring. Mirrors [`crate::decode`]: one thread we own (the decode producer) //! plus a shutdown flag; the realtime callback thread is owned by AAudio. //! +//! **Two planes, one pipeline.** A session runs Opus on `0xC9` (48 kHz, 5 ms frames — what every +//! host has always spoken) **or** lossless PCM on `0xD3` at the negotiated rate and depth +//! (`design/hi-res-audio.md`), never both, and which one is a session-wide fact settled in the +//! handshake — [`punktfunk_core::client::NativeClient::audio_codec`] — not a per-packet one. +//! Everything below reads it once through [`SessionAudio`]. The two planes share the jitter ring, +//! the A/V sync loop and the gap tracker *unchanged*, because they share a datagram header; only +//! the payload decode differs, and the concealment — a lossless format has no PLC to borrow +//! (§4.5), so [`punktfunk_core::audio::pcm::PcmConceal`] stands in for libopus's. +//! //! **The device is not assumed to work.** Opening AAudio is a negotiation with a vendor HAL, and //! this plane used to treat it as a formality: one Exclusive attempt, one Shared retry, and from //! there everything was taken on trust. Three separate failures all came out as "the app has no @@ -15,6 +24,18 @@ //! / `audio_perf` / `audio_reopen` pin any of it from `adb shell setprop`, for the device that //! reports silence and cannot be handed a custom build. //! +//! **The ladder's rate dimension.** Every rung used to ask for 48 kHz, so rejecting a stream whose +//! GRANTED rate differed was free. With a negotiated rate it is not: a device that will not grant +//! 96 kHz would fail every rung and the supervisor would disable audio for the whole session, +//! which is the one outcome the design calls unacceptable. So the rung carries the rate it asked +//! for, [`arm`] compares against THAT rather than a constant, and the ladder ends with a rung that +//! asks for nothing at all (AAudio's own choice) for the HAL that refuses an explicit request but +//! is natively at the rate we wanted. What the ladder deliberately does NOT contain is a 48 kHz +//! rung on a 96 kHz session: opening one would mean either playing the wire at double speed or +//! resampling it behind the user's back, and §9's rule is "say so and fall back, not resample +//! quietly". The fallback that keeps such a device in audio therefore happens BEFORE the `Hello` +//! — see [`output_rate_is_openable`], which is why that function exists. +//! //! The layout is the host-RESOLVED channel count (`NativeClient::audio_channels`, negotiated at //! connect), so an older/clamping host that can only capture stereo is decoded + played as stereo. //! 2 = stereo / 6 = 5.1 / 8 = 7.1, in the canonical wire order FL FR FC LFE RL RR SL SR. @@ -69,11 +90,20 @@ struct LiveStream { rung: OpenRung, } -/// One rung of the AAudio open ladder — a sharing mode and a performance mode tried together. +/// One rung of the AAudio open ladder — a sharing mode, a performance mode and the sample rate +/// they are tried at. +/// +/// `rate` is `Some(hz)` for an explicit request (AAudio's contract: an explicitly-set rate is +/// honoured or the open FAILS — it never silently substitutes) and `None` for AAUDIO_UNSPECIFIED, +/// which lets the HAL name its own. The unspecified rung is not a licence to play at whatever came +/// back: [`arm`] accepts it only when the granted rate equals the session's, so it rescues the +/// device that refuses an explicit 48 000/96 000 while already running at it, and rejects the one +/// that would have handed us a different rate. #[derive(Clone, Copy, Debug, PartialEq, Eq)] struct OpenRung { sharing: AudioSharingMode, perf: AudioPerformanceMode, + rate: Option, } /// Why [`decode_loop`] returned — only one of them is worth reopening the device for. @@ -86,13 +116,103 @@ enum DecodeExit { Disconnected, /// The connector closed: no more audio is coming, so there is nothing to reopen FOR. SessionClosed, - /// The plane cannot run at all (the Opus decoder would not build). Reopening the DEVICE would - /// not change that, so it is not a reason to walk the ladder again. + /// The plane cannot run at all (the decoder would not build — libopus refusing the negotiated + /// rate is the only way this happens today, since the PCM arm cannot fail). Reopening the + /// DEVICE would not change that, so it is not a reason to walk the ladder again. Fatal, } -const SAMPLE_RATE: i32 = 48_000; -/// Decoded-chunk hand-off depth: 64 × 5 ms = 320 ms slack (matches the core's AUDIO_QUEUE). +/// The audio format this session RESOLVED, read once from the connector and threaded through the +/// whole plane. +/// +/// Gathered into one value rather than passed as five parameters because the fields are only +/// meaningful together: a rate without the codec cannot tell a 48 kHz lossless session from a +/// 48 kHz Opus one, and those two agree on every other resolved value. +/// +/// ⚠ Everything here is what the HOST resolved, never what this client asked for. A client that +/// requests 96 kHz, is answered 48 kHz and opens at 96 kHz anyway is `design/hi-res-audio.md` +/// §4.3's failure one end further along — a session that audits clean at both ends and plays the +/// wrong content. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +struct SessionAudio { + /// [`punktfunk_core::quic::AUDIO_CODEC_OPUS`] (`0xC9`) or + /// [`punktfunk_core::quic::AUDIO_CODEC_PCM`] (`0xD3`) — what SELECTS the decoder, and the only + /// field that can. + codec: u8, + /// The resolved sample rate: 48 000 on every Opus session, 48 000 or 96 000 on `0xD3`. + rate_hz: u32, + /// The resolved sample depth (16 or 24) — the stride `0xD3` payloads are unpacked at. + /// Meaningless on the Opus plane, which decodes to f32 regardless. + bits: u8, + /// The resolved, normalized channel count (2 / 6 / 8). + channels: usize, + /// How much audio one datagram carries. Negotiated from the path MTU on `0xD3` (at 96 kHz / + /// 24-bit the default MTU ceiling only leaves room for 2 ms), and the Opus plane's fixed 5 ms + /// otherwise — folded to one field here so nothing downstream has to branch to size a buffer. + frame_us: u32, +} + +impl SessionAudio { + fn of(client: &NativeClient) -> SessionAudio { + let codec = client.audio_codec; + let is_pcm = codec == punktfunk_core::quic::AUDIO_CODEC_PCM; + // A zero rate is inexpressible off the wire (`Welcome::decode` folds both absence and a + // literal 0 to the legacy 48 kHz), but every buffer figure below divides by `rate/1000`, + // so a 0 that ever DID reach here would be a division by zero on the decode thread. One + // clamp, at the one place the value enters this module. + let rate_hz = if client.audio_sample_rate_hz == 0 { + punktfunk_core::audio::SAMPLE_RATE_HZ + } else { + client.audio_sample_rate_hz + }; + SessionAudio { + codec, + rate_hz, + bits: client.audio_bits, + channels: punktfunk_core::audio::normalize_channels(client.audio_channels) as usize, + // Same reasoning as the rate: an old host sends no `audio_frame_us` at all and a + // hostile one could send 0, and this number divides nothing but sizes everything. + // + // Capped at the longest rung of `FRAME_US_LADDER` (which is also the Opus plane's + // 5 ms) because the decode scratch is sized from that rung and clamps its copies to + // it: an unclamped `frame_us` would let a `Welcome` claim frames the scratch cannot + // hold, and the ring would then be reserved for a size the loop can never deliver. + // A conforming host only ever names a rung, so this bites nobody real. + frame_us: match (is_pcm, u32::from(client.audio_frame_us)) { + (true, us) if us > 0 => us.min(punktfunk_core::audio::pcm::FRAME_US_LADDER[0]), + // The Opus plane's frames are the protocol's fixed 5 ms (host `audio_thread`). + _ => OPUS_FRAME_US, + }, + } + } + + /// True when this session runs the lossless `0xD3` plane rather than Opus on `0xC9`. + fn is_pcm(&self) -> bool { + self.codec == punktfunk_core::quic::AUDIO_CODEC_PCM + } + + /// Interleaved f32 samples per millisecond at this rate and layout — the unit every + /// ms-denominated jitter-ring depth is expressed in. + /// + /// Integer samples per millisecond is load-bearing, not a rounding convenience: 48 000 → 48 + /// and 96 000 → 96 are exact, and it is why the ladder is 48/96 kHz only (44 100 → 44.1 + /// truncates to 44, a silent 2.3 % error in every depth figure — §4.1). + /// [`punktfunk_core::audio::JitterPolicy::new_at_rate`] carries the matching tripwire. + fn per_ms(&self) -> usize { + (self.rate_hz as usize / 1000) * self.channels + } + + /// Interleaved samples in ONE frame of this plane — what the ring reserves per queued chunk + /// and what the decode-scratch assertion is written against. + fn frame_samples(&self) -> usize { + (self.rate_hz as usize * self.frame_us as usize / 1_000_000) * self.channels + } +} + +/// The `0xC9` plane's frame duration: fixed by the protocol at 5 ms (the host's `audio_thread`), +/// not negotiated. Only `0xD3` carries `audio_frame_us`. +const OPUS_FRAME_US: u32 = 5_000; +/// Decoded-chunk hand-off depth: 64 frames of slack (matches the core's AUDIO_QUEUE). const RING_CHUNKS: usize = 64; /// How long [`arm`] waits for a freshly started stream's FIRST data callback before writing the /// rung off. Generous: a LowLatency stream calls back every few ms, and even a legacy path with a @@ -109,9 +229,11 @@ const REOPEN_SETTLE_MS: u64 = 250; /// disconnects permanently (unplugged, claimed by another app for good) settles into silence /// instead of a forever loop of opens on the session's audio thread. const REOPEN_ATTEMPTS: u32 = 8; -/// Opus packets decoded with AAudio never having taken a single sample before we call it: ~1 s at -/// the protocol's 5 ms frames. -const DEAD_STREAM_WARN_PACKETS: u64 = 200; +/// Packets decoded with AAudio never having taken a single sample before we call it — expressed +/// as a DURATION, because the two planes do not agree on what a packet is worth: 5 ms on Opus, as +/// little as 2 ms on a 96 kHz/24-bit `0xD3` session. A packet count would have meant ~0.4 s there +/// and a warning that fires before a slow HAL has finished waking. +const DEAD_STREAM_WARN_MS: u64 = 1_000; // --- Jitter-ring depths now come from the SHARED policy (`punktfunk_core::audio::JitterTuning`). -- // They used to be four Android-only constants here. The rationale for Android being DEEPER than the @@ -128,25 +250,70 @@ const DEAD_STREAM_WARN_PACKETS: u64 = 200; /// Throttle the AAudio XRun-driven HW-buffer grow check (cheap, but no need to poll every quantum). const XRUN_CHECK_EVERY: u32 = 128; -/// Opus decoder for the audio plane: a plain stereo decoder (the validated path) or a multistream -/// decoder for 5.1/7.1, both behind one `decode_float`. Built from the host-RESOLVED channel count -/// via the shared layout table. Mirrors the Linux client's `AudioDec`. +/// Why one arriving frame could not be turned into samples. Small on purpose — the decode loop +/// only logs it and moves on to the next packet. +#[derive(Debug)] +enum DecodeErr { + Opus(opus::Error), + /// A `0xD3` payload that is not a whole number of samples at the negotiated depth — a + /// truncated or hostile datagram. There is no partial-frame reading of it that is not a + /// permanent desync of every sample after it, so it is dropped whole. + Ragged, +} + +impl std::fmt::Display for DecodeErr { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + DecodeErr::Opus(e) => write!(f, "opus: {e}"), + DecodeErr::Ragged => write!(f, "PCM payload is not a whole number of samples"), + } + } +} + +/// Decoder for the audio plane: a plain Opus stereo decoder (the validated path), an Opus +/// multistream decoder for 5.1/7.1, or — on the lossless `0xD3` plane — no decoder at all, since +/// interleaved little-endian samples are a stride unpack. All three sit behind one `decode_float` +/// / `conceal` pair so the loop below never branches on the plane. Built from the host-RESOLVED +/// format; mirrors the Linux client's `AudioDec` and core's own `AudioPcmState`. enum AudioDec { Stereo(opus::Decoder), Surround(opus::MSDecoder), + /// The `0xD3` plane. `conceal` rides here rather than next to the Opus arms because it is the + /// thing a lossless format cannot borrow: `AudioGapTracker` feeds libopus PLC on `0xC9`, and + /// there is nothing in a raw frame from which to synthesize its successor (§4.5). `scratch` + /// exists because `pcm::to_f32` clears and reserves its output — it cannot write into the + /// fixed slice the loop hands out — so the samples are staged and then COPIED in, clamped. + Pcm { + bits: u8, + scratch: Vec, + conceal: punktfunk_core::audio::pcm::PcmConceal, + }, } impl AudioDec { - fn new(channels: u8) -> Result { + fn new(fmt: SessionAudio) -> Result { + let channels = fmt.channels as u8; + if fmt.is_pcm() { + return Ok(AudioDec::Pcm { + bits: fmt.bits, + scratch: Vec::with_capacity(fmt.frame_samples()), + conceal: punktfunk_core::audio::pcm::PcmConceal::new(), + }); + } + // The negotiated rate, not a constant — even though on this plane it is always 48 000. + // libopus accepts only 8/12/16/24/48 kHz and rejects 96 000 outright, which is the entire + // reason `0xD3` exists, so passing it through costs nothing and makes libopus itself the + // validator: a host that claimed Opus at a rate it cannot open fails loudly HERE (one + // "audio disabled" line naming the codec) instead of decoding at the wrong rate. if channels == 2 { Ok(AudioDec::Stereo(opus::Decoder::new( - SAMPLE_RATE as u32, + fmt.rate_hz, opus::Channels::Stereo, )?)) } else { let l = punktfunk_core::audio::layout_for(channels, false); Ok(AudioDec::Surround(opus::MSDecoder::new( - SAMPLE_RATE as u32, + fmt.rate_hz, l.streams, l.coupled, l.mapping, @@ -154,15 +321,82 @@ impl AudioDec { } } + /// Turn one arriving frame into interleaved f32 in `out`, returning **per-channel** samples + /// (the unit both planes' callers count in, and the unit concealment is sized from). + /// + /// `out` is a fixed slice and is never grown: on the PCM arm the staged samples are copied in + /// clamped to what fits, which is what makes an oversized or malformed datagram a truncated + /// frame rather than an overrun on the decode thread. fn decode_float( &mut self, input: &[u8], out: &mut [f32], - fec: bool, - ) -> Result { + channels: usize, + ) -> Result { match self { - AudioDec::Stereo(d) => d.decode_float(input, out, fec), - AudioDec::Surround(d) => d.decode_float(input, out, fec), + AudioDec::Stereo(d) => d.decode_float(input, out, false).map_err(DecodeErr::Opus), + AudioDec::Surround(d) => d.decode_float(input, out, false).map_err(DecodeErr::Opus), + AudioDec::Pcm { + bits, + scratch, + conceal, + } => { + // No host emits an empty `0xD3` payload — PCM has no DTX — but a torn datagram + // can present as one, and it must NOT reach `PcmConceal::accept`: accepting an + // empty frame would clear the last good frame and leave the next loss with + // nothing to conceal from. + if input.is_empty() { + return Ok(0); + } + let n = punktfunk_core::audio::pcm::to_f32(input, *bits, scratch) + .ok_or(DecodeErr::Ragged)?; + let n = n.min(out.len()); + out[..n].copy_from_slice(&scratch[..n]); + // The next loss is concealed from what the ring actually RECEIVED — the staged + // prefix, not the decoded frame. They differ only for an oversized datagram no + // conforming host sends, and taking the staged length keeps the concealment + // source bounded by the same fixed buffer as everything else. + conceal.accept(&out[..n]); + Ok(n / channels.max(1)) + } + } + } + + /// Synthesize ONE concealed frame into `out` for a packet that never arrived, returning + /// per-channel samples (0 = nothing to build from yet, so the caller should let the ring + /// carry the gap rather than emit an uninitialized buffer). + /// + /// Opus interpolates from the decoder's own state (empty input = libopus PLC). `0xD3` has no + /// such state, so [`punktfunk_core::audio::pcm::PcmConceal`] repeats-and-fades the last good + /// frame and decays a sustained gap to silence — a clean dropout beats a warble. + fn conceal( + &mut self, + out: &mut [f32], + frame_samples: usize, + channels: usize, + ) -> Result { + // libopus synthesizes into a slice sized by the LAST decoded frame; asking for more than + // that is asking it to invent audio the stream never had, and asking with 0 is asking + // before anything has decoded. + let plc = (frame_samples * channels).min(out.len()); + match self { + AudioDec::Stereo(d) if plc > 0 => d + .decode_float(&[], &mut out[..plc], false) + .map_err(DecodeErr::Opus), + AudioDec::Surround(d) if plc > 0 => d + .decode_float(&[], &mut out[..plc], false) + .map_err(DecodeErr::Opus), + AudioDec::Stereo(_) | AudioDec::Surround(_) => Ok(0), + AudioDec::Pcm { + scratch, conceal, .. + } => { + if !conceal.conceal(scratch) { + return Ok(0); // nothing has arrived yet — nothing to repeat + } + let n = scratch.len().min(out.len()); + out[..n].copy_from_slice(&scratch[..n]); + Ok(n / channels.max(1)) + } } } } @@ -175,10 +409,12 @@ impl AudioDec { /// log line below. One publisher, one reading — a second copy is a second thing to go stale. #[derive(Default)] struct Counters { - opus_decoded: AtomicU64, // Opus packets decoded OK (~200/s at 5 ms frames) - pcm_written: AtomicU64, // PCM frames copied out to AAudio (device clock is pulling) - underruns: AtomicU64, // callbacks that emitted silence (ring not primed / drained) - target_ms: AtomicU64, // the policy's LIVE target depth (it grows on this device's underruns) + // Wire frames decoded OK — Opus packets off `0xC9` (~200/s at 5 ms) or PCM frames off `0xD3` + // (up to 500/s at 2 ms). One counter for both planes because only one of them ever runs. + frames_decoded: AtomicU64, + pcm_written: AtomicU64, // PCM frames copied out to AAudio (device clock is pulling) + underruns: AtomicU64, // callbacks that emitted silence (ring not primed / drained) + target_ms: AtomicU64, // the policy's LIVE target depth (it grows on this device's underruns) /// Data callbacks since the process started, primed or not. Distinct from `pcm_written` /// (which only counts SERVED reads) because that is exactly the distinction the start /// watchdog needs: a device that is pulling but un-primed still ticks this, a stream that @@ -241,17 +477,17 @@ fn is_tv_device() -> bool { } /// Owned by [`crate::session::SessionHandle`]: the supervisor thread that owns the AAudio stream -/// and the Opus decode loop for as long as the session lives. +/// and the decode loop for as long as the session lives. pub struct AudioPlayback { shutdown: Arc, join: Option>, } impl AudioPlayback { - /// Spawn the audio supervisor: it opens AAudio (48 kHz/f32, the host-resolved channel layout) - /// by walking [`open_ladder`], runs the Opus decode loop against it, and reopens it if the - /// device disconnects. `None` only if the thread itself could not be spawned — an open failure - /// is reported by the supervisor (the caller leaves video streaming either way). + /// Spawn the audio supervisor: it opens AAudio at the host-RESOLVED format by walking + /// [`open_ladder`], runs the decode loop against it, and reopens it if the device disconnects. + /// `None` only if the thread itself could not be spawned — an open failure is reported by the + /// supervisor (the caller leaves video streaming either way). /// /// `game_audio` (the experimental low-latency mode) tags the stream usage=Game for the HAL's /// game-audio routing; off, the stream is untagged as it was before the overhaul. `is_tv` is @@ -261,14 +497,15 @@ impl AudioPlayback { game_audio: bool, is_tv: bool, ) -> Option { - // Build playback from the host-RESOLVED channel count (never the request): 2 = stereo / - // 6 = 5.1 / 8 = 7.1, canonical wire order FL FR FC LFE RL RR SL SR. - let channels = punktfunk_core::audio::normalize_channels(client.audio_channels) as usize; + // Everything about the format comes from what the host RESOLVED, never from what this + // device asked for: the channel count (2 = stereo / 6 = 5.1 / 8 = 7.1, canonical wire + // order FL FR FC LFE RL RR SL SR), the plane, the rate, the depth and the frame duration. + let fmt = SessionAudio::of(&client); let shutdown = Arc::new(AtomicBool::new(false)); let sd = shutdown.clone(); let join = std::thread::Builder::new() .name("pf-audio".into()) - .spawn(move || supervise(client, game_audio, is_tv, channels, &sd)) + .spawn(move || supervise(client, game_audio, is_tv, fmt, &sd)) .ok()?; Some(AudioPlayback { shutdown, @@ -277,6 +514,68 @@ impl AudioPlayback { } } +/// Would this device open a playback stream at `rate_hz`? Asked **before** the `Hello`, from +/// [`crate::session::connect`], and the reason it is asked there at all. +/// +/// AAudio's contract is that an explicitly-requested rate is honoured or the open FAILS — it never +/// silently substitutes. Which means a device that will not grant 96 kHz cannot be rescued *after* +/// negotiation: the wire would already be carrying 96 kHz frames, the plane is never renegotiated +/// mid-session (§6), and the only ways to play them on a 48 kHz stream are double speed or a +/// resampler nobody asked for. §7 states the rule directly — *"a client that cannot open a 96 kHz +/// output must not set `CLIENT_CAP_AUDIO_HIRES`"* — and this is how this client knows. +/// +/// The probe is the most permissive rung the ladder would ever reach (Shared + no performance +/// hint), so a `true` here means SOME rung can open it; it is not a promise that the Exclusive one +/// will. It is also a measurement at one instant: a route change between here and playback can +/// still invalidate it, which is why [`open_ladder`] carries the rate too. +/// +/// Opened and immediately dropped — never started, no data callback, so nothing is routed and no +/// audio focus is taken. Only called when the user actually asked for a non-default format +/// (`rate_hz != 48 000`), so an ordinary session pays nothing for it. +/// +/// ⚠ Never `request_start` this stream. The ndk wrapper's `Drop` **unwraps** `AAudioStream_close`'s +/// status, so closing a stream the HAL is unhappy about panics rather than logging — which is why +/// [`open_any`] stops a rung before dropping it. A stream that was opened and never started closes +/// cleanly from OPEN, so this probe has nothing to tear down. +pub fn output_rate_is_openable(rate_hz: u32, channels: u8) -> bool { + let built = AudioStreamBuilder::new().map(|b| { + b.direction(AudioDirection::Output) + .sample_rate(rate_hz as i32) + .channel_count(punktfunk_core::audio::normalize_channels(channels) as i32) + // The same f32 device format playback uses — see `try_open`. The wire depth is a wire + // fact and never reaches AAudio, so probing at 24-bit would be probing the wrong thing. + .format(AudioFormat::PCM_Float) + .sharing_mode(AudioSharingMode::Shared) + .performance_mode(AudioPerformanceMode::None) + .open_stream() + }); + match built { + Ok(Ok(stream)) => { + // Belt and braces: the contract says an explicit rate is granted or the open fails, + // but this is the one place cheap enough to check rather than trust, and a HAL that + // lied here would otherwise have talked us into negotiating a wire we cannot play. + let granted = stream.sample_rate(); + if granted != rate_hz as i32 { + log::warn!( + "audio: probe asked AAudio for {rate_hz} Hz and was granted {granted} Hz — treating the rate as unavailable" + ); + return false; + } + true + } + Ok(Err(e)) => { + log::info!("audio: this device will not open a {rate_hz} Hz output ({e})"); + false + } + Err(e) => { + // No builder at all is a broken AAudio, not a verdict about the rate. Say no: the + // caller's fallback is the legacy 48 kHz plane, which is the safe answer either way. + log::warn!("audio: AAudio stream builder unavailable for the {rate_hz} Hz probe ({e})"); + false + } + } +} + impl Drop for AudioPlayback { fn drop(&mut self) { self.shutdown.store(true, Ordering::SeqCst); @@ -303,75 +602,70 @@ impl Drop for AudioPlayback { /// the entire audio plane on it. Phones and handhelds — where the depths might one day come down, /// and where MMAP is exercised by every other app on the device — keep Exclusive first. /// +/// **Why rate is the OUTERMOST dimension.** Every sharing/performance mode is tried at the +/// session's negotiated rate before anything is tried at another: a Shared, resampled stream at +/// the RIGHT rate is worth more than an MMAP stream at the wrong one, because the wrong one is not +/// mis-tuned — it is the wrong audio. The second (and last) rate rung asks for nothing at all +/// (AAUDIO_UNSPECIFIED), for the HAL that refuses an explicit request but is natively at the rate +/// we wanted; [`arm`] still holds it to the session's rate, so it can only ever rescue, never +/// mislabel. +/// +/// **What is deliberately NOT here: a 48 kHz rung on a 96 kHz session.** It would open on almost +/// any device — and then the wire carries 96 kHz frames the stream would play at double speed, or +/// we would resample them behind the user's back, which is exactly what §9 forbids ("say so and +/// fall back, not resample quietly"). Mid-session the plane cannot be renegotiated either — the +/// host never switches tags under a client whose device is already open (§6). So the fallback that +/// keeps such a device in audio has to happen BEFORE the `Hello`, and it does: +/// [`output_rate_is_openable`] downgrades the REQUEST so this session is never a 96 kHz one in the +/// first place. +/// /// **Overrides.** `debug.punktfunk.audio_sharing` (`exclusive`|`shared`) and /// `debug.punktfunk.audio_perf` (`lowlatency`|`none`) pin the ladder to one sharing/performance /// mode, so a device that reports no audio can be bisected with `adb shell setprop` instead of a -/// rebuild — the same reasoning as `debug.punktfunk.no_av_sync`. -fn open_ladder(is_tv: bool) -> Vec { +/// rebuild — the same reasoning as `debug.punktfunk.no_av_sync`. There is deliberately no rate +/// override: a pinned rate that disagreed with the wire would produce the mislabelled playback the +/// whole design is written to prevent, and it is not a knob a field tester could use safely. +fn open_ladder(is_tv: bool, fmt: SessionAudio) -> Vec { use AudioPerformanceMode::{LowLatency, None as PerfNone}; use AudioSharingMode::{Exclusive, Shared}; - let mut rungs = match sysprop(c"debug.punktfunk.audio_sharing").as_deref() { - Some("exclusive") => vec![ - OpenRung { - sharing: Exclusive, - perf: LowLatency, - }, - OpenRung { - sharing: Exclusive, - perf: PerfNone, - }, - ], - Some("shared") => vec![ - OpenRung { - sharing: Shared, - perf: LowLatency, - }, - OpenRung { - sharing: Shared, - perf: PerfNone, - }, - ], - _ if is_tv => vec![ - OpenRung { - sharing: Shared, - perf: LowLatency, - }, - OpenRung { - sharing: Shared, - perf: PerfNone, - }, - ], - _ => vec![ - OpenRung { - sharing: Exclusive, - perf: LowLatency, - }, - OpenRung { - sharing: Shared, - perf: LowLatency, - }, - OpenRung { - sharing: Shared, - perf: PerfNone, - }, - ], - }; + let mut modes: Vec<(AudioSharingMode, AudioPerformanceMode)> = + match sysprop(c"debug.punktfunk.audio_sharing").as_deref() { + Some("exclusive") => vec![(Exclusive, LowLatency), (Exclusive, PerfNone)], + Some("shared") => vec![(Shared, LowLatency), (Shared, PerfNone)], + _ if is_tv => vec![(Shared, LowLatency), (Shared, PerfNone)], + _ => vec![ + (Exclusive, LowLatency), + (Shared, LowLatency), + (Shared, PerfNone), + ], + }; // Not every device honours LowLatency (it is a request, like everything else on the builder), // and a HAL that mishandles it is exactly the sort we are laddering around — so `none` has to // be reachable as a forced choice, not only as the last rung. match sysprop(c"debug.punktfunk.audio_perf").as_deref() { - Some("none") => rungs.iter_mut().for_each(|r| r.perf = PerfNone), - Some("lowlatency") | Some("low") => rungs.iter_mut().for_each(|r| r.perf = LowLatency), + Some("none") => modes.iter_mut().for_each(|m| m.1 = PerfNone), + Some("lowlatency") | Some("low") => modes.iter_mut().for_each(|m| m.1 = LowLatency), _ => {} } - rungs.dedup(); + modes.dedup(); + let mut rungs = Vec::with_capacity(modes.len() * 2); + for rate in [Some(fmt.rate_hz as i32), None] { + for &(sharing, perf) in &modes { + rungs.push(OpenRung { + sharing, + perf, + rate, + }); + } + } rungs } /// Everything an open attempt needs that does not vary between rungs. struct OpenCtx<'a> { - channels: usize, - ms: usize, + /// The session's resolved format — what the ring is sized in and what [`arm`] holds an + /// unspecified-rate rung to. + fmt: SessionAudio, tuning: punktfunk_core::audio::JitterTuning, hard_cap_max: usize, game_audio: bool, @@ -394,9 +688,16 @@ enum ArmError { /// Log the configuration a stream actually came up with. /// /// The GRANTED modes, which need not be the ones asked for: AAudio may resolve an Exclusive -/// request to Shared and LowLatency to None, and `rate != 48000` or `perf != LowLatency` means it -/// quietly fell to a resampled legacy path with different burst behaviour. Printing both sides is -/// what lets a field log distinguish that from plain jitter. +/// request to Shared and LowLatency to None, and `perf != LowLatency` means it fell to a legacy +/// path with different burst behaviour. Printing both sides is what lets a field log distinguish +/// that from plain jitter. +/// +/// The RATE used to be diagnostic here too — a granted rate other than 48 000 was the tell for +/// that legacy path. It is now load-bearing instead: [`arm`] refuses any rung whose granted rate +/// is not the one the session negotiated, because playing a 96 kHz wire through a 48 kHz stream is +/// not a tuning problem, it is the wrong audio (§9). So this line can only ever print the rate the +/// session resolved — which is exactly why it still prints it: it is the field-log proof that the +/// device really opened at the rate the `Welcome` claimed. fn log_started(live: &LiveStream, proven: bool) { let s = &live.stream; log::info!( @@ -432,7 +733,7 @@ fn open_any(ladder: &[OpenRung], ctx: &OpenCtx) -> Option { continue; } }; - match arm(&live, ctx.channels, ctx.counters, true) { + match arm(&live, ctx.fmt, ctx.counters, true) { Ok(()) => { log_started(&live, true); return Some(live); @@ -460,7 +761,7 @@ fn open_any(ladder: &[OpenRung], ctx: &OpenCtx) -> Option { "audio: no rung proved it was pulling — falling back to {rung:?} unproven; if this device is silent, this line is where to look" ); let live = try_open(rung, ctx).ok()?; - match arm(&live, ctx.channels, ctx.counters, false) { + match arm(&live, ctx.fmt, ctx.counters, false) { Ok(()) => { log_started(&live, false); Some(live) @@ -478,32 +779,43 @@ fn open_any(ladder: &[OpenRung], ctx: &OpenCtx) -> Option { /// permanent silence behind a healthy-looking log: /// /// 1. **The grant differs from the request.** The data callback casts AAudio's buffer to `f32` and -/// writes `num_frames * channels` of them, so a stream that came back with a different layout, -/// rate or format is not merely mis-tuned — it is an out-of-bounds write on a realtime thread. +/// writes `num_frames * channels` of them, so a stream that came back with a different layout +/// or format is not merely mis-tuned — it is an out-of-bounds write on a realtime thread. /// The NDK contract says an explicitly-requested value is honoured or the open fails, so this /// should be unreachable; "should be unreachable" is not a licence to trust a HAL about the /// length of a buffer we are about to write. /// 2. **`request_start` fails.** The old code gave up on the spot rather than trying the next rung, /// so one grumpy configuration disabled audio for the whole session. /// 3. **The stream starts and never calls back.** Nothing detected this, and it is the failure that -/// matters most: the decode thread cheerfully decodes Opus into a device that will never play -/// it, every counter looks plausible, and the only symptom is silence. +/// matters most: the decode thread cheerfully decodes into a device that will never play it, +/// every counter looks plausible, and the only symptom is silence. +/// +/// ⚠ **The rate check is compared against the RUNG, not a constant, and it is no longer only a +/// memory-safety check.** Every rung used to ask for 48 kHz, so `!= 48000` could only mean a HAL +/// misbehaving. Now the session negotiates its rate, so this comparison is also the §9 rule — *a +/// client that opens its device and gets a rate other than the resolved one must say so and fall +/// back, not resample quietly* — and rejecting the rung is how it says so. A rung that asked for +/// nothing (AAUDIO_UNSPECIFIED) is held to the SESSION's rate: it exists to rescue a HAL that +/// refuses explicit requests while already running at the rate we wanted, never to accept whatever +/// the HAL felt like. /// /// `prove_pulling` runs (3); the last-resort reopen in [`open_any`] passes `false`, having already /// decided that an unproven stream beats no stream. fn arm( live: &LiveStream, - channels: usize, + fmt: SessionAudio, counters: &Counters, prove_pulling: bool, ) -> Result<(), ArmError> { let s = &live.stream; + let channels = fmt.channels; + let want_rate = live.rung.rate.unwrap_or(fmt.rate_hz as i32); if s.channel_count() != channels as i32 - || s.sample_rate() != SAMPLE_RATE + || s.sample_rate() != want_rate || s.format() != AudioFormat::PCM_Float { return Err(ArmError::Unusable(format!( - "granted rate={} ch={} fmt={:?}, asked {SAMPLE_RATE}/{channels}/PCM_Float", + "granted rate={} ch={} fmt={:?}, needed {want_rate}/{channels}/PCM_Float", s.sample_rate(), s.channel_count(), s.format(), @@ -549,28 +861,39 @@ fn supervise( client: Arc, game_audio: bool, is_tv: bool, - channels: usize, + fmt: SessionAudio, shutdown: &AtomicBool, ) { - // Fold this Opus→AAudio thread into the client's hot-thread set so the ADPF session the decode - // thread opens also keeps audio decode on a fast core (registered before the video pump's first - // frame arrives, so it's captured when that session is created). No-op below API 33. Done once - // for the thread, not once per generation — it is the same thread throughout. + // Fold this decode→AAudio thread into the client's hot-thread set so the ADPF session the + // decode thread opens also keeps audio decode on a fast core (registered before the video + // pump's first frame arrives, so it's captured when that session is created). No-op below API + // 33. Done once for the thread, not once per generation — it is the same thread throughout. client.register_hot_thread(); - // Interleaved f32 samples per millisecond at this layout (48 kHz × channels); the ms- - // denominated jitter-ring depths scale by it. - let ms = (SAMPLE_RATE as usize / 1000) * channels; + // Interleaved f32 samples per millisecond at the RESOLVED rate and layout; every ms-denominated + // jitter-ring depth scales by it. 96 kHz doubles it, which is the whole reason it stopped being + // a constant. + let ms = fmt.per_ms(); let tuning = punktfunk_core::audio::JitterTuning::AAUDIO; let counters = Arc::new(Counters::default()); // The A/V sync hand-off: the realtime callback owns the ring (so it publishes the depth and // consumes the target), the decode thread owns the timestamps (so it computes the target). - // Two atomics, because the callback must not block on the thread that decodes Opus. + // Two atomics, because the callback must not block on the thread that decodes. let sync: Arc = Arc::default(); // Either signal counts. Kotlin's `FEATURE_LEANBACK` is the authoritative one; the sysprop // catches a device reached through some path that did not pass the flag, and neither answering // simply keeps the phone ladder. - let ladder = open_ladder(is_tv || is_tv_device()); - log::info!("audio: open ladder {ladder:?}"); + let ladder = open_ladder(is_tv || is_tv_device(), fmt); + // The one line that says what this session actually resolved — the `Welcome`'s answer, not the + // request. A report of "hi-res is on but it sounds the same" is triaged from here: `codec=0` + // means the host declined and the session is ordinary Opus, and the host's own log says why. + log::info!( + "audio: plane codec={} rate={} bits={} ch={} frame_us={} — open ladder {ladder:?}", + fmt.codec, + fmt.rate_hz, + fmt.bits, + fmt.channels, + fmt.frame_us, + ); // An escape hatch for the reopen itself: if reopening ever turns out to fight a device (a HAL // that disconnects in a loop), the field can pin the old give-up-on-disconnect behaviour // without a rebuild rather than living with a restart storm. @@ -584,8 +907,7 @@ fn supervise( while !shutdown.load(Ordering::Relaxed) { let disconnected = Arc::new(AtomicBool::new(false)); let ctx = OpenCtx { - channels, - ms, + fmt, tuning, // Worst transient the ring can hold before the policy trims it. hard_cap_max: tuning.hard_cap_ms as usize * ms, @@ -612,8 +934,20 @@ fn supervise( continue; } None => { + // Name the format, because at 96 kHz it is the likeliest cause and the cure is a + // setting rather than a rebuild. It should be near-unreachable — `connect` proves + // the rate is openable BEFORE the `Hello` (see `output_rate_is_openable`), so + // getting here on a hi-res session means the device changed underneath one that + // did open, and the reopen attempts above have already ridden out the settle. log::error!( - "audio: no AAudio configuration on the ladder could be opened and started — audio disabled for this session (video unaffected)" + "audio: no AAudio configuration on the ladder could be opened and started at {} Hz / {} ch — audio disabled for this session (video unaffected){}", + fmt.rate_hz, + fmt.channels, + if fmt.rate_hz == punktfunk_core::audio::SAMPLE_RATE_HZ { + "" + } else { + "; this device would not give us the rate the host resolved — turn hi-res audio off in Settings to run this session at 48 kHz" + }, ); return; } @@ -627,7 +961,7 @@ fn supervise( shutdown, &disconnected, &counters, - channels, + fmt, &sync, ); let _ = live.stream.request_stop(); @@ -646,14 +980,27 @@ fn supervise( } } log::info!( - "audio: stopped (opus={} pcm_frames={} underruns={} generations={})", - counters.opus_decoded.load(Ordering::Relaxed), + "audio: stopped ({}={} pcm_frames={} underruns={} generations={})", + plane_counter_key(fmt), + counters.frames_decoded.load(Ordering::Relaxed), counters.pcm_written.load(Ordering::Relaxed), counters.underruns.load(Ordering::Relaxed), generation + 1, ); } +/// What the decoded-frame counter is called in the log lines. Kept plane-specific rather than +/// renamed to something neutral so that the `opus=` an existing field report or triage note greps +/// for still means exactly what it always meant — and a lossless session is visibly a different +/// line rather than the same one with a surprising rate. +fn plane_counter_key(fmt: SessionAudio) -> &'static str { + if fmt.is_pcm() { + "pcm" + } else { + "opus" + } +} + /// Sleep up to `total_ms`, in slices, giving up early once `shutdown` is set. /// /// The supervisor's backoffs run on the same thread `AudioPlayback::drop` joins, so a plain @@ -675,13 +1022,13 @@ fn nap(shutdown: &AtomicBool, total_ms: u64) { /// nothing survives a failed try to reuse. fn try_open(rung: OpenRung, ctx: &OpenCtx) -> ndk::audio::Result { let OpenCtx { - channels, - ms, + fmt, tuning, hard_cap_max, game_audio, .. } = *ctx; + let channels = fmt.channels; let (tx, rx) = sync_channel::>(RING_CHUNKS); // Recycle free-list: drained PCM buffers go BACK to the decode thread to be refilled, so // the realtime callback never frees heap (Android's Scudo allocator has unbounded free() @@ -694,15 +1041,35 @@ fn try_open(rung: OpenRung, ctx: &OpenCtx) -> ndk::audio::Result { let cb_counters = ctx.counters.clone(); let cb_sync = ctx.sync.clone(); // Pre-reserve the ring so `extend` never reallocates on the realtime thread. Worst - // transient before the trim below = the hard cap plus one full channel of 5 ms (480-f32) - // frames — the punktfunk protocol always sends 5 ms Opus frames (host `audio_thread`); a - // larger frame would force a one-time realloc, asserted (not silently corrupted) in - // `decode_loop`. - let mut ring: VecDeque = VecDeque::with_capacity(hard_cap_max + RING_CHUNKS * 5 * ms); - // Shared de-jitter policy — prime depth, drift correction, de-prime hysteresis. The - // hysteresis this replaces was Android-only; Linux and Windows carried the instant - // `if ring.is_empty()` re-prime until now. - let mut policy = punktfunk_core::audio::JitterPolicy::new(tuning, channels as u8); + // transient before the trim below = the hard cap plus one full channel of the plane's OWN + // frame — 5 ms on Opus (the protocol's fixed size, host `audio_thread`), the negotiated + // `audio_frame_us` on `0xD3`, which at 96 kHz/24-bit is 2 ms. Sized from the resolved format + // rather than from a 5 ms constant: at 96 kHz a 5 ms reserve is DOUBLE what it should be, + // which is merely wasteful, but the same constant used the other way round (a plane whose + // frames were longer than the reserve) would force a one-time realloc on the RT thread — + // asserted, not silently corrupted, in `decode_loop`. + let mut ring: VecDeque = + VecDeque::with_capacity(hard_cap_max + RING_CHUNKS * fmt.frame_samples()); + // Shared de-jitter policy — prime depth, drift correction, de-prime hysteresis — told the + // RESOLVED format on both axes, because it is denominated in both: + // + // - `new_at_rate`: every depth, target, shed threshold and the `buffer_ms`/`target_ms` this + // client reports are `ms × per_ms` with `per_ms` an INTEGER count of samples per + // millisecond, so a 96 kHz session's figures must be in 96-sample milliseconds or every one + // of them is half what the tuning asked for. + // - `set_frame_us`: two of its decisions are denominated in FRAMES, not milliseconds — the + // floor under the effective target (a device quantum plus one frame) and the smooth shed + // (drop exactly one frame) — and both were written when 5 ms was the only frame this + // protocol had. Left at the default a 96 kHz/24-bit session would shed 2.5 frames at a time + // and crossfade across a whole one, which is not a crossfade. + // + // Microseconds throughout: the ladder has sub-millisecond rungs and `audio_frame_us` is the + // negotiated figure, so routing it through integer ms would truncate 2 500 µs to 2. An Opus + // session passes 5 000, which is the constructor's own default — so the ordinary session is + // bit-identical by construction rather than by a branch. + let mut policy = + punktfunk_core::audio::JitterPolicy::new_at_rate(tuning, channels as u8, fmt.rate_hz); + policy.set_frame_us(fmt.frame_us); let mut cb_count: u32 = 0; // callbacks since open (throttles the XRun grow check) let mut last_xrun: i32 = 0; // last AAudio XRun count we grew the buffer for let callback = move |s: &AudioStream, data: *mut c_void, num_frames: i32| { @@ -713,7 +1080,9 @@ fn try_open(rung: OpenRung, ctx: &OpenCtx) -> ndk::audio::Result { let want = num_frames as usize * channels; // SAFETY: AAudio provides `num_frames * channel_count` F32 slots at `data`, and // `arm` refused this stream unless the GRANTED channel count and format match the - // `channels`/`PCM_Float` this cast assumes. + // `channels`/`PCM_Float` this cast assumes. Unchanged by the lossless plane: the + // DEVICE format stays f32 whatever the wire depth is (see `try_open`'s builder), so + // this cast is as sound at 24-bit as it was at 16. let out = unsafe { std::slice::from_raw_parts_mut(data as *mut f32, want) }; // Drain decoded chunks into the ring WITHOUT freeing on the RT thread: `drain(..)` // empties each Vec but keeps its capacity, then the empty buffer is handed back for @@ -731,9 +1100,12 @@ fn try_open(rung: OpenRung, ctx: &OpenCtx) -> ndk::audio::Result { policy.set_sync_target(cb_sync.target()); cb_sync.publish_depth(ring.len()); // Jitter buffer: the shared policy decides prime/silence, trims a burst, and — - // new here — sheds ONE crossfaded 5 ms frame when the depth average has sat above - // target long enough to be drift rather than jitter. Without that shed this ring - // had no way back down: it clamped at 120 ms and stayed pinned there. + // new here — sheds ONE crossfaded frame when the depth average has sat above target + // long enough to be drift rather than jitter. Without that shed this ring had no + // way back down: it clamped at 120 ms and stayed pinned there. "One frame" is a + // REAL frame of this session, not a fixed 5 ms — `set_frame_us` above told the + // policy the negotiated length, and it also caps the seam crossfade at half of it, + // so a 2 ms lossless frame is not faded across its whole length. let step = policy.step(ring.len(), want); if step.drop_front > 0 { punktfunk_core::audio::crossfade_drop(&mut ring, step.drop_front, step.crossfade); @@ -778,13 +1150,27 @@ fn try_open(rung: OpenRung, ctx: &OpenCtx) -> ndk::audio::Result { let builder = AudioStreamBuilder::new()? .direction(AudioDirection::Output) - .sample_rate(SAMPLE_RATE) // The wire order (FL FR FC LFE RL RR SL SR) is the standard AAudio/Android channel // order, so this is an IDENTITY mapping — no permute. AAudio infers the 5.1/7.1 mask // from `channel_count` (the ndk crate's builder exposes no setChannelMask); the host - // captures + Opus-encodes in exactly this order. + // captures + encodes in exactly this order. .channel_count(channels as i32) + // ⚠ The DEVICE format is f32 on BOTH planes, deliberately — this is not an oversight + // left over from the Opus-only era. Core decodes each plane to interleaved f32 (libopus + // `decode_float`; `pcm::to_f32` normalises 16/24-bit codes by their full scale), so a + // 24-bit session already arrives as floats and asking AAudio for PCM_I24_PACKED would + // mean quantising them BACK — a second rounding, of the very samples the plane exists to + // deliver unrounded. It would also be unreachable here: that format is API 31, above this + // client's minSdk-28 floor. The wire depth is a WIRE fact; it never reaches the HAL. .format(AudioFormat::PCM_Float); + // The rate is per-rung: an explicit request (honoured or the open fails — AAudio never + // substitutes silently) or, on the last rate rung, nothing at all, letting the HAL name its + // own. `arm` holds an unspecified rung to the session's rate afterwards, so "let AAudio + // choose" can rescue a stubborn HAL but can never quietly change what we are playing. + let builder = match rung.rate { + Some(hz) => builder.sample_rate(hz), + None => builder, + }; // Tag the stream as game audio (usage=Game / content=Movie): the audio HAL applies // its low-latency game-audio routing/policy and it's grouped correctly with the // game-mode profile. Advisory — ignored where the device has no such policy. Part of @@ -818,9 +1204,9 @@ fn try_open(rung: OpenRung, ctx: &OpenCtx) -> ndk::audio::Result { }) } -/// Producer: `next_audio` → Opus `decode_float` → push interleaved f32 into the ring channel. -/// Buffers come from (and return to) the realtime callback's recycle free-list so the steady state -/// is allocation-free on both threads. +/// Producer: `next_audio` → decode (libopus, or a PCM stride unpack) → push interleaved f32 into +/// the ring channel. Buffers come from (and return to) the realtime callback's recycle free-list so +/// the steady state is allocation-free on both threads. /// /// Runs on the supervisor's thread and returns when the session ends, the playback is dropped, or /// the device disconnects — [`DecodeExit`] says which, because only one of them is worth reopening @@ -831,23 +1217,49 @@ fn decode_loop( shutdown: &AtomicBool, disconnected: &AtomicBool, counters: &Counters, - channels: usize, + fmt: SessionAudio, sync: &punktfunk_core::audio::AudioSyncCell, ) -> DecodeExit { let tx = &live.tx; let free_rx = &live.free_rx; - // Interleaved f32 samples per millisecond at this layout — the ring's 5 ms reserve check below. - let ms = (SAMPLE_RATE as usize / 1000) * channels; - // Opus decode scratch: worst-case 120 ms frame (5760 samples/ch) × channels. - let pcm_scratch = 5760 * channels; - let mut dec = match AudioDec::new(channels as u8) { + let channels = fmt.channels; + // Interleaved f32 samples per millisecond at the RESOLVED rate — the unit `buffer_ms` and the + // ring-reserve assertion below are both expressed in. + let ms = fmt.per_ms(); + // Decode scratch, sized for the LARGEST frame the running plane can hand us — and only for + // that plane, because they differ by more than an order of magnitude: + // + // - Opus: libopus's largest legal frame is 120 ms (5760 samples/ch), and it is always 48 kHz. + // - `0xD3`: the longest rung of `pcm::FRAME_US_LADDER` at the negotiated rate. The frame + // duration is chosen from the path MTU at session start and this plane is never fragmented, + // so nothing longer can arrive from a conforming host. + // + // Sizing BOTH from the Opus worst case (the `5760 * channels` this replaces) would have been + // 24× too big at 96 kHz, and — the reason the task called it out — sizing both from the PCM + // one would be far too SMALL for Opus. Either way the copies into it are clamped, so a + // non-conforming host truncates a frame rather than overrunning the decode thread's buffer. + let scratch_samples = if fmt.is_pcm() { + punktfunk_core::audio::pcm::samples_per_frame( + fmt.rate_hz, + punktfunk_core::audio::pcm::FRAME_US_LADDER[0], + channels as u8, + ) + } else { + 5760 * channels + }; + let mut dec = match AudioDec::new(fmt) { Ok(d) => d, Err(e) => { - log::error!("audio: opus decoder init: {e} — audio disabled"); + log::error!( + "audio: decoder init for codec={} rate={} ch={}: {e} — audio disabled", + fmt.codec, + fmt.rate_hz, + channels, + ); return DecodeExit::Fatal; } }; - let mut pcm = vec![0f32; pcm_scratch]; + let mut pcm = vec![0f32; scratch_samples]; let mut window_peak = 0f32; // loudest |sample| since the last log — tells a tone from silence let mut gaps = punktfunk_core::audio::AudioGapTracker::new(); let mut frame_samples = 0usize; // per-channel samples of the last decoded frame — the PLC unit @@ -867,20 +1279,29 @@ fn decode_loop( // dropped on the floor here for the plane's whole existence, which is why audio ran at whatever // depth its jitter ring settled at with nothing ever placing it against the picture. let av_sync_enabled = av_sync_enabled(); - let mut av = punktfunk_core::audio::AvSync::new(channels as u8); + // At the RESOLVED rate, for the same reason `JitterPolicy` is: this type's proposal is + // denominated in the ring's own samples-per-millisecond, so the two have to agree about what a + // millisecond is or every correction is out by the rate ratio. + let mut av = punktfunk_core::audio::AvSync::new_at_rate(channels as u8, fmt.rate_hz); let video_e2e = client.video_e2e_shared(); let av_offset_out = client.audio_av_offset_shared(); let buffer_ms_out = client.audio_buffer_ms_shared(); if !av_sync_enabled { log::info!("audio: A/V sync disabled (PUNKTFUNK_NO_AV_SYNC / debug.punktfunk.no_av_sync)"); } - // Both flags are polled at the 5 ms `next_audio` timeout, so a disconnect is noticed within one - // packet time even on a silent link. + // One tick = one frame of THIS plane, which is what makes the drought arm below conceal at the + // rate the callback drains at rather than racing it or falling behind. 5 ms on Opus (unchanged); + // as little as 2 ms on a 96 kHz/24-bit `0xD3` session, where a 5 ms tick would have synthesized + // 2 ms of cover per 5 ms of silence and lost ground for the whole drought. Also the poll period + // for both exit flags, so a disconnect is still noticed within one packet time on a silent link. + let tick = Duration::from_micros(fmt.frame_us.max(1) as u64); + // The dead-stream warning is a DURATION, not a packet count — see `DEAD_STREAM_WARN_MS`. + let dead_stream_warn_packets = (DEAD_STREAM_WARN_MS * 1000 / fmt.frame_us.max(1) as u64).max(1); while !shutdown.load(Ordering::Relaxed) { if disconnected.load(Ordering::Relaxed) { return DecodeExit::Disconnected; } - match client.next_audio(Duration::from_millis(5)) { + match client.next_audio(tick) { Ok(pkt) => { // Place this frame against the picture it belongs with, BEFORE it is queued: // `buffered_ahead` is everything that must still play first, so the depth read here @@ -908,46 +1329,56 @@ fn decode_loop( // concealing it a second time here would insert samples it never carried and push // everything after them later. let already = drought.packet(); - // Conceal lost packets (a seq gap) with libopus PLC before decoding the one that - // arrived: empty input synthesizes `frame_samples` of interpolation per missing - // packet — an inaudible fade instead of the click a hard gap makes in the ring. + // Conceal lost packets (a seq gap) before decoding the one that arrived: one + // synthesized frame per missing packet — an inaudible fade instead of the click a + // hard gap makes in the ring. libopus interpolates from its own state on `0xC9`; + // `0xD3` has none to interpolate from, so `PcmConceal` repeats-and-fades and + // decays a run to silence (§4.5). `AudioDec::conceal` hides which. for _ in 0..gaps.missing_before(pkt.seq).saturating_sub(already) { - let plc = frame_samples * channels; - if plc == 0 { + if frame_samples == 0 { break; // no decoded frame yet to size the concealment from } - if let Ok(samples) = dec.decode_float(&[], &mut pcm[..plc], false) { - let mut buf = free_rx - .try_recv() - .unwrap_or_else(|_| Vec::with_capacity(pcm_scratch)); - buf.clear(); - buf.extend_from_slice(&pcm[..samples * channels]); - match tx.try_send(buf) { - Ok(()) | Err(TrySendError::Full(_)) => {} - Err(TrySendError::Disconnected(_)) => return DecodeExit::Shutdown, + match dec.conceal(&mut pcm, frame_samples, channels) { + Ok(0) => break, // nothing to build from — let the ring carry the gap + Ok(samples) => { + let mut buf = free_rx + .try_recv() + .unwrap_or_else(|_| Vec::with_capacity(scratch_samples)); + buf.clear(); + buf.extend_from_slice(&pcm[..samples * channels]); + match tx.try_send(buf) { + Ok(()) | Err(TrySendError::Full(_)) => {} + Err(TrySendError::Disconnected(_)) => return DecodeExit::Shutdown, + } } + Err(_) => break, } } - match dec.decode_float(&pkt.data, &mut pcm, false) { + match dec.decode_float(&pkt.data, &mut pcm, channels) { Ok(samples) => { frame_samples = samples; let n = samples * channels; for &s in &pcm[..n] { window_peak = window_peak.max(s.abs()); } - // The ring's pre-reservation in `start` assumes the protocol's 5 ms (≤480-f32/ch) - // frames; a larger frame would force a one-time realloc on the RT thread. Catch a - // future host frame-size change here in debug, not as a silent audio glitch. + // The ring's pre-reservation in `try_open` is one frame of THIS plane per + // queued chunk; a larger frame would force a one-time realloc on the RT + // thread. Catch a host that changed its frame size — or a `Welcome` whose + // `audio_frame_us` disagrees with what it then sends — here in debug, + // rather than as a silent audio glitch. debug_assert!( - n <= 5 * ms, - "audio frame {n} f32 exceeds the 5 ms ring reserve" + n <= fmt.frame_samples(), + "audio frame {n} f32 exceeds the {} f32 ring reserve ({} µs at {} Hz)", + fmt.frame_samples(), + fmt.frame_us, + fmt.rate_hz, ); - let count = counters.opus_decoded.fetch_add(1, Ordering::Relaxed) + 1; + let count = counters.frames_decoded.fetch_add(1, Ordering::Relaxed) + 1; // Reuse a recycled buffer if the callback handed one back; only allocate when the // free-list is momentarily empty (startup / after a backpressure drop). let mut buf = free_rx .try_recv() - .unwrap_or_else(|_| Vec::with_capacity(pcm_scratch)); + .unwrap_or_else(|_| Vec::with_capacity(scratch_samples)); buf.clear(); buf.extend_from_slice(&pcm[..n]); match tx.try_send(buf) { @@ -959,11 +1390,11 @@ fn decode_loop( // sample. `arm`'s watchdog catches it at open, so reaching this means the // device stopped pulling AFTER it started — say so loudly, because from // the outside it is indistinguishable from "the app has no sound". - if count == DEAD_STREAM_WARN_PACKETS + if count == dead_stream_warn_packets && counters.pcm_written.load(Ordering::Relaxed) == 0 { log::error!( - "audio: {count} Opus packets decoded but AAudio has not taken one sample — {:?} opened into a device that is not playing", + "audio: {count} frames decoded ({DEAD_STREAM_WARN_MS} ms) but AAudio has not taken one sample — {:?} opened into a device that is not playing", live.rung, ); } @@ -976,7 +1407,8 @@ fn decode_loop( // `underruns` bought with a climbing `plc_ms` is a link in trouble, // not a link that is fine. log::info!( - "audio: opus={count} pcm_frames={} underruns={} buffer_ms={} target_ms={} av_ms={} plc_ms={} peak={window_peak:.3}", + "audio: {}={count} pcm_frames={} underruns={} buffer_ms={} target_ms={} av_ms={} plc_ms={} peak={window_peak:.3}", + plane_counter_key(fmt), counters.pcm_written.load(Ordering::Relaxed), counters.underruns.load(Ordering::Relaxed), (depth / ms.max(1)) as u64, @@ -987,24 +1419,24 @@ fn decode_loop( window_peak = 0.0; } } - Err(e) => log::debug!("audio: opus decode: {e}"), + Err(e) => log::debug!("audio: decode: {e}"), } } Err(PunktfunkError::NoFrame) => { - // Nothing on the wire. If the ring is draining with it, conceal from the decoder's - // own state — the same libopus interpolation the loss path uses, bounded by this - // preset's de-prime fuse so a genuinely dead stream is not papered over. ONE frame - // per tick, not a burst: this arm fires every 5 ms, which is the rate the callback - // drains at, so concealment keeps pace with playout instead of racing ahead of a - // depth reading it has already invalidated. `frame_samples` is 0 until something - // has decoded — there is no state to extrapolate from before then. + // Nothing on the wire. If the ring is draining with it, conceal — the same + // synthesis the loss path uses, bounded by this preset's de-prime fuse so a + // genuinely dead stream is not papered over. ONE frame per tick, not a burst: + // this arm fires once per `tick`, which is one frame of this plane and therefore + // the rate the callback drains at, so concealment keeps pace with playout instead + // of racing ahead of a depth reading it has already invalidated. `frame_samples` + // is 0 until something has decoded — there is no state to extrapolate from + // before then. let depth_ms = (sync.depth() / ms.max(1)) as u32; if frame_samples > 0 && drought.conceal(last_packet.elapsed(), depth_ms) { - let plc = frame_samples * channels; - if let Ok(samples) = dec.decode_float(&[], &mut pcm[..plc], false) { + if let Ok(samples @ 1..) = dec.conceal(&mut pcm, frame_samples, channels) { let mut buf = free_rx .try_recv() - .unwrap_or_else(|_| Vec::with_capacity(pcm_scratch)); + .unwrap_or_else(|_| Vec::with_capacity(scratch_samples)); buf.clear(); buf.extend_from_slice(&pcm[..samples * channels]); match tx.try_send(buf) { diff --git a/clients/android/native/src/session/connect.rs b/clients/android/native/src/session/connect.rs index 8ab520b5..5b9e92cc 100644 --- a/clients/android/native/src/session/connect.rs +++ b/clients/android/native/src/session/connect.rs @@ -102,9 +102,64 @@ fn force_parts_sysprop() -> bool { false } +/// Resolve the audio format this `Hello` should ASK for, from what Kotlin's setting requested — +/// after proving this device can actually open it. +/// +/// This is `design/hi-res-audio.md` §7's rule made mechanical: *"a client that cannot open a +/// 96 kHz output must not set `CLIENT_CAP_AUDIO_HIRES`"*. It has to happen here, before the +/// handshake, because after it there is no recovery: AAudio grants an explicitly-requested rate or +/// fails the open (it never substitutes), the host does not renegotiate the plane mid-session +/// (§6), and the only ways to play a 96 kHz wire through a 48 kHz stream are double speed or a +/// resampler nobody asked for — which §9 forbids in as many words ("say so and fall back, not +/// resample quietly"). So the fall back happens where falling back is still free: in the request. +/// +/// The ladder is 96 kHz → 48 kHz → the legacy pair. Dropping the RATE keeps the depth, so a device +/// that refuses 96 kHz still gets 48 kHz/24-bit lossless rather than being pushed all the way back +/// to Opus — the depth is where the plane earns its bandwidth anyway. +/// +/// Only the 96 kHz request is probed. 48 kHz is universally supported, and the DEPTH never reaches +/// AAudio at all (the device is opened as f32 on both planes — see `crate::audio`), so there is +/// nothing about 16-vs-24-bit for a probe to discover. An ordinary session therefore opens no +/// stream here and pays nothing. +fn resolve_requested_audio_format(rate_hz: u32, bits: u8, channels: u8) -> (u32, u8) { + let default = ( + punktfunk_core::audio::SAMPLE_RATE_HZ, + punktfunk_core::audio::pcm::BITS_16, + ); + // A format core would not carry (or Kotlin's `0` for "unset") is the legacy pair, not an + // error: the request is a preference, and an unrecognized one must not block a connect. + if !punktfunk_core::audio::pcm::depth_is_supported(bits) + || !matches!(rate_hz, punktfunk_core::audio::SAMPLE_RATE_HZ | 96_000) + { + return default; + } + if rate_hz == punktfunk_core::audio::SAMPLE_RATE_HZ || audio_rate_is_openable(rate_hz, channels) + { + return (rate_hz, bits); + } + log::warn!( + "audio: this device will not open a {rate_hz} Hz output, so the session asks for {} Hz / {bits}-bit instead — the wire is only ever offered a format this client has proved it can play", + punktfunk_core::audio::SAMPLE_RATE_HZ, + ); + (punktfunk_core::audio::SAMPLE_RATE_HZ, bits) +} + +#[cfg(target_os = "android")] +fn audio_rate_is_openable(rate_hz: u32, channels: u8) -> bool { + crate::audio::output_rate_is_openable(rate_hz, channels) +} + +/// Off-device (the host `cargo build --workspace` leg, where there is no AAudio at all): nothing +/// can be proved, so nothing is claimed. The caller falls back to the legacy rate, which is the +/// safe answer for a build that never runs on a phone anyway. +#[cfg(not(target_os = "android"))] +fn audio_rate_is_openable(_rate_hz: u32, _channels: u8) -> bool { + false +} + /// `NativeBridge.nativeConnect(host, port, w, h, hz, certPem, keyPem, pinHex, bitrateKbps, -/// compositorPref, gamepadPref, hdrEnabled, audioChannels, preferredCodec, timeoutMs, launch, -/// deviceName): Long`. +/// compositorPref, gamepadPref, hdrEnabled, audioChannels, audioRateHz, audioBits, preferredCodec, +/// timeoutMs, launch, deviceName): Long`. /// `launch` (empty ⇒ none) is a store-qualified library id to boot straight into a game. /// `deviceName` (empty ⇒ none) rides the Hello as `name` — what the host's pending-approval list /// and trust store show for this device (Kotlin passes `Build.MODEL`, its `nativePair` convention). @@ -113,6 +168,12 @@ fn force_parts_sysprop() -> bool { /// `bitrateKbps` 0 = host default. `compositorPref`/`gamepadPref` are `CompositorPref`/`GamepadPref` /// wire bytes (0 = Auto; unknown → Auto). `audioChannels` is the requested surround layout (2/6/8; /// normalized, anything else → stereo) — the host clamps it and the resolved count drives playback. +/// `audioRateHz`/`audioBits` are the audio FORMAT asked for: `48000`/`16` — or `0`/`0`, or anything +/// unrecognized — is the legacy Opus request every build has made, and any other supported pair asks +/// the host for the lossless `0xD3` plane. Only a request; the host's five-condition gate may answer +/// Opus regardless, and this device may not be able to open the rate at all, which is what +/// [`resolve_requested_audio_format`] settles HERE rather than letting the session negotiate a wire +/// it cannot play. /// `preferredCodec` is the soft codec preference wire byte (0 = Auto). `timeoutMs` is the handshake /// budget: the normal path passes a short value, the no-PIN "request access" path a long one (≥ the /// host's approval-park window) so a slow operator approval lands on this same parked connection @@ -137,6 +198,8 @@ pub extern "system" fn Java_io_unom_punktfunk_kit_NativeBridge_nativeConnect<'lo multi_slice_ok: jboolean, frame_parts_ok: jboolean, audio_channels: jint, + audio_rate_hz: jint, + audio_bits: jint, video_codecs: jint, preferred_codec: jint, timeout_ms: jint, @@ -222,7 +285,21 @@ pub extern "system" fn Java_io_unom_punktfunk_kit_NativeBridge_nativeConnect<'lo height: height as u32, refresh_hz: refresh_hz as u32, }; - match NativeClient::connect( + // Requested surround layout (2 = stereo / 6 = 5.1 / 8 = 7.1); anything else is stereo. The + // host clamps it and echoes the resolved count in `connector.audio_channels`, which drives the + // decoder + AAudio layout (read in `crate::audio::AudioPlayback::start`). + let audio_channels = + punktfunk_core::audio::normalize_channels(audio_channels.clamp(0, u8::MAX as jint) as u8); + // The audio format, downgraded to something this device has PROVED it can open before the + // `Hello` carries it — see `resolve_requested_audio_format` for why it cannot wait until + // playback. `clamp` first: a negative jint from a corrupted setting must not wrap into a + // plausible rate. + let (audio_rate_hz, audio_bits) = resolve_requested_audio_format( + audio_rate_hz.max(0) as u32, + audio_bits.clamp(0, u8::MAX as jint) as u8, + audio_channels, + ); + match NativeClient::connect_with_audio_format( &host, port as u16, mode, @@ -248,11 +325,15 @@ pub extern "system" fn Java_io_unom_punktfunk_kit_NativeBridge_nativeConnect<'lo } else { 0 }), - // Requested surround layout (2 = stereo / 6 = 5.1 / 8 = 7.1). The host clamps to what it can - // capture and echoes the resolved count in `connector.audio_channels`, which drives the - // decoder + AAudio layout (read in `crate::audio::AudioPlayback::start`). Anything else - // normalizes to stereo here. - punktfunk_core::audio::normalize_channels(audio_channels.clamp(0, u8::MAX as jint) as u8), + audio_channels, + // The audio format this session ASKS for (resolved above). A non-default pair is what + // makes core set `CLIENT_CAP_AUDIO_HIRES` in the `Hello` — capable AND the user turned it + // on, the `VIDEO_CAP_444` precedent — and it is answered by the host re-formatting the + // wire, so it must never be advertised on a device that cannot open the output. The host + // may still decline; `connector.audio_codec`/`audio_sample_rate_hz`/`audio_bits` are what + // actually happened, and `crate::audio` opens the device from those, never from these. + audio_rate_hz, + audio_bits, // Codecs this device can decode, ranked on the Kotlin side (`VideoDecoders.decodableCodecBits`: // H.264 + HEVC always, AV1 when a real `video/av01` decoder exists — AMediaCodec is // mime-driven, see `codec_mime`). Mask to the known bits and fall back to the pre-AV1 @@ -490,3 +571,59 @@ pub extern "system" fn Java_io_unom_punktfunk_kit_NativeBridge_nativePair<'local }) .resolve::() } + +#[cfg(test)] +mod tests { + use super::*; + use punktfunk_core::audio::pcm::{BITS_16, BITS_24}; + use punktfunk_core::audio::SAMPLE_RATE_HZ; + + /// The rule this leg exists to enforce: the `Hello` never asks for an audio format this device + /// has not proved it can open, because after the handshake there is no way back — AAudio grants + /// an explicit rate or fails the open, the host does not renegotiate the plane mid-session, and + /// playing a 96 kHz wire through a 48 kHz stream is not a fallback, it is the wrong audio. + /// + /// Off-device (this test's target) `audio_rate_is_openable` answers `false` for everything, so + /// what is pinned here is the DOWNGRADE, which is the half that has to be right: a device that + /// cannot do 96 kHz still gets a lossless session at 48 kHz rather than being pushed all the + /// way back to Opus, and the depth — the thing lossless is actually for — survives. + #[test] + fn an_unopenable_rate_is_downgraded_before_the_hello_and_keeps_its_depth() { + // The legacy pair passes through and probes nothing — a default session's `Hello` must + // stay byte-identical to every build before the lossless plane existed. + assert_eq!( + resolve_requested_audio_format(SAMPLE_RATE_HZ, BITS_16, 2), + (SAMPLE_RATE_HZ, BITS_16) + ); + // 48 kHz is never probed, so 48/24 lossless survives even where nothing can be opened. + assert_eq!( + resolve_requested_audio_format(SAMPLE_RATE_HZ, BITS_24, 2), + (SAMPLE_RATE_HZ, BITS_24) + ); + // 96 kHz IS probed, is refused here, and drops to 48 kHz with the depth intact. + assert_eq!( + resolve_requested_audio_format(96_000, BITS_24, 2), + (SAMPLE_RATE_HZ, BITS_24) + ); + } + + /// A settings string, a profile written by a newer build, or a corrupted preference must never + /// reach the wire as a format the plane cannot carry — and must never block a connect either. + /// Both halves resolve to the legacy pair, which every host can answer. + #[test] + fn an_unrepresentable_request_falls_back_instead_of_failing() { + for (rate, bits) in [ + (0, 0), // Kotlin's "unset" + (44_100, BITS_24), // §4.1 — breaks the integer samples-per-ms arithmetic + (192_000, BITS_24), // above the ladder + (SAMPLE_RATE_HZ, 32), // 32-bit float is deliberately not on the wire + (SAMPLE_RATE_HZ, 8), // not a depth this plane carries + ] { + assert_eq!( + resolve_requested_audio_format(rate, bits, 2), + (SAMPLE_RATE_HZ, BITS_16), + "{rate} Hz / {bits}-bit should have fallen back to the legacy pair" + ); + } + } +} diff --git a/clients/android/native/src/session/planes.rs b/clients/android/native/src/session/planes.rs index f10248dd..ded4f010 100644 --- a/clients/android/native/src/session/planes.rs +++ b/clients/android/native/src/session/planes.rs @@ -174,12 +174,13 @@ pub extern "system" fn Java_io_unom_punktfunk_kit_NativeBridge_nativeStopVideo( } /// `NativeBridge.nativeVideoStats(handle): DoubleArray?` — drain ~1 s of decode stats for the HUD -/// (unified stats spec, `design/stats-unification.md`). Returns 35 doubles +/// (unified stats spec, `design/stats-unification.md`). Returns 38 doubles /// `[fps, mbps, e2eP50Ms, e2eP95Ms, latValid, skewCorrected, width, height, refreshHz, framesLost, /// bitDepth, colorPrimaries, colorTransfer, chromaFormatIdc, hostNetP50Ms, decodeP50Ms, hostP50Ms, /// netP50Ms, lostWindow, skippedWindow, fecWindow, framesWindow, dispValid, displayP50Ms, /// e2eDispP50Ms, e2eDispP95Ms, paceP50Ms, latchP50Ms, presentsWindow, presenterActive, -/// feedP50Ms, codecP50Ms, skippedOverflowWindow, audioBufferMs, audioAvOffsetMs]` +/// feedP50Ms, codecP50Ms, skippedOverflowWindow, audioBufferMs, audioAvOffsetMs, audioCodec, +/// audioRateHz, audioBits]` /// (the flags are 1.0/0.0; indexes 0–21 match the previous 22-double layout — 0–13 the original /// 14-double one with the latency pair re-based to the end-to-end capture→decoded headline, 14/15 /// the stage p50s tiling it: `host+network` = capture→received, `decode` = received→decoded; 16/17 @@ -202,8 +203,13 @@ pub extern "system" fn Java_io_unom_punktfunk_kit_NativeBridge_nativeStopVideo( /// parked-AU overflow subset of the window's `skipped` at 19 (decoder fell behind, vs benign /// newest-wins pacing); 33/34 are the AUDIO plane's latency — the playback ring's live depth in ms /// and the A/V sync loop's smoothed offset in ms (positive = audio behind the picture) — both live -/// gauges rather than windowed samples, like the cumulative drop total at 9), or `null` when no -/// decode thread is running. +/// gauges rather than windowed samples, like the cumulative drop total at 9; 35–37 are the audio +/// FORMAT the host resolved at the handshake — `audioCodec` (`0` = Opus on `0xC9`, `2` = lossless +/// PCM on `0xD3`), the resolved rate in Hz and the resolved depth in bits. Static for the session, +/// and here because `design/hi-res-audio.md` §10 requires a surface for the RESOLVED format rather +/// than the requested one: a session that spends 4.6 Mbps and a session whose host quietly +/// declined look identical from the outside, which is §4.3's failure wearing a UI hat), or `null` +/// when no decode thread is running. /// Poll ~1 Hz from the UI; each call /// resets the measurement window. Not android-gated — pure `jni` + connector reads, so it links on /// the host build too (Kotlin only ever calls it on device). @@ -227,7 +233,7 @@ pub extern "system" fn Java_io_unom_punktfunk_kit_NativeBridge_nativeVideoStats< .drain(h.client.frames_dropped(), h.client.fec_recovered_shards()); let mode = h.client.mode(); let color = h.client.color; - let buf: [f64; 35] = [ + let buf: [f64; 38] = [ snap.fps, snap.mbps, snap.e2e_p50_ms, @@ -290,6 +296,16 @@ pub extern "system" fn Java_io_unom_punktfunk_kit_NativeBridge_nativeVideoStats< // high" report had no instrument behind it at all. h.client.audio_buffer_ms() as f64, h.client.audio_av_offset_ms() as f64, + // The audio format the host RESOLVED (`Welcome`), not what this device asked for. + // A lossless session and a session whose host declined lossless are indistinguishable + // from the outside — same picture, same latency figures, one of them quietly spending + // 2.3–4.6 Mbps of the link on nothing — so the HUD has to be able to name which + // (`design/hi-res-audio.md` §10, and §4.3 for why it matters). Static for the session: + // the plane is settled at the handshake and the host never switches it underneath a + // client whose output device is already open. + h.client.audio_codec as f64, + h.client.audio_sample_rate_hz as f64, + h.client.audio_bits as f64, ]; let arr = env.new_double_array(buf.len())?; arr.set_region(env, 0, &buf)?; diff --git a/crates/punktfunk-core/src/audio.rs b/crates/punktfunk-core/src/audio.rs index 483c90d6..d439fa2d 100644 --- a/crates/punktfunk-core/src/audio.rs +++ b/crates/punktfunk-core/src/audio.rs @@ -630,12 +630,8 @@ const SHRINK_QUIET_MS: u32 = 30_000; /// The same, while the A/V sync loop is actively asking for a shallower ring — see the branch in /// [`JitterPolicy::note_read`] that selects between them. const SHRINK_QUIET_SYNC_MS: u32 = 5_000; -/// Post-read depth below which a served callback counts as a NEAR-MISS: the device got its -/// samples, but less than one protocol frame was left in hand, so the next callback starves -/// unless a packet lands inside one frame time. On a healthy link the post-read depth hovers a -/// whole target above this, which is what makes a near-miss evidence of real delivery jitter — -/// the same evidence as an underrun, except nobody heard it yet. -const NEAR_MISS_MARGIN_MS: u32 = FRAME_MS; +// The NEAR-MISS margin is ONE PROTOCOL FRAME, so it is [`JitterPolicy::frame_samples`] rather +// than a constant — see the use site in `step`. /// How long a shrink remains a PROBE, in consumed audio: an underrun or near-miss inside this /// window means the shrink was wrong, and the previous target is restored at once instead of /// being re-learned three audible underruns at a time. @@ -720,7 +716,7 @@ pub struct JitterPolicy { /// without diverging in the meantime. sync_target: Option, /// Set by [`step`](Self::step) when the read it authorised leaves less than - /// [`NEAR_MISS_MARGIN_MS`] buffered; consumed by [`note_read`](Self::note_read). + /// one protocol frame buffered; consumed by [`note_read`](Self::note_read). near_miss: bool, /// A near-miss already grew the target this window — one step per window, so a single /// bunching episode (which lands as a RUN of consecutive near-misses while the ring refills) @@ -950,7 +946,17 @@ impl JitterPolicy { let after = depth.saturating_sub(out.drop_front); self.near_miss = self.primed && after >= want - && after - want < NEAR_MISS_MARGIN_MS as usize * self.per_ms; + // Post-read depth below which a served callback counts as a NEAR-MISS: the device got + // its samples, but less than ONE PROTOCOL FRAME was left in hand, so the next callback + // starves unless a packet lands inside one frame time. On a healthy link the post-read + // depth hovers a whole target above this, which is what makes a near-miss evidence of + // real delivery jitter — the same evidence as an underrun, except nobody heard it yet. + // + // Denominated in the RESOLVED frame, not a fixed 5 ms. Against a 2 ms lossless frame a + // frozen 5 ms margin stops meaning "one packet in hand" and starts meaning two and a + // half, so it would grow the target on a ring that was never close to starving — + // inverting the thing it exists to detect. Identical on every Opus session. + && after - want < self.frame_samples(); // Hollow: the depth AVERAGE runs a debt against the target — the promise has been raised // but the depth was never re-banked (see `DEPRIME_DEBT_MS`). Judged on the average, not // this instant: a single late packet empties the ring for a callback without making it @@ -2207,6 +2213,35 @@ mod tests { assert!(z.frame_samples() >= 1); } + /// The near-miss margin is "less than one packet left in hand". Frozen at 5 ms it would mean + /// two and a half packets on a 2 ms lossless frame — growing the target on a ring that was + /// never close to starving, which inverts what the near-miss detects. Identical on Opus. + #[test] + fn the_near_miss_margin_is_one_negotiated_frame() { + let pm = per_ms(2); + let want = 5 * pm; + + // A depth one sample short of a full frame in hand is a near miss… + let mut p = JitterPolicy::new(JitterTuning::PIPEWIRE, 2); + p.set_frame_us(2_000); + p.step(60 * pm, want); // prime + p.note_read(false); + p.step(want + 2 * pm - 1, want); + assert!(p.near_miss, "under one 2 ms frame in hand is a near miss"); + + // …and a full frame in hand is not. Under the old fixed 5 ms margin this depth would + // have counted, and the target would have grown for nothing. + let mut q = JitterPolicy::new(JitterTuning::PIPEWIRE, 2); + q.set_frame_us(2_000); + q.step(60 * pm, want); + q.note_read(false); + q.step(want + 2 * pm, want); + assert!( + !q.near_miss, + "a whole 2 ms frame in hand is not a near miss" + ); + } + fn per_ms_at(rate: u32, channels: u8) -> usize { (rate / 1000) as usize * channels as usize } @@ -2654,7 +2689,7 @@ mod tests { assert!(p.is_primed()); let base = p.target_ms(); // Serve the callback with less than one frame left over: depth = want + (margin − 1). - p.step(want + NEAR_MISS_MARGIN_MS as usize * pm - 1, want); + p.step(want + FRAME_MS as usize * pm - 1, want); p.note_read(false); // NOT short — the device got its samples assert_eq!( p.target_ms(),