diff mbox

[FFmpeg-devel,v2] avfilter/vf_delogo: make the interp value compute method simple

Message ID 20191007051246.93315-1-lq@chinaffmpeg.org
State Superseded
Headers show

Commit Message

Liu Steven Oct. 7, 2019, 5:12 a.m. UTC
because the interp >= 0UL comparison of an unsigned value is always true
fix CID: 1454642

Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
---
 libavfilter/vf_delogo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Lance Wang Oct. 7, 2019, 3:07 p.m. UTC | #1
On Mon, Oct 07, 2019 at 01:12:46PM +0800, Steven Liu wrote:
> because the interp >= 0UL comparison of an unsigned value is always true
> fix CID: 1454642
> 
> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> ---
>  libavfilter/vf_delogo.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavfilter/vf_delogo.c b/libavfilter/vf_delogo.c
> index 376c5e850f..3678548353 100644
> --- a/libavfilter/vf_delogo.c
> +++ b/libavfilter/vf_delogo.c
> @@ -168,7 +168,7 @@ static void apply_delogo(uint8_t *dst, int dst_linesize,
>                   botleft[x-logo_x1-1]  +
>                   botleft[x-logo_x1+1]) * weightb;
>              weight = (weightl + weightr + weightt + weightb) * 3U;
> -            interp = ROUNDED_DIV(interp, weight);
> +            interp = (interp + weight >> 1) / weight;

By the macros definition:
#define ROUNDED_DIV(a,b) (((a)>0 ? (a) + ((b)>>1) : (a) - ((b)>>1))/(b))

I feel it should be:
interp = (interp - weight >> 1) / weight;

instead of: 
interp = (interp + weight >> 1) / weight;


>  
>              if (y >= logo_y+band && y < logo_y+logo_h-band &&
>                  x >= logo_x+band && x < logo_x+logo_w-band) {
> -- 
> 2.17.2 (Apple Git-113)
> 
> 
> 
> _______________________________________________
> 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".
Michael Niedermayer Oct. 7, 2019, 7:29 p.m. UTC | #2
On Mon, Oct 07, 2019 at 01:12:46PM +0800, Steven Liu wrote:
> because the interp >= 0UL comparison of an unsigned value is always true
> fix CID: 1454642
> 
> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> ---
>  libavfilter/vf_delogo.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

this changes fate-filter-delogo, if thats intended it needs to be updated

thx

[...]
mypopy@gmail.com Oct. 8, 2019, 2:47 a.m. UTC | #3
On Mon, Oct 7, 2019 at 11:14 PM Limin Wang <lance.lmwang@gmail.com> wrote:
>
> On Mon, Oct 07, 2019 at 01:12:46PM +0800, Steven Liu wrote:
> > because the interp >= 0UL comparison of an unsigned value is always true
> > fix CID: 1454642
> >
> > Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> > ---
> >  libavfilter/vf_delogo.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavfilter/vf_delogo.c b/libavfilter/vf_delogo.c
> > index 376c5e850f..3678548353 100644
> > --- a/libavfilter/vf_delogo.c
> > +++ b/libavfilter/vf_delogo.c
> > @@ -168,7 +168,7 @@ static void apply_delogo(uint8_t *dst, int dst_linesize,
> >                   botleft[x-logo_x1-1]  +
> >                   botleft[x-logo_x1+1]) * weightb;
> >              weight = (weightl + weightr + weightt + weightb) * 3U;
> > -            interp = ROUNDED_DIV(interp, weight);
> > +            interp = (interp + weight >> 1) / weight;
>
> By the macros definition:
> #define ROUNDED_DIV(a,b) (((a)>0 ? (a) + ((b)>>1) : (a) - ((b)>>1))/(b))
>
I've update the ROUNDED_DIV to:
#define ROUNDED_DIV(a,b) (((a)>=0 ? (a) + ((b)>>1) : (a) -
((b)>>1))/(b)), I guess
you didn't update the code with this change
> I feel it should be:
> interp = (interp - weight >> 1) / weight;
>
> instead of:
> interp = (interp + weight >> 1) / weight;
>

> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Lance Wang Oct. 8, 2019, 3:02 a.m. UTC | #4
On Tue, Oct 08, 2019 at 10:47:56AM +0800, mypopy@gmail.com wrote:
> On Mon, Oct 7, 2019 at 11:14 PM Limin Wang <lance.lmwang@gmail.com> wrote:
> >
> > On Mon, Oct 07, 2019 at 01:12:46PM +0800, Steven Liu wrote:
> > > because the interp >= 0UL comparison of an unsigned value is always true
> > > fix CID: 1454642
> > >
> > > Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> > > ---
> > >  libavfilter/vf_delogo.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/libavfilter/vf_delogo.c b/libavfilter/vf_delogo.c
> > > index 376c5e850f..3678548353 100644
> > > --- a/libavfilter/vf_delogo.c
> > > +++ b/libavfilter/vf_delogo.c
> > > @@ -168,7 +168,7 @@ static void apply_delogo(uint8_t *dst, int dst_linesize,
> > >                   botleft[x-logo_x1-1]  +
> > >                   botleft[x-logo_x1+1]) * weightb;
> > >              weight = (weightl + weightr + weightt + weightb) * 3U;
> > > -            interp = ROUNDED_DIV(interp, weight);
> > > +            interp = (interp + weight >> 1) / weight;
> >
> > By the macros definition:
> > #define ROUNDED_DIV(a,b) (((a)>0 ? (a) + ((b)>>1) : (a) - ((b)>>1))/(b))
> >
> I've update the ROUNDED_DIV to:
> #define ROUNDED_DIV(a,b) (((a)>=0 ? (a) + ((b)>>1) : (a) -
> ((b)>>1))/(b)), I guess
> you didn't update the code with this change

Yeah, I'll fetch and update master.

> > I feel it should be:
> > interp = (interp - weight >> 1) / weight;
> >
> > instead of:
> > interp = (interp + weight >> 1) / weight;
> >
> 
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> 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/libavfilter/vf_delogo.c b/libavfilter/vf_delogo.c
index 376c5e850f..3678548353 100644
--- a/libavfilter/vf_delogo.c
+++ b/libavfilter/vf_delogo.c
@@ -168,7 +168,7 @@  static void apply_delogo(uint8_t *dst, int dst_linesize,
                  botleft[x-logo_x1-1]  +
                  botleft[x-logo_x1+1]) * weightb;
             weight = (weightl + weightr + weightt + weightb) * 3U;
-            interp = ROUNDED_DIV(interp, weight);
+            interp = (interp + weight >> 1) / weight;
 
             if (y >= logo_y+band && y < logo_y+logo_h-band &&
                 x >= logo_x+band && x < logo_x+logo_w-band) {