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(),