ruuda / hound

A wav encoding and decoding library in Rust
https://codeberg.org/ruuda/hound
Apache License 2.0
491 stars 65 forks source link

Add S24_LE wav files and tests #41

Closed fletcherw closed 4 years ago

fletcherw commented 4 years ago

The second patch in the series gets read_wav_wave_format_extensible_pcm_24bit_4byte() to pass, but the other test will need some deeper changes to work (potentially adding a frame size field which is block_align / num_channels ?)

ruuda commented 4 years ago

Thanks! This looks good to me so far.

the other test will need some deeper changes to work (potentially adding a frame size field which is block_align / num_channels ?)

WavSpecEx already has a bytes_per_sample field, and as you show that works out with WAVEFORMATEXTENSIBLE. I think what we can do is relax the check you mentioned earlier, and then pass the number of bytes per sample down into these methods that construct the WavSpecExc: https://github.com/ruuda/hound/blob/10673f865ad05fbb019e3ebf12fb42868affe875/src/read.rs#L511-L514

Or we could even already construct the WavSpecEx in read_fmt_chunk; all three read_wave_format_* methods set bytes_per_sample to spec.bits_per_sample / 8 anyway. We would set it to block_align / n_channels instead now.

Do you want to add that to this pull request?

fletcherw commented 4 years ago

Got both tests passing. I wasn't sure what your format preferences were, since it seems like you're not using cargo fmt generally. I just ran it on the lines that I modified, hopefully that looks good to you. I'm going to take a look at writing relatively soon as well.

fletcherw commented 4 years ago

Fixed some build errors on old rust.

ruuda commented 4 years ago

Got both tests passing. I wasn't sure what your format preferences were, since it seems like you're not using cargo fmt generally. I just ran it on the lines that I modified, hopefully that looks good to you.

I don’t have a strong preference on formatting, it looks good to me. Thanks for only formatting the modified lines and not the entire file.