[FFmpeg-devel] [PATCH] lavfi: port decimate libmpcodecs filter

[Stefano Sabatini]

On date Sunday 2012-08-19 20:22:20 +0200, Nicolas George encoded:
> Le tridi 3 fructidor, an CCXX, Stefano Sabatini a écrit :
> > >From 7379459869bb671282e7f50d4bf7ba44c05ef0a5 Mon Sep 17 00:00:00 2001
> > From: Stefano Sabatini <stefasab at gmail.com>
> > Date: Sat, 17 Mar 2012 15:49:51 +0100
> > Subject: [PATCH] lavfi: port decimate libmpcodecs filter
> > 
> > ---
> >  configure                 |    1 +
> >  doc/filters.texi          |   34 +++++++
> >  libavfilter/Makefile      |    1 +
> >  libavfilter/allfilters.c  |    1 +
> >  libavfilter/diff.c        |   35 +++++++
> >  libavfilter/diff.h        |   27 +++++
> >  libavfilter/vf_decimate.c |  243 +++++++++++++++++++++++++++++++++++++++++++++
> >  libavfilter/x86/Makefile  |    2 +
> >  libavfilter/x86/diff.c    |   63 ++++++++++++
> >  9 files changed, 407 insertions(+), 0 deletions(-)
> >  create mode 100644 libavfilter/diff.c
> >  create mode 100644 libavfilter/diff.h
> >  create mode 100644 libavfilter/vf_decimate.c
> >  create mode 100644 libavfilter/x86/diff.c
[...]
> > diff --git a/libavfilter/diff.c b/libavfilter/diff.c
> > new file mode 100644
> > index 0000000..4e748ec
> > --- /dev/null
> > +++ b/libavfilter/diff.c
> > @@ -0,0 +1,35 @@
> > +/*
> 
> > + * Copyright (c) 2003 Rich Felker
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> 
> The original code is under GPL, but you clearly reimplemented it. Probably
> alter the copyright notice.

Yes, I removed the copyright notice, code is trivial anyway.

> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > + */
> > +
> > +#include "diff.h"
> > +
> > +int ff_diff_8x8_c(const uint8_t *cur, int cur_linesize,
> > +                  const uint8_t *ref, int ref_linesize)
> > +{
> > +    int x, y, d = 0;
> > +
> 
> > +    for (y = 8; y; y--) {
> > +        for (x = 8; x; x--)
> > +            d += abs(cur[x] - ref[x]);
> 
> Looks strange: the data will be accessed at indices 1..8, instead of 0..7.
> And in reverse order, which may be not good for the cache. Maybe switch to a
> more natural loop?

Good catch (this also fixes difference with ASM code).

[...]
> > +typedef struct {
> > +    int lo, hi;                    ///< lower and higher threshold number of differences
> > +                                   /// values for 8x8 blocks
> 
> > +    float frac;                    ///< threshold of changed pixels over the total fraction
> 
> Maybe store it as an integer? That would help making the filter bit-exact.

Please elaborate, also I'd like to keep compatibility with mp=decimate.

[...] 
> > +/**
> > + * Return 1 in case the two frames are different, 0 otherwise.
> > + */
> > +static int diff_planes(AVFilterContext *ctx,
> > +                       uint8_t *cur, int cur_linesize,
> > +                       uint8_t *ref, int ref_linesize,
> > +                       int w, int h)
> > +{
> > +    DecimateContext *decimate = ctx->priv;
> > +
> > +    int x, y;
> > +    int d, c = 0;
> > +    int t = (w/16)*(h/16)*decimate->frac;
> > +
> > +    /* compute difference for blocks of 8x8 bytes */
> 
> > +    for (y = 0; y < h-7; y += 4) {
> > +        for (x = 8; x < w-7; x += 4) {
> > +            d = decimate->diff(cur+x+y*cur_linesize, cur_linesize,
> > +                               ref+x+y*ref_linesize, ref_linesize);
> 
> I wonder if the benefit of the MMX optimization of the diff function is not
> completely wasted by the fact that all computations are done four times.

Again, since this is a port I'd prefer to keep the original filter
logic.

[...]
> > +AVFilter avfilter_vf_decimate = {
> > +    .name        = "decimate",
> > +    .description = NULL_IF_CONFIG_SMALL("Remove near-duplicate frames."),
> > +    .init        = init,
> > +    .uninit      = uninit,
> > +
> > +    .priv_size = sizeof(DecimateContext),
> > +    .query_formats = query_formats,
> > +
> > +    .inputs = (const AVFilterPad[]) {
> > +        {
> > +            .name             = "default",
> > +            .type             = AVMEDIA_TYPE_VIDEO,
> > +            .get_video_buffer = ff_null_get_video_buffer,
> > +            .config_props     = config_input,
> > +            .start_frame      = start_frame,
> > +            .draw_slice       = draw_slice,
> > +            .end_frame        = end_frame,
> 
> > +            .min_perms        = AV_PERM_READ,
> 
> You need PRESERVE.

Fixed.
[...]

Patch updated.
-- 
FFmpeg = Faithful and Fanciful Most Pure Extensive Goblin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavfi-port-decimate-libmpcodecs-filter.patch
Type: text/x-diff
Size: 18495 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120819/c781e4ab/attachment.bin>