change unparseable frame handling, from a fatal error and...

change unparseable frame handling, from a fatal error and FLAC__STREAM_DECODER_UNPARSEABLE_STREAM state, to a soft error FLAC__STREAM_DECODER_ERROR_STATUS_UNPARSEABLE_STREAM

doc/html/changelog.html

@@ -66,7 +66,7 @@
 				General:
 				<ul>
 					<li>Large file (&gt;2GB) support everywhere</li>
-					<li>Better recovery for corrupted files</li>
+					<li>Much better recovery for corrupted files</li>
 				</ul>
 			</li>
 			<li>
@@ -84,7 +84,7 @@
 			<li>
 				flac:
 				<ul>
-					<li>Improved the <a href="documentation.html#flac_options_decode_through_errors"><span class="argument">-F</span></a> to allow decoding of FLAC files whose metadata is corrupted.</li>
+					<li>Improved the <a href="documentation.html#flac_options_decode_through_errors"><span class="argument">-F</span></a> to allow decoding of FLAC files whose metadata is corrupted, and other kinds of corruption.</li>
 					<li>Added a new option <a href="documentation.html#flac_options_tag_from_file"><span class="argument">--tag-from-file</span></a> for setting a tag from file (e.g. for importing a cuesheet as a tag).</li>
 					<li>Added support for encoding from non-compressed AIFF-C (<a href="https://sourceforge.net/tracker/?func=detail&amp;atid=113478&amp;aid=1090933&amp;group_id=13478">SF #1090933</a>).</li>
 					<li>Importing of non-CDDA-compliant cuesheets now only issues a warning, not an error (see <a href="http://www.hydrogenaudio.org/forums/index.php?showtopic=31282">here</a>).</li>
@@ -139,6 +139,9 @@
 							<li><b>Added</b> FLAC__*_encoder_set_apodization()</li>
 							<li><b>Added</b> FLAC__metadata_object_cuesheet_calculate_cddb_id()</li>
 							<li><b>Added</b> FLAC__metadata_get_cuesheet()</li>
+							<li><b>Changed</b> FLAC__StreamDecoderState: removed state FLAC__STREAM_DECODER_UNPARSEABLE_STREAM</li>
+							<li><b>Changed</b> FLAC__StreamDecoderErrorStatus: new error code FLAC__STREAM_DECODER_ERROR_STATUS_UNPARSEABLE_STREAM</li>
+							<li>The above two changes mean that when the decoder encounters what it thinks are unparseable frames from a future decoder, instead of returning a fatal error with the FLAC__STREAM_DECODER_UNPARSEABLE_STREAM state, it just calls the error callback with FLAC__STREAM_DECODER_ERROR_STATUS_UNPARSEABLE_STREAM and leaves the behavior up to the application.</li>
 						</ul>
 					</li>
 					<li>

include/FLAC/stream_decoder.h

@@ -213,9 +213,6 @@ typedef enum {
 	FLAC__STREAM_DECODER_ABORTED,
 	/**< The decoder was aborted by the read callback. */
 
-	FLAC__STREAM_DECODER_UNPARSEABLE_STREAM,
-	/**< The decoder encountered reserved fields in use in the stream. */
-
 	FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR,
 	/**< An error occurred allocating memory. */
 
@@ -284,7 +281,20 @@ typedef enum {
 extern FLAC_API const char * const FLAC__StreamDecoderWriteStatusString[];
 
 
-/** Possible values passed in to the FLAC__StreamDecoder error callback.
+/** Possible values passed back to the FLAC__StreamDecoder error callback.
+ *  \c FLAC__STREAM_DECODER_ERROR_STATUS_LOST_SYNC is the generic catch-
+ *  all.  The rest could be caused by bad sync (false synchronization on
+ *  data that is not the start of a frame) or corrupted data.  The error
+ *  itself is the decoder's best guess at what happened assuming a correct
+ *  sync.  For example \c FLAC__STREAM_DECODER_ERROR_STATUS_BAD_HEADER
+ *  could be caused by a correct sync on the start of a frame, but some
+ *  data in the frame header was corrupted.  Or it could be the result of
+ *  syncing on a point the stream that looked like the starting of a frame
+ *  but was not.  \c FLAC__STREAM_DECODER_ERROR_STATUS_UNPARSEABLE_STREAM
+ *  could be because the decoder encountered a valid frame made by a future
+ *  version of the encoder which it cannot parse, or because of a false
+ *  sync making it appear as though an encountered frame was generated by
+ *  a future encoder.
  */
 typedef enum {
 
@@ -294,9 +304,12 @@ typedef enum {
 	FLAC__STREAM_DECODER_ERROR_STATUS_BAD_HEADER,
 	/**< The decoder encountered a corrupted frame header. */
 
-	FLAC__STREAM_DECODER_ERROR_STATUS_FRAME_CRC_MISMATCH
+	FLAC__STREAM_DECODER_ERROR_STATUS_FRAME_CRC_MISMATCH,
 	/**< The frame's data did not match the CRC in the footer. */
 
+	FLAC__STREAM_DECODER_ERROR_STATUS_UNPARSEABLE_STREAM
+	/**< The decoder encountered reserved fields in use in the stream. */
+
 } FLAC__StreamDecoderErrorStatus;
 
 /** Maps a FLAC__StreamDecoderErrorStatus to a C string.
@@ -747,16 +760,14 @@ FLAC_API FLAC__bool FLAC__stream_decoder_reset(FLAC__StreamDecoder *decoder);
  *
  *  As the decoder needs more input it will call the read callback.
  *  Depending on what was decoded, the metadata or write callback will be
- *  called with the decoded metadata block or audio frame, unless an error
- *  occurred.  If the decoder loses sync it will call the error callback
- *  instead.
+ *  called with the decoded metadata block or audio frame.
  *
  *  Unless there is a fatal read error or end of stream, this function
  *  will return once one whole frame is decoded.  In other words, if the
  *  stream is not synchronized or points to a corrupt frame header, the
  *  decoder will continue to try and resync until it gets to a valid
  *  frame, then decode one frame, then return.  If the decoder points to
- *  frame whose frame CRC in the frame footer does not match the
+ *  a frame whose frame CRC in the frame footer does not match the
  *  computed frame CRC, this function will issue a
  *  FLAC__STREAM_DECODER_ERROR_STATUS_FRAME_CRC_MISMATCH error to the
  *  error callback, and return, having decoded one complete, although
@@ -767,11 +778,10 @@ FLAC_API FLAC__bool FLAC__stream_decoder_reset(FLAC__StreamDecoder *decoder);
  * \assert
  *    \code decoder != NULL \endcode
  * \retval FLAC__bool
- *    \c false if any read or write error occurred (except
- *    \c FLAC__STREAM_DECODER_ERROR_STATUS_LOST_SYNC), else \c true;
- *    in any case, check the decoder state with
- *    FLAC__stream_decoder_get_state() to see what went wrong or to
- *    check for lost synchronization (a sign of stream corruption).
+ *    \c false if any fatal read, write, or memory allocation error
+ *    occurred (meaning decoding must stop), else \c true; for more
+ *    information about the decoder, check the decoder state with
+ *    FLAC__stream_decoder_get_state().
  */
 FLAC_API FLAC__bool FLAC__stream_decoder_process_single(FLAC__StreamDecoder *decoder);
 
@@ -783,18 +793,16 @@ FLAC_API FLAC__bool FLAC__stream_decoder_process_single(FLAC__StreamDecoder *dec
  *
  *  As the decoder needs more input it will call the read callback.
  *  As each metadata block is decoded, the metadata callback will be called
- *  with the decoded metadata.  If the decoder loses sync it will call the
- *  error callback.
+ *  with the decoded metadata.
  *
  * \param  decoder  An initialized decoder instance.
  * \assert
  *    \code decoder != NULL \endcode
  * \retval FLAC__bool
- *    \c false if any read or write error occurred (except
- *    \c FLAC__STREAM_DECODER_ERROR_STATUS_LOST_SYNC), else \c true;
- *    in any case, check the decoder state with
- *    FLAC__stream_decoder_get_state() to see what went wrong or to
- *    check for lost synchronization (a sign of stream corruption).
+ *    \c false if any fatal read, write, or memory allocation error
+ *    occurred (meaning decoding must stop), else \c true; for more
+ *    information about the decoder, check the decoder state with
+ *    FLAC__stream_decoder_get_state().
  */
 FLAC_API FLAC__bool FLAC__stream_decoder_process_until_end_of_metadata(FLAC__StreamDecoder *decoder);
 
@@ -806,18 +814,16 @@ FLAC_API FLAC__bool FLAC__stream_decoder_process_until_end_of_metadata(FLAC__Str
  *
  *  As the decoder needs more input it will call the read callback.
  *  As each metadata block and frame is decoded, the metadata or write
- *  callback will be called with the decoded metadata or frame.  If the
- *  decoder loses sync it will call the error callback.
+ *  callback will be called with the decoded metadata or frame.
  *
  * \param  decoder  An initialized decoder instance.
  * \assert
  *    \code decoder != NULL \endcode
  * \retval FLAC__bool
- *    \c false if any read or write error occurred (except
- *    \c FLAC__STREAM_DECODER_ERROR_STATUS_LOST_SYNC), else \c true;
- *    in any case, check the decoder state with
- *    FLAC__stream_decoder_get_state() to see what went wrong or to
- *    check for lost synchronization (a sign of stream corruption).
+ *    \c false if any fatal read, write, or memory allocation error
+ *    occurred (meaning decoding must stop), else \c true; for more
+ *    information about the decoder, check the decoder state with
+ *    FLAC__stream_decoder_get_state().
  */
 FLAC_API FLAC__bool FLAC__stream_decoder_process_until_end_of_stream(FLAC__StreamDecoder *decoder);
 
@@ -854,13 +860,12 @@ FLAC_API FLAC__bool FLAC__stream_decoder_process_until_end_of_stream(FLAC__Strea
  * \assert
  *    \code decoder != NULL \endcode
  * \retval FLAC__bool
- *    \c false if any read or write error occurred (except
- *    \c FLAC__STREAM_DECODER_ERROR_STATUS_LOST_SYNC), or if the decoder
+ *    \c false if any fatal read, write, or memory allocation error
+ *    occurred (meaning decoding must stop), or if the decoder
  *    is in the FLAC__STREAM_DECODER_SEARCH_FOR_METADATA or
- *    FLAC__STREAM_DECODER_READ_METADATA state, else \c true;
- *    in any case, check the decoder state with
- *    FLAC__stream_decoder_get_state() to see what went wrong or to
- *    check for lost synchronization (a sign of stream corruption).
+ *    FLAC__STREAM_DECODER_READ_METADATA state, else \c true; for more
+ *    information about the decoder, check the decoder state with
+ *    FLAC__stream_decoder_get_state().
  */
 FLAC_API FLAC__bool FLAC__stream_decoder_skip_single_frame(FLAC__StreamDecoder *decoder);
 

src/libFLAC/seekable_stream_decoder.c

@@ -91,6 +91,7 @@ typedef struct FLAC__SeekableStreamDecoderPrivate {
 	FLAC__bool ignore_seek_table_block;
 	FLAC__Frame last_frame; /* holds the info of the last frame we seeked to */
 	FLAC__uint64 target_sample;
+	unsigned unparseable_frame_count; /* used to tell whether we're decoding a future version of FLAC or just got a bad sync */
 } FLAC__SeekableStreamDecoderPrivate;
 
 /***********************************************************************
@@ -217,6 +218,7 @@ FLAC_API FLAC__SeekableStreamDecoderState FLAC__seekable_stream_decoder_init(FLA
 
 	decoder->private_->do_md5_checking = decoder->protected_->md5_checking;
 	decoder->private_->got_stream_info = false;
+	decoder->private_->unparseable_frame_count = 0;
 
 	/* We initialize the FLAC__MD5Context even though we may never use it.  This
 	 * is because md5 checking may be turned on to start and then turned off if
@@ -755,6 +757,18 @@ FLAC__StreamDecoderReadStatus read_callback_(const FLAC__StreamDecoder *decoder,
 		return FLAC__STREAM_DECODER_READ_STATUS_END_OF_STREAM;
 	}
 	else if(*bytes > 0) {
+		/* While seeking, it is possible for our seek to land in the
+		 * middle of audio data that looks exactly like a frame header
+		 * from a future version of an encoder.  When that happens, our
+		 * error callback will get an
+		 * FLAC__STREAM_DECODER_UNPARSEABLE_STREAM and increment its
+		 * unparseable_frame_count.  But there is a remote possibility
+		 * that it is properly synced at such a "future-codec frame",
+		 * so to make sure, we wait to see many "unparseable" errors in
+		 * a row before bailing out.
+		 */
+		if(seekable_stream_decoder->protected_->state == FLAC__SEEKABLE_STREAM_DECODER_SEEKING && seekable_stream_decoder->private_->unparseable_frame_count > 20)
+			return FLAC__STREAM_DECODER_READ_STATUS_ABORT;
 		if(seekable_stream_decoder->private_->read_callback(seekable_stream_decoder, buffer, bytes, seekable_stream_decoder->private_->client_data) != FLAC__SEEKABLE_STREAM_DECODER_READ_STATUS_OK) {
 			seekable_stream_decoder->protected_->state = FLAC__SEEKABLE_STREAM_DECODER_READ_ERROR;
 			return FLAC__STREAM_DECODER_READ_STATUS_ABORT;
@@ -859,6 +873,8 @@ void error_callback_(const FLAC__StreamDecoder *decoder, FLAC__StreamDecoderErro
 
 	if(seekable_stream_decoder->protected_->state != FLAC__SEEKABLE_STREAM_DECODER_SEEKING)
 		seekable_stream_decoder->private_->error_callback(seekable_stream_decoder, status, seekable_stream_decoder->private_->client_data);
+	else if(status == FLAC__STREAM_DECODER_ERROR_STATUS_UNPARSEABLE_STREAM)
+		seekable_stream_decoder->private_->unparseable_frame_count++;
 }
 
 FLAC__bool seek_to_absolute_sample_(FLAC__SeekableStreamDecoder *decoder, FLAC__uint64 stream_length, FLAC__uint64 target_sample)
@@ -1040,32 +1056,16 @@ FLAC__bool seek_to_absolute_sample_(FLAC__SeekableStreamDecoder *decoder, FLAC__
 				return false;
 			}
 		}
-		/* Now we need to get a frame.  It is possible for our seek
-		 * to land in the middle of audio data that looks exactly like
-		 * a frame header from a future version of an encoder.  When
-		 * that happens, FLAC__stream_decoder_process_single() will
-		 * return false and the state will be
-		 * FLAC__STREAM_DECODER_UNPARSEABLE_STREAM.  But there is a
-		 * remote possibility that it is properly synced at such a
-		 * "future-codec frame", so to make sure, we wait to see
-		 * several "unparseable" errors in a row before bailing out.
+		/* Now we need to get a frame.  First we need to reset our
+		 * unparseable_frame_count; if we get too many unparseable
+		 * frames in a row, the read callback will return
+		 * FLAC__STREAM_DECODER_READ_STATUS_ABORT, causing
+		 * FLAC__stream_decoder_process_single() to return false.
 		 */
-		{
-			unsigned unparseable_count;
-			FLAC__bool got_a_frame = false;
-			for (unparseable_count = 0; !got_a_frame && unparseable_count < 10; unparseable_count++) {
-				if(FLAC__stream_decoder_process_single(decoder->private_->stream_decoder))
-					got_a_frame = true;
-				else if(decoder->private_->stream_decoder->protected_->state == FLAC__STREAM_DECODER_UNPARSEABLE_STREAM)
-					/* try again.  we don't want to flush the decoder since that clears the bitbuffer */
-					decoder->private_->stream_decoder->protected_->state = FLAC__STREAM_DECODER_SEARCH_FOR_FRAME_SYNC;
-				else /* it's a real error */
-					break;
-			}
-			if (!got_a_frame) {
-				decoder->protected_->state = FLAC__SEEKABLE_STREAM_DECODER_SEEK_ERROR;
-				return false;
-			}
+		decoder->private_->unparseable_frame_count = 0;
+		if(!FLAC__stream_decoder_process_single(decoder->private_->stream_decoder)) {
+			decoder->protected_->state = FLAC__SEEKABLE_STREAM_DECODER_SEEK_ERROR;
+			return false;
 		}
 		/* our write callback will change the state when it gets to the target frame */
 		/* actually, we could have got_a_frame if our decoder is at FLAC__SEEKABLE_STREAM_DECODER_END_OF_STREAM so we need to check for that also */

src/libFLAC/stream_decoder.c

@@ -151,7 +151,6 @@ FLAC_API const char * const FLAC__StreamDecoderStateString[] = {
 	"FLAC__STREAM_DECODER_READ_FRAME",
 	"FLAC__STREAM_DECODER_END_OF_STREAM",
 	"FLAC__STREAM_DECODER_ABORTED",
-	"FLAC__STREAM_DECODER_UNPARSEABLE_STREAM",
 	"FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR",
 	"FLAC__STREAM_DECODER_ALREADY_INITIALIZED",
 	"FLAC__STREAM_DECODER_INVALID_CALLBACK",
@@ -172,7 +171,8 @@ FLAC_API const char * const FLAC__StreamDecoderWriteStatusString[] = {
 FLAC_API const char * const FLAC__StreamDecoderErrorStatusString[] = {
 	"FLAC__STREAM_DECODER_ERROR_STATUS_LOST_SYNC",
 	"FLAC__STREAM_DECODER_ERROR_STATUS_BAD_HEADER",
-	"FLAC__STREAM_DECODER_ERROR_STATUS_FRAME_CRC_MISMATCH"
+	"FLAC__STREAM_DECODER_ERROR_STATUS_FRAME_CRC_MISMATCH",
+	"FLAC__STREAM_DECODER_ERROR_STATUS_UNPARSEABLE_STREAM"
 };
 
 /***********************************************************************
@@ -1399,7 +1399,7 @@ FLAC__bool read_frame_(FLAC__StreamDecoder *decoder, FLAC__bool *got_a_frame, FL
 
 	if(!read_frame_header_(decoder))
 		return false;
-	if(decoder->protected_->state == FLAC__STREAM_DECODER_SEARCH_FOR_FRAME_SYNC)
+	if(decoder->protected_->state == FLAC__STREAM_DECODER_SEARCH_FOR_FRAME_SYNC) /* means we didn't sync on a valid header */
 		return true;
 	if(!allocate_output_(decoder, decoder->private_->frame.header.blocksize, decoder->private_->frame.header.channels))
 		return false;
@@ -1435,10 +1435,8 @@ FLAC__bool read_frame_(FLAC__StreamDecoder *decoder, FLAC__bool *got_a_frame, FL
 		 */
 		if(!read_subframe_(decoder, channel, bps, do_full_decode))
 			return false;
-		if(decoder->protected_->state != FLAC__STREAM_DECODER_READ_FRAME) {
-			decoder->protected_->state = FLAC__STREAM_DECODER_SEARCH_FOR_FRAME_SYNC;
+		if(decoder->protected_->state == FLAC__STREAM_DECODER_SEARCH_FOR_FRAME_SYNC) /* means bad sync or got corruption */
 			return true;
-		}
 	}
 	if(!read_zero_padding_(decoder))
 		return false;
@@ -1813,8 +1811,9 @@ FLAC__bool read_frame_header_(FLAC__StreamDecoder *decoder)
 	}
 
 	if(is_unparseable) {
-		decoder->protected_->state = FLAC__STREAM_DECODER_UNPARSEABLE_STREAM;
-		return false;
+		decoder->private_->error_callback(decoder, FLAC__STREAM_DECODER_ERROR_STATUS_UNPARSEABLE_STREAM, decoder->private_->client_data);
+		decoder->protected_->state = FLAC__STREAM_DECODER_SEARCH_FOR_FRAME_SYNC;
+		return true;
 	}
 
 	return true;
@@ -1824,6 +1823,7 @@ FLAC__bool read_subframe_(FLAC__StreamDecoder *decoder, unsigned channel, unsign
 {
 	FLAC__uint32 x;
 	FLAC__bool wasted_bits;
+	unsigned i;
 
 	if(!FLAC__bitbuffer_read_raw_uint32(decoder->private_->input, &x, 8, read_callback_, decoder)) /* MAGIC NUMBER */
 		return false; /* the read_callback_ sets the state for us */
@@ -1858,24 +1858,29 @@ FLAC__bool read_subframe_(FLAC__StreamDecoder *decoder, unsigned channel, unsign
 			return false;
 	}
 	else if(x < 16) {
-		decoder->protected_->state = FLAC__STREAM_DECODER_UNPARSEABLE_STREAM;
-		return false;
+		decoder->private_->error_callback(decoder, FLAC__STREAM_DECODER_ERROR_STATUS_UNPARSEABLE_STREAM, decoder->private_->client_data);
+		decoder->protected_->state = FLAC__STREAM_DECODER_SEARCH_FOR_FRAME_SYNC;
+		return true;
 	}
 	else if(x <= 24) {
 		if(!read_subframe_fixed_(decoder, channel, bps, (x>>1)&7, do_full_decode))
 			return false;
+		if(decoder->protected_->state == FLAC__STREAM_DECODER_SEARCH_FOR_FRAME_SYNC) /* means bad sync or got corruption */
+			return true;
 	}
 	else if(x < 64) {
-		decoder->protected_->state = FLAC__STREAM_DECODER_UNPARSEABLE_STREAM;
-		return false;
+		decoder->private_->error_callback(decoder, FLAC__STREAM_DECODER_ERROR_STATUS_UNPARSEABLE_STREAM, decoder->private_->client_data);
+		decoder->protected_->state = FLAC__STREAM_DECODER_SEARCH_FOR_FRAME_SYNC;
+		return true;
 	}
 	else {
 		if(!read_subframe_lpc_(decoder, channel, bps, ((x>>1)&31)+1, do_full_decode))
 			return false;
+		if(decoder->protected_->state == FLAC__STREAM_DECODER_SEARCH_FOR_FRAME_SYNC) /* means bad sync or got corruption */
+			return true;
 	}
 
 	if(wasted_bits && do_full_decode) {
-		unsigned i;
 		x = decoder->private_->frame.subframes[channel].wasted_bits;
 		for(i = 0; i < decoder->private_->frame.header.blocksize; i++)
 			decoder->private_->output[channel][i] <<= x;
@@ -1938,8 +1943,9 @@ FLAC__bool read_subframe_fixed_(FLAC__StreamDecoder *decoder, unsigned channel,
 			subframe->entropy_coding_method.data.partitioned_rice.contents = &decoder->private_->partitioned_rice_contents[channel];
 			break;
 		default:
-			decoder->protected_->state = FLAC__STREAM_DECODER_UNPARSEABLE_STREAM;
-			return false;
+			decoder->private_->error_callback(decoder, FLAC__STREAM_DECODER_ERROR_STATUS_UNPARSEABLE_STREAM, decoder->private_->client_data);
+			decoder->protected_->state = FLAC__STREAM_DECODER_SEARCH_FOR_FRAME_SYNC;
+			return true;
 	}
 
 	/* read residual */
@@ -2014,8 +2020,9 @@ FLAC__bool read_subframe_lpc_(FLAC__StreamDecoder *decoder, unsigned channel, un
 			subframe->entropy_coding_method.data.partitioned_rice.contents = &decoder->private_->partitioned_rice_contents[channel];
 			break;
 		default:
-			decoder->protected_->state = FLAC__STREAM_DECODER_UNPARSEABLE_STREAM;
-			return false;
+			decoder->private_->error_callback(decoder, FLAC__STREAM_DECODER_ERROR_STATUS_UNPARSEABLE_STREAM, decoder->private_->client_data);
+			decoder->protected_->state = FLAC__STREAM_DECODER_SEARCH_FOR_FRAME_SYNC;
+			return true;
 	}
 
 	/* read residual */