fix(android): give the renderer its own USB connection, and add a real-world self test
ci / docs-site (pull_request) Successful in 1m13s
apple / swift (pull_request) Successful in 1m17s
windows / build (aarch64-pc-windows-msvc) (pull_request) Successful in 1m3s
apple / screenshots (pull_request) Skipped
ci / web (pull_request) Successful in 1m26s
ci / rust-arm64 (pull_request) Successful in 1m28s
android / android (pull_request) Successful in 4m5s
windows / build (x86_64-pc-windows-msvc) (pull_request) Successful in 3m21s
ci / rust (pull_request) Successful in 13m11s

**The bug.** The renderer was handed `HidUsbLink`'s file descriptor. That link's
own comment states the hazard exactly — "only one thread may drive a
connection's UsbRequests (requestWait() returns ANY completed request; a second
waiter would steal the reader's completions)" — and it is just as true of the
usbfs reap underneath: the isochronous ring and the HID reader were reaping each
other's URB completions. The standalone harness works because it owns its
descriptor by construction, which is precisely why it could never have caught
this. `DsCapture` now opens a dedicated connection via `openAuxConnection()` and
closes it only after the render thread is joined.

**The test.** Nothing exercised the CLIENT path without a host, so the two things
most likely to be wrong were invisible: whether the descriptor handed over is
exclusively ours, and whether the claim succeeds on this kernel. Neither is
unit-testable and a harness proves neither.

`nativePadAudioSelfTest` drives the voice coils with a tone through the real
path — the same aux connection, claim, sink and write loop the renderer uses —
and is triggered by `adb shell setprop debug.punktfunk.pad_audio_selftest 3`,
matching this repo's existing debug.punktfunk.* convention. It runs INSTEAD of
the renderer for that capture, never alongside it: two engines on one descriptor
is the fault being tested for, and I nearly shipped it into the test itself.

