[flac-dev] [PATCH] stream_encoder : Improve selection of residual accumulator width

[Miroslav Lichvar]

On Thu, Jun 19, 2014 at 03:30:06PM +0200, Miroslav Lichvar wrote:
> But, as we have seen with unusual data the residual signal can be
> wider than bps. The FLAC format specification doesn't seem to mention
> this. Should it be treated as a valid FLAC stream?

I think it would be interesting to know how common are such streams. I
patched flac to print a warning on decoding or testing when this is
detected, but didn't find any files with this problem in my (small)
music collection.

If someone has a large collection and some cycles to spare, can you please 
consider compiling flac from git with the attached patch and see if
you have any files that fail with "flac -t" ?

With the known problem file (snippet6.wav) encoded by 1.3.0 it prints
this:

WARNING: residual -11025151 wider than bps 24
WARNING: residual 41873263 wider than bps 24
WARNING: residual -67175215 wider than bps 24
WARNING: residual 69950995 wider than bps 24
WARNING: residual -67108864 wider than bps 24                                                                                      
...
WARNING: residual 11227392 wider than bps 24
WARNING: residual -8754288 wider than bps 24
snippet6.flac: ERROR, MD5 signature mismatch

Thanks,

-- 
Miroslav Lichvar
-------------- next part --------------
diff --git a/src/libFLAC/stream_decoder.c b/src/libFLAC/stream_decoder.c
index ddd8979..82318ae 100644
--- a/src/libFLAC/stream_decoder.c
+++ b/src/libFLAC/stream_decoder.c
@@ -99,7 +99,7 @@ static FLAC__bool read_subframe_constant_(FLAC__StreamDecoder *decoder, unsigned
 static FLAC__bool read_subframe_fixed_(FLAC__StreamDecoder *decoder, unsigned channel, unsigned bps, const unsigned order, FLAC__bool do_full_decode);
 static FLAC__bool read_subframe_lpc_(FLAC__StreamDecoder *decoder, unsigned channel, unsigned bps, const unsigned order, FLAC__bool do_full_decode);
 static FLAC__bool read_subframe_verbatim_(FLAC__StreamDecoder *decoder, unsigned channel, unsigned bps, FLAC__bool do_full_decode);
-static FLAC__bool read_residual_partitioned_rice_(FLAC__StreamDecoder *decoder, unsigned predictor_order, unsigned partition_order, FLAC__EntropyCodingMethod_PartitionedRiceContents *partitioned_rice_contents, FLAC__int32 *residual, FLAC__bool is_extended);
+static FLAC__bool read_residual_partitioned_rice_(FLAC__StreamDecoder *decoder, unsigned predictor_order, unsigned partition_order, FLAC__EntropyCodingMethod_PartitionedRiceContents *partitioned_rice_contents, FLAC__int32 *residual, FLAC__bool is_extended, int bps);
 static FLAC__bool read_zero_padding_(FLAC__StreamDecoder *decoder);
 static FLAC__bool read_callback_(FLAC__byte buffer[], size_t *bytes, void *client_data);
 #if FLAC__HAS_OGG
@@ -2572,7 +2572,7 @@ FLAC__bool read_subframe_fixed_(FLAC__StreamDecoder *decoder, unsigned channel,
 	switch(subframe->entropy_coding_method.type) {
 		case FLAC__ENTROPY_CODING_METHOD_PARTITIONED_RICE:
 		case FLAC__ENTROPY_CODING_METHOD_PARTITIONED_RICE2:
-			if(!read_residual_partitioned_rice_(decoder, order, subframe->entropy_coding_method.data.partitioned_rice.order, &decoder->private_->partitioned_rice_contents[channel], decoder->private_->residual[channel], /*is_extended=*/subframe->entropy_coding_method.type == FLAC__ENTROPY_CODING_METHOD_PARTITIONED_RICE2))
+			if(!read_residual_partitioned_rice_(decoder, order, subframe->entropy_coding_method.data.partitioned_rice.order, &decoder->private_->partitioned_rice_contents[channel], decoder->private_->residual[channel], /*is_extended=*/subframe->entropy_coding_method.type == FLAC__ENTROPY_CODING_METHOD_PARTITIONED_RICE2, bps))
 				return false;
 			break;
 		default:
@@ -2651,7 +2651,7 @@ FLAC__bool read_subframe_lpc_(FLAC__StreamDecoder *decoder, unsigned channel, un
 	switch(subframe->entropy_coding_method.type) {
 		case FLAC__ENTROPY_CODING_METHOD_PARTITIONED_RICE:
 		case FLAC__ENTROPY_CODING_METHOD_PARTITIONED_RICE2:
-			if(!read_residual_partitioned_rice_(decoder, order, subframe->entropy_coding_method.data.partitioned_rice.order, &decoder->private_->partitioned_rice_contents[channel], decoder->private_->residual[channel], /*is_extended=*/subframe->entropy_coding_method.type == FLAC__ENTROPY_CODING_METHOD_PARTITIONED_RICE2))
+			if(!read_residual_partitioned_rice_(decoder, order, subframe->entropy_coding_method.data.partitioned_rice.order, &decoder->private_->partitioned_rice_contents[channel], decoder->private_->residual[channel], /*is_extended=*/subframe->entropy_coding_method.type == FLAC__ENTROPY_CODING_METHOD_PARTITIONED_RICE2, bps))
 				return false;
 			break;
 		default:
@@ -2703,7 +2703,7 @@ FLAC__bool read_subframe_verbatim_(FLAC__StreamDecoder *decoder, unsigned channe
 	return true;
 }
 
-FLAC__bool read_residual_partitioned_rice_(FLAC__StreamDecoder *decoder, unsigned predictor_order, unsigned partition_order, FLAC__EntropyCodingMethod_PartitionedRiceContents *partitioned_rice_contents, FLAC__int32 *residual, FLAC__bool is_extended)
+FLAC__bool read_residual_partitioned_rice_(FLAC__StreamDecoder *decoder, unsigned predictor_order, unsigned partition_order, FLAC__EntropyCodingMethod_PartitionedRiceContents *partitioned_rice_contents, FLAC__int32 *residual, FLAC__bool is_extended, int bps)
 {
 	FLAC__uint32 rice_parameter;
 	int i;
@@ -2744,6 +2744,15 @@ FLAC__bool read_residual_partitioned_rice_(FLAC__StreamDecoder *decoder, unsigne
 			u = (partition_order == 0 || partition > 0)? partition_samples : partition_samples - predictor_order;
 			if(!decoder->private_->local_bitreader_read_rice_signed_block(decoder->private_->input, residual + sample, u, rice_parameter))
 				return false; /* read_callback_ sets the state for us */
+#if 1
+			for (i = sample; (unsigned)i < sample + u; i++)  {
+				if (abs(residual[i]) > (1 << (bps - 1))) {
+					fprintf(stderr, "WARNING: residual %d wider than bps %d\n",
+							residual[i], bps);
+					residual[i]--; /* corrupt the value to fail decoding test */
+				}
+			}
+#endif
 			sample += u;
 		}
 		else {