Fix garbled playback: sample loops, pitch, mixing, and sample offsets #1

Merged
yeold merged 3 commits from yeold/wip into main 2026-09-02 15:27:10 +00:00 AGit
Owner

Six independent defects behind the garbled audio: non-looping samples buzzing on two bytes (word-vs-byte repeat length), everything an octave sharp (missing x2 in the Paula period formula), ~84% of samples hard-clipping (4 channels summed into a +-127 range), wrong sample offsets on modules storing unplayed patterns, loop wrap points, and unclamped portamento. Also: 64KB position wrapping, unapplied finetune, audio-clock sequencing instead of a jittery timer, a dropped-DMA-refill race, an output volume control, and on-screen load-failure reasons. Verified against openmpt123 across all 8 four-channel modules (centroid ratio 1.00 throughout, song length within 0.04%) and on hardware with zero buffer underruns. See the commit messages for the full breakdown.

Six independent defects behind the garbled audio: non-looping samples buzzing on two bytes (word-vs-byte repeat length), everything an octave sharp (missing x2 in the Paula period formula), ~84% of samples hard-clipping (4 channels summed into a +-127 range), wrong sample offsets on modules storing unplayed patterns, loop wrap points, and unclamped portamento. Also: 64KB position wrapping, unapplied finetune, audio-clock sequencing instead of a jittery timer, a dropped-DMA-refill race, an output volume control, and on-screen load-failure reasons. Verified against openmpt123 across all 8 four-channel modules (centroid ratio 1.00 throughout, song length within 0.04%) and on hardware with zero buffer underruns. See the commit messages for the full breakdown.
Fixes furi_check crash on launch: furi_hal_speaker_acquire() already
enables TIM16's bus, so audio_pwm_dac.c must not enable it again.

Reworks mod_parser.c/mixer.c to stream sample PCM from the SD card
during playback instead of loading it into RAM, since real MOD files
(e.g. a 293KB file tested) exceed the Flipper's ~130KB free heap.
Adds a per-channel read-ahead cache, bumps the audio ring buffer and
worker thread stack accordingly, and fixes a display line that was
being clipped off-screen.

Known issue: audio still sounds garbled during playback (tempo is
correct). Root cause not yet found - suspect either a bug in the
streaming/cache path in mixer.c or a real-time SD-read latency issue
underrunning the audio buffer. Temporary diagnostic instrumentation
(mixer_get_stats/reset_stats, the SD-card debug log in
tick_timer_callback) is included and should be removed once resolved.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TNcczMGJiSP9Lk85F7Mtcj
Playback was garbled for several independent reasons. All were found by
rendering modules on the host and comparing against a reference player
(openmpt123), which the streaming design made possible to do off-device
by stubbing the storage API.

- Non-looping samples were played as a two-byte loop. MOD stores repeat
  length in words, and a stored length of 1 word is the conventional
  "not looped" marker; the parser doubles it to bytes, so the mixer's
  `loop_length > 1` test caught every non-looping instrument and buzzed
  on its first two bytes. This was the largest single defect: it moved
  the rendered spectral centroid from 0.38x the reference's to 1.00x.

- Everything played exactly one octave sharp. Paula's period-to-rate
  conversion is clock / (2 * period), and the factor of two was missing.

- Four channels each spanning +-127 were summed into a range clamped at
  +-127, hard-clipping ~84% of output samples. The mixer now renders
  signed 16-bit, where four full-scale channels sum to exactly full
  scale, and the PWM driver does the single final requantisation.

- Looped samples wrapped at the end of the sample data rather than at
  loop_start + loop_length.

- Portamento was unclamped, letting periods walk to 0 or five figures.

- Sample position was Q16.16 packed in one uint32, so it wrapped on
  samples over 64KB. It is now a separate frame index and fraction.

- Per-instrument finetune was parsed but never applied.

The sequencer now runs off the audio clock (mixer_render advances ticks
as it renders) instead of a wall-clock timer that could block for a whole
buffer on the mixer's mutex. Tempo is sample-accurate: total song length
matches openmpt123 to 0.04% over four minutes.

In the audio driver: a lost-refill race where both DMA interrupts
arriving before the worker was scheduled left one half replaying stale
audio; a worker stack too small for its own scratch buffer; a missing
guard so stop() after a failed start joined a null thread; and TIM16's
bus enable now checks furi_hal_bus_is_enabled(), so it works both on
firmware that enables it inside furi_hal_speaker_acquire() and on
firmware that does not, disabling only what it enabled.

Adds an output volume control (left/right, nine steps of 0.5x, default
2x), since MOD material sits near -20 dBFS RMS and is quiet at unity.

Verified on hardware (firmware 1.4.3): zero buffer underruns, with ~18 SD
refills/sec averaging under 2ms against the 64ms each half-buffer allows.
That settles the streaming read-latency question this branch left open,
so the temporary instrumentation is removed; the driver's underrun
counter stays, and now surfaces on screen if it is ever non-zero.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bw6fFXMfaUw9bzQiX3zf3U
Modules routinely store patterns beyond the range the order table
actually plays — leftovers from composing. The loader sized the sample
data region from the highest pattern index in the first song_length
order entries, but the file stores patterns up to the highest index in
the whole 128-byte table, and sample PCM begins after all of them.

When those differ, every sample's file offset lands early by the
difference and each instrument plays a neighbouring sample's bytes,
while the sequencer's timing stays perfectly correct — audible as a
module that is in time but playing the wrong sounds throughout. Of the
modules to hand, cream_of_the_earth.mod was off by 7168 bytes (7
patterns) and preludesynth.mod by 5120 (5).

Only the patterns playback can reach are loaded into RAM, as before; the
full stored count is used solely to locate the sample area, so this costs
no extra memory.

Checked against openmpt123 across every 4-channel module available, as
envelope correlation and spectral centroid ratio:

  apoplexy      0.995 / 0.999      cream_of_the_earth  0.999 / 1.000
  astraltr      0.899 / 1.003      gluppobe            0.948 / 1.003
  cartoon...    0.984 / 0.992      spacedeb            0.999 / 0.999
  chiptune      0.997 / 0.999      waterfal            0.982 / 0.998

cream_of_the_earth was 0.627 / 1.113 before this change.

Also reports why a load failed, on screen rather than only to a log the
device can't show: a 6-, 8- or 28-channel MOD was indistinguishable from
a corrupt file, both just leaving "No module loaded".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bw6fFXMfaUw9bzQiX3zf3U
yeold merged commit 78aec24979 into main 2026-09-02 15:27:10 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
yeold/flipper-mod-player!1
No description provided.