diff mbox

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

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

Commit Message

Limin Wang Sept. 6, 2019, 3:28 p.m. UTC
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. UTC | #1
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

[...]
Limin Wang Oct. 12, 2019, 1:45 a.m. UTC | #2
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".
diff mbox

Patch

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;