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

Paul B Mahol onemda at gmail.com
Mon Mar 20 16:23:10 EET 2017


On 3/20/17, Dave Rice <dave at dericed.com> wrote:
>
>> On Mar 20, 2017, at 10:13 AM, Paul B Mahol <onemda at gmail.com> wrote:
>>
>> On 3/20/17, Dave Rice <dave at dericed.com> wrote:
>>> 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.
>>
>> I do not care as long its GPL.
>
> Gerion's last post on this thread appears to resolve all review comments but
> indicated that he would create a FATE test for the filter. Since the patch
> has been reviewed, I suggest that the missing FATE test could be a later
> patch and not block consideration of merging the signature filter. As noted,
> it is written with GPL.

Then wait until someone else appears and like to commit this code.


More information about the ffmpeg-devel mailing list