diff mbox

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

Message ID 1964063.rjEffV081J@gump
State Superseded
Headers show

Commit Message

Gerion Entrup Jan. 3, 2017, 3:29 p.m. UTC
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.
> 
> > +@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

Comments

Moritz Barsnick Jan. 3, 2017, 3:58 p.m. UTC | #1
On Tue, Jan 03, 2017 at 16:29:44 +0100, Gerion Entrup wrote:
> The matching is not calculated automatically but with a commandline switch. I've tried
> to reformulate it in a better way.

Very good, I understand it now. :-)

> 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).

Forget it, I believe this is only relevant to libavformat. (Background
is that, using internal av_*() functions, you can handle input and
output "files" as arbitrary URLs, with whatever libavformat supports:
file:, ftp://, and so on. You can't use this code in libavfilter
though, I believe.)

> > 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.)

> Done in most cases.

Perfect.

> Thank you for the suggestions. Attached the new patch and diff to the last patch.
(Thanks for the diff-diff.)

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.

Gruß,
Moritz
diff mbox

Patch

diff --git a/doc/filters.texi b/doc/filters.texi
index 8174b5b..1722d65 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -12546,10 +12546,10 @@  saturation maximum: %@{metadata:lavfi.signalstats.SATMAX@}
 @anchor{signature}
 @section signature
 
-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.
+Calculates the MPEG-7 Video Signature. The filter can handle more than one
+input. In this case the matching between the inputs can be calculated additionally.
+The filter always passes through the first input. The signature of each stream can
+be written into a file.
 
 It accepts the following options:
 
@@ -12563,10 +12563,10 @@  Available values are:
 @item off
 Disable the calculation of a matching (default).
 @item full
-Calculate the mathing for the whole video and output whether the whole video
+Calculate the matching for the whole video and output whether the whole video
 matches or only parts.
 @item fast
-Calculate as long as a matching is found or the video ends. Should be faster in
+Calculate only until a matching is found or the video ends. Should be faster in
 some cases.
 @end table
 
@@ -12624,7 +12624,7 @@  ffmpeg -i input.mkv -vf signature=filename=signature.bin -map 0:v -f null -
 @end example
 
 @item
-To detect whether two videos matches and store the signatures in XML format in
+To detect whether two videos match and store the signatures in XML format in
 signature0.xml and signature1.xml:
 @example
 ffmpeg -i input1.mkv -i input2.mkv -filter_complex "[0:v][1:v] signature=nb_inputs=2:detectmode=full:format=xml:filename=signature%d.xml" -map :v -f null -
diff --git a/libavfilter/signature.h b/libavfilter/signature.h
index 8c8f0a8..4728606 100644
--- a/libavfilter/signature.h
+++ b/libavfilter/signature.h
@@ -69,7 +69,7 @@  typedef struct {
     const Block* blocks;
 } ElemCat;
 
