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

[Stefano Sabatini]

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?

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

>  
>      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?
-- 
FFmpeg = Fiendish & Fundamental Monstrous Plastic Elastic Gangster