Coping with damaged flac files

[mewkiz]

Coping with damaged flac files

[mewmew]

All crc mismatch messages lead to a program abort, which is not ideal.

Turning CRC mismatches into warnings, rather than fatal errors seems reasonable. We'll push this change momentarily.

Do you agree that making the decoder more robust against damaged flac files is a reasonable goal?

As for CRC, sure. However, for the later two cases, it seems better to try to get a working FLAC file when it is broken is incorrectly encoded. Otherwise we may end up with something similar to quirks mode in HTML, which is a mess. Better to stay true to the FLAC standard.

As far as I'm aware, it should never be the case that a valid FLAC file has different number of samples in the side and left channels (i.e. in correlate). So, the FLAC files exhibiting this property must be incorrectly encoded or damaged.

For the LPC case, it seems your file does not have subframe.NSamples == len(subframe.Samples) which is strange. Would you please verify if this is the case?

I.e. add the following to decodeLPC:

if subframe.NSamples != len(subframe.Samples) {
	log.Fatalf("subframe sample count mismatch; expected %d, got %d", subframe.NSamples, len(subframe.Samples))
}

[mewmew]

Turning CRC mismatches into warnings, rather than fatal errors seems reasonable. We'll push this change momentarily.

Done as of rev 8c55685.

[mehrvarz]

for the later two cases, it seems better to try to get a working FLAC file when it is broken is incorrectly encoded. Otherwise we may end up with something similar to quirks mode in HTML, which is a mess. Better to stay true to the FLAC standard.

Let us not call it quirks mode. Quirks mode is about dealing with human ignorance. Here we have flipping bits. Why not call it error correction? Or maybe better "error resilience". Apparently mp3 decoders never panic on such issues. They attempt to be robust about damages and try to (sometimes audibly) skip over. I find this preferable.

Turning CRC mismatches into warnings

Thank you. However, this change alone may not be sufficient. What this does is that...

frame, err := flacstream.ParseNext()

may now return incomplete number of subframes or samples. You either get "len(frame.Subframes) < channels" or "len(frame.Subframes[n].Samples) < int(frame.BlockSize). Without any precaution this will likely crash the consumer of the API. Some guidance is expedient.

Btw, you can use this to find damaged flac files (I just learned):

find "path2musicfolder" -name \*.flac -type f -exec flac -t "$1" {} \+