[FFmpeg-devel] [PATCH] add signature filter for MPEG7 video signature

[Gerion Entrup]

On Montag, 21. März 2016 11:55:47 CET Clément Bœsch wrote:
> On Mon, Mar 21, 2016 at 12:27:13AM +0100, Gerion Entrup wrote:
> > On Sonntag, 20. März 2016 16:52:36 CET Clément Bœsch wrote:
> > > On Sun, Mar 20, 2016 at 12:00:13PM +0100, Gerion Entrup wrote:
> > > > Good day,
> > > > 
> > > > I attached the patch. The MPEG7 video signature is a method to
> > > > calculate a
> > > > fingerprint of a video and to compare two or more of this
> > > > fingerprints.
> > > > Most of the standard is implemented. I've done this work as part of my
> > > > bachelor thesis.
> > > > 
> > > > The method is designed mainly for fast caculation of the fingerprint,
> > > > so
> > > > it is blockbased and not very robust to some geometric modification.
> > > > 
> > > > 
> > > > Further details can be found at
> > > > http://epubs.surrey.ac.uk/531590/1/MPEG-7%20Video%20Signature%20Author
> > > > %27s
> > > > %20Copy.pdf
> > > 
> > > use a @see in the code (typically in the @file section)
> > 
> > Wrap the url, too?
> 
> What do you mean?
The URL is longer than 80 chars, so it could be wanted that it is wrapped in
the source. But because of your enquiry, I guess, this is not necessary.
> 
> > > > +Calculates the MPEG-7 Video Signature. The filter could handle more
> > > > than
> > 
> > one input. In this case the matching between the inputs could be
> > calculated. The filter passthrough the first input. The output is written
> > in XML.> 
> > > > +
> > > 
> > > please wrap the text, same below
> > 
> > Also wrap the commands in the example section?
> 
> Not the verbatim/pre/blockcode, just the text
> 
> > > > diff --git a/libavfilter/signature.h b/libavfilter/signature.h
> > > > new file mode 100644
> > > > index 0000000..6f0584b
> > > > --- /dev/null
> > > > +++ b/libavfilter/signature.h
> > > > @@ -0,0 +1,569 @@
> > > > +/*
> > > > + * Copyright (c) 2016 Gerion Entrup
> > > > + *
> > > > + * This file is part of FFmpeg.
> > > > + *
> > > > + * FFmpeg is free software; you can redistribute it and/or modify
> > > > + * it under the terms of the GNU General Public License as published
> > > > by
> > > > + * the Free Software Foundation; either version 2 of the License, or
> > > > + * (at your option) any later version.
> > > > + *
> > > > + * 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 General Public License for more details.
> > > > + *
> > > > + * You should have received a copy of the GNU 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.
> > > > + */
> > > > +
> > > 
> > > GPL?
> > 
> > This is intended.
> 
> don't forget to update the LICENSE file.
I'll do it in the next iteration.
> 
> > > > +/**
> > > > + * @file
> > > > + * MPEG-7 video signature calculation and lookup filter
> > > > + */
> > > > +
> > > > +#include <float.h>
> > > > +#include "libavutil/opt.h"
> > > > +#include "libavutil/timestamp.h"
> > > > +#include "avfilter.h"
> > > > +#include "internal.h"
> > > > 
> > > > +#include <stdbool.h>
> > > 
> > > please no, use ints.
> > 
> > Done. For interest, why? This is not related to performance, I guess.
> > I find bools a lot of more readable.
> 
> first, because we don't use it anywhere else in the code, so for
> consistency at least. There is also risk of incompatibility. And then you
> end up using bool in various places assuming you know the size, where it
> could actually be platform dependant. So typically using it with
> AV_OPT_TYPE_BOOL could actually lead to invalid size writing/reading
> (similar issue as with using enum instead of ints).
> 
> [...]
Ok, understood. Thank you.