[Flac-dev] Re: Problem with CRAM and flac-1.1.2

[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 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,
	Josh Green