-typedef struct FineSignature{
+typedef struct FineSignature {
     struct FineSignature* next;
     struct FineSignature* prev;
     uint64_t pts;
@@ -79,15 +79,15 @@  typedef struct FineSignature{
     uint8_t framesig[SIGELEM_SIZE/5];
 } FineSignature;
 
-typedef struct CourseSignature{
+typedef struct CoarseSignature {
     uint8_t data[5][31]; /* 5 words with min. 243 bit */
     struct FineSignature* first; /* associated Finesignatures */
     struct FineSignature* last;
-    struct CourseSignature* next;
-} CourseSignature;
+    struct CoarseSignature* next;
+} CoarseSignature;
 
 /* lookup types */
-typedef struct MatchingInfo{
+typedef struct MatchingInfo {
     double meandist;
     double framerateratio; /* second/first */
     int score;
@@ -111,11 +111,11 @@  typedef struct {
     FineSignature* finesiglist;
     FineSignature* curfinesig;
 
-    CourseSignature* coursesiglist;
-    CourseSignature* courseend; /* needed for xml export */
+    CoarseSignature* coursesiglist;
+    CoarseSignature* courseend; /* needed for xml export */
     /* helpers to store the alternating signatures */
-    CourseSignature* curcoursesig1;
-    CourseSignature* curcoursesig2;
+    CoarseSignature* curcoursesig1;
+    CoarseSignature* curcoursesig2;
 
     int coursecount; /* counter from 0 to 89 */
     int midcourse;   /* whether it is a coursesignature beginning from 45 + i * 90 */
diff --git a/libavfilter/signature_lookup.c b/libavfilter/signature_lookup.c
index 3a75770..23c0ff1 100644
--- a/libavfilter/signature_lookup.c
+++ b/libavfilter/signature_lookup.c
@@ -39,7 +39,7 @@ 
 
 static void fill_l1distlut(uint8_t lut[])
 {
-    int i,j,tmp_i,tmp_j,count;
+    int i, j, tmp_i, tmp_j,count;
     uint8_t dist;
 
     for (i = 0, count = 0; i < 242; i++) {
@@ -91,7 +91,7 @@  static unsigned int get_l1dist(AVFilterContext *ctx, SignatureContext *sc, const
 {
     unsigned int i;
     unsigned int dist = 0;
-    uint8_t f,s;
+    uint8_t f, s;
 
     for (i = 0; i < SIGELEM_SIZE/5; i++) {
         if (first[i] != second[i]) {
@@ -112,7 +112,7 @@  static unsigned int get_l1dist(AVFilterContext *ctx, SignatureContext *sc, const
  * calculates the jaccard distance and evaluates a pair of course signatures as good
  * @return 0 if pair is bad, 1 otherwise
  */
-static int get_jaccarddist(SignatureContext *sc, CourseSignature *first, CourseSignature *second)
+static int get_jaccarddist(SignatureContext *sc, CoarseSignature *first, CoarseSignature *second)
 {
     int jaccarddist, i, composdist = 0, cwthcount = 0;
     for (i = 0; i < 5; i++) {
@@ -137,7 +137,7 @@  static int get_jaccarddist(SignatureContext *sc, CourseSignature *first, CourseS
  * step through the coursesignatures as long as a good candidate is found
  * @return 0 if no candidate is found, 1 otherwise
  */
-static int find_next_coursecandidate(SignatureContext *sc, CourseSignature *secondstart, CourseSignature **first, CourseSignature **second, int start)
+static int find_next_coursecandidate(SignatureContext *sc, CoarseSignature *secondstart, CoarseSignature **first, CoarseSignature **second, int start)
 {
     /* go one coursesignature foreword */
     if (!start) {
@@ -173,7 +173,7 @@  static int find_next_coursecandidate(SignatureContext *sc, CourseSignature *seco
  */
 static MatchingInfo* get_matching_parameters(AVFilterContext *ctx, SignatureContext *sc, FineSignature *first, FineSignature *second)
 {
-    FineSignature *f,*s;
+    FineSignature *f, *s;
     size_t i, j, k, l, hmax = 0, score;
     int framerate, offset, l1dist;
     double m;
@@ -421,7 +421,7 @@  static MatchingInfo evaluate_parameters(AVFilterContext *ctx, SignatureContext *
     int fcount = 0, goodfcount = 0, gooda = 0, goodb = 0;
     double meandist, minmeandist = bestmatch.meandist;
     int tolerancecount = 0;
-    FineSignature *a,*b, *aprev, *bprev;
+    FineSignature *a, *b, *aprev, *bprev;
     int status = STATUS_NULL;
 
     for (; infos != NULL; infos = infos->next) {
@@ -532,7 +532,7 @@  static void sll_free(MatchingInfo *sll)
 
 static MatchingInfo lookup_signatures(AVFilterContext *ctx, SignatureContext *sc, StreamContext *first, StreamContext *second, int mode)
 {
-    CourseSignature *cs, *cs2;
+    CoarseSignature *cs, *cs2;
     MatchingInfo *infos;
     MatchingInfo bestmatch;
     MatchingInfo *i;
diff --git a/libavfilter/vf_signature.c b/libavfilter/vf_signature.c
index ee01914..6db515c 100644
--- a/libavfilter/vf_signature.c
+++ b/libavfilter/vf_signature.c
@@ -41,28 +41,28 @@ 
 #define BLOCK_LCM (int64_t) 476985600
 
 static const AVOption signature_options[] = {
-    { "detectmode", "set the detectmode, possible values: off (default), full, fast",
+    { "detectmode", "set the detectmode",
         OFFSET(mode),         AV_OPT_TYPE_INT,    {.i64 = MODE_OFF}, 0, NB_LOOKUP_MODE-1, FLAGS, "mode" },
         { "off",  NULL, 0, AV_OPT_TYPE_CONST, {.i64 = MODE_OFF},  0, 0, .flags = FLAGS, "mode" },
         { "full", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = MODE_FULL}, 0, 0, .flags = FLAGS, "mode" },
         { "fast", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = MODE_FAST}, 0, 0, .flags = FLAGS, "mode" },
-    { "nb_inputs",  "set number of inputs",
+    { "nb_inputs",  "number of inputs",
         OFFSET(nb_inputs),    AV_OPT_TYPE_INT,    {.i64 = 1},        1, INT_MAX,          FLAGS },
-    { "filename",   "set filename for output files",
+    { "filename",   "filename for output files",
         OFFSET(filename),     AV_OPT_TYPE_STRING, {.str = ""},       0, NB_FORMATS-1,     FLAGS },
-    { "format",     "set output format, possible values: binary (default), xml",
+    { "format",     "set output format",
         OFFSET(format),       AV_OPT_TYPE_INT,    {.i64 = FORMAT_BINARY}, 0, 1,           FLAGS , "format" },
         { "binary", 0, 0, AV_OPT_TYPE_CONST, {.i64=FORMAT_BINARY}, 0, 0, FLAGS, "format" },
         { "xml",    0, 0, AV_OPT_TYPE_CONST, {.i64=FORMAT_XML},    0, 0, FLAGS, "format" },
-    { "th_d",       "set threshold to detect one word as similar",
+    { "th_d",       "threshold to detect one word as similar",
         OFFSET(thworddist),   AV_OPT_TYPE_INT,    {.i64 = 9000},     1, INT_MAX,          FLAGS },
-    { "th_dc",      "set threshold to detect all words as similar",
+    { "th_dc",      "threshold to detect all words as similar",
         OFFSET(thcomposdist), AV_OPT_TYPE_INT,    {.i64 = 60000},    1, INT_MAX,          FLAGS },
-    { "th_xh",      "set threshold to detect frames as similar",
+    { "th_xh",      "threshold to detect frames as similar",
         OFFSET(thl1),         AV_OPT_TYPE_INT,    {.i64 = 116},      1, INT_MAX,          FLAGS },
     { "th_di",      "minimum length of matching sequence in frames",
         OFFSET(thdi),         AV_OPT_TYPE_INT,    {.i64 = 0},        0, INT_MAX,          FLAGS },
-    { "th_it",      "set threshold for relation of good to all frames",
+    { "th_it",      "threshold for relation of good to all frames",
         OFFSET(thit),         AV_OPT_TYPE_DOUBLE, {.dbl = 0.5},    0.0, 1.0,              FLAGS },
     { NULL }
 };
@@ -113,7 +113,7 @@  static uint64_t get_block_sum(StreamContext *sc, uint64_t intpic[32][32], const
 {
     uint64_t sum = 0;
 
-    int x0,y0,x1,y1;
+    int x0, y0, x1, y1;
 
     x0 = b->up.x;
     y0 = b->up.y;
@@ -122,11 +122,11 @@  static uint64_t get_block_sum(StreamContext *sc, uint64_t intpic[32][32], const
 
     if (x0-1 >= 0 && y0-1 >= 0) {
         sum = intpic[y1][x1] + intpic[y0-1][x0-1] - intpic[y1][x0-1] - intpic[y0-1][x1];
-    }else if(x0-1 >= 0){
+    } else if (x0-1 >= 0) {
         sum = intpic[y1][x1] - intpic[y1][x0-1];
-    }else if(y0-1 >= 0){
+    } else if (y0-1 >= 0) {
         sum = intpic[y1][x1] - intpic[y0-1][x1];
-    }else{
+    } else {
         sum = intpic[y1][x1];
     }
     return sum;
@@ -171,7 +171,7 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
     int f = 0, g = 0, w = 0;
     int32_t dh1 = 1, dh2 = 1, dw1 = 1, dw2 = 1, a, b;
     int64_t denom;
-    int i,j,k,ternary;
+    int i, j, k, ternary;
     uint64_t blocksum;
     int blocksize;
     int64_t th; /* threshold */
@@ -321,7 +321,7 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
     /* coursesignature */
     if (sc->coursecount == 0) {
         if (sc->curcoursesig2) {
-            sc->curcoursesig1 = av_mallocz(sizeof(CourseSignature));
+            sc->curcoursesig1 = av_mallocz(sizeof(CoarseSignature));
             if (!sc->curcoursesig1)
                 return AVERROR(ENOMEM);
             sc->curcoursesig1->first = fs;
@@ -331,7 +331,7 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
     }
     if (sc->coursecount == 45) {
         sc->midcourse = 1;
-        sc->curcoursesig2 = av_mallocz(sizeof(CourseSignature));
+        sc->curcoursesig2 = av_mallocz(sizeof(CoarseSignature));
         if (!sc->curcoursesig2)
             return AVERROR(ENOMEM);
         sc->curcoursesig2->first = fs;
@@ -383,8 +383,8 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
 static int xml_export(AVFilterContext *ctx, StreamContext *sc, const char* filename)
 {
     FineSignature* fs;
-    CourseSignature* cs;
-    int i,j;
+    CoarseSignature* cs;
+    int i, j;
     FILE* f;
     unsigned int pot3[5] = { 3*3*3*3, 3*3*3, 3*3, 3, 1 };
 
@@ -490,9 +490,9 @@  static int binary_export(AVFilterContext *ctx, StreamContext *sc, const char* fi
 {
     FILE* f;
     FineSignature* fs;
-    CourseSignature* cs;
+    CoarseSignature* cs;
     uint32_t numofsegments = (sc->lastindex + 44)/45;
-    int i,j;
+    int i, j;
     PutBitContext buf;
     /* buffer + header + coursesignatures + finesignature */
     int len = (512 + 6 * 32 + 3*16 + 2 +
@@ -598,12 +598,12 @@  static int request_frame(AVFilterLink *outlink)
 
         ret = ff_request_frame(ctx->inputs[i]);
 
-        /* return if unexspected error occurs in input stream */
+        /* return if unexpected error occurs in input stream */
         if (ret < 0 && ret != AVERROR_EOF)
             return ret;
 
         /* export signature at EOF */
-        if (ret == AVERROR_EOF){
+        if (ret == AVERROR_EOF) {
             /* export if wanted */
             if (strlen(sic->filename) > 0) {
                 if (export(ctx, sc, i) < 0)
@@ -670,7 +670,7 @@  static av_cold int init(AVFilterContext *ctx)
             return AVERROR(ENOMEM);
         sc->curfinesig = NULL;
 
-        sc->coursesiglist = av_mallocz(sizeof(CourseSignature));
+        sc->coursesiglist = av_mallocz(sizeof(CoarseSignature));
         if (!sc->coursesiglist)
             return AVERROR(ENOMEM);
         sc->curcoursesig1 = sc->coursesiglist;
@@ -701,7 +701,7 @@  static av_cold void uninit(AVFilterContext *ctx)
     StreamContext *sc;
     void* tmp;
     FineSignature* finsig;
-    CourseSignature* cousig;
+    CoarseSignature* cousig;
     int i;