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

[Jean Delvare]

Hi Stefano,

On Wed, 3 Jul 2013 08:54:59 +0200, Stefano Sabatini wrote:
> 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?

This is the plan, yes, in a subsequent patch. The current code has other
issues when horizontal and vertical subsampling factors differ, which
need to be fixed first. This is on my list, but not at the top of it
because these issues do not affect my own workflow (I work with YUV
4:2:0 input files.)

-- 
Jean Delvare