Skip to content

fix: clipped audio and leaked files on the device - #3

Merged
kolebayev merged 2 commits into
devfrom
kolebayev/converter-fixes
Aug 21, 2026
Merged

kolebayev merged 2 commits into
devfrom
kolebayev/converter-fixes

Conversation

@kolebayev

Copy link
Copy Markdown
Owner

Summary

Two defects that both cost the user real audio on the device. A lossy
conversion could decode back above full scale and clip on playback, and
removing a track left its file on the iPod for good — a 74 GB device had
2 GB of audio no database record pointed at.

Changes

  • convert.rs: after a lossy encode, measure what the file actually
    decodes to and re-encode it attenuated when it lands above 0 dBFS.
    Nothing about the source predicts the overshoot — the master that
    prompted this peaks at −0.18 dBFS, has a true peak of −0.05 dBTP, and
    still comes back out of AAC at +1.45 — so the encoded result is what
    gets measured. Lossless targets are exact and skip the pass.
  • convert_check grows --aac/--mp3: its lossless-only default could
    not exercise the new path at all.
  • library.rs: removals queue the device path and save() sweeps it
    once itdb_write returns. Deleting before the write would leave the
    on-disk database pointing at audio that is gone, which is worse than
    the leak; a path a surviving record still claims is skipped.
  • library_roundtrip.rs: a test that drives the real FFI — imports two
    tracks, removes one, asserts the file survives until the write lands
    and is gone after it, and that the other track keeps its audio.

Test plan

CI does not run on pull requests based on dev, so the suite was run
locally on the head of this branch:

  • cargo test — 46 unit + 13 round-trip, green
  • npm test — 125, green
  • npm run typecheck — clean
  • cargo clippy --all-targets -- -D warnings — clean
  • Verified the new round-trip test fails with the sweep disabled, so it
    guards the behaviour rather than passing by construction.
  • Measured against the album that surfaced this: AAC 256 went from
    +1.45 dBFS with 1895 samples past full scale to −0.51 dBFS with none,
    MP3 320 from +0.72 dBFS to −0.93 dBFS, and an ALAC conversion still
    nulls to -inf against its FLAC source.

A lossy encoder is free to reconstruct a waveform louder than the one it
was handed, and every one of them does: a master peaking at -0.18 dBFS
comes back out of AAC at +1.45. The iPod's output stage is fixed point,
so those samples saturate and land as short bright clicks.

Nothing about the source predicts it — this master's true peak is
-0.05 dBTP — so measure the encoded file instead and re-encode it
attenuated when it decodes back above full scale. Lossless targets are
exact and skip the pass entirely.

convert_check grows --aac/--mp3 because the lossless default it had
could not exercise the new path at all.
gpod_remove_track frees the database record and nothing else, so every
removal left its file on the device for good. A 74 GB iPod had 2 GB of
audio no record pointed at.

Queue the device path at removal and sweep it once itdb_write returns.
The ordering is the whole point: deleting first and then failing to
write leaves the on-disk database pointing at audio that is gone, which
is worse than the leak. A path a surviving record still claims is
skipped — libgpod does not check either.
@kolebayev
kolebayev merged commit 58aa7fa into dev Aug 21, 2026
3 of 4 checks passed
@kolebayev
kolebayev deleted the kolebayev/converter-fixes branch August 21, 2026 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant