[FFmpeg-devel] [PATCH 2/6] idcinvideo: if decoding fails return error

[Paul B Mahol]

On 10/17/12, Stefano Sabatini <stefasab at gmail.com> wrote:
> On date Wednesday 2012-10-17 15:55:15 +0000, Paul B Mahol encoded:
>> While here return meaningful error codes.
>>
>> Signed-off-by: Paul B Mahol <onemda at gmail.com>
>> ---
>>  libavcodec/idcinvideo.c     | 14 +++++++++-----
>>  tests/ref/fate/id-cin-video |  1 -
>>  2 files changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/libavcodec/idcinvideo.c b/libavcodec/idcinvideo.c
>> index eedc4fc..9185f88 100644
>> --- a/libavcodec/idcinvideo.c
>> +++ b/libavcodec/idcinvideo.c
>> @@ -173,7 +173,7 @@ static av_cold int idcin_decode_init(AVCodecContext
>> *avctx)
>>      return 0;
>>  }
>>
>> -static void idcin_decode_vlcs(IdcinContext *s)
>> +static int idcin_decode_vlcs(IdcinContext *s)
>>  {
>>      hnode *hnodes;
>>      long x, y;
>> @@ -192,7 +192,7 @@ static void idcin_decode_vlcs(IdcinContext *s)
>>                  if(!bit_pos) {
>>                      if(dat_pos >= s->size) {
>>                          av_log(s->avctx, AV_LOG_ERROR, "Huffman decode
>> error.\n");
>> -                        return;
>> +                        return -1;
>
> Uhm return AVERROR_INVALIDDATA?

Not much point, see bellow.
>
>>                      }
>>                      bit_pos = 8;
>>                      v = s->buf[dat_pos++];
>> @@ -207,6 +207,8 @@ static void idcin_decode_vlcs(IdcinContext *s)
>>              prev = node_num;
>>          }
>>      }
>> +
>> +    return 0;
>>  }
>>
>>  static int idcin_decode_frame(AVCodecContext *avctx,
>> @@ -217,6 +219,7 @@ static int idcin_decode_frame(AVCodecContext *avctx,
>>      int buf_size = avpkt->size;
>>      IdcinContext *s = avctx->priv_data;
>>      const uint8_t *pal = av_packet_get_side_data(avpkt,
>> AV_PKT_DATA_PALETTE, NULL);
>> +    int ret;
>>
>>      s->buf = buf;
>>      s->size = buf_size;
>> @@ -224,12 +227,13 @@ static int idcin_decode_frame(AVCodecContext
>> *avctx,
>>      if (s->frame.data[0])
>>          avctx->release_buffer(avctx, &s->frame);
>>
>> -    if (avctx->get_buffer(avctx, &s->frame)) {
>> +    if ((ret = avctx->get_buffer(avctx, &s->frame))) {
>>          av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n");
>> -        return -1;
>> +        return ret;
>>      }
>>
>> -    idcin_decode_vlcs(s);
>> +    if (idcin_decode_vlcs(s))
>> +        return AVERROR_INVALIDDATA;
>
> While at it you could propagate the error code.

no much point, it always fails with same error code.
>
>>
>>      if (pal) {
>>          s->frame.palette_has_changed = 1;
>> diff --git a/tests/ref/fate/id-cin-video b/tests/ref/fate/id-cin-video
>> index 241cae5..f55544f 100644
>> --- a/tests/ref/fate/id-cin-video
>> +++ b/tests/ref/fate/id-cin-video
>> @@ -104,4 +104,3 @@
>>  1,      78750,      78750,     1575,     6300, 0xe3bfa403
>>  0,         51,         51,        1,   230400, 0x488de02d
>>  1,      80325,      80325,     1575,     6300, 0x2c5bd9c9
>> -0,         52,         52,        1,   230400, 0x488de02d
>
> Is this expected? Could you explain it in the commit log?

Last video frame is truncated, previously you would get error msg if decoding
failed but it would be ingnored - aka no error detection/resilence.

> --
> FFmpeg = Fiendish & Fundamental Monstrous Plastic Elastic Gangster
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>