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

Dave Rice dave at dericed.com
Mon Mar 20 15:36:51 EET 2017


On Jan 6, 2017, at 1:34 PM, Gerion Entrup <gerion.entrup.ffdev at 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



More information about the ffmpeg-devel mailing list