Message ID | 1588850057-24236-4-git-send-email-lance.lmwang@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | [FFmpeg-devel,v2,1/5] avcodec/mpegvideo_enc: reindent code | expand |
Context | Check | Description |
---|---|---|
andriy/default | pending | |
andriy/make | success | Make finished |
andriy/make_fate | success | Make fate finished |
On Thu, 7 May 2020, lance.lmwang@gmail.com wrote: > From: Limin Wang <lance.lmwang@gmail.com> > > Signed-off-by: Limin Wang <lance.lmwang@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
On Thu, May 07, 2020 at 08:34:44PM +0200, Marton Balint wrote: > > > On Thu, 7 May 2020, lance.lmwang@gmail.com wrote: > > > From: Limin Wang <lance.lmwang@gmail.com> > > > > Signed-off-by: Limin Wang <lance.lmwang@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-request@ffmpeg.org with subject "unsubscribe".
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); 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; } /**