[FFmpeg-devel,v2,1/2] avcodec/v210dec: removed the duplicated 'if' condition

Submitted by lance.lmwang@gmail.com on Sept. 6, 2019, 3:28 p.m.

Details

Message ID 20190906152829.639-1-lance.lmwang@gmail.com
State New
Headers show

Commit Message

lance.lmwang@gmail.com Sept. 6, 2019, 3:28 p.m.
From: Limin Wang <lance.lmwang@gmail.com>

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

Comments

Michael Niedermayer Oct. 11, 2019, 7:07 p.m.
On Fri, Sep 06, 2019 at 11:28:28PM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavcodec/v210dec.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/libavcodec/v210dec.c b/libavcodec/v210dec.c
> index 5a33d8c..6ce18aa 100644
> --- a/libavcodec/v210dec.c
> +++ b/libavcodec/v210dec.c
> @@ -98,8 +98,7 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
>              return AVERROR_INVALIDDATA;
>          }
>      }
> -    if (   avctx->codec_tag == MKTAG('C', '2', '1', '0')
> -        && avpkt->size > 64
> +    if (avctx->codec_tag == MKTAG('C', '2', '1', '0')
>          && AV_RN32(psrc) == AV_RN32("INFO")
>          && avpkt->size - 64 >= stride * avctx->height)
>          psrc += 64;

Iam undecided on this, the change is correct but iam not sure
it makes the code easier to understand also it makes the code
less robust. For example the check is just unneeded as long
as the types of the variables arent changed

thx

[...]
lance.lmwang@gmail.com Oct. 12, 2019, 1:45 a.m.
On Fri, Oct 11, 2019 at 09:07:38PM +0200, Michael Niedermayer wrote:
> On Fri, Sep 06, 2019 at 11:28:28PM +0800, lance.lmwang@gmail.com wrote:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavcodec/v210dec.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/libavcodec/v210dec.c b/libavcodec/v210dec.c
> > index 5a33d8c..6ce18aa 100644
> > --- a/libavcodec/v210dec.c
> > +++ b/libavcodec/v210dec.c
> > @@ -98,8 +98,7 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
> >              return AVERROR_INVALIDDATA;
> >          }
> >      }
> > -    if (   avctx->codec_tag == MKTAG('C', '2', '1', '0')
> > -        && avpkt->size > 64
> > +    if (avctx->codec_tag == MKTAG('C', '2', '1', '0')
> >          && AV_RN32(psrc) == AV_RN32("INFO")
> >          && avpkt->size - 64 >= stride * avctx->height)
> >          psrc += 64;
> 
> Iam undecided on this, the change is correct but iam not sure
> it makes the code easier to understand also it makes the code
> less robust. For example the check is just unneeded as long
> as the types of the variables arent changed

thanks for the feedback, please ignore the patch anyway. I'll update patch#2 only.

> 
> thx
> 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> I know you won't believe me, but the highest form of Human Excellence is
> to question oneself and others. -- Socrates



> _______________________________________________
> 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".

Patch hide | download patch | download mbox

diff --git a/libavcodec/v210dec.c b/libavcodec/v210dec.c
index 5a33d8c..6ce18aa 100644
--- a/libavcodec/v210dec.c
+++ b/libavcodec/v210dec.c
@@ -98,8 +98,7 @@  static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
             return AVERROR_INVALIDDATA;
         }
     }
-    if (   avctx->codec_tag == MKTAG('C', '2', '1', '0')
-        && avpkt->size > 64
+    if (avctx->codec_tag == MKTAG('C', '2', '1', '0')
         && AV_RN32(psrc) == AV_RN32("INFO")
         && avpkt->size - 64 >= stride * avctx->height)
         psrc += 64;