[FFmpeg-cvslog] r21439 - trunk/libavcodec/ac3dec.c

[Reimar Döffinger]

On Mon, Jan 25, 2010 at 12:55:33AM +0100, jbr wrote:
> Author: jbr
> Date: Mon Jan 25 00:55:33 2010
> New Revision: 21439
> 
> Log:
> Only check frame size if the header is valid.
> 
> Modified:
>    trunk/libavcodec/ac3dec.c
> 
> Modified: trunk/libavcodec/ac3dec.c
> ==============================================================================
> --- trunk/libavcodec/ac3dec.c	Sun Jan 24 23:47:50 2010	(r21438)
> +++ trunk/libavcodec/ac3dec.c	Mon Jan 25 00:55:33 2010	(r21439)
> @@ -1237,7 +1237,7 @@ static int ac3_decode_frame(AVCodecConte
>      err = parse_frame_header(s);
>  
>      /* check that reported frame size fits in input buffer */
> -    if(s->frame_size > buf_size) {
> +    if(!err && s->frame_size > buf_size) {
>          av_log(avctx, AV_LOG_ERROR, "incomplete frame\n");
>          err = AAC_AC3_PARSE_ERROR_FRAME_SIZE;
>      }

Well, maybe that's better, but I think all that error handling is worth reconsidering.
Because even now, the crc check will override the frame sync error - not to mention
that I can't see why the CRC case gets a special case handling when there is nothing
special with it.
Also AAC_AC3_PARSE_ERROR_FRAME_SIZE prints 2 messages by default, which IMO makes
little sense as well.
Now just extracting the AAC_AC3_PARSE_ERROR_SYNC handling out right behind the
parse_frame_header seems like it would be a easy, reliable and maintainable
solutions, but it would also be possible to make the errors use powers of two
and or them together or sort them by how critical they are and use FFMAX for example.