On Thu, May 07, 2020 at 08:34:44PM +0200, Marton Balint wrote:
>
>
> 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.
thanks for review, will update to add fail_nomem label.
>
> 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".
--
Thanks,
Limin Wang
_______________________________________________
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".
