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

[Gerion Entrup]

On Dienstag, 3. Januar 2017 11:33:48 CET Moritz Barsnick wrote:
> On Mon, Jan 02, 2017 at 23:52:58 +0100, Gerion Entrup wrote:
> > +Calculates the MPEG-7 Video Signature. The filter could handle more than one
> > +input. In this case the matching between the inputs could be calculated. The
> > +filter passthrough the first input. The signature of each stream could be written
> > +into a file.
> 
> "Could" means "könnte" in German (I assume that's you first language),
> and that's probably not you intent here. I don't even understand what
> you man. "Can" (and "could") implies optional. What does it really do?
> 
> You also need to fix some verbs. Language-wise, this could be correct,
> but it still does not tell what really happens and what is optional:
> 
>   Calculates the MPEG-7 Video Signature. The filter can handle more
>   than one input. In this case the matching between the inputs is
>   calculated. The filter passes through the first input. The signature
>   of each stream can be written into a file.
The matching is not calculated automatically but with a commandline switch. I've tried
to reformulate it in a better way.


> > +Calculate the mathing for the whole video and output whether the whole video
>                  ^matching
> > +Calculate as long as a matching is found or the video ends. Should be faster in
> > +some cases.
> 
> "as long as" or "only until"? It sounds like you mean the latter.
> 
> > + at item filename
> > +Set the path to which the output is written. If there is more than one input,
> > +the path must be a prototype, i.e. must contain %d or %0nd (where n is a positive
> > +integer), that will be replaced with the input number. If no filename is
> > +specified, no output will be written. This is the default.
> 
> Question: Is this path technically (in ffmpeg terms) a URL/URI? If so,
> that might need to be mentioned.
I don't understand fully what your question is.

The output path is either a standard filepath or (if muliple inputs exist) similar to
the path specification in other parts of FFmpeg (e.g. the image2 muxer).


> > +typedef struct CourseSignature{
> 
> The English opposite of "fine" is "coarse", not "course". :)
Oops.


> > +    { "format",     "set output format, possible values: binary (default), xml",
>                                            ^ possible values and default are
>                                              automatically documented for you,
>                                              see "ffmpeg -h filter=filtername".
Nice.

 
> > +    { "th_d",       "set threshold to detect one word as similar",
> > +    { "th_dc",      "set threshold to detect all words as similar",
> > +    { "th_xh",      "set threshold to detect frames as similar",
> 
> You can probably omit the word "set" here, that's what options are for
> anyway.
Done in most cases.


> > +typedef struct MatchingInfo{
> 
> Bracket style.
> 
> > +    int i,j,tmp_i,tmp_j,count;
> 
> Add some spaces, please.
> 
> > +    int x0,y0,x1,y1;
> 
> Spaces.
> 
> > +    }else if(x0-1 >= 0){
> > +    }else if(y0-1 >= 0){
> > +    }else{
> 
> Bracket style (spaces).
> 
> > +        /* return if unexspected error occurs in input stream */
>                         ^ unexpected
> > +        if (ret == AVERROR_EOF){
> 
> Bracket style.
Hopefully fixed the spaces and brackets in all cases.

Thank you for the suggestions. Attached the new patch and diff to the last patch.

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