[FFmpeg-devel] [PATCH] IFF: Add error checking to byterun1 decoder

[Ronald S. Bultje]

Hi,

On Sun, May 16, 2010 at 6:35 PM, Sebastian Vater
<cdgs.basty at googlemail.com> wrote:
> Stefano Sabatini a ?crit :
>> On date Monday 2010-05-17 00:08:55 +0200, Sebastian Vater encoded:
>>
>>> @@ -306,7 +313,7 @@ static int decode_frame_byterun1(AVCodecContext *avctx,
>>> ? ? ?const uint8_t *buf = avpkt->data;
>>> ? ? ?int buf_size = avpkt->size;
>>> ? ? ?const uint8_t *buf_end = buf+buf_size;
>>> - ? ?int y, plane;
>>> + ? ?int y, plane, err;
>>>
>>> ? ? ?if (avctx->reget_buffer(avctx, &s->frame) < 0){
>>> ? ? ? ? ?av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n");
>>> @@ -319,7 +326,11 @@ static int decode_frame_byterun1(AVCodecContext *avctx,
>>> ? ? ? ? ? ? ? ? ?uint8_t *row = &s->frame.data[0][ y*s->frame.linesize[0] ];
>>> ? ? ? ? ? ? ? ? ?memset(row, 0, avctx->width);
>>> ? ? ? ? ? ? ? ? ?for (plane = 0; plane < avctx->bits_per_coded_sample; plane++) {
>>> - ? ? ? ? ? ? ? ? ? ?buf += decode_byterun(s->planebuf, s->planesize, buf, buf_end);
>>> + ? ? ? ? ? ? ? ? ? ?if ((err = decode_byterun(s->planebuf, s->planesize, buf, buf_end)) < 0) {
>>> + ? ? ? ? ? ? ? ? ? ? ? ?av_log(avctx, AV_LOG_ERROR, "IFF byterun1 stream truncated\n");
>>> + ? ? ? ? ? ? ? ? ? ? ? ?return err;
>>> + ? ? ? ? ? ? ? ? ? ?}
>>> + ? ? ? ? ? ? ? ? ? ?buf += err;
>>>
>>
>> buf += err looks strange, buf += ret should look saner.
>>
>
> Fixed by changing err(or) to res(ult).
[..]
> @@ -336,7 +351,11 @@ static int decode_frame_byterun1(AVCodecContext *avctx,
>      } else {
>          for(y = 0; y < avctx->height ; y++ ) {
>              uint8_t *row = &s->frame.data[0][y*s->frame.linesize[0]];
> -            buf += decode_byterun(row, avctx->width, buf, buf_end);
> +            if ((res = decode_byterun(row, avctx->width, buf, buf_end)) < 0) {
> +                av_log(avctx, AV_LOG_ERROR, "IFF byterun1 stream truncated\n");
> +                return res;
> +            }
> +            buf += res;
>          }
>      }

You could also pass &buf as a function argument, have decode_byterun()
update buf internally, so this duplication is removed also. Then
return 0 on success or <0 on error like any other FFmpeg function.

And I agree with Stefano that the log msg should be in
decode_byterun() also, a goto here is fine with me.

Ronald