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

[Gerion Entrup]

On Donnerstag, 5. Januar 2017 02:26:23 CET Michael Niedermayer wrote:
> On Wed, Jan 04, 2017 at 05:05:41PM +0100, Gerion Entrup wrote:
> > On Dienstag, 3. Januar 2017 16:58:32 CET Moritz Barsnick wrote:
> > > > > The English opposite of "fine" is "coarse", not "course". :)
> > > > Oops.
> > > 
> > > You still have a few "courses". (The actual variables, not the types. I
> > > don't care *too* much, but might be better for consistency.)
> > You're right. Fixed version attached.
> > 
> > 
> > > From my side - mostly style and docs - it looks fine. Technically, I
> > > can't judge too much. You went through a long review cycle already, so
> > > I assume it's fine for those previous reviewers. They have the last
> > > call anyway.
> > 
> > What about FATE? I'm willing to write a test, but don't know the best way. 
> > There are official conditions, whether the signature is standard compliant. I've 
> > written a python script to proof that (see previous emails). Nevertheless the 
> > checks take relatively long and need 3GB inputvideo, so I assume this is not 
> > usable for FATE.
> 
> yes, a 3gb reference is not ok for fate
> 
> 
> > 
> > One way would be, to take a short input video, take the calculated signature 
> > and use this as reference, but the standard allow some bitflips and my code
> > has some of them in comparison to the reference signatures.
> 
> then the fate test could/should compare and pass if its within what
> we expect of our code. (which may be stricter than the reference
> allowance)
> there are already tests that do similar for comparing PCM/RAW
Ok, will try to create a test the next days.

 
> > +#define OFFSET(x) offsetof(SignatureContext, x)
> 
> > +#define FLAGS AV_OPT_FLAG_VIDEO_PARAM
> 
> should contin also  AV_OPT_FLAG_FILTERING_PARAM
Done.


> > +static int export(AVFilterContext *ctx, StreamContext *sc, int input)
> > +{
> > +    SignatureContext* sic = ctx->priv;
> > +    char filename[1024];
> > +
> > +    if (sic->nb_inputs > 1) {
> 
> > +        /* error already handled */
> > +        av_get_frame_filename(filename, sizeof(filename), sic->filename, input);
> 
> its more robust to use a av_assert*() on the return code if its assumed
> to be never failing than just a comment as a comment cannot be
> automatcially checked for validity currently.
I chose av_assert0 because the check is done only nb_inputs times.

The attached patch also fixes the file writing for every frame the one input is
longer than the other.

Gerion
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-add-signature-filter-for-MPEG7-video-signature.patch
Type: text/x-patch
Size: 81590 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170106/966539ef/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: changes.diff
Type: text/x-patch
Size: 1430 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170106/966539ef/attachment-0001.bin>