diff mbox

[FFmpeg-devel,v2] libavfilter/vf_blackdetect.c

Message ID 9e2b12db-81f6-d80d-5ed7-8c07e308f9b8@gmail.com
State Superseded
Headers show

Commit Message

jb Dec. 9, 2017, 6:28 p.m. UTC 
Hello!

I wanted to apologize for the mess! Last Monday I already sended this 
patch, but the first was not correct, and on top the email format was 
not right to. So I hope now everything is ok.

Here is again the description:

This patch unify vf_blackdetect with af_silencedetect. Now the logging 
prints *black_start* and *black_end* in separate lines. This is the same 
behavior like af_silencedetect and it is also more useful for monitoring 
stream. It works in that way, that when the black duration passes the 
duration limit the log massage is: *black_start: 7.56 *and when the last 
black frame comes, the massage: *black_end: 25 | black_duration: 17.44 
*pop up.

I did compiling tests on MacOS/Windows and Ubuntu, all works fine. I 
also make a fate test, it runs to, but I was not able to run this command:

tests/fate.sh fate-suite/

It always says something with path not fund... Is it really necessary to 
run a fate test, even with a little change like this?

Regards

Jonathan
From b6b6e4ab885f9b35a6696492286e504a4b3d6d92 Mon Sep 17 00:00:00 2001
From: Jonathan <jonbae77@gmail.com>
Date: Mon, 4 Dec 2017 16:05:48 +0100
Subject: [PATCH] unify blackdetect with af_silencedetect. Is more useful for
 monitoring streams.

---
 libavfilter/vf_blackdetect.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

--
2.15.1

Comments

Carl Eugen Hoyos Dec. 10, 2017, 12:48 p.m. UTC | #1 
2017-12-09 19:28 GMT+01:00 Jonathan Baecker <jonbae77@gmail.com>:

> -static void check_black_end(AVFilterContext *ctx)
> +static void check_black(AVFilterContext *ctx)

Why do you rename this function?

Please remove the superfluous brackets in the "if()"
you change.

Carl Eugen
jb Dec. 10, 2017, 1:12 p.m. UTC | #2 
Am 10.12.2017 um 13:48 schrieb Carl Eugen Hoyos:
> 2017-12-09 19:28 GMT+01:00 Jonathan Baecker <jonbae77@gmail.com>:
>
>> -static void check_black_end(AVFilterContext *ctx)
>> +static void check_black(AVFilterContext *ctx)
> Why do you rename this function?
I rename them because before the function only got called, if the last 
black frame passes. Now the function get called already when the first 
black frame comes. So for me are more generic name is more logic. But if 
you want I can change them back!
> Please remove the superfluous brackets in the "if()"
> you change.
I did not touch them, but yes I will remove them! Thanks for command!
jb Jan. 4, 2018, 9:57 p.m. UTC | #3 
Hello,
after my last update, I got not any response. Is there still something 
wrong with the patch?

Regards
Jonathan
diff mbox

Patch

diff --git a/libavfilter/vf_blackdetect.c b/libavfilter/vf_blackdetect.c
index 06ef9988d..92ea39b33 100644
--- a/libavfilter/vf_blackdetect.c
+++ b/libavfilter/vf_blackdetect.c
@@ -38,6 +38,7 @@  typedef struct BlackDetectContext {
     int64_t black_end;               ///< pts end time of the last black picture
     int64_t last_picref_pts;         ///< pts of the last input picture
     int black_started;
+    int black_match;

     double       picture_black_ratio_th;
     double       pixel_black_th;
@@ -107,15 +108,20 @@  static int config_input(AVFilterLink *inlink)
     return 0;
 }

-static void check_black_end(AVFilterContext *ctx)
+static void check_black(AVFilterContext *ctx)
 {
     BlackDetectContext *blackdetect = ctx->priv;
     AVFilterLink *inlink = ctx->inputs[0];

-    if ((blackdetect->black_end - blackdetect->black_start) >= blackdetect->black_min_duration) {
+    if ((blackdetect->last_picref_pts - blackdetect->black_start) >= blackdetect->black_min_duration && blackdetect->black_match == 0) {
         av_log(blackdetect, AV_LOG_INFO,
-               "black_start:%s black_end:%s black_duration:%s\n",
-               av_ts2timestr(blackdetect->black_start, &inlink->time_base),
+               "black_start: %s \n",
+               av_ts2timestr(blackdetect->black_start, &inlink->time_base));
+        blackdetect->black_match = 1;
+    }
+    if ((blackdetect->black_end - blackdetect->black_start) >= blackdetect->black_min_duration && blackdetect->black_started == 1) {
+        av_log(blackdetect, AV_LOG_INFO,
+               "black_end: %s | black_duration: %s \n",
                av_ts2timestr(blackdetect->black_end,   &inlink->time_base),
                av_ts2timestr(blackdetect->black_end - blackdetect->black_start, &inlink->time_base));
     }
@@ -131,7 +137,7 @@  static int request_frame(AVFilterLink *outlink)
     if (ret == AVERROR_EOF && blackdetect->black_started) {
         // FIXME: black_end should be set to last_picref_pts + last_picref_duration
         blackdetect->black_end = blackdetect->last_picref_pts;
-        check_black_end(ctx);
+        check_black(ctx);
     }
     return ret;
 }
@@ -163,15 +169,17 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
         if (!blackdetect->black_started) {
             /* black starts here */
             blackdetect->black_started = 1;
+            blackdetect->black_match = 0;
             blackdetect->black_start = picref->pts;
             av_dict_set(&picref->metadata, "lavfi.black_start",
                 av_ts2timestr(blackdetect->black_start, &inlink->time_base), 0);
         }
+        check_black(ctx);
     } else if (blackdetect->black_started) {
         /* black ends here */
-        blackdetect->black_started = 0;
         blackdetect->black_end = picref->pts;
-        check_black_end(ctx);
+        check_black(ctx);
+        blackdetect->black_started = 0;
         av_dict_set(&picref->metadata, "lavfi.black_end",
             av_ts2timestr(blackdetect->black_end, &inlink->time_base), 0);
     }