flac icon indicating copy to clipboard operation
flac copied to clipboard

flac seems to write malformed AIFF-C, both none and sowt

Open H2Swine opened this issue 1 year ago • 6 comments

In the course of testing input file handling after this one - used: 1.4.3 and including https://github.com/xiph/flac/actions/runs/12089446877 which I think is the most recent?

Running the following on the attached .flac:

flac --force-aiff-c-none-format -dfo aiffcnone.aiff short.flac
flac --force-aiff-c-sowt-format -dfo aiffcsowt.aiff short.flac

... gets me the two .aiff files in the attachment.

Then try flac --no-padding aiffc*.aiff :

WARNING: FORM chunk size of file aiffcnone.aiff does not agree with filesize
aiffcnone.aiff: wrote 2222 bytes, ratio=0,135
WARNING: FORM chunk size of file aiffcsowt.aiff does not agree with filesize
aiffcsowt.aiff: wrote 2222 bytes, ratio=0,135

With --keep-foreign-metadata-if-present added:

WARNING: FORM chunk size of file aiffcnone.aiff does not agree with filesize
aiffcnone.aiff: WARNING reading foreign metadata: invalid AIFF file: unexpected EOF (012)
aiffcnone.aiff: wrote 2306 bytes, ratio=0,140
WARNING: FORM chunk size of file aiffcsowt.aiff does not agree with filesize
aiffcsowt.aiff: WARNING reading foreign metadata: invalid AIFF file: unexpected EOF (012)
aiffcsowt.aiff: wrote 2306 bytes, ratio=0,140

aiffc.zip

H2Swine avatar Dec 02 '24 15:12 H2Swine

I just made a pull request for a potential fix for this issue. Could you try again with the binary under https://github.com/xiph/flac/actions/runs/12135436767 (as soon as it is finished)

ktmf01 avatar Dec 03 '24 08:12 ktmf01

It seems to resolve the invalid files creation, but I discovered a few things that may or may not be related. First, how it behaves on those files now:

  • Trying to --keep-foreign-metadata-if-present --force-aiff-format on a .flac with AIFC foreign metadata (none or sowt), throws this error: ERROR verifying foreign metadata restore from withforeignmetadata.aifcsowt.flac to outfile.aiff: stored main chunk length differs from written length Sure it isn't strange that it objects, but maybe it should throw a different one?
  • Similar happens when trying to force AIFC formats on a .flac with AIFF foreign metadata. But if trying to "force none on a sowt or vice versa", then it gives the error that stored foreign format block differs from written block. Perhaps the file is being restored to a different format than that of the original file.

So it looks like the decoder knows that AIFC-none and AIFC-sowt are different, but not that old-fashion AIFF and AIFC are different formats?

Doing the same with --keep-foreign-metadata-if-present --force-[the wave and rf64 formats], reveal the same as in the first item above: ERROR verifying foreign metadata restore from flacfile.flac to outfile.<extension>: stored main chunk length differs from written length. Maybe it should throw the other error.

Then a couple of issues I discovered when testing

  • I see that flac.exe now outputs .aifc, but -o outfile.aifc (sans any foreign metadata preservation) does not try to force AIFF-C?
  • Running --keep-foreign-metadata-if-present on .flac files generated from metadata-less .aiff, gives the following warning: WARNING reading foreign metadata: invalid WAVE file: missing "fmt " chunk (024) . Sure it isn't supposed to have that ...

H2Swine avatar Dec 03 '24 17:12 H2Swine

Also, it might leave suspicious or corrupted decoded files upon throwing those errors, but that is maybe expected.

H2Swine avatar Dec 03 '24 18:12 H2Swine

So it looks like the decoder knows that AIFC-none and AIFC-sowt are different, but not that old-fashion AIFF and AIFC are different formats?

This is 'first come, first served' in case of decoding AIFF to AIFF-C or vice versa, the total length of the file is different and this is encountered first. In the case of mixing up sowt and NONE, the file length is the same, and the first error is a different COMM chunk.

I am not sure changing that is practical, but I can take a look.

Then a couple of issues I discovered when testing

  • I see that flac.exe now outputs .aifc, but -o outfile.aifc (sans any foreign metadata preservation) does not try to force AIFF-C?

Yes, that is correct. I forgot to add that apparently. However, I wonder, in case a filename ends in aifc, should flac default to NONE, sowt or throw an error that this is ambiguous.

  • Running --keep-foreign-metadata-if-present on .flac files generated from metadata-less .aiff, gives the following warning: WARNING reading foreign metadata: invalid WAVE file: missing "fmt " chunk (024) . Sure it isn't supposed to have that ...

I'll take a look, should be an easy fix.

ktmf01 avatar Dec 03 '24 18:12 ktmf01

I wonder, in case a filename ends in aifc, should flac default to NONE, sowt or throw an error that this is ambiguous.

My first reaction is, "decoder chooses". Because we have already gotten used to it doing so for -o outfile.wav. But then, what to choose? On one hand why choose aifc for integer LPCM if not for sowt? On the other hand, why choose sowt on an 8-bit signal except for creating test files?

Watch out or you are in for a --prefer-output-format=[ordered list] ;-)

H2Swine avatar Dec 03 '24 20:12 H2Swine

But then, what to choose?

Apparently: at least not sowt for anything but 16-bit - it seems also that Apple tried to deprecate sowt even more so than the attempts at having CAF take over for AIFF.

I still think the decoder should choose, because that is what we are used to from .wav. It seems "safest" to choose NONE. Maybe with the exception of "sowt for 16-bit" - or even "sowt for CDDA".

H2Swine avatar Jul 04 '25 13:07 H2Swine