Another pass at Auto CPU and Audio Rate Control #59
Conversation
Replace linear rate control with a PI controller that: - Smooths error signal (0.9) to filter jitter from proportional term - Uses quadratic integral weighting for faster convergence far from 50% - Clamps integral to ±1% for persistent hardware drift correction Parameters: d=0.5%, ki=0.00005, smooth=0.9, buffer=4 frames.
There was a problem hiding this comment.
Pull request overview
This PR refines auto CPU scaling and audio rate control for better performance and stability on handheld devices. The changes replace the rate meter system with a dual-timescale PI controller for audio synchronization, improve auto CPU frequency selection with smarter boost/reduce algorithms, and update documentation to reflect the new approach.
Key Changes
- Audio Rate Control: Replaced rate meter measurement system with a dual-timescale PI controller that separates proportional (fast jitter response) from integral (slow drift correction) on different timescales to prevent oscillation
- Auto CPU Scaling: Enhanced granular frequency scaling with minimum frequency filtering (400 MHz), aggressive boost with no step limit, conservative reduction with 2-step max, and extended startup grace period (5 seconds at max frequency)
- Documentation Updates: Revised algorithm descriptions, parameters, and tuning guidance to reflect the PI controller approach (though multiple inconsistencies remain between docs and code)
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| workspace/miyoomini/platform/platform.c | Updated sample rate policy to respect core's requested rate instead of forcing max |
| workspace/all/minarch/minarch.c | Removed rate meter, added SND_newFrame() call, refined auto CPU logic with new constants, improved panic handling, fixed vsync cadence maintenance |
| workspace/all/common/rate_meter.h | Removed entire rate meter header (replaced by PI controller) |
| workspace/all/common/rate_meter.c | Removed entire rate meter implementation |
| workspace/all/common/audio_resampler.h | Removed SND_RESAMPLER_MAX_DEVIATION safety clamp documentation |
| workspace/all/common/audio_resampler.c | Removed safety clamp implementation (no longer needed with PI controller) |
| workspace/all/common/api.h | Updated SND_Snapshot structure with PI controller state, replaced rate meter functions with SND_newFrame() |
| workspace/all/common/api.c | Implemented dual-timescale PI controller with per-frame integral updates, removed rate meter code and swing-based buffer sizing, simplified to fixed 5-frame buffer |
| tests/unit/all/common/test_rate_meter.c | Removed rate meter unit tests |
| makefile.qa | Removed rate_meter_test from test executables |
| docs/auto-cpu-scaling.md | Updated documentation with PI controller parameters and auto CPU tuning details (multiple parameter mismatches with code) |
| docs/audio-rate-control.md | Rewrote algorithm documentation to describe PI controller approach, simplified from dual correction to unified control |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
docs/auto-cpu-scaling.md
Outdated
| // Rate control gains (api.c) | ||
| #define SND_RATE_CONTROL_D 0.005f // 0.5% - proportional gain | ||
| #define SND_RATE_CONTROL_KI 0.00005f // integral gain (drift correction) | ||
| #define SND_RATE_CONTROL_SMOOTH 0.9f // error smoothing factor |
There was a problem hiding this comment.
The constant name and value are incorrect. The code uses SND_ERROR_AVG_ALPHA with value 0.003f, but the documentation shows SND_RATE_CONTROL_SMOOTH with value 0.9f. These should be updated to match the actual implementation.
docs/auto-cpu-scaling.md
Outdated
| |-----------|---------|-------| | ||
| | Rate control d | 0.5% | Handles timing jitter (paper's recommendation) | | ||
| | Base correction | SDL refresh_rate / core fps | Static display/core mismatch correction | | ||
| | Rate control d | 0.5% | Proportional gain (tune per device class) | |
There was a problem hiding this comment.
The documented value for Rate control d is inconsistent. It shows "0.5%" here but the actual code value is 1.0% (0.010f). This should be updated to "1.0%" to match the implementation.
| | Rate control d | 0.5% | Proportional gain (tune per device class) | | |
| | Rate control d | 1.0% | Proportional gain (tune per device class) | |
docs/auto-cpu-scaling.md
Outdated
| | Base correction | SDL refresh_rate / core fps | Static display/core mismatch correction | | ||
| | Rate control d | 0.5% | Proportional gain (tune per device class) | | ||
| | Rate control ki | 0.00005 | Integral gain (tune globally) | | ||
| | Error smoothing | 0.9 | Filters jitter from P term (fixed) | |
There was a problem hiding this comment.
The error smoothing value is incorrect. It shows "0.9" but the actual code uses SND_ERROR_AVG_ALPHA with value 0.003f. This should be updated to "0.003 (~333 frame average)" to match the implementation.
| | Error smoothing | 0.9 | Filters jitter from P term (fixed) | | |
| | Error smoothing | 0.003 (~333 frame average) | Filters jitter from P term (fixed) | |
docs/auto-cpu-scaling.md
Outdated
| | Rate control d | 0.5% | Proportional gain (tune per device class) | | ||
| | Rate control ki | 0.00005 | Integral gain (tune globally) | | ||
| | Error smoothing | 0.9 | Filters jitter from P term (fixed) | | ||
| | Integral clamp | ±1% | Max drift correction (fixed) | |
There was a problem hiding this comment.
The integral clamp value is incorrect. It shows "±1%" but the actual code uses SND_INTEGRAL_CLAMP with value 0.02f (±2%). This should be updated to "±2%" to match the implementation.
| | Integral clamp | ±1% | Max drift correction (fixed) | | |
| | Integral clamp | ±2% | Max drift correction (fixed) | |
docs/auto-cpu-scaling.md
Outdated
| | Error smoothing | 0.9 | Filters jitter from P term (fixed) | | ||
| | Integral clamp | ±1% | Max drift correction (fixed) | | ||
| | Resampler safety clamp | 5% | Prevents extreme pitch shifts from bugs | | ||
| | Audio buffer | 4 frames (~67ms) | Effective latency ~33ms at 50% equilibrium | |
There was a problem hiding this comment.
The audio buffer size is incorrect. It shows "4 frames (~67ms)" but the code was updated to use SND_BUFFER_FRAMES = 5 (~83ms at 60fps). This should be updated to "5 frames (~83ms)" and the latency calculation should be updated accordingly (~42ms at 50% equilibrium).
| | Audio buffer | 4 frames (~67ms) | Effective latency ~33ms at 50% equilibrium | | |
| | Audio buffer | 5 frames (~83ms) | Effective latency ~42ms at 50% equilibrium | |
docs/auto-cpu-scaling.md
Outdated
| // Resampler safety clamp (audio_resampler.h) | ||
| #define SND_RESAMPLER_MAX_DEVIATION 0.05f // ±5% absolute maximum (catches bugs) |
There was a problem hiding this comment.
The resampler safety clamp (SND_RESAMPLER_MAX_DEVIATION) was removed in this PR (see audio_resampler.h/c changes), but this documentation still references it. This section should be removed or updated to reflect that the safety clamp no longer exists.
| // Resampler safety clamp (audio_resampler.h) | |
| #define SND_RESAMPLER_MAX_DEVIATION 0.05f // ±5% absolute maximum (catches bugs) |
docs/auto-cpu-scaling.md
Outdated
| | Rate control ki | 0.00005 | Integral gain (tune globally) | | ||
| | Error smoothing | 0.9 | Filters jitter from P term (fixed) | | ||
| | Integral clamp | ±1% | Max drift correction (fixed) | | ||
| | Resampler safety clamp | 5% | Prevents extreme pitch shifts from bugs | |
There was a problem hiding this comment.
The resampler safety clamp (SND_RESAMPLER_MAX_DEVIATION) was removed in this PR (see audio_resampler.h/c changes), but the parameters table still references it. This line should be removed.
| | Resampler safety clamp | 5% | Prevents extreme pitch shifts from bugs | |
docs/auto-cpu-scaling.md
Outdated
| // Base correction (api.c, calculated at init from SDL) | ||
| float base_correction = display_hz / core_fps; // e.g., 0.9935 for -0.65% mismatch | ||
| // Rate control gains (api.c) | ||
| #define SND_RATE_CONTROL_D 0.005f // 0.5% - proportional gain |
There was a problem hiding this comment.
The documented value for SND_RATE_CONTROL_D is incorrect. The code defines it as 0.010f (1.0%), but the documentation shows 0.005f (0.5%). This should be updated to match the actual implementation.
| #define SND_RATE_CONTROL_D 0.005f // 0.5% - proportional gain | |
| #define SND_RATE_CONTROL_D 0.010f // 1.0% - proportional gain |
- Tune parameters for handheld timing variance (d=0.010, 5-frame buffer) - Add SND_newFrame() to update integral once per frame, preventing over-accumulation with per-sample audio callbacks - Fix miyoomini sample rate policy to respect core's native rate - Fix vsync skip bug: always flip even when core skips rendering
nchapman
force-pushed
the
feature/autocpu-refinements
branch
from
9783358 to
4f88319
Compare
Should be even better now!