Invalid decoding of 24-bit FLAC files

[Uwe Klotz]

Bug when decoding a 24-bit FLAC file:
https://www.mixxx.org/forums/viewtopic.php?f=3&t=11654&p=38302&hilit=stability#p38302

Playing this file in Mixxx produces a distorted output and weird waveform, both with SoundSourceFLAC as well as SoundSourceSndFile.

When decoding the file with "flac -d -o 24_bps.wav 24_bps.flac" (v1.3.2) the resulting WAV file is ok. Opening the FLAC file directly in Audacity using the same FLAC version for decoding shows a very weird waveform. See the attached example file and screenshots.

Even though this might not be a Mixxx issue, we should track this bug. Other 24-bit FLAC files are decoded correctly, e.g. http://www.linnrecords.com/recording-test-files.aspx

[Chris Duncan (zosoled)]

For my song files which also exhibit this behavior, they were originally encoded with FLAC 1.3.0, if that helps. foobar2000 experience this issue, their dev stated this:

"On this file, libFLAC returns 24-bit samples improperly padded to 32-bit, with most significant bits filled with garbage instead of correct sign expansion. [...] This problem isn't 24-bit specific at all, the same issue could potentially occur with 16-bit files. [...] I still don't believe that this is a legit behavior of FLAC tools and anybody else converting FLAC decoder's output to floating-point values will run into the same problem."

sauce: https://hydrogenaud.io/index.php/topic,61792.msg559045.html#msg559045

[Chris Duncan (zosoled)]

Per that sample file in particular:

"
in this clip, sample 0 of channel 0 is 0 and sample 1 of channel 0 is -364 (0xfffe94 in the wav file). the first FLAC subframe of channel 0 is a FIXED subframe with order 1, meaning the first sample (0) is stored raw and the second sample (-364) is stored as a residual value "sample[1]-sample[0]" == -364 - 0 == -364

but that is not what is actually in 24_bps.flac, the first residual sample it has encoded is 16776852 (0xfffe94), which is exactly what I would expect if the client passed in bad data (explained next), and also explains why the compression ratio is bad (because 16776852 takes more bits to store than -364)

when samples are passed in to FLAC__stream_encoder_process(), they must be 32-bit signed ints regardless of the sample resolution. so to pass in -364, the sample value should be 0xfffffe94, but if the client is not sign-extending properly, it will pass in 0xfffe94 and due to the above mentioned magic, it will all appear to work (magic extends even to the md5 summing).

now this could have been avoided if libFLAC did range checking on the input but this noticeably slows things down, which I am hesitant to do for something that should be caught in testing. (maybe it could be done in debug builds.)"

[Uwe Klotz (uklotzde-deactivatedaccount)]

Thank you for your explanations, Chris! With these insights I found the workaround, PR will follow.

This is not a bug in Mixxx and actually libFLAC should take care of these issues internally.

[Uwe Klotz (uklotzde-deactivatedaccount)]

https://github.com/mixxxdj/mixxx/pull/1660

[Swiftb0y (swiftb0y)]

Mixxx now uses GitHub for bug tracking. This bug has been migrated to:
https://github.com/mixxxdj/mixxx/issues/9275