Re: [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.lmw...@gmail.com wrote:

From: Limin Wang <lance.lmw...@gmail.com>

Signed-off-by: Limin Wang <lance.lmw...@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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".