search

dbry / WavPack

WavPack encode/decode library, command-line programs, and several plugins
BSD 3-Clause "New" or "Revised" License
371 stars 67 forks source link

WavpackUnpackSamples memset with OPEN_2CH_MAX flag #177

Closed arnaud-neny closed 8 months ago

arnaud-neny commented 9 months ago

Hi, the memset in the WavpackUnpackSamples is using the default number of channels to clear the output buffer. Shouldn't it be using the reduced_channels if there are any? Thx!

dbry commented 9 months ago

Hi, and thanks for the heads-up on this!

Yes, that's definitely wrong, and I verified that it crashes if the reduced channels feature is used. However, even after I fixed that crash the function still returned errors. It may be that the reduced channels feature has been broken for a long time (even that memset() was over 4 years ago!).

If the fix is too complex I might consider deprecating the feature. Is this something that you are using, or considering using?

arnaud-neny commented 9 months ago

I might just be able to get around this by just using the first two channels.

dbry commented 9 months ago

I might just be able to get around this by just using the first two channels.

Depending on your exact application, the "tiny decoder" might work for you. It always works in the reduced channels mode because it's designed for embedded platforms with limited resources (it's used on Rockbox). Unpacking multichannel files requires the entire multi-frame block to be loaded at the same time, whereas the tiny decoder decodes on the fly and skips any subsequent frames.

Tiny Decoder

Sorry for the trouble!

dbry commented 9 months ago

So I did a little more digging and built old versions and found that this feature (OPEN_2CH_MAX) has been broken since at least version 4.40, which was 2006! I suspect that nobody is really using it (or at least not using it with multichannel files) and I'm thinking of getting rid of it. It might be a chore to fix (especially with the new multithreading) and removing it would clean up the code a little.

Besides, playing just the front channels of a multichannel stream really is not ideal.

Were you able to find a way around not having it?

arnaud-neny commented 9 months ago

I didn't really try yet, maybe i'll try to simply mix down on read.

dbry commented 9 months ago

Okay, I looked even closer and it turns out once I fixed the memset crash the mode was working fine (producing the correct output) but was just falsely generating errors. I fixed that and everything seems to be working fine now.

Let me know if you have further trouble, and thanks again for reporting this!