[FFmpeg-devel] [PATCH 2/2 v2] delogo: Take SAR into account

[Stefano Sabatini]

Note:
lavfi/delogo: take SAR into account

as subject would be more consistent with (my own) conventions.

On date Wednesday 2013-06-26 14:57:34 +0200, Jean Delvare encoded:
> When interpolating, weights are based on relative distances, which
> assume square pixels. If a non-1:1 sample aspect ratio is used, it
> should be taken into account when comparing distances, because the
> human eye and brain care about the picture as it is displayed, not
> stored.
> 
> Signed-off-by: Jean Delvare <khali at linux-fr.org>
> ---
> No functional change since v1, but rebasing was needed.
> 
>  libavfilter/version.h   |    2 +-
>  libavfilter/vf_delogo.c |   14 ++++++++------
>  2 files changed, 9 insertions(+), 7 deletions(-)
> 
> --- ffmpeg.orig/libavfilter/vf_delogo.c	2013-06-25 21:56:27.049663266 +0200
> +++ ffmpeg/libavfilter/vf_delogo.c	2013-06-25 22:00:58.615920845 +0200
> @@ -56,7 +56,7 @@
>   */
>  static void apply_delogo(uint8_t *dst, int dst_linesize,
>                           uint8_t *src, int src_linesize,
> -                         int w, int h,
> +                         int w, int h, AVRational sar,
>                           int logo_x, int logo_y, int logo_w, int logo_h,
>                           int band, int show, int direct)
>  {
> @@ -67,6 +67,7 @@ static void apply_delogo(uint8_t *dst, i
>      uint8_t *topleft, *botleft, *topright;
>      int xclipl, xclipr, yclipt, yclipb;
>      int logo_x1, logo_x2, logo_y1, logo_y2;

> +    int sar_num = sar.num ? : 1;

GNU extension, check:
http://en.wikipedia.org/wiki/%3F:#C

Potentially non portable.

Also you can modify the sar variable in place, and avoid the
additional temporary sar_num.

>  
>      xclipl = FFMAX(-logo_x, 0);
>      xclipr = FFMAX(logo_x+logo_w-w, 0);
> @@ -93,11 +94,11 @@ static void apply_delogo(uint8_t *dst, i
>               xdst = dst+logo_x1+1,
>               xsrc = src+logo_x1+1; x < logo_x2-1; x++, xdst++, xsrc++) {
>  
> -            /* Weighted interpolation based on relative distances */
> -            weightl = (uint64_t)              (logo_x2-1-x) * (y-logo_y1) * (logo_y2-1-y);
> -            weightr = (uint64_t)(x-logo_x1)                 * (y-logo_y1) * (logo_y2-1-y);
> -            weightt = (uint64_t)(x-logo_x1) * (logo_x2-1-x)               * (logo_y2-1-y);
> -            weightb = (uint64_t)(x-logo_x1) * (logo_x2-1-x) * (y-logo_y1);
> +            /* Weighted interpolation based on relative distances, taking SAR into account */

> +            weightl = (uint64_t)              (logo_x2-1-x) * (y-logo_y1) * (logo_y2-1-y) * sar.den;

I'd expect
(logo_x2-1-x)*sar.num * (y-logo_y1)*sar.den * (logo_y2-1-y)*sar.den

but maybe I'm missing something...

> +            weightr = (uint64_t)(x-logo_x1)                 * (y-logo_y1) * (logo_y2-1-y) * sar.den;
> +            weightt = (uint64_t)(x-logo_x1) * (logo_x2-1-x)               * (logo_y2-1-y) * sar_num;
> +            weightb = (uint64_t)(x-logo_x1) * (logo_x2-1-x) * (y-logo_y1)                 * sar_num;
>  
>              interp =
>                  (topleft[src_linesize*(y-logo_y  -yclipt)]   +
> @@ -239,6 +240,7 @@ static int filter_frame(AVFilterLink *in
>                       in ->data[plane], in ->linesize[plane],
>                       FF_CEIL_RSHIFT(inlink->w, hsub),
>                       FF_CEIL_RSHIFT(inlink->h, vsub),
> +                     in->sample_aspect_ratio,
>                       s->x>>hsub, s->y>>vsub,
>                       FF_CEIL_RSHIFT(s->w, hsub),
>                       FF_CEIL_RSHIFT(s->h, vsub),
> --- ffmpeg.orig/libavfilter/version.h	2013-06-25 18:13:56.702312485 +0200
> +++ ffmpeg/libavfilter/version.h	2013-06-25 21:59:34.304666042 +0200
> @@ -31,7 +31,7 @@
>  
>  #define LIBAVFILTER_VERSION_MAJOR  3
>  #define LIBAVFILTER_VERSION_MINOR  77
> -#define LIBAVFILTER_VERSION_MICRO 101
> +#define LIBAVFILTER_VERSION_MICRO 102

No need to bump version for this.
-- 
FFmpeg = Fierce & Fancy Most Perfectionist Elected Game