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

[Justin Ruggles]

Reimar D?ffinger wrote:

> 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.


I'm open to changing it.

> 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.

After looking through the error conditions I think I came up with
something that works better.  The errors from the parser are handled
first, and if there are no parser errors, the buf_size check is done,
and if that is ok, the crc check is done.

This way frame sync error prints once, frame size error prints once, and
it is a bit simpler I think since no error conditions override
eachother.  Patch attached.

Thanks,
Justin


-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ac3_error_handling.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20100125/53f98e82/attachment.asc>