Underruns are deliberately not a failure condition — that is producer pacing.
The pass condition is data reaching the bus.
This commit is contained in:
enricobuehler
2026-08-03 00:38:58 +02:00
parent 2f1ef44191
commit e32bd30c85
5 changed files with 197 additions and 3 deletions
@@ -97,6 +97,15 @@ class DsCapture(
/** True once [PadAudioHook.start] has run for the current capture, so it fires exactly once. */
@Volatile private var padAudioStarted = false
/**
* The renderer's OWN connection to the pad.
*
* It must not share [usb]'s descriptor: two transfer engines on one usbfs descriptor reap each
* other's completions (see [HidUsbLink.openAuxConnection]), which strands both the HID reader
* and the audio ring. Closed only after the hook's stop has returned.
*/
@Volatile private var padAudioConn: android.hardware.usb.UsbDeviceConnection? = null
val isActive: Boolean get() = model != null
/** First attached Sony USB pad, for the permission flow. Needs no permission to enumerate. */
@@ -135,7 +144,11 @@ class DsCapture(
// joined, so ordering this first is what makes the borrow sound.
if (padAudioStarted) {
padAudioStarted = false
// stop() joins the render thread, so nothing is using the descriptor after it returns
// — only then is it safe to close the connection that owns it.
pad?.let { padAudio?.stop(it.index) }
padAudioConn?.close()
padAudioConn = null
}
val m = model
if (m != null) {
@@ -161,11 +174,38 @@ class DsCapture(
pad = it
// The wire index exists from here on, and the host addresses pad audio by it. Fired on
// the link thread, once per capture.
if (!padAudioStarted) {
val fd = usb.fileDescriptor
if (!padAudioStarted && padAudio != null) {
// A dedicated connection, NOT usb.fileDescriptor — see padAudioConn.
val conn = usb.openAuxConnection()
val fd = conn?.fileDescriptor ?: -1
if (fd >= 0) {
padAudioConn = conn
padAudioStarted = true
padAudio?.start(it.index, fd)
// Real-world self test, opt-in: `adb shell setprop debug.punktfunk.pad_audio_selftest 3`
// drives the voice coils for N seconds through the actual client path before
// the renderer takes over — the one check that proves the descriptor, the
// interface claim and the write path all work on THIS device, without needing
// a host to be streaming. Same convention as debug.punktfunk.force_parts.
val secs = runCatching {
Class.forName("android.os.SystemProperties")
.getMethod("get", String::class.java, String::class.java)
.invoke(null, "debug.punktfunk.pad_audio_selftest", "0") as String
}.getOrNull()?.toIntOrNull() ?: 0
if (secs > 0) {
// Diagnostic mode: the self test OWNS this descriptor for the capture, and
// the renderer must not also drive it — two engines on one usbfs
// descriptor reap each other's completions, which is precisely the fault
// this test exists to expose.
Thread({
val r = NativeBridge.nativePadAudioSelfTest(fd, secs, 60)
Log.i(TAG, "pad audio self-test → ${if (r > 0) "PASS ($r frames)" else "FAIL ($r)"}")
}, "pf-pad-selftest").start()
} else {
padAudio?.start(it.index, fd)
}
} else {
conn?.close()
Log.w(TAG, "pad audio: could not open a second USB connection")
}
}
Log.i(TAG, "captured $m → wire pad ${it.index}")
@@ -92,6 +92,26 @@ class HidUsbLink(
/** First attached matching device, or null. Does not need USB permission to enumerate. */
fun findDevice(): UsbDevice? = usb.deviceList.values.firstOrNull(config.deviceMatch)
/**
* Open a SECOND connection to the same device, for a consumer that needs its own descriptor.
*
* **Not a convenience — a correctness requirement.** `UsbDeviceConnection.requestWait()`
* returns *any* completed request on that connection, and the same is true of the usbfs reap
* ioctl underneath it: two independent transfer engines sharing one descriptor steal each
* other's completions. This link's reader owns its connection exclusively (see the note on
* [outQueue]), so anything else driving transfers on this device — the isochronous audio
* renderer — must open its own.
*
* usbfs allows the same device to be opened many times, and claims are per (descriptor,
* interface), so a claim made on this connection does not conflict with one made on that.
*
* The caller owns the returned connection and must close it.
*/
fun openAuxConnection(): UsbDeviceConnection? {
val dev = device ?: return null
return usb.openDevice(dev)
}
/**
* The open connection's usbfs file descriptor, or -1 when the link is not running.
*
@@ -367,6 +367,15 @@ object NativeBridge {
*/
external fun nativeStopPadAudio(handle: Long, pad: Int)
/**
* Drive the pad with a test tone through the real render path — no host, no session.
*
* [fd] must come from a connection **nothing else is driving transfers on**: two engines on
* one usbfs descriptor reap each other's completions. Blocks for roughly [seconds]; run it off
* the main thread. Returns sample frames written, or negative on failure.
*/
external fun nativePadAudioSelfTest(fd: Int, seconds: Int, hz: Int): Int
/**
* Is a mic capture actually RUNNING — i.e. did [nativeStartMic] open a stream, and has
* [nativeStopMic] not been called since? Offer the in-stream mute control on THIS rather than
+100
View File
@@ -280,6 +280,106 @@ mod sink {
}
}
// ---- the self test ------------------------------------------------------------------------------
/// Drive the pad directly with a synthetic tone, through **the real client path**.
///
/// This exists because the two things most likely to be wrong here cannot be unit-tested and are
/// invisible without a host: whether the descriptor Kotlin handed over is one this renderer may
/// drive exclusively, and whether the interface claim succeeds on this kernel. A standalone
/// harness proves neither — it owns its descriptor by construction, which is exactly the condition
/// that was violated when this renderer was handed the HID link's fd and the two engines began
/// stealing each other's URB completions.
///
/// Opens the sink the same way [`render`] does and writes a sine into the voice-coil pair, which
/// is felt rather than heard. Returns sample frames written, or a negative [`SelfTest`] code.
///
/// # Safety
///
/// `fd` must be a live usbfs descriptor whose connection outlives the call, and which **nothing
/// else is driving transfers on**.
#[cfg(target_os = "android")]
pub(crate) unsafe fn self_test(fd: i32, seconds: i32, hz: i32) -> i32 {
// SAFETY: the caller's contract.
let dev = unsafe { sink::device(fd) };
let mut playback = match sink::open(&dev) {
Ok(p) => p,
Err(e) => {
log::warn!("pad audio self-test: could not open the stream: {e}");
return SelfTest::OPEN_FAILED;
}
};
log::info!(
"pad audio self-test: {} ch {} at {} Hz, {} us in flight",
playback.channels(),
playback.format(),
playback.rate(),
playback.schedule().in_flight_us()
);
let rate = playback.rate();
let channels = playback.channels() as usize;
let frames_per_chunk = (rate as usize / 1000).max(1);
let mut chunk = vec![0i16; frames_per_chunk * channels];
let mut phase = 0.0f32;
let step = std::f32::consts::TAU * hz.clamp(20, 500) as f32 / rate as f32;
let total = u64::from(rate) * seconds.clamp(1, 30) as u64;
let mut written = 0u64;
while written < total {
for frame in chunk.chunks_mut(channels) {
let sample = (phase.sin() * 16_384.0) as i16;
phase += step;
if phase >= std::f32::consts::TAU {
phase -= std::f32::consts::TAU;
}
frame.fill(0);
// Channels 2 and 3 are the voice coils; the speaker pair stays silent so a pass is
// unambiguously FELT rather than merely audible.
for c in 2..channels {
frame[c] = sample;
}
}
if let Err(e) = playback.write_interleaved(&chunk) {
log::warn!("pad audio self-test: write failed after {written} frames: {e}");
return SelfTest::WRITE_FAILED;
}
written += frames_per_chunk as u64;
}
let _ = playback.drain(Duration::from_millis(500));
let stats = playback.stats();
log::info!(
"pad audio self-test: {} frames, {} urbs, {} underruns, {} short bytes, {} urb errors",
playback.frames_written(),
stats.urbs_completed,
stats.underruns,
stats.short_bytes,
stats.urb_errors
);
// Underruns are a producer-pacing property and deliberately NOT a failure here: the question
// this answers is whether the client can drive the pad at all. Data reaching the bus is the
// pass condition.
if stats.urb_errors > 0 || playback.frames_written() == 0 {
return SelfTest::NO_DATA;
}
playback.frames_written().min(i32::MAX as u64) as i32
}
/// Negative results from [`self_test`]. Positive values are sample frames written.
#[cfg(target_os = "android")]
pub(crate) struct SelfTest;
#[cfg(target_os = "android")]
impl SelfTest {
/// The claim or stream open failed — the OEM-kernel case, or a descriptor another engine owns.
pub(crate) const OPEN_FAILED: i32 = -1;
/// The stream opened but a write failed part-way.
pub(crate) const WRITE_FAILED: i32 = -2;
/// It ran, but nothing reached the bus.
pub(crate) const NO_DATA: i32 = -3;
}
// ---- the renderer worker -----------------------------------------------------------------------
/// A running renderer: the stop flag and the thread, joined on drop.
@@ -512,6 +512,31 @@ pub extern "system" fn Java_io_unom_punktfunk_kit_NativeBridge_nativeStartPadAud
})
}
/// `NativeBridge.nativePadAudioSelfTest(fd, seconds, hz): Int` — drive the pad directly with a
/// tone through the real client render path, with no host and no session involved.
///
/// The check a standalone harness cannot make: it owns its descriptor by construction, so it can
/// never reveal that the client handed the renderer a descriptor something else was already
/// driving. Returns sample frames written, or negative on failure (see `pad_audio::SelfTest`).
#[no_mangle]
#[cfg(target_os = "android")]
pub extern "system" fn Java_io_unom_punktfunk_kit_NativeBridge_nativePadAudioSelfTest(
_env: JNIEnv,
_this: JObject,
fd: jni::sys::jint,
seconds: jni::sys::jint,
hz: jni::sys::jint,
) -> jni::sys::jint {
jni_guard(-1, || {
if fd < 0 {
return -1;
}
// SAFETY: Kotlin holds the owning UsbDeviceConnection open across this call and drives no
// other transfers on it (it opens a dedicated connection for exactly this).
unsafe { crate::pad_audio::self_test(fd, seconds, hz) }
})
}
/// `NativeBridge.nativeStopPadAudio(handle, pad)` — stop tier-A pad audio and join its thread.
///
/// Returns only once the render thread is joined, which is the point: Kotlin may close the