[FFmpeg-devel] libavfilter-soc: negate filter artifacts

[Stefano Sabatini]

On date Friday 2009-07-24 01:58:07 +0200, Stefano Sabatini encoded:
> On date Monday 2009-07-20 05:55:41 +0200, Vitor Sessak encoded:
> > Stefano Sabatini wrote:
> >> Hi,
> >>
> >> I wonder what's the point of the default offesets assigned in the config_props:
> >> static int config_props(AVFilterLink *link)
> >> {
> >>     NegContext *neg = link->dst->priv;
> >>
> >>     avcodec_get_chroma_sub_sample(link->format, &neg->hsub, &neg->vsub);
> >>
> >>     switch(link->format) {
> >>     case PIX_FMT_YUVJ444P:
> >>     case PIX_FMT_YUVJ422P:
> >>     case PIX_FMT_YUVJ420P:
> >>     case PIX_FMT_YUVJ440P:
> >>         neg->offY  =
> >>         neg->offUV = 0;
> >>         break;
> >>     default:
> >>         neg->offY  = -4;
> >>         neg->offUV = 1;
> >>     }
> >>
> >>     return 0;
> >> }
> >
> > I think the problem here is that YUV (and not YUVJ) has Y data in the  
> > 16...235 range. So ideally one would negate 235 <-> 16 (see  
> > imgconvert.c:47). But as you show in your message, the code is buggy.
> 
> Check the patch attached if it looks reasonable.
> 
> Also in order to make the else if .. switch just one time, I'm
> considering to use a pointer to draw_slice for each different
> draw_slice algorithm used (see attached), I still have to see how that
> would perform with respect to the current solution.
> 
> Regards.
> -- 
> FFmpeg = Furious & Foolish Moronic Perennial Esoteric Generator

> Index: ffmpeg-vfilters/ffmpeg/libavfilter/vf_negate.c
> ===================================================================
> --- ffmpeg-vfilters.orig/ffmpeg/libavfilter/vf_negate.c	2009-07-21 01:38:12.000000000 +0200
> +++ ffmpeg-vfilters/ffmpeg/libavfilter/vf_negate.c	2009-07-24 01:30:56.000000000 +0200
> @@ -46,22 +46,16 @@
>  
>      avcodec_get_chroma_sub_sample(link->format, &neg->hsub, &neg->vsub);
>  
> -    switch(link->format) {
> -    case PIX_FMT_YUVJ444P:
> -    case PIX_FMT_YUVJ422P:
> -    case PIX_FMT_YUVJ420P:
> -    case PIX_FMT_YUVJ440P:
> -        neg->offY  =
> -        neg->offUV = 0;
> -        break;
> -    default:
> -        neg->offY  = -4;
> -        neg->offUV = 1;
> -    }
> -
>      return 0;
>  }
>  
> +/*
> + * When computing the inverted value for YUV colorspaces we use the equation:
> + * MID - X = X' - MID => X' = MID * 2 - X
> + * the following array contains the MID * 2 value for each YUB component
> + */
> +static unsigned int yuv_range_mid2_per_plane[3]  = { 251, 271, 240 };
> +
>  static void draw_slice(AVFilterLink *link, int y, int h)
>  {
>      NegContext *neg = link->dst->priv;
> @@ -79,13 +73,15 @@
>              inrow  += in-> linesize[0];
>              outrow += out->linesize[0];
>          }
> -    } else {
> +    } else if (link->format == PIX_FMT_YUVJ444P ||
> +               link->format == PIX_FMT_YUVJ422P ||
> +               link->format == PIX_FMT_YUVJ420P) {
>          /* luma plane */
>          inrow  = in-> data[0] + y * in-> linesize[0];
>          outrow = out->data[0] + y * out->linesize[0];
>          for(i = 0; i < h; i ++) {
>              for(j = 0; j < link->w; j ++)
> -                outrow[j] = 255 - inrow[j] + neg->offY;
> +                outrow[j] = 255 - inrow[j];
>              inrow  += in-> linesize[0];
>              outrow += out->linesize[0];
>          }
> @@ -97,7 +93,32 @@
>  
>              for(i = 0; i < h >> neg->vsub; i ++) {
>                  for(j = 0; j < link->w >> neg->hsub; j ++)
> -                    outrow[j] = 255 - inrow[j] + neg->offUV;
> +                    outrow[j] = 255 - inrow[j];
> +                inrow  += in-> linesize[plane];
> +                outrow += out->linesize[plane];
> +            }
> +        }
> +    } else {
> +#define MID2(plane)   yuv_range_mid2_per_plane[plane]
> +        /* luma plane */
> +        inrow  = in-> data[0] + y * in-> linesize[0];
> +        outrow = out->data[0] + y * out->linesize[0];
> +        for(i = 0; i < h; i++) {
> +            for(j = 0; j < link->w; j++)
> +                outrow[j] = MID2(0) - inrow[j];
> +            inrow  += in-> linesize[0];
> +            outrow += out->linesize[0];
> +        }
> +
> +        /* chroma planes */
> +        for(plane = 1; plane < 3; plane ++) {
> +            inrow  = in-> data[plane] + (y >> neg->vsub) * in-> linesize[plane];
> +            outrow = out->data[plane] + (y >> neg->vsub) * out->linesize[plane];
> +
> +            for(i = 0; i < h >> neg->vsub; i ++) {
> +                for(j = 0; j < link->w >> neg->hsub; j ++) {
> +                    outrow[j] = MID2(plane) - inrow[j];
> +                }
>                  inrow  += in-> linesize[plane];
>                  outrow += out->linesize[plane];
>              }

This clearly can be factorized as I realized just today, writing
patches at late night doesn't help...

Regards.
-- 
FFmpeg = Freak Faboulous Mysterious Patchable Elastic God