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

[Michael Niedermayer]

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


[...]

> +#define OFFSET(x) offsetof(SignatureContext, x)

> +#define FLAGS AV_OPT_FLAG_VIDEO_PARAM

should contin also  AV_OPT_FLAG_FILTERING_PARAM

[...]
> +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.

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170105/7c730ef7/attachment.sig>