[FFmpeg-devel] [PATCH v2 4/5] avcodec/mpegvideo: return more specific error codes for ff_mpv_common_init()

[Marton Balint]

On Thu, 7 May 2020, lance.lmwang at gmail.com wrote:

> From: Limin Wang <lance.lmwang at gmail.com>
>
> Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
> ---
> libavcodec/mpegvideo.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
> index 22cab28..b5ddb1b 100644
> --- a/libavcodec/mpegvideo.c
> +++ b/libavcodec/mpegvideo.c
> @@ -889,7 +889,7 @@ static void clear_context(MpegEncContext *s)
>  */
> av_cold int ff_mpv_common_init(MpegEncContext *s)
> {
> -    int i, ret;
> +    int i, ret = AVERROR(ENOMEM);

This initialization is unnecessary here, you will overwrite ret later.

>     int nb_slices = (HAVE_THREADS &&
>                      s->avctx->active_thread_type & FF_THREAD_SLICE) ?
>                     s->avctx->thread_count : 1;
> @@ -907,7 +907,7 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
>     if (s->avctx->pix_fmt == AV_PIX_FMT_NONE) {
>         av_log(s->avctx, AV_LOG_ERROR,
>                "decoding to AV_PIX_FMT_NONE is not supported.\n");
> -        return -1;
> +        return AVERROR(EINVAL);
>     }
>
>     if (nb_slices > MAX_THREADS || (nb_slices > s->mb_height && s->mb_height)) {
> @@ -923,7 +923,7 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
>
>     if ((s->width || s->height) &&
>         av_image_check_size(s->width, s->height, 0, s->avctx))
> -        return -1;
> +        return AVERROR(EINVAL);
>
>     dct_init(s);
> 
> @@ -954,7 +954,7 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
>     if (!s->new_picture.f)
>         goto fail;
> 
> -    if (init_context_frame(s))
> +    if ((ret = init_context_frame(s)))
>         goto fail;
>
>     s->parse_context.state = -1;
> @@ -971,7 +971,7 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
>                 if (!s->thread_context[i])
>                     goto fail;
>             }
> -            if (init_duplicate_context(s->thread_context[i]) < 0)
> +            if ((ret = init_duplicate_context(s->thread_context[i])) < 0)
>                 goto fail;
>             s->thread_context[i]->start_mb_y =
>                 (s->mb_height * (i) + nb_slices / 2) / nb_slices;
> @@ -979,7 +979,7 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
>                 (s->mb_height * (i + 1) + nb_slices / 2) / nb_slices;
>         }
>     } else {
> -        if (init_duplicate_context(s) < 0)
> +        if ((ret = init_duplicate_context(s)) < 0)
>             goto fail;
>         s->start_mb_y = 0;
>         s->end_mb_y   = s->mb_height;
> @@ -990,7 +990,7 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
>     return 0;
>  fail:
>     ff_mpv_common_end(s);
> -    return -1;
> +    return ret

Not all goto fail set ret. I think it is cleaner if you add a new label 
"fail_nomem" before the fail label and set ret to ENOMEM there, and use 
that label for ENOMEM cases.

Regards,
Marton