[FFmpeg-devel] [PATCH v3] lavfi/delogo: take SAR into account

[Stefano Sabatini]

On date Monday 2013-07-01 16:28:59 +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>
> ---
> Changes since v2:
>  * Adjusted the subject.
>  * Don't use non-portable "? :" construct.
>  * Alter sar.den in place.
>  * Alter sar.den in the caller rather than in apply_delogo(), so we
>    only do it once per frame.
> 
>  libavfilter/vf_delogo.c |   20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> --- ffmpeg.orig/libavfilter/vf_delogo.c	2013-06-28 09:41:50.291495611 +0200
> +++ ffmpeg/libavfilter/vf_delogo.c	2013-06-28 10:45:42.496634606 +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)
>  {
> @@ -93,11 +93,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;
> +            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)]   +
> @@ -217,6 +217,7 @@ static int filter_frame(AVFilterLink *in
>      int vsub0 = desc->log2_chroma_h;
>      int direct = 0;
>      int plane;
> +    AVRational sar;
>  
>      if (av_frame_is_writable(in)) {
>          direct = 1;
> @@ -231,6 +232,11 @@ static int filter_frame(AVFilterLink *in
>          av_frame_copy_props(out, in);
>      }
>  
> +    sar = in->sample_aspect_ratio;
> +    /* Assume square pixels if SAR is unknown */
> +    if (!sar.num)
> +        sar.num = sar.den = 1;
> +
>      for (plane = 0; plane < 4 && in->data[plane]; plane++) {
>          int hsub = plane == 1 || plane == 2 ? hsub0 : 0;
>          int vsub = plane == 1 || plane == 2 ? vsub0 : 0;
> @@ -239,7 +245,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),
> -                     s->x>>hsub, s->y>>vsub,
> +                     sar, s->x>>hsub, s->y>>vsub,
>                       FF_CEIL_RSHIFT(s->w, hsub),
>                       FF_CEIL_RSHIFT(s->h, vsub),
>                       s->band>>FFMIN(hsub, vsub),

Shouldn't you consider chroma subsampling as well?
-- 
FFmpeg = Funny & Fantastic Mind-dumbing Pitiful Egregious Generator