On Jan 6, 2017, at 1:34 PM, Gerion Entrup <gerion.entrup.ff...@flump.de> wrote: > > 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.
Just bumping this thread. I've been using the patch and find it very helpful and would like to see it in libavfilter. Dave Rice _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel