Decode mid/side stereo blocks + fix interleave bounds - #6
Open
tomaszcarter wants to merge 2 commits into
Open
Conversation
Previously, stereo NCW files with MidSide channel encoding (common in
Kontakt sample libraries — e.g. Legendary, where 100% of files use it)
would panic in v0.1.2 ("mid/side compression not implemented") or, on
main, silently produce a WAV with garbled stereo because the M/S → L/R
transform was never applied.
This commit:
- Tracks the per-block ChannelEncoding flag during decode_samples and,
for 2-channel files, applies the lossless integer M/S → L/R recovery
(same formula FLAC uses: shifted_mid = (mid << 1) | (side & 1);
L = (shifted_mid + side) >> 1; R = (shifted_mid - side) >> 1).
- Clamps the interleave loop to the shortest channel length to tolerate
an off-by-one in the existing overflow_samples bookkeeping that
panicked on some files (e.g. bass_clean_1.ncw).
- Drops two stray dbg!() prints in read_i32_block.
Tested by converting 459 NCW files from the Legendary Kontakt library
to 24-bit / 96 kHz stereo WAVs; output verified non-silent and
spectrally plausible (mean ~ -22 dB, peak ~ -1 dB).
The NCW decoder occasionally yields i32 sample values one or two LSBs outside the nominal bit-depth range — most often after delta accumulation or the new M/S transform on near-clipping material. hound rejects those with Error::TooWide and aborts the entire file. Clamp samples to [-2^(b-1), 2^(b-1) - 1] for the declared bits_per_sample before writing. Effect on output is inaudible (handful of clipped LSBs per file) and lets the conversion finish. Tested by re-running the Legendary library batch: 459/459 NCW files now convert end-to-end (was 445/459 before this patch).
Author
|
Pushed a follow-up commit (8580dd3) that saturates samples to the destination bit-depth before handing them to hound. Without it, ~3% of real-world files (14/459 in my Legendary test set) abort with |
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
1 participant
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
ChannelEncoding::MidSide(currentlydecode_samplesreads the flag but never applies the transform — output WAV is garbled).index out of bounds) on some files whereheader.num_samplesexceeded the actual per-channel sample count by 1.dbg!()calls inread_i32_block.The M/S → L/R formula is the FLAC integer one (lossless roundtrip):
Motivation
The published
ncw-convertcrate (v0.1.2) panics with"mid/side compression not implemented yet!"on every M/S-encoded file. In real-world Kontakt libraries this is the common encoding for stereo samples — e.g. 459/459 files in the Legendary library are M/S — so the converter is effectively unusable for stereo content today.Test plan
examples/ncw-convertbinary against this branch.afinfoandffmpeg volumedetect— non-silent, mean ≈ -22 dB, peak ≈ -1 dB, stereo image audibly correct.🤖 Generated with Claude Code