diff mbox series

[FFmpeg-devel,5/5] avcodec/utils: simplify, remove duplicate code

Message ID 1588251554-3665-5-git-send-email-lance.lmwang@gmail.com
State Accepted
Commit 79e3c4dd7485e469b2d0e29e2775d4e37b55bfa7
Headers show
Series [FFmpeg-devel,1/5] avcodec/v4l2_m2m_enc: reindent code | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Lance Wang April 30, 2020, 12:59 p.m. UTC 
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavcodec/utils.c | 29 +++--------------------------
 1 file changed, 3 insertions(+), 26 deletions(-)

Comments

Andreas Rheinhardt April 30, 2020, 1:18 p.m. UTC | #1 
lance.lmwang@gmail.com:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavcodec/utils.c | 29 +++--------------------------
>  1 file changed, 3 insertions(+), 26 deletions(-)
> 
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index e77090daef..91b271a717 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -584,37 +584,14 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code
>      avctx->internal = avci;
>  
>      avci->to_free = av_frame_alloc();
> -    if (!avci->to_free) {
> -        ret = AVERROR(ENOMEM);
> -        goto free_and_end;
> -    }
> -
>      avci->compat_decode_frame = av_frame_alloc();
> -    if (!avci->compat_decode_frame) {
> -        ret = AVERROR(ENOMEM);
> -        goto free_and_end;
> -    }
> -
>      avci->buffer_frame = av_frame_alloc();
> -    if (!avci->buffer_frame) {
> -        ret = AVERROR(ENOMEM);
> -        goto free_and_end;
> -    }
> -
>      avci->buffer_pkt = av_packet_alloc();
> -    if (!avci->buffer_pkt) {
> -        ret = AVERROR(ENOMEM);
> -        goto free_and_end;
> -    }
> -
>      avci->ds.in_pkt = av_packet_alloc();
> -    if (!avci->ds.in_pkt) {
> -        ret = AVERROR(ENOMEM);
> -        goto free_and_end;
> -    }
> -
>      avci->last_pkt_props = av_packet_alloc();
> -    if (!avci->last_pkt_props) {
> +    if (!avci->to_free      || !avci->compat_decode_frame ||
> +        !avci->buffer_frame || !avci->buffer_pkt          ||
> +        !avci->ds.in_pkt    || !avci->last_pkt_props) {
>          ret = AVERROR(ENOMEM);
>          goto free_and_end;
>      }
> 
If you write this in the following way

if (!(avci->to_free = av_frame_alloc())             ||
    !(avci->compat_decode_frame = av_frame_alloc()) || ...) {
    ret = AVERROR(ENOMEM);
    goto free_and_end;
}

then shortcircuit evaluation will make sure that an error is detected
immediately (like now), but without the code duplication.

- Andreas
Lance Wang April 30, 2020, 1:28 p.m. UTC | #2 
On Thu, Apr 30, 2020 at 03:18:15PM +0200, Andreas Rheinhardt wrote:
> lance.lmwang@gmail.com:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavcodec/utils.c | 29 +++--------------------------
> >  1 file changed, 3 insertions(+), 26 deletions(-)
> > 
> > diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> > index e77090daef..91b271a717 100644
> > --- a/libavcodec/utils.c
> > +++ b/libavcodec/utils.c
> > @@ -584,37 +584,14 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code
> >      avctx->internal = avci;
> >  
> >      avci->to_free = av_frame_alloc();
> > -    if (!avci->to_free) {
> > -        ret = AVERROR(ENOMEM);
> > -        goto free_and_end;
> > -    }
> > -
> >      avci->compat_decode_frame = av_frame_alloc();
> > -    if (!avci->compat_decode_frame) {
> > -        ret = AVERROR(ENOMEM);
> > -        goto free_and_end;
> > -    }
> > -
> >      avci->buffer_frame = av_frame_alloc();
> > -    if (!avci->buffer_frame) {
> > -        ret = AVERROR(ENOMEM);
> > -        goto free_and_end;
> > -    }
> > -
> >      avci->buffer_pkt = av_packet_alloc();
> > -    if (!avci->buffer_pkt) {
> > -        ret = AVERROR(ENOMEM);
> > -        goto free_and_end;
> > -    }
> > -
> >      avci->ds.in_pkt = av_packet_alloc();
> > -    if (!avci->ds.in_pkt) {
> > -        ret = AVERROR(ENOMEM);
> > -        goto free_and_end;
> > -    }
> > -
> >      avci->last_pkt_props = av_packet_alloc();
> > -    if (!avci->last_pkt_props) {
> > +    if (!avci->to_free      || !avci->compat_decode_frame ||
> > +        !avci->buffer_frame || !avci->buffer_pkt          ||
> > +        !avci->ds.in_pkt    || !avci->last_pkt_props) {
> >          ret = AVERROR(ENOMEM);
> >          goto free_and_end;
> >      }
> > 
> If you write this in the following way
> 
> if (!(avci->to_free = av_frame_alloc())             ||
>     !(avci->compat_decode_frame = av_frame_alloc()) || ...) {
>     ret = AVERROR(ENOMEM);
>     goto free_and_end;
> }
> 
> then shortcircuit evaluation will make sure that an error is detected
> immediately (like now), but without the code duplication.

Yeah, it's better, I'll update it.

> 
> - Andreas
> _______________________________________________
> 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 mbox series

Patch

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index e77090daef..91b271a717 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -584,37 +584,14 @@  int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code
     avctx->internal = avci;
 
     avci->to_free = av_frame_alloc();
-    if (!avci->to_free) {
-        ret = AVERROR(ENOMEM);
-        goto free_and_end;
-    }
-
     avci->compat_decode_frame = av_frame_alloc();
-    if (!avci->compat_decode_frame) {
-        ret = AVERROR(ENOMEM);
-        goto free_and_end;
-    }
-
     avci->buffer_frame = av_frame_alloc();
-    if (!avci->buffer_frame) {
-        ret = AVERROR(ENOMEM);
-        goto free_and_end;
-    }
-
     avci->buffer_pkt = av_packet_alloc();
-    if (!avci->buffer_pkt) {
-        ret = AVERROR(ENOMEM);
-        goto free_and_end;
-    }
-
     avci->ds.in_pkt = av_packet_alloc();
-    if (!avci->ds.in_pkt) {
-        ret = AVERROR(ENOMEM);
-        goto free_and_end;
-    }
-
     avci->last_pkt_props = av_packet_alloc();
-    if (!avci->last_pkt_props) {
+    if (!avci->to_free      || !avci->compat_decode_frame ||
+        !avci->buffer_frame || !avci->buffer_pkt          ||
+        !avci->ds.in_pkt    || !avci->last_pkt_props) {
         ret = AVERROR(ENOMEM);
         goto free_and_end;
     }