WSOLA + Low Pass Filter - #19512
Conversation
1cb4db9 to
2c19a32
Compare
|
Could you add some information to the PR please. A summary would be good, what exactly are you trying to solve. |
|
I like the idea of combining the separate FF audio options into one option, but surely it could be done without this many modifications.. I mean are the |
2c19a32 to
a3a7a33
Compare
|
I'm inclined to just close this, tbh. |
|
Hi guys, sorry I meant to open this up against my own fork to test the CI. I'm still working on this, but yes I will add a description to the PR. I've been focused on testing against all the systems I have so I just glossed over the fact that I opened an actual PR against upstream. |
Yes- there's a few commits I was going to mention in the PR body that we can drop or split into different PRs. Apologies for the mess, I'm happy to take feedback and adjust. EDIT: Actually to expand on this in particular, and something I'm going to add into the PR body- the reason this was initially added is because a lot of the systems that RA targets have very little RAM, so there's an argument to keep this out of the build. I can include some numbers to support this soon. However an argument not gate it by build is that this only consumes the RAM when it's active, so a person on a low RAM device could just not enable the time-stretch audio. |
|
Please take a look at the updated PR description. I included a technical details section for thoroughness though I apologize it's largely LLM written since I've been kind of panicking since I didn't realize I opened a PR against the actual project until just now. As stated, some commits in this PR can and probably should be split into some separate PRs (or dropped), especially the use-after-free bug fix. Please let me know if you'd like me to do that and I can. I'm currently rebasing on the latest master and then I need to do some manual testing/listening on Windows again after my latest changes. EDIT: Actually realized that upstream already fixed the use-after-free bug while rebasing. |
2c79640 to
050b320
Compare
|
@sonninnos I looked at your question more carefully and yes it was possible to simplify some of the configuration logic. In the common makefile I set an ifneq to make the default opt-in. ctr, vita, wii, wiiu, ngc (stretcher only), lfx000, miyoo (both) opt out because of the RAM considerations (though we can potentially enable it though it would probably need on-device tsting. openpandora and ps2's griffin branch opt in because they don't use |
|
I'm glad to see a bespoke time-stretching algo instead of adding the same big lib dependency that everyone always reaches for first. |
5b5659a to
95396ed
Compare
I've been replaying some old Pokemon games (as you can probably see from testing) and was really dissatisfied with the complete lack of audio filtering in basically any emulators. After finding some satisfying changes with melonDS using WSOLA I spent a lot of time manually tuning this in RA so I hope it can benefit a lot of people across basically all the cores. |
27e0400 to
dd8f9d1
Compare
|
big fan of this project! as a common 2x fast forwarder, I would love to keep the audio on! thank you for your effort on this! |
c1c060a to
da45d63
Compare
2dec8ce to
90e559b
Compare
|
I think this PR is good to review/test- I'll keep rebasing as conflicts or other merge issues come up. As stated in the PR description I was able to test on Linux, Steam Deck, Mac, and Windows, and then in an Android emulator, but testing on an actual Android device would be ideal. More other devices would also be good because I found small issues in basically all of them. Ideally there should be no crackling sounds on any sped up gameplay, either when fully at fast forward or when toggling between fast forward and regular speed. The low-pass filter also shouldn't be engaged at normal speed, which can be easily tested by setting it to something low like 8000Hz and making sure that normal speed gameplay sounds normal. |
a16f156 to
90f866b
Compare
Changes playback rate while preserving pitch, by overlap-adding windowed frames chosen for waveform similarity so consecutive frames splice on matching phase. Nothing calls it yet; the audio driver wiring follows. Not internally synchronised: a write and a read must never run concurrently. In RetroArch both happen inside audio_driver_flush(), whose every call site holds audio_driver_state_lock(). Two suites are registered in Makefile.test. The pumping test is the one that matters: it drives noise at ratio 3 and measures the envelope at the hop rate, separating a working search (0.03-0.05) from one disabled (0.31-0.35) or inverted (0.63-0.65). Costs roughly 15ms per second of output at 48kHz, about 1.5% of one x86-64 core, flat across speeds since the work scales with output frames rather than emulation speed. The search is double-precision throughout, so targets without an FPU decline it. Makefile.common gates the module on HAVE_AUDIO_TIMESTRETCH = 1 and only qb was setting it, so every hand-maintained define list was excluded by accident. Turn it on next to each HAVE_DSP_FILTER = 1, which is the same set. Declining it, with the reason in each file: 3DS, Vita, Wii, Wii U and GameCube on the ~290KB of rings; Miyoo and LFX000 because armv5te / arm926ej-s has no FPU. PSP, Xbox 360 and PS3 never enable DSP filters either. The cost is paid only by a user who selects the mode - the setting defaults to Discard - which is how RetroArch already treats DSP and video filters.
Fast-forward audio through the WSOLA time-stretcher still sounds "crunchy" in listening tests. This ports the low-pass from the same author's unmerged melonDS branch, which pairs the stretcher with a cutoff that scales inversely with speed, specifically to take the edge off. Nothing calls it yet; the audio driver wiring follows. Fourth-order Butterworth (two cascaded RBJ biquads, stereo, independent state per channel), cutoff smoothed toward its target rather than stepped, passthrough at wide-open, and running - but not writing back - even while bypassed, so its state never falls out of step with the signal. Coefficients are interpolated across each block rather than replaced in one go: a transposed direct-form biquad's state encodes its past under the coefficients that produced it, so a step leaves the two inconsistent and the filter rings. Gated on its own HAVE_AUDIO_LOWPASS rather than sharing the stretcher's flag: five platforms decline the stretcher for its ~290KB of rings, and this is 168 bytes of state, so 3DS, Vita, Wii, Wii U and GameCube keep the filter. Miyoo and LFX000 decline both, for the reason they already declined the stretcher - no FPU, and this is double throughout. test_low_pass.c measures real attenuation in dB rather than asserting on shape alone.
audio_fastforward_mute and audio_fastforward_speedup were never independently meaningful: runloop.c already made mute win over speedup. Replace both with audio_fastforward_mode, a four-valued enum, and add a Time-Stretch value that nothing implements yet - the driver wiring follows, and until then it falls through to Discard. Discard is zero, so a configuration that has never seen this setting behaves exactly as before. Migration follows the quit_press_twice -> confirm_quit precedent. Mute is checked before speedup because mute wins today, and the translation is skipped when the new key is already present, so an explicit audio_fastforward_mode always beats a stale bool. Both old keys are unset on save. Like that precedent, this covers the main configuration only - a per-core or per-content override carrying an old key is dropped rather than migrated. msg_hash.h keeps MENU_LABEL(AUDIO_FASTFORWARD_MUTE) and its speedup counterpart declared although no setting backs them: the packed translation headers under intl/ hold a flat ids[] array positionally correlated with a separate string table, so removing an id without its paired string member is not a local edit; removing them properly is a Crowdin round-trip.
The dropdown builder enumerates every uint setting min-to-max, so the per-setting left/right handlers could not reverse what the list shows. Add ST_UI_TYPE_UINT_COMBOBOX_DESC, which walks the same range backwards. Nothing uses it yet; the fast-forward low-pass cutoff follows. ui_type rather than a new SD_FLAG: the flags field is a full uint16_t with no spare bit, widening it would grow every setting's descriptor, and list order is a presentation concern, which is what ui_type is for. The dropdown OK handler needs the same treatment, since it derives a uint setting's value from the entry index assuming the list was built ascending from offset_by. A descending combobox lists the maximum first, so every selection applied a value from the wrong end of the range - picking the largest value set one near the smallest. Step down from the maximum for that ui_type, and clamp to the minimum so a range whose span is not a whole multiple of the step cannot underrun.
Runs core audio through the WSOLA stretcher when the new mode is selected, so fast-forward and slow-motion keep their pitch instead of being chopped up or resampled to a higher one. Discard, Mute and Speed Up are untouched. The stretcher sits before the resampler, at the core's input rate; the resampler goes on doing rate conversion and dynamic rate control as it did. The stretch ratio is measured, not requested: an exponential average of frames arriving per flush, over frames the device says it wants. In steady state consumption must equal arrival, so that is the ratio, and it stays correct whatever speed the host actually achieves - which the requested fastforward_ratio does not. A fill-error trim steers ring occupancy toward its target rather than letting it drift to either end. Deliberately not audio_driver_fastforward_ratio_mult(), which the Speed Up path uses: that averages wall-clock intervals, this averages frame counts. Slow-motion goes through the same path with a ratio below 1.0, which expands rather than pitching down. Expansion is bounded by the output scratch, so beyond roughly 2.5x the audio stops slowing further while video continues to. The write_raw fast path has to decline a stretch-engaged flush: it takes core audio straight to the driver and has no hook for the stretcher, so without that clause Time-Stretch would do nothing at all on coreaudio (the macOS and iOS default) and sdl3_audio. Both also implement a normal write(), and the resamplers are allocated at init regardless, so falling through is safe. The rings are allocated on first use and freed at driver deinit, so a user who never selects this mode never pays the ~290KB. Allocation failure falls back to the discard path rather than to silence.
Wires audio_low_pass into the driver, so the "crunchy" edge left on fast-forward audio by the time-stretcher is taken off by a cutoff that scales inversely with the emulation speed. New audio_fastforward_lowpass setting, off by default: reference cutoff in Hz, divided by the current speed multiplier while fast-forwarding under Speed Up or Time-Stretch. The cutoff list reads better descending - its largest value is the mildest setting - so the row uses ST_UI_TYPE_UINT_COMBOBOX_DESC, with the per-setting left/right handlers swapped to match, and OFF is a sentinel above the top of the range, as the unmerged melonDS branch of this work does, rather than zero below the bottom of it. Zero would clamp to the minimum, which is the strongest filter rather than none, so a zero in a configuration file is read as off too. The range runs to 48000 Hz rather than 22000: the reference is divided by the emulation speed, so the lower ceiling could not produce a gentle result - at 3x it still gave a 7kHz cutoff on a 32768Hz core whose own bandwidth is 14.7kHz. The sublabel says how to choose a reference, since the useful range is not obvious from the numbers alone. The Qt companion UI renders the descending type through the same uint combobox, walking the range the other way up; the value rides along as the item's data rather than being derived from its index, so only the walk changes. The filter is applied after the stretcher in both the int16 and the float flush arm, on a buffer the driver owns rather than the core's own. audio_driver_time_stretch() no longer calls audio_driver_fastforward_ratio_mult() internally. That call has side effects and must run at most once per flush, so the caller now computes it once and shares it between the stretcher, the Speed Up ratio and the low-pass filter's speed.
Every artefact at a fast-forward engage came from the stretcher starting cold: the speed estimate had to converge, the ring had to prime, output had to hand over from the fallback path, and overlap-add had to ramp up from silence. Run it whenever the mode is selected instead. At normal speed the ratio is 1.0, which measures transparent, so a speed change becomes a ratio change, which is what WSOLA does smoothly. The flush still hands normal-speed audio straight to the resampler, so audio_driver_time_stretch_idle() feeds the ring without reading it; left unfed, every engage overlap-added onto stale audio from the last release. Toggling every 150ms: 7, 10 and 1 splices per run before, 0, 1 and 0 after. The ratio itself, traced across a transition: - Seed the interval average from the first interval; blending up from zero pinned the ratio at its clamp for ~90ms. Speed Up shares the fix. - Average frames as well as intervals, so the estimate is total time over total frames whatever the core's batch size. - Slew-limit the ratio and discard the first intervals after an edge; the burst when the frame limiter lifts read as 13.4x against 3x. - Bound the fill trim around the measured speed and pace emission on a credit against real time; the trim used to run the ratio ~25% low. - Mark a refused write as a discontinuity via audio_time_stretch_resync(), which keeps the buffered history. - Charge the overlap-add accumulator with one hop after a reposition, so the first hop does not ramp up from silence.
Opening the menu pauses the core, and the audio stream ended wherever the waveform happened to be: a step at both ends, heard as a click. Every frame has already gone to the device by the time the pause is known, so a plain ramp has only last_out to decay. Keep a copy of the last frames of output instead, find the period that best matches the last few milliseconds, and repeat it under a raised cosine, as a packet- loss concealer does. The period is scored by distance, since correlation is blind to level. Pause peak 654 -> 7 on the tone core; resume 1149 -> 11. The ramp hangs off retroarch_menu_running() and its finished counterpart, which the menu hotkey reaches where CMD_EVENT_MENU_TOGGLE does not. It reaches the driver ahead of what the resampler and the stretcher still hold, so pause_mute_frames drops those leftovers; the ramp back up is spent on the core's first frames, not on the menu's silence; and the state is cleared with the driver. A core with its own audio thread kept handing frames over after the tail, since menu_pause_libretro never sets RUNLOOP_FLAG_PAUSED. audio_driver_pause_fade() publishes core_silenced, which gates the core's input paths and only those, and holds the state lock for the write. The ramp out follows what the runloop will do, not the setting alone: under netplay the core keeps running behind the menu. The resume is not gated, since the setting can be turned off from inside the menu it paused; a resume with no pause behind it does nothing. The fast-forward handover is cross-faded at equal power over 256 frames rather than dipped through silence, the speed estimate is anchored at that edge, and the low-pass runs at every speed so it is never spliced in or out. All of it runs on the int16 path as well as the float one. write_raw drivers get the resume ramp and the mute but no tail: they hand the driver the core's buffer at the input rate, so there is nothing at the output rate to keep as history.
Loading a state splices the game's own audio, and the frontend does the splicing on top of a device that may already have run dry: the undo backup and the core's unserialize both run on the main thread inside the load task's callback, and on mupen64plus-next/SM64 they cost 27-45ms together, for a main-loop gap of 47-66ms against a default 64ms of device buffer. Two PulseAudio underruns and a splice is what that sounds like. Use the pause tail and the resume ramp the menu already uses, so the splice and whatever gap the stall leaves both land in silence. On the same content and tap, the worst |d2| against the local scale at a load went from 28-411 (median 226 and 45 over two runs) to 12-38 (median 17 and 28) - the remainder is at the level of SM64's own transients. Silencing the core across the load also shortens the stall it is covering, since the core's audio thread is no longer resampling against the same memory bus: the blocking work measured 4.6-27.0ms over ten loads before and 3.4-8.1ms after. The other three ways the frontend throws the game's audio away mid-waveform get the same bracket. A core reset re-initialises the machine, an undo pays the same two serialisations as the load it is undoing, and a load from the RAM slot is a load. All run on the thread that would otherwise be feeding the device. audio_driver_jump_fade_begin/_end name the bracket rather than repeating its invariant at four call sites - the stream may already be down, because the jump was made from the menu or while paused, and then the ramp back up has to stay with whatever took it down. core_reset() is bracketed at the CMD_EVENT_RESET site rather than inside it, so netplay's own reset path is untouched.
audio_driver_fastforward_ratio_mult() clamps its return to AUDIO_MIN_RATIO, which sizes the resampler's output buffers and says nothing about how fast the emulator can run. The stretcher synthesises up to AUDIO_STRETCH_MAX_RATIO, 32, but above 16x the estimate it was handed saturated: speed read a flat 16.00 with the core at 20x, the ring overran and audio_time_stretch_resync() spliced audibly. Only uncapped fast-forward gets there; fastforward_ratio = "0" swings 1.7x-19.9x inside one burst on a Steam Deck. Give the stretcher its own estimate, stretch_speed_mult, floored at 1/AUDIO_STRETCH_MAX_RATIO, and fix what seeing the real speed exposes: - trim_hi falls toward the measured speed as the ring empties, the mirror of the adaptive trim_lo; outrunning the core is affordable only with a reserve to spend. - The output credit accrues consumed_exact rather than (int)consumed; half a frame per flush is ~3% at 20x, and the credit paces the device. - The ratio slew may rise faster while the ring is above target, since an uncapped edge has no configured ratio to anchor to and the ring fills long before 8% a flush reaches 20x. Only the rise is relaxed. At ratio 0 on FF7, resyncs per run go from a median of 11 to 0.5 and the ring no longer fills; capped 3x is unchanged. stretch_speed_mult lives inside HAVE_AUDIO_TIMESTRETCH, so the estimate is recorded through a macro that stores when the stretcher is built and drops its argument otherwise; --disable-audio_timestretch still compiles.
audio_driver_ff_discard_bound() resamples only what the device can accept, but it was reached only while is_fastforward was set, and that is already false by the time the release is flushed. The device is still pinned from the burst, nothing bounds the write, and the non-blocking driver truncates it mid-waveform: some 57ms of audio dropped at every release, heard as a click on any content with sound. Gate on AUDIO_FLAG_NONBLOCK instead, which runloop.c holds for six frames after the release, exactly the window where the device is still full. The discard slack goes with it: the surplus is the part the driver truncates, and it bought nothing. Bounding the release still discards it, and in the recovery window runloop.c opens under VRR those frames are ordinary real-time audio. Hold them instead: a carry FIFO on audio_st leads the next flush, and dynamic rate control takes the latency back out as it already does for a device above its setpoint, over a few seconds at a pitch offset well under a cent. Gated on normal speed, a non-blocking driver, audio_sync and rate control being in circuit, since without DRC the stream would stay permanently behind; capped at 8192 frames, oldest first; and independent of the audio mode and the low-pass. The speed estimate counts frames as they arrive, before the carry can re-present them. With 200ms holds and 150ms gaps, clicks per run go from 13, 11, 17, 11 to 3, 1, 2, 1, with no release step above 0.05.
At a fast-forward edge the speed estimate is anchored to the configured fastforward_ratio rather than measured, because an anchored edge is known and the running average needs some thirty flushes to cross between speeds. The setting is a ceiling, though, not a promise. A heavy core on a handheld sits under it: melonDS DS capped at 3.0 runs at 1.7-2.1x on a Steam Deck. The estimate then starts every hold 50% wrong and spends the hold walking down to the truth. Traced per flush over twelve holds of 200ms, the synthesis ratio is 16-23% away from the measured speed, and a two-second hold does not finish converging either. Anchor to the speed the previous hold actually reached instead, keeping the configured ratio as the ceiling and as the fallback for the first hold after a core loads. The measurement is already there - avg_flush_delta is the observed flush interval, and its quotient with the 1x interval is the speed - so this is a matter of banking it at the release before the next anchor overwrites it. Uncapped fast-forward gains an anchor it never had. With ratio 0 there was nothing to anchor to, so the estimate began each hold at 1.0; the first hold still does, and every hold after it starts from the last one's speed. Worst ratio error per hold, melonDS on the Deck, twelve holds of 200ms: before 3.00 every hold, 16-23%, median 16.8% after 3.00 then 2.05, 1.77, 1.76 ..., 6.8-13.6%, median 11.1% The estimate also paces the stretcher's output, which is where this stops being only a matter of pitch: an overestimate hands the device that much less than real time and drains it. Underruns per 25s of the same content: audio_latency 64ms 82, 87 -> 11, 13 audio_latency 128ms 26, 25 -> 3, 4 FF7 is unchanged - clicks 2, 3, 2 against 2, 1, 3, no release step above 0.05 in 60 edges either way, median step 0.011 both - which is the case worth checking, since uncapped fast-forward had no anchor before this. What remains is the speed genuinely varying inside a hold, 1.65-2.05 over one session, which the average follows over some thirty flushes - most of a 200ms hold. That is a question of how fast the estimate converges, not of where it starts.
audio_driver_time_stretch() paces its output on a credit that pays out real time as estimated - stretch_arrival_avg times sp_mult per flush. Nothing feeds the device's occupancy back into that. The room bound says how full the device may get, never how empty it may run, so whatever the estimate and the device's true drain disagree by integrates into the occupancy, and the equilibrium is wherever the drift stops: empty. Traced against PipeWire on a 3x hold. Output settled at 47.8-47.9k frames/s against a 48k drain. PipeWire pulls 2048 frames at a time from the 3072-frame buffer, so occupancy is a sawtooth of that amplitude, and its base slid down to 0-250 frames within a few seconds of the hold and stayed there; from then on every pull that landed wrong came up short and the process callback filled silence - 492 short pulls and 6854 frames of silence in one 60 s hold, counted in pwire_playback_process_cb(). It builds over the hold, which is why holding is worse than tapping. The hole after a release is the same mechanism: the hold leaves the device empty and the first pull after the release drains it, 500-850 frames of silence within 300 ms of every release. Close the loop. Allow output beyond the credit in proportion to how far the device sits below a setpoint of 60% of its buffer. The occupancy is averaged over the arrival window, since a single reading is mostly the driver's pull sawtooth; the gain is per unit of time, with a 200 ms constant, so it converges the same at any speed; and the extra is capped at 5% of one flush's real-time output, which is the surplus the ring-fill trim can supply without draining the ring. The credit is floored at zero after a read so the correction is not paid back. 60% puts the sawtooth's base near 27% here and its top near 93%, so the device neither empties nor saturates - a saturated device makes the reads bunch up behind it, which the ring trim turns into ratio wobble. Same scripted holds, FF7 on swanstation, driver-reported underruns and fabricated silence frames, before and after: 3x, 60 s hold 492 / 6854 -> 0 / 0 8x, 12 s 102 / 4952 -> 0 / 0 uncapped (~25x), 12 s 10 / 378 -> 0 / 0 32 ms latency, 15 s 124 / 938 -> 0 / 0 pulse driver, 15 s 0 / 0 -> 0 / 0, base 313 -> 980 frames sine core, 2 x 12 s 74, 76 -> 0, 0 release, silence / 300 ms 500-850 -> 0 tapping 200/150 ms x20 798 -> 0 Output rate lands on 48000 exactly, the occupancy median rises from ~1250 to ~1850 frames, and the join-step distribution and ratio jitter are unchanged on every row. Only write_avail() and buffer_size are used, which rate control already relies on, so nothing here is driver-specific.
Between the last write of a pause and the first write of resumed core audio the runloop hands nothing over. With the menu that is ~34 ms: it feeds silence at frame rate while it is open, so the device sits healthy until the close, then drains through the hole. Rate control is what refills it afterwards, at 0.5% of real time, so the base of the driver's pull sawtooth sits near zero for seconds. Traced against PipeWire, which pulls 2048 frames at a time from a 3072-frame buffer, every menu close was followed by a short pull every ~85 ms - 4-8 short pulls and 400-680 frames of fabricated silence in the first 600 ms, on each of 15 closes, rapid or single. Tapping the menu key makes it continuous, which is how it was heard: a crackle that survived the fast-forward pacing fix. Have audio_driver_pause_fade(false) arm a flag, and let the first audio_driver_flush() after it fill the device with silence before writing. Silence is free - the device has been playing it - and the surplus occupancy is what rate control already takes out. It has to be the first flush rather than the resume itself: a top-up at the resume is undone by a pull landing inside the hole. The write is chunked so it never exceeds a driver's per-write limit and bounded by write_avail(), so a blocking driver cannot wait on it. Same scripted toggles through the menu hotkey path, short pulls / silence frames per resume: rapid, 300 ms open, 11 closes 2-8 / 400-680 -> 0 / 0 single, 1.5 s open, 4 closes 4-8 / 470-570 -> 0-2 / 0-74 What is left on a single close is normal play's own margin. Save-state loads and the pause hotkey resume through the same call and inherit this.
Under the threaded pipeline every fast-forward engage dropped the batches produced between the edge and the consumer's next pass: the ring is full at normal speed by design, and the producer stopped waiting the moment the flag flipped. The consumer then paced itself on room for the unstretched chunk, so nothing above the emulation speed fit through the ring. The wrapper also ignored the non-blocking state, leaving the device write blocking during fast-forward and with audio sync off. The producer now waits for a consumer pass at any speed, the consumer skips the device-room wait while fast-forward is held, and the wrapper applies the requested state to the wrapped driver from the audio thread. Cores with their own audio callback keep blocking writes.
HAVE_AUDIO_TIMESTRETCH and HAVE_AUDIO_LOWPASS gated on an explicit 1, so every platform Makefile had to restate the default. Gate on "not 0" instead, following the HAVE_BUILTINSPIRV_CROSS block in the same file, and keep only the platforms that opt out. 3DS, Vita, Wii, Wii U and GameCube decline the stretcher; lfx000 and miyoo decline both, having no FPU. openpandora and the PS2 griffin build keep explicit defines because neither reaches Makefile.common.
…ed pipeline The threaded producer only measured the speed multiplier in Speedup mode, so with the time-stretcher selected the consumer's flush read the initial 1.0. The stretcher then paced its output at the core's rate during fast-forward, the non-blocking write truncated the surplus, and every driver crackled for the length of the hold. Measure whenever the stretcher is selected, at every speed, as the inline flush already does, and reset at normal speed the same way.
…iver The pause tail is written from the main thread, and through the thread wrapper it reached a driver parked by CMD_EVENT_AUDIO_STOP. WASAPI answers a write to a stopped device with -1, which the wrapper took as its own thread having failed: it freed the driver while the producer went on calling write_avail on the freed context. The wrapper now refuses writes while parked, only ends its thread on a failed write of its own, and clears the driver context once it has freed it. The wrapper also applied a requested blocking state only at the top of its loop, but the thread spends nearly all its time inside the pipeline pass below that. At a fast-forward release the flush ran with the flag clear, so no discard bound and no carry, through a driver still non-blocking into a device the burst had left full: a click at every release under Threaded Audio. The state is applied on the audio thread right before its write as well. With audio sync off the driver is non-blocking for good, and the producer, waiting for a consumer pass whenever the ring was full, was paced by the consumer waiting for the device to drain a chunk: at 75 Hz vsync with 60 fps content the ring filled within a second and the frontend was held to 60 fps, judder on every frame, while the runloop believed audio was not pacing it. At normal speed with audio sync off the producer now drops at a full ring, as it did before the pipeline was kept non-blocking through fast-forward and as the inline path drops at the device. The consumer keeps waiting for device room, so a device smaller than a publish is fed as it drains rather than overrun. AUDIO_FLAG_NONBLOCK was set whether or not the state reached the driver. A core with its own audio callback keeps its wrapper blocking on purpose, so with audio sync off every flush ran the discard bound against a driver that would have taken the whole write, and audio that master delivered in full was thrown away. The flag is recorded only when the driver is handed the state, and frames the bound drops at normal speed are counted as offered and refused so the warning about a device smaller than a frame's worth can fire again.
While fast-forward has the device pinned full the driver is non-blocking, so the 512-frame tail written at a pause from there kept only what had drained since the last flush and cut the ramp at a gain of 0.5 or more, on the loudest content. The ramp now spans the room the device reports, and a pause cancels a cross-fade still in flight, which would otherwise re-inject its far side ahead of the ramp back up. Three paths changed the core's state without the bracket. CMD_EVENT_AUDIO_REINIT inside the paused menu tore down the record that the core is held, so the close that followed came up unramped; the handler re-publishes the pause after the drivers come back. The pause hotkey pressed twice in the menu spent the resume ramp on menu silence; that branch now leaves the ramp to the menu's close. The paused rewind step and the BSV checkpoint seek flipped RUNLOOP_FLAG_PAUSED directly and left stale rewound audio in the tail history; both go through audio_driver_pause_fade. A netplay session ending or starting behind the menu likewise stopped or resumed the core with no tail or ramp, and now goes through the same call. Under the threaded pipeline the ring behind the tail still held up to three frames of the core's audio, and the consumer dropped ring content only on the runloop's own pause flag, which neither the menu nor a state load sets. The stale frames were flushed behind the tail at full level past the mute, and a state load resumed before the consumer had flushed anything, so the whole ring played through the resume ramp. The producer and consumer now count the bytes they put into and take out of the ring, the pause records where it fell in that stream, and the consumer skips everything up to that point. The resume ramp was armed at the resume and spent by whatever flush came next. Under the threaded pipeline that was the menu's last frame of silence when the menu was closed from inside it, and the core's audio came back at full level. The resume now records that a ramp is owed, the producer marks where the first core audio after it starts, and the consumer never takes a chunk across that point and arms the ramp on reaching it.
The 256-frame cross-fade at a fast-forward edge hides the stretcher's reserve changing hands, but it was armed in every audio mode, so Discard and Speed Up put a notch in a continuous waveform at each edge, and a pending fade took write_raw drivers (coreaudio, sdl3_audio) through the frontend resampler for one flush per edge. It is armed only with the stretcher engaged. The stretcher emits only what the device reports room for, so on a driver without write_avail (PS2, or the thread wrapper over one) a Time-Stretch hold played silence. The mode is declined there and the flush falls back to the discard path. Both flush arms consumed the head of the release carry before the resampler ran, and the resampler's input still pointed into the carry whenever the low-pass had not copied it out, so every bounded flush of the post-release window under VRR resampled the wrong frames. The carry is consumed after the read, and frames still deferred at a re-engage are dropped, since the idle feed already wrote them into the stretcher's ring. In Speed Up the estimate resets on every normal-speed flush, so the engage flush reached the estimator with no interval, reseeded the average to 1x over the anchor the edge had just written, and returned 1.0, sending that flush to the non-blocking write at the core's full rate. The anchor is kept and answered from, and it and its ceiling come from runloop_get_fastforward_ratio, which honours a core's RETRO_ENVIRONMENT_SET_FASTFORWARDING_OVERRIDE. Slow-motion is paced by the audio write blocking on the extra output the slowed ratio produces. With Fast-Forward Audio set to Time-Stretch the stretcher emitted only what the device had room for, the write returned at once and the core ran at full speed with "Slow-Motion" on screen. In slow-motion the stretcher now runs at the setting's ratio, emits the whole slowed output and leaves the device's occupancy loop out of it, so the write waits on the device as it does without it. On the threaded pipeline the producer measured the flush interval on the main thread with no lock, while the consumer anchored the same estimate under the state lock at every edge, and the producer's reseed landed before the consumer saw the release, so the speed a hold reached was read as 1x and never banked. On 32-bit targets a torn read of the double multiplier could also turn the output credit to NaN. The estimator's side of the edge now runs on whichever thread measures, ahead of the reset, and the producer publishes the whole estimate the stretcher needs through the atomics that already carried its multiplier.
The migration from the audio_fastforward_mute and audio_fastforward_speedup bools only ever set MUTE or SPEEDUP, never DISCARD, so a negating core override left the mode at MUTE, and a positive override was written into the main config at exit by config_save_file. The mode is derived completely from the old bools whenever either is present, mute winning, then speedup, otherwise DISCARD, and logged as the quit_press_twice migration is. Makefile.common enables the stretcher and the low-pass unless a platform sets their flags to 0. The RS-90's JZ4725B has no FPU and the PS2's R5900 emulates doubles in software, so a Time-Stretch hold ran the WSOLA search tens of times slower than real time and stalled emulation. Both opt out as Miyoo and LFX000 already do. The descending combobox's list builder falls back to 9999 for a setting without an enforced maximum, but the selection handler fell back to 0 and would have stored the minimum for every row. Latent: the one descending setting enforces its maximum. A stale audio_fastforward_mode value in a build without the stretcher left its menu row blank. A default case labels it Discard, which is how audio_driver_flush already treats it.
… code Mostly no behaviour change, with the one exception noted at the end. audio_time_stretch.c and audio_low_pass.c drop their private clamp and max helpers for MAX/MIN, and their tests share one goertzel_mag in test_goertzel.h. In audio_driver.c the device's frame byte size, the exponential moving average and the pause write's saturating int16 conversion each go through one helper instead of being written out at every site, two guards that could never be false are dropped from audio_driver_time_stretch, and comments that described what the code used to do are cut. Two loops lose per-iteration work. The WSOLA similarity search copies its reference window into a linear buffer once per search instead of re-reading it through the ring's masked index for every candidate, and the pause tail's period search walks the history ring with a decrement-and-wrap step instead of a modulo per frame. The sums stay in the same order, so the results are unchanged. The exception, and it follows from having the reference window in hand: audio_stretch_find_best_offset() now scores its seed. It seeded best_k with the nominal offset but left best_score at -1e30 without ever scoring it, so the first coarse candidate won every tie. On digital silence the score is 0 for every candidate, the seed included, and best_k therefore slid to the bottom of the search window - leaving natural_pos, the reference the next search matches against, a further AUDIO_STRETCH_SEARCH_RADIUS frames behind analysis_pos than it should be. Real content is unaffected: candidates score differently there, so the seed loses on merit as before, and the chosen offsets measure identical on a sine at 0.5x, 1x and 3x.
90f866b to
b04aead
Compare
|
Hi, will use this PR to come up with a new PR on a branch and try to make it fit RetroArch's mold and system in the way I think it could fit. I'd ask that you keep this freezed for now while I make that happen |
Sounds good! Let me know if you need anything from me. A lot of the small details are directly as a result of testing and getting crackling sounds in edge cases when pausing or pressing and releasing speed up, so I hope we can make this work in your rewrite. |
|
It should be implemented now on master. Check it out and give me your feedback |
Guidelines
C89_BUILD=1Description
The main goal of this PR is to solve issues with the current 2 main options when changing gameplay speed (mostly fast forward). Increasing the pitch obviously distorts the sound a lot, and discarding samples adds a lot of harsh crackling.
The two main additions this PR makes are a WSOLA resampling filter, which is an existing algorithm to resample the audio to keep the pitch the same despite being played back faster. This is most of what's required on most systems. The other thing is the Butterworth low-pass filter, which helps cut the edge on some high pitched, short sounds that can still persist, especially on older systems with more simple audio, like GB or DS.
Testing
I tested this PR many cores and games on a Linux desktop, Steam Deck, MacOS, Windows (the same desktop as Linux, just dual booted), and in an Android emulator on Linux, (not on real hardware on Android). I found small issues on all of these platforms, which is partially why the code changes are so extensive. The cores/games I most extensively tested were:
FCEUmm - SMB1
mGBA - Pokemon Emerald
MelonDS DS - Pokemon Black
Azahar - Pokemon Alpha Sapphire
Mupen64 - Ocarina of Time
SwanStation - Final Fantasy VII
I tested more games and cores here and there, but these were some of the games and cores that showed the most obvious issues after first passes at the code.
Separable Sections
This PR can be split into separate reviewable sections, and some commits/decisions can easily be dropped:
2a7cc15 audio: end the stream on a ramp, and bring it back on one
26cf657 audio: bracket the frontend's state jumps with the pause fade
1a09da3 ceiling
796bf6f deferred release
4af5628 speed anchor
8866ef6 occupancy steering
cf4e517 resume refill
050b320 non-blocking threaded pipeline
Right now the descending combobox change is not easily separable, but I can rip it out. The low-pass filter setting that most people will set is either OFF or somewhere between 40k-48k, depending on the game/core they're playing, and it was annoying me that by default the list was ascending and was hard to pick. This is just UI and has nothing to do with the actual audio work.
Technical Details
(Partially LLM written for thoroughness)
Inside
audio_driver_flush(), on both the float and the deterministic int16 arms:Both new filters run on interleaved s16 before the resampler. That is the difference from Speed Up: Speed Up scales
src_data.ratio, which is what shifts the pitch, while Time-Stretch leaves the resampler ratio alone and changes the frame count ahead of it.write_rawhas no hook for either filter, so its gate declines a flush while the stretcher or the low-pass is engaged and that flush takes the normal path.WSOLA time-stretcher (
libretro-common/audio/audio_time_stretch.c)The ratio is not simply the speed multiplier.
audio_time_stretch_ratio()takes measured input arrival per flush, the device's output demand and the current ring fill, and returns the ratio at which consumption matches arrival. A trim term then steers ring occupancy: gain 0.25, bounded to [0.95, 1.10], with a floor that widens to 0.75 while the ring is cold so a fresh start fills in a few flushes rather than a few hundred. Ratio movement is slew-limited to 1.08x per flush, because the measured speed overshoots badly the moment the frame limiter lifts and following it drains the ring.At a fast-forward edge the estimate is anchored rather than measured — the running average otherwise needs ~30 flushes to cross between speeds and is audibly wrong for all of them. The anchor is the speed the host actually reached on the previous hold, bounded by the configured ratio, so a core the machine cannot run at the configured speed does not spend the hold walking the estimate down to the truth.
Low-pass filter (
libretro-common/audio/audio_low_pass.c)Settings and config migration
audio_fastforward_muteandaudio_fastforward_speedupare replaced by a singleaudio_fastforward_mode(enum fastforward_audio_mode: Discard, Mute, Speed Up, Time-Stretch). Discard is 0, so an unmigrated configuration keeps the behaviour it had, and the default is unchanged for everyone.Migration runs in
config_load_file()and only whenaudio_fastforward_modeis absent, so an explicit new key always wins;config_save_file()then unsets the two dead keys.audio_fastforward_lowpassis a reference cutoff in Hz whose OFF value is a sentinel at the top of the range (AUDIO_FASTFORWARD_LOWPASS_OFF, 49000) rather than 0, so the descending list shows OFF first; a 0 read from an older config maps to the sentinel. The range runs past any core's Nyquist on purpose, since the reference is divided by speed and the mild end needs headroom.FASTFORWARD_AUDIO_MAXexcludes Time-Stretch when it is compiled out, so the menu row never offers a mode that would silently fall back to Discard.Build flags
HAVE_AUDIO_TIMESTRETCHandHAVE_AUDIO_LOWPASS, both defaulting to yes inqb/config.params.sh. Per-platform opt-outs:That is what accounts for most of the file count: 23 platform Makefiles, the MSVC/Xcode project files and
griffin.call just list the two new objects.Tests
Three suites under
libretro-common/audio/test/, wired intoMakefile.testand run bymake -f Makefile.test:test_time_stretch— the stretcher end to end.test_time_stretch_ratio— the ratio and trim control loop in isolation.test_low_pass— coefficient design, smoothing and bypass.They link
-lmexplicitly: both new files call libm, andLIBCHECK_LIBSonly happens to carry it on distributions where libcheck itself depends on it.Licensing
The WSOLA and low-pass implementations are my own, written first for melonDS and then ported to mGBA and Azahar. I haven't opened PRs yet for mGBA and Azahar, those are still a WIP. Those branches carry each project's per-file licence header by that project's convention; none has been merged upstream, and the code is relicensed here by me for inclusion in libretro-common under its usual terms. The provenance note is repeated at the top of both new headers. I am happy to adjust licensing details or provide more info about this if needed.
Known gaps
write_rawdrivers get the resume ramp but not the pause tail.coreaudioandsdl3_audioare the only two drivers with a non-NULLwrite_raw; that path hands the driver the core's own buffer at the input rate and the driver resamples, so there is nothing at output rate to keep as history to ramp from. The gate already declines a stretch- or low-pass-engaged flush, so the uncovered case is narrow: coreaudio or sdl3_audio, an int16 core, no DSP filter, no mixer voice, and mode Discard or Mute (or Speed Up with the low-pass off). Falling through to the normal path once a ramp is armed does not fix it — the tail needs history from before the pause, and a late fall-through leaves the search with nothing, which is the DC ramp the tail work exists to replace.The fast-forward edge dip is compiled out where the stretcher is. The edge detection itself is outside
#ifdef HAVE_AUDIO_TIMESTRETCH, but thefade_out_framesarming inside it is not, so the seven targets above get no dip under Speed Up — where the resampler ratio still steps abruptly at the edge. It splits cleanly (arming the dip needs onlylast_outandfade_out_frames, and the consumer already runs unguarded), and the cost would be 256 frames ofcosf/sinfper speed change. I have left it alone because I cannot test those seven targets; happy to add it if a maintainer with the hardware wants it.Related Issues
[Any issues this pull request may be addressing]
Related Pull Requests
I've already opened a PR in the melonDS standalone repo which is where I first experimented with this, and after porting it to the melonds-ds repo I realized it's probably better to solve this more generally in RetroArch.
Reviewers
[If possible @mention all the people that should review your pull request]