Repository navigation
fix(audio): route legacy vibration PCM with per-port timing - #6860
Conversation
MarcZX10
left a comment
There was a problem hiding this comment.
The implementation connects the legacy type-10 output path to the existing AudioOut2 PCM conversion when a controller audio device is available, and provides frame-timed per-port haptic envelopes otherwise. The queue is keyed by the AudioOut port handle, clears on drain/close, and combines sources per motor without altering the separate AudioOut2 hold. The PR documents the host approximation and its hardware limits, with focused Linux/Windows test evidence in the description. Current conventions, intake, eligibility and report checks pass; platform CI jobs are skipped, so I am relying on the reported local runs for those configurations.
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
jotitadev
left a comment
There was a problem hiding this comment.
Native Linux run, plus one behaviour difference from main when SDL audio cannot initialize.
Machine: Intel i7-8700, Ubuntu 26.04, GCC 15.2, CMake 4.2.3, Release. Head 6ae8b93fd merged on main 143f13055.
- Build clean, no new warnings. The 38 audio/Pad tests (
^(audio_out|guest_pad)) pass. Non-GPU suite (-E "^agc|gpu|vulkan|video_out"): 667/669,main648/650; the same two fail on both (file_writer_binary,file_writer_text). - Test-only control on
main(the three new test files and the CMake registration, production code frommain): the eightaudio_out_vibration_{s16,f32,batch_*,mono_*,gain_*}cases fail,muted,silence,drain,exandpad_controlpass. Same picture as youraa53ed390control. - ASan (
ANYPS5_ENABLE_ASAN=ON, RelWithDebInfo, leak detection on): the 22 focused tests (audio_out_vibration_*,audio_out2_pad_mix,guest_pad,guest_pad_vibration_mode) are clean.
Behaviour difference: on main, sceAudioOutOpen with type 10 never touches SDL, and a main/BGM port that cannot open a device stays open without sound (openDevice returns false). AudioOut2Context.cpp also returns quietly when SDL audio init fails during pad discovery. With this PR, a type-10 open on such a host throws from openVibrationDevice, so a title that opens the legacy vibration port at startup (the PPSA99902 caller) stops where main keeps running without sound.
Reproduced with audio_out_last_output_time_tests, which already opens a type-10 port, with its setenv("SDL_AUDIODRIVER", "dummy") removed locally:
| environment | main |
this PR |
|---|---|---|
SDL_AUDIODRIVER=bogus |
passes | aborts: AudioOut: could not initialize controller audio: Audio target 'bogus' not available |
SDL_AUDIODRIVER=pulseaudio PULSE_SERVER=unix:/nonexistent |
passes | aborts: ... Could not setup connection to PulseAudio |
host default over SSH (no usable output; SDL falls back to ALSA, Unknown PCM default) |
passes | passes (no controller card found, envelope path) |
The description lists this as intended, so this is only to ask: should init/enumeration failure fall back to the motor envelope path, like main ports and AudioOut2 pad discovery do, rather than throw? A named-open failure on a card that was found is a different case.
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
@jotitadev Thanks for the native Linux controls and ASan run. I have linked them in the description with attribution. I independently confirmed the initialization behavior on Windows GCC 15.2 and Ubuntu 24.04 GCC 13.3 using the actual library: The difference from main is deliberate: successful discovery with no controller sound card chooses the documented motor approximation; a failed initialization or enumeration cannot establish that absence. I kept those errors explicit rather than silently changing routes. The existing That is our proposed host failure policy, not a claim about console behavior. I recommend retaining the throws under the contribution guide's exact-behavior-or-throw rule. If maintainers want discovery failure to select the motor route too, that should be an explicit policy decision with corresponding tests and debt documentation; I have not changed that behavior in response to the review. The same choice would not automatically apply to named-card open failure. AI-assisted: Codex; author responsible. |
6ae8b93 to
a2d6c16
Compare
|
Another title that needs this: Dead Cells (PPSA15552) opens a type 10 port with 256 stereo S16 frames at 48000 Hz, the same layout as PPSA99902, and sends its combat haptics through it. On main those samples are dropped, so the controller never rumbles. I ran it on Windows 11 with a DualSense over Bluetooth, using a local change that turns that port's PCM into motor rumble (per-channel RMS, the AudioOut2 mapping, without your per-port timeline). With that change, the combat haptics reached the controller: up to 120 rumble updates a second with RMS around 0.12 to 0.65, and they stopped when the haptic audio stopped. I have not run this PR's branch itself. Over Bluetooth there is no controller sound card, so this PR's rumble path is the one that applies. |
|
Thanks for the second title data point. It confirms that the generic legacy type-10 stereo S16/48 kHz path is used beyond Sonic Unleashed. One evidence gap remains: the local validation you describe predates this PR head, and the new device fixture uses one 48,000-frame legacy output call. It does not exercise the reported 256-frame cadence repeatedly through the per-port timeline. Could you either run the current PR head in Dead Cells or add a deterministic repeated 256-frame legacy-output case? That would check the newly confirmed workload directly. |
|
@MarcZX10 Added the repeated-call case in 166e84bed1: two persistent type-10 stereo S16/48 kHz ports, 32 successive 256-frame outputs each, checking PCM/silence, interval boundaries, mixing and drain isolation. At that exact head, all AudioOut/Pad groups pass three times: 117 Windows / 129 Ubuntu passes. Resetting each queue deadline makes the new case fail 3/3 while the older timing case still passes 3/3; restoring the exact source and full build gives 12 focused passes. The updated PR body records pins, commands and limitations. This addresses the component coverage gap; no new Dead Cells or physical-controller run is claimed. |
|
Checked the legacy type-10 routing (S16/F32, mono/stereo, gains, drain/close isolation), the per-port timeline and its tests, including the repeated 256-frame cadence case; the full Build covers Windows and Linux. Merged. |
What
Closes #6824. Legacy type-10 PCM currently returns an accepted frame count without reaching controller output. The public Sonic Unleashed Recompiled / PPSA99902 caller opens 256 stereo S16 frames at 48 kHz, tries F32 if opening fails, and submits through
sceAudioOutOutput. The failing main fixture reportslegacy vibration output published the wrong controller state; this is a component result, not a newly observed title startup stop.The timing and RMS motor projection are documented host approximations, not measured console semantics. Native controller hardware, Bluetooth transport and the existing single-controller routing limitation remain unverified. This is independent of AudioOut2-only #5448/#180. Closed #6839 used an 80 ms hold; its author withdrew it in the #6824 duration discussion. This implementation keeps duration per legacy port instead.
Tested
Current review update (2026-10-18 UTC): head
166e84bed1, incorporating maineb1a15b5cc4d90ddb0993b964b7646ad71cd7832. Addresses MarcZX10's repeated-output coverage request. The new test commit changes only the existing device fixture and its CTest registration; upstream changes are retained through the preceding main merge.audio_out_vibration_device_cadenceopens two persistent legacy type-10 stereo S16/48 kHz ports and submits 32 successive 256-frame calls per port (64 calls). It checks accepted frame counts, source identity, alternating PCM energy/silence, 5,333 us grain durations, nonoverlapping intervals per port, both motors at every recorded start/end boundary, source mixing and drain/close isolation. The real AudioOut producer calls the captured Pad callback and actualPadAudioHapticQueue; SDL device discovery/output are mocked. The boundary oracle allows host scheduling gaps and has no wall-time precision threshold.Pad: overlapping legacy haptic intervalsThe control changes only queue-deadline accumulation to
now + buffer duration, leaving the fixture unchanged. Thus the new repeated-call case catches a defect that the older single-submission timing case misses. Production was restored byte-for-byte (SHA-256 verified); no control is committed. Selected passing tests have no skips/timeouts and maximum durations 6.55 s Windows / 6.41 s Linux, below the 30 s requirement. Tests use SDL dummy/mock audio; no physical controller, Dead Cells replay or fresh game-playability claim.Cppcheck 2.20 then clang-tidy 23.1.3 completed on the production AudioOut TU and changed fixture in 6.029 analyzer seconds, no parser/configuration failures or timeouts. The compilation database selects the actual
libSceAudioOutproduction command. Zero selected clang-tidy findings; one Cppcheck byte-buffer cast portability advisory inprepareBuffer, whose complete body is byte-identical to main. No suppression, automatic fix or static-clean claim.Later main
fc623324a2changes rect-list shader/test behavior and an AGC recorder test, without AudioOut/Pad or contribution-policy changes, and merges cleanly in a local merge-tree check. Results retain the exact166e/eb1pins; no combined later-main runtime result is claimed. Earlier Linux/hardware/title evidence keeps its original scope below.Earlier validation at a2d6c16 and predecessor pins
Earlier validated head
a2d6c16bcd, based on main1c523bdb48. Full relinker/library builds pass on Windows 11 Enterprise 26200 / WinLibs GCC 15.2r7 and Ubuntu 24.04 WSL2 / GCC 13.3, Release, on Ryzen 9 9950X3D with the normal CPU guard.The four existing contribution commits were consolidated into one after backing up the published head. The runtime conflict was main's newer
checkedBufferSize: both ordinary PCM and expanded quad/Pulse controller PCM now use that existing helper, preserving main's signed 32-bit size limit before conversion/allocation. Routing, per-port timing, discovery failures and the three dedicated vibration fixtures are retained.bf16a3f7f0(main1b0c7cfa02)The main control replaces only
AudioOut.cppwith main production code, retaining the same fixtures and Pad implementation. All 48 failures are the specific assertionlegacy vibration output published the wrong controller state, after accepting the frame count. There are no timeout/abort/skip results in these selections. Longest passing test in the broader matrix: 6.55 seconds. Frozen validated fork checkpoint.The final rebase adds only main's unrelated AMM-range change; all ten contribution files are byte-identical to that checkpoint. Full builds were repeated at
a2d6c16bcd, followed by three runs each of the changedguest_amm_placementunit, actual S16 motor handoff, mocked quad-device routing and AudioOut2 mixing: 12 passes per OS. The prior audio matrix and main controls retain their explicit pins.The fixtures cover mono/stereo S16/F32; single/batched output; gain, mute, silence, drain and Ex isolation; quad/Pulse channel and speaker isolation; named-open/discovery failures; real drain waiting; source mixing/removal; grain-local energy; and separate AudioOut2/regular motor state. Deterministic timeline cases exercise 5,333 us and 200,000 us durations, exact boundaries and silent grains.
Cppcheck 2.20 then clang-tidy 23.1.3 analyzed six changed translation units at the current head, using the verified production AudioOut compilation command. One Cppcheck portability advisory is on main's unchanged
prepareBufferbyte-buffer cast; zero selected clang-tidy findings, no parser/configuration error or timeout, 8.728 analyzer seconds. The local command selector and its self-tests were corrected before this run; earlier mock-command analysis is superseded. No upstream tooling configuration or formatting was added. Conventions and whitespace checks pass.cmake -E env SDL_AUDIODRIVER=dummy ctest --test-dir build -V -j1 --repeat until-fail:3 --timeout 30 -R '^(audio_out|guest_pad)'Tests use SDL dummy audio or a mock named device and the actual component libraries. Physical controller, speaker, Bluetooth, GPU and game-playability coverage is unverified. Public-source/dump revision identity is not established.
Independent review and host-audio failure policy
jotitadev's earlier native Linux review, at published
6ae8b93fdmerged onto main143f13055, reported all 38 then-registered audio/Pad tests passing, eight main-producer output controls failing, and 22 focused ASan cases clean. Those results belong to the reviewer and retain that older pin.The reviewed host policy is unchanged: successful discovery with no controller sound card selects the documented motor approximation. Initialization/enumeration failures throw because discovery is incomplete; a named-card open failure also throws. Failed port slots remain reusable. Main/BGM and AudioOut2 retain their existing failure handling. The current mock cases exercise those errors; earlier real-host initialization controls and the policy discussion remain in our review reply. This stricter type-10 behavior is an explicit host policy, with console semantics unmeasured.
Overlap ordering remains independent of the AudioOut2-only work and the separate queue-pacing/Sys-port changes. The described producer control confirms the missing legacy output remains in main; the wider shared-file intersections remain recorded in the existing overlap report.
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)