Repository navigation
fix: clipped audio and leaked files on the device - #3
Merged
Merged
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 actuallydecodes 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_checkgrows--aac/--mp3: its lossless-only default couldnot exercise the new path at all.
library.rs: removals queue the device path andsave()sweeps itonce
itdb_writereturns. Deleting before the write would leave theon-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 twotracks, 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 runlocally on the head of this branch:
cargo test— 46 unit + 13 round-trip, greennpm test— 125, greennpm run typecheck— cleancargo clippy --all-targets -- -D warnings— cleanguards the behaviour rather than passing by construction.
+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
-infagainst its FLAC source.