A user of CRAM (http://swami.sourceforge.net/cram.php) sent in a bug report related to decoding of CRAM files. This issue occurs with flac-1.1.2 but not previous versions (such as flac-1.1.1). Note that the same file is used for this test (hopefully ruling out any issue with the encoder). Details of the issue: When calling FLAC__stream_decoder_process_single() the error callback is triggered with the error FLAC__STREAM_DECODER_ERROR_STATUS_BAD_HEADER (the error callback is from line 1806 of stream_decoder.c in read_frame_). One thing I noticed is that FLAC__stream_decoder_process_single() still returns TRUE when our read callback returns FLAC__STREAM_DECODER_ABORTED (this was previously causing an endless loop in CRAM, since it didn't detect that an error had occurred). This behavior doesn't seem quite right and is contrary to what is described in the API docs (says the only error which returns TRUE is FLAC__STREAM_DECODER_ERROR_STATUS_LOST_SYNC). Seems like returning FALSE might be more intuitive. Some details of how CRAM uses FLAC: - using libFLAC stream encoder/decoder - many samples are encoded/decoded in the same file using the same encoder/decoder instance (the individual sample formats may differ but the block size is fixed currently) - CRAM uses its own container format (EBML) - FLAC header is forged in the read_callback (isn't actually written to CRAM file) If anyone has any ideas on how this issue can be resolved I'd appreciate the input. In particular this is related to some change between version 1.1.1 and 1.1.2 of FLAC. I'll continue to try and narrow down what exactly it is. Best regards, Josh Green
On Tue, 2006-07-25 at 06:37 +0200, Josh Green wrote:> A user of CRAM (http://swami.sourceforge.net/cram.php) sent in a bug > report related to decoding of CRAM files. This issue occurs with > flac-1.1.2 but not previous versions (such as flac-1.1.1). Note that > the same file is used for this test (hopefully ruling out any issue with > the encoder). > > Details of the issue: > > When calling FLAC__stream_decoder_process_single() the error callback is > triggered with the error FLAC__STREAM_DECODER_ERROR_STATUS_BAD_HEADER > (the error callback is from line 1806 of stream_decoder.c in > read_frame_). One thing I noticed is that > FLAC__stream_decoder_process_single() still returns TRUE when our read > callback returns FLAC__STREAM_DECODER_ABORTED (this was previously > causing an endless loop in CRAM, since it didn't detect that an error > had occurred). This behavior doesn't seem quite right and is contrary > to what is described in the API docs (says the only error which returns > TRUE is FLAC__STREAM_DECODER_ERROR_STATUS_LOST_SYNC). Seems like > returning FALSE might be more intuitive. > > Some details of how CRAM uses FLAC: > - using libFLAC stream encoder/decoder > - many samples are encoded/decoded in the same file using the same > encoder/decoder instance (the individual sample formats may differ but > the block size is fixed currently) > - CRAM uses its own container format (EBML) > - FLAC header is forged in the read_callback (isn't actually written to > CRAM file) > > If anyone has any ideas on how this issue can be resolved I'd appreciate > the input. In particular this is related to some change between version > 1.1.1 and 1.1.2 of FLAC. I'll continue to try and narrow down what > exactly it is. Best regards, > Josh GreenReplying to my own post to add some additional findings. The issue is being triggered by the code beginning at line 1721 in stream_decoder.c. Some variable values at this point: is_known_variable_blocksize_stream = 1 In the forged STREAMINFO header CRAM specifies a min and max of 16 and 65535 respectively. CRAM does not currently change the block size during the life of the encoder, but may in the future (if some algorithm is discovered for determining optimal block sizes for different samples). blocksize_hint = 0 At line 1721 in version 1.1.2 can be found: if(is_known_variable_blocksize_stream) { if(blocksize_hint) { In the previous version it was: if(blocksize_hint && is_known_variable_blocksize_stream) { So it appears that the stream decoder does not like the fact that STREAMINFO is indicating a variable block size but no blocksize_hint was found. Looking at the switch statement towards the top of read_frame_header_() for the block size it looks like blocksize_hint is only set on 'case 7:' (16 bit blocksize from end of header), seems in the CRAM case it is using 'case 5:' (576/1152/2304/4608). Any suggestions on the best way to resolve this? I suppose one solution might be to just set the min/max block sizes to the fixed value in the forged STREAMINFO and deal with this issue in the future when variable block size encoding is desired. This is kind of a pain on the part of the CRAM decoder though, since the STREAMINFO isn't actually stored and therefore the first FLAC frame needs to be read to determine what the block size is, or add an additional field to the CRAM format for specifying the block size. Ideally it would be nice to allow the decoder to handle variable block sizes though. Is this a bug in FLAC or is CRAM mis-using it? Thanks again for any help on this. Best regards, Josh Green
Replying to myself once again, since I seem to have found the answer I was looking for. Sorry for the noise. It seems CRAM is indeed misusing FLAC, since stated in the Notes section of the FLAC format for the FRAME_HEADER is the fact that only block size values 0110 and 0111 can be used for variable block size data (i.e., the block size is specified as an 8 or 16 bit value at the end of the header): http://flac.sourceforge.net/format.html#frame_header Not quite sure the best way to fix this. I'd love to hear any ideas.. CRAM files usually consist of several audio samples all packed into the same file (with binary segments compressed with zlib). FLAC is used to store only the compressed audio and no other FLAC headers (STREAMINFO, etc) are actually found in a CRAM file. A single sample could (and is currently) compressed using a single block size. It might be advantageous to change the block size between audio samples though, depending on the individual sample parameters (sample rate, length, etc), I have yet to determine a good method for guessing a good block size though. Seems like it might be a little bit of a waste to add 2 bytes to every frame in this case (but I imagine that's probably a trivial amount). I suppose the FLAC stream decoder could be re-created for each sample with the appropriate fixed block size STREAMINFO which is forged, but that leaves it up to CRAM to determine what block size is stored in the FLAC FRAME_HEADER and seems a little messy anyways. Any input appreciated. Best regards, Josh Green
sorry if I did not reply to this, answers below: --- Josh Green <josh@resonance.org> wrote:> On Tue, 2006-07-25 at 06:37 +0200, Josh Green wrote: > > A user of CRAM (http://swami.sourceforge.net/cram.php) sent in a > bug > > report related to decoding of CRAM files. This issue occurs with > > flac-1.1.2 but not previous versions (such as flac-1.1.1). Note > that > > the same file is used for this test (hopefully ruling out any issue > with > > the encoder). > > > > Details of the issue: > > > > When calling FLAC__stream_decoder_process_single() the error > callback is > > triggered with the error > FLAC__STREAM_DECODER_ERROR_STATUS_BAD_HEADER > > (the error callback is from line 1806 of stream_decoder.c in > > read_frame_). One thing I noticed is that > > FLAC__stream_decoder_process_single() still returns TRUE when our > read > > callback returns FLAC__STREAM_DECODER_ABORTED (this was previously > > causing an endless loop in CRAM, since it didn't detect that an > error > > had occurred). This behavior doesn't seem quite right and is > contrary > > to what is described in the API docs (says the only error which > returns > > TRUE is FLAC__STREAM_DECODER_ERROR_STATUS_LOST_SYNC). Seems like > > returning FALSE might be more intuitive.the API docs are more clear now for the return code, but the bottom line is you have to check the return value and the state to know exactly what happened after the call.> > Some details of how CRAM uses FLAC: > > - using libFLAC stream encoder/decoder > > - many samples are encoded/decoded in the same file using the same > > encoder/decoder instance (the individual sample formats may differ > but > > the block size is fixed currently) > > - CRAM uses its own container format (EBML) > > - FLAC header is forged in the read_callback (isn't actually > written to > > CRAM file) > > > > If anyone has any ideas on how this issue can be resolved I'd > appreciate > > the input. In particular this is related to some change between > version > > 1.1.1 and 1.1.2 of FLAC. I'll continue to try and narrow down what > > exactly it is. Best regards, > > Josh Green > > > Replying to my own post to add some additional findings. > > The issue is being triggered by the code beginning at line 1721 in > stream_decoder.c. Some variable values at this point: > > is_known_variable_blocksize_stream = 1 > > In the forged STREAMINFO header CRAM specifies a min and max of 16 > and > 65535 respectively. CRAM does not currently change the block size > during the life of the encoder, but may in the future (if some > algorithm > is discovered for determining optimal block sizes for different > samples). > > blocksize_hint = 0 > > At line 1721 in version 1.1.2 can be found: > > if(is_known_variable_blocksize_stream) { > if(blocksize_hint) { > > > In the previous version it was: > > if(blocksize_hint && is_known_variable_blocksize_stream) { > > > So it appears that the stream decoder does not like the fact that > STREAMINFO is indicating a variable block size but no blocksize_hint > was > found. Looking at the switch statement towards the top of > read_frame_header_() for the block size it looks like blocksize_hint > is > only set on 'case 7:' (16 bit blocksize from end of header), seems in > the CRAM case it is using 'case 5:' (576/1152/2304/4608). > > Any suggestions on the best way to resolve this? I suppose one > solution > might be to just set the min/max block sizes to the fixed value in > the > forged STREAMINFO and deal with this issue in the future when > variable > block size encoding is desired. This is kind of a pain on the part > of > the CRAM decoder though, since the STREAMINFO isn't actually stored > and > therefore the first FLAC frame needs to be read to determine what the > block size is, or add an additional field to the CRAM format for > specifying the block size. Ideally it would be nice to allow the > decoder to handle variable block sizes though. > > Is this a bug in FLAC or is CRAM mis-using it? Thanks again for any > help on this. Best regards,hopefully I'm understanding this correctly... according to the FLAC spec, if min and max blocksize are differeny, then the blocksize bits in each frame header must be 0110 or 0111. the other values imply to the decoder that it is a fixed-blocksize stream. Josh ____________________________________________________________________________________ Sponsored Link $420k for $1,399/mo. Think You Pay Too Much For Your Mortgage? Find Out! www.LowerMyBills.com/lre