[FFmpeg-devel,v1] avfilter/vf_freezedetect: add force_discard option to force discard freeze/non-freeze frame

Submitted by lance.lmwang@gmail.com on Sept. 30, 2019, 3:22 p.m.

Details

Message ID 20190930152218.6860-1-lance.lmwang@gmail.com
State New
Headers show

Commit Message

lance.lmwang@gmail.com Sept. 30, 2019, 3:22 p.m.
From: Limin Wang <lance.lmwang@gmail.com>

 How to tested it, please try with the following command:
 ./ffmpeg  -f lavfi -i "smptebars=duration=5:size=1280x720:rate=30,freezedetect=d=1:f=0"  -f null -
 frame=  150 fps=0.0 q=-0.0 Lsize=N/A time=00:00:05.00 bitrate=N/A speed= 232x

 ./ffmpeg  -f lavfi -i "smptebars=duration=5:size=1280x720:rate=30,freezedetect=d=1:f=-1"  -f null -
 frame=  120 fps=0.0 q=-0.0 Lsize=N/A time=00:00:04.00 bitrate=N/A speed= 211x

 ./ffmpeg  -f lavfi -i "smptebars=duration=5:size=1280x720:rate=30,freezedetect=d=1:f=1"  -f null -
 frame=   30 fps=0.0 q=-0.0 Lsize=N/A time=00:00:01.00 bitrate=N/A speed=93.9x

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 doc/filters.texi              | 10 ++++++++++
 libavfilter/vf_freezedetect.c | 23 ++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)

Comments

Moritz Barsnick Oct. 1, 2019, 7:47 a.m.
On Mon, Sep 30, 2019 at 23:22:18 +0800, lance.lmwang@gmail.com wrote:
> +                if ( s->force_discard > 0 && frozen)
> +                    s->drop_count++;
> +                else if ( s->force_discard < 0 && frozen && s->drop_count < 0) {
> +                    s->drop_count = 0;
> +                }

"if (s" (drop the space after the opening bracket).

I also don't quite understand why you use no brackets around the if()
block, but around the else block.

> +            if (s->force_discard > 0) {
> +                s->drop_count = 0;
> +            } else if ( s->force_discard < 0)
> +                s->drop_count--;

Same here regarding the brackets. Quite confusing, both blocks are
one-liners.

> -        return ff_filter_frame(outlink, frame);
> +        if (s->drop_count > 0 || s->drop_count < 0) {
> +            av_frame_free(&frame);
> +        } else
> +            return ff_filter_frame(outlink, frame);

Same here.

Moritz
Marton Balint Oct. 1, 2019, 7:34 p.m.
On Mon, 30 Sep 2019, lance.lmwang@gmail.com wrote:

> From: Limin Wang <lance.lmwang@gmail.com>
>
> How to tested it, please try with the following command:
> ./ffmpeg  -f lavfi -i "smptebars=duration=5:size=1280x720:rate=30,freezedetect=d=1:f=0"  -f null -
> frame=  150 fps=0.0 q=-0.0 Lsize=N/A time=00:00:05.00 bitrate=N/A speed= 232x
>
> ./ffmpeg  -f lavfi -i "smptebars=duration=5:size=1280x720:rate=30,freezedetect=d=1:f=-1"  -f null -
> frame=  120 fps=0.0 q=-0.0 Lsize=N/A time=00:00:04.00 bitrate=N/A speed= 211x
>
> ./ffmpeg  -f lavfi -i "smptebars=duration=5:size=1280x720:rate=30,freezedetect=d=1:f=1"  -f null -
> frame=   30 fps=0.0 q=-0.0 Lsize=N/A time=00:00:01.00 bitrate=N/A speed=93.9x
>
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
> doc/filters.texi              | 10 ++++++++++
> libavfilter/vf_freezedetect.c | 23 ++++++++++++++++++++++-
> 2 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 6ed1c8fa75..2be8b93c53 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -10678,6 +10678,8 @@ timestamp of the first frame of the freeze. The
> @code{lavfi.freezedetect.freeze_duration} and
> @code{lavfi.freezedetect.freeze_end} metadata keys are set on the first frame
> after the freeze.
> +In addition, you can choose to discard the freeze/non-freezee frames instead of
> +report by metadata.
> 
> The filter accepts the following options:
> 
> @@ -10689,6 +10691,14 @@ specified value) or as a difference ratio between 0 and 1. Default is -60dB, or
> 
> @item duration, d
> Set freeze duration until notification (default is 2 seconds).
> +
> +@item force_discard, f
> +Set force to discard or keep freeze frame.
> + 0:  do nothing
> +-1:  discard non-freeze frame
> + 1:  discard freeze frame

I don't see how discarding non-frozen frames is useful in any way, so that 
option should be removed unless you have something sensible in mind.

I would also like to point out that freezedetect does not buffer any 
frames, so the first few frames (the detection interval) of a freeze won't 
be dropped, which is not what the user might expect when it wants to drop 
frozen frames. At least you should document this.

> +
> +Default is 0
> @end table
> 
> @anchor{frei0r}
> diff --git a/libavfilter/vf_freezedetect.c b/libavfilter/vf_freezedetect.c
> index cc086afee6..fc08c235f7 100644
> --- a/libavfilter/vf_freezedetect.c
> +++ b/libavfilter/vf_freezedetect.c
> @@ -45,6 +45,9 @@ typedef struct FreezeDetectContext {
>
>     double noise;
>     int64_t duration;            ///< minimum duration of frozen frame until notification
> +
> +    int force_discard;           ///< 0: no discard, -1: discard non-freeze frame, 1: discard freeze frame
> +    int drop_count;

Why don't you simply use s->frozen to see if the current frame needs to be 
dropped or not? That would be much easier to follow.

> } FreezeDetectContext;
> 
> #define OFFSET(x) offsetof(FreezeDetectContext, x)
> @@ -56,6 +59,8 @@ static const AVOption freezedetect_options[] = {
>     { "noise",               "set noise tolerance",                       OFFSET(noise),  AV_OPT_TYPE_DOUBLE,   {.dbl=0.001},     0,       1.0, V|F },
>     { "d",                   "set minimum duration in seconds",        OFFSET(duration),  AV_OPT_TYPE_DURATION, {.i64=2000000},   0, INT64_MAX, V|F },
>     { "duration",            "set minimum duration in seconds",        OFFSET(duration),  AV_OPT_TYPE_DURATION, {.i64=2000000},   0, INT64_MAX, V|F },
> +    { "f",                   "set frame discard",                      OFFSET(force_discard), AV_OPT_TYPE_INT,  {.i64=0},         -1,        1, V|F },
> +    { "force_discard",       "set frame discard",                      OFFSET(force_discard), AV_OPT_TYPE_INT,  {.i64=0},         -1,        1, V|F },

A name like "discard" should be enough, no need for short version, it is 
only used for consistency with silencedetect.

>
>     {NULL}
> };
> @@ -115,6 +120,7 @@ static int config_input(AVFilterLink *inlink)
>     if (!s->sad)
>         return AVERROR(EINVAL);
> 
> +    s->drop_count = 0;
>     return 0;
> }
> 
> @@ -184,10 +190,22 @@ static int activate(AVFilterContext *ctx)
>                     set_meta(s, frame, "lavfi.freezedetect.freeze_end", av_ts2timestr(frame->pts, &inlink->time_base));
>                 }
>                 s->frozen = frozen;
> +                if ( s->force_discard > 0 && frozen)
> +                    s->drop_count++;
> +                else if ( s->force_discard < 0 && frozen && s->drop_count < 0) {
> +                    s->drop_count = 0;
> +                }
> +            } else {
> +                if ( s->force_discard < 0)
> +                    s->drop_count--;
>             }
>         }
>
>         if (!frozen) {
> +            if (s->force_discard > 0) {
> +                s->drop_count = 0;
> +            } else if ( s->force_discard < 0)
> +                s->drop_count--;
>             av_frame_free(&s->reference_frame);
>             s->reference_frame = av_frame_clone(frame);
>             s->reference_n = s->n;
> @@ -196,7 +214,10 @@ static int activate(AVFilterContext *ctx)
>                 return AVERROR(ENOMEM);
>             }
>         }
> -        return ff_filter_frame(outlink, frame);
> +        if (s->drop_count > 0 || s->drop_count < 0) {
> +            av_frame_free(&frame);
> +        } else
> +            return ff_filter_frame(outlink, frame);
>     }
>
>     FF_FILTER_FORWARD_STATUS(inlink, outlink);
> -- 
> 2.21.0
>

Regards,
Marton
lance.lmwang@gmail.com Oct. 7, 2019, 2:28 p.m.
On Tue, Oct 01, 2019 at 09:47:19AM +0200, Moritz Barsnick wrote:
> On Mon, Sep 30, 2019 at 23:22:18 +0800, lance.lmwang@gmail.com wrote:
> > +                if ( s->force_discard > 0 && frozen)
> > +                    s->drop_count++;
> > +                else if ( s->force_discard < 0 && frozen && s->drop_count < 0) {
> > +                    s->drop_count = 0;
> > +                }
> 
> "if (s" (drop the space after the opening bracket).
> 
> I also don't quite understand why you use no brackets around the if()
> block, but around the else block.
> 
> > +            if (s->force_discard > 0) {
> > +                s->drop_count = 0;
> > +            } else if ( s->force_discard < 0)
> > +                s->drop_count--;
> 
> Same here regarding the brackets. Quite confusing, both blocks are
> one-liners.
> 
> > -        return ff_filter_frame(outlink, frame);
> > +        if (s->drop_count > 0 || s->drop_count < 0) {
> > +            av_frame_free(&frame);
> > +        } else
> > +            return ff_filter_frame(outlink, frame);
> 
> Same here.
Thanks for the catch, I'll fix it for the update patch.


> 
> Moritz
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
lance.lmwang@gmail.com Oct. 7, 2019, 2:36 p.m.
On Tue, Oct 01, 2019 at 09:34:48PM +0200, Marton Balint wrote:
> 
> 
> On Mon, 30 Sep 2019, lance.lmwang@gmail.com wrote:
> 
> >From: Limin Wang <lance.lmwang@gmail.com>
> >
> >How to tested it, please try with the following command:
> >./ffmpeg  -f lavfi -i "smptebars=duration=5:size=1280x720:rate=30,freezedetect=d=1:f=0"  -f null -
> >frame=  150 fps=0.0 q=-0.0 Lsize=N/A time=00:00:05.00 bitrate=N/A speed= 232x
> >
> >./ffmpeg  -f lavfi -i "smptebars=duration=5:size=1280x720:rate=30,freezedetect=d=1:f=-1"  -f null -
> >frame=  120 fps=0.0 q=-0.0 Lsize=N/A time=00:00:04.00 bitrate=N/A speed= 211x
> >
> >./ffmpeg  -f lavfi -i "smptebars=duration=5:size=1280x720:rate=30,freezedetect=d=1:f=1"  -f null -
> >frame=   30 fps=0.0 q=-0.0 Lsize=N/A time=00:00:01.00 bitrate=N/A speed=93.9x
> >
> >Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> >---
> >doc/filters.texi              | 10 ++++++++++
> >libavfilter/vf_freezedetect.c | 23 ++++++++++++++++++++++-
> >2 files changed, 32 insertions(+), 1 deletion(-)
> >
> >diff --git a/doc/filters.texi b/doc/filters.texi
> >index 6ed1c8fa75..2be8b93c53 100644
> >--- a/doc/filters.texi
> >+++ b/doc/filters.texi
> >@@ -10678,6 +10678,8 @@ timestamp of the first frame of the freeze. The
> >@code{lavfi.freezedetect.freeze_duration} and
> >@code{lavfi.freezedetect.freeze_end} metadata keys are set on the first frame
> >after the freeze.
> >+In addition, you can choose to discard the freeze/non-freezee frames instead of
> >+report by metadata.
> >
> >The filter accepts the following options:
> >
> >@@ -10689,6 +10691,14 @@ specified value) or as a difference ratio between 0 and 1. Default is -60dB, or
> >
> >@item duration, d
> >Set freeze duration until notification (default is 2 seconds).
> >+
> >+@item force_discard, f
> >+Set force to discard or keep freeze frame.
> >+ 0:  do nothing
> >+-1:  discard non-freeze frame
> >+ 1:  discard freeze frame
> 
> I don't see how discarding non-frozen frames is useful in any way,
> so that option should be removed unless you have something sensible
> in mind.

When the input file is big, I like to extract the freeze frame
after the running sometimes, it's useful to help check whether 
it's expect for the detecting result.

> 
> I would also like to point out that freezedetect does not buffer any
> frames, so the first few frames (the detection interval) of a freeze
> won't be dropped, which is not what the user might expect when it
> wants to drop frozen frames. At least you should document this.

OK, will update the document to note it.

> 
> >+
> >+Default is 0
> >@end table
> >
> >@anchor{frei0r}
> >diff --git a/libavfilter/vf_freezedetect.c b/libavfilter/vf_freezedetect.c
> >index cc086afee6..fc08c235f7 100644
> >--- a/libavfilter/vf_freezedetect.c
> >+++ b/libavfilter/vf_freezedetect.c
> >@@ -45,6 +45,9 @@ typedef struct FreezeDetectContext {
> >
> >    double noise;
> >    int64_t duration;            ///< minimum duration of frozen frame until notification
> >+
> >+    int force_discard;           ///< 0: no discard, -1: discard non-freeze frame, 1: discard freeze frame
> >+    int drop_count;
> 
> Why don't you simply use s->frozen to see if the current frame needs
> to be dropped or not? That would be much easier to follow.

OK, I'll consider it how to simplify it by your advise.

> 
> >} FreezeDetectContext;
> >
> >#define OFFSET(x) offsetof(FreezeDetectContext, x)
> >@@ -56,6 +59,8 @@ static const AVOption freezedetect_options[] = {
> >    { "noise",               "set noise tolerance",                       OFFSET(noise),  AV_OPT_TYPE_DOUBLE,   {.dbl=0.001},     0,       1.0, V|F },
> >    { "d",                   "set minimum duration in seconds",        OFFSET(duration),  AV_OPT_TYPE_DURATION, {.i64=2000000},   0, INT64_MAX, V|F },
> >    { "duration",            "set minimum duration in seconds",        OFFSET(duration),  AV_OPT_TYPE_DURATION, {.i64=2000000},   0, INT64_MAX, V|F },
> >+    { "f",                   "set frame discard",                      OFFSET(force_discard), AV_OPT_TYPE_INT,  {.i64=0},         -1,        1, V|F },
> >+    { "force_discard",       "set frame discard",                      OFFSET(force_discard), AV_OPT_TYPE_INT,  {.i64=0},         -1,        1, V|F },
> 
> A name like "discard" should be enough, no need for short version,
> it is only used for consistency with silencedetect.

OK, will update it.

> 
> >
> >    {NULL}
> >};
> >@@ -115,6 +120,7 @@ static int config_input(AVFilterLink *inlink)
> >    if (!s->sad)
> >        return AVERROR(EINVAL);
> >
> >+    s->drop_count = 0;
> >    return 0;
> >}
> >
> >@@ -184,10 +190,22 @@ static int activate(AVFilterContext *ctx)
> >                    set_meta(s, frame, "lavfi.freezedetect.freeze_end", av_ts2timestr(frame->pts, &inlink->time_base));
> >                }
> >                s->frozen = frozen;
> >+                if ( s->force_discard > 0 && frozen)
> >+                    s->drop_count++;
> >+                else if ( s->force_discard < 0 && frozen && s->drop_count < 0) {
> >+                    s->drop_count = 0;
> >+                }
> >+            } else {
> >+                if ( s->force_discard < 0)
> >+                    s->drop_count--;
> >            }
> >        }
> >
> >        if (!frozen) {
> >+            if (s->force_discard > 0) {
> >+                s->drop_count = 0;
> >+            } else if ( s->force_discard < 0)
> >+                s->drop_count--;
> >            av_frame_free(&s->reference_frame);
> >            s->reference_frame = av_frame_clone(frame);
> >            s->reference_n = s->n;
> >@@ -196,7 +214,10 @@ static int activate(AVFilterContext *ctx)
> >                return AVERROR(ENOMEM);
> >            }
> >        }
> >-        return ff_filter_frame(outlink, frame);
> >+        if (s->drop_count > 0 || s->drop_count < 0) {
> >+            av_frame_free(&frame);
> >+        } else
> >+            return ff_filter_frame(outlink, frame);
> >    }
> >
> >    FF_FILTER_FORWARD_STATUS(inlink, outlink);
> >-- 
> >2.21.0
> >
> 
> Regards,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Patch hide | download patch | download mbox

diff --git a/doc/filters.texi b/doc/filters.texi
index 6ed1c8fa75..2be8b93c53 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -10678,6 +10678,8 @@  timestamp of the first frame of the freeze. The
 @code{lavfi.freezedetect.freeze_duration} and
 @code{lavfi.freezedetect.freeze_end} metadata keys are set on the first frame
 after the freeze.
+In addition, you can choose to discard the freeze/non-freezee frames instead of
+report by metadata.
 
 The filter accepts the following options:
 
@@ -10689,6 +10691,14 @@  specified value) or as a difference ratio between 0 and 1. Default is -60dB, or
 
 @item duration, d
 Set freeze duration until notification (default is 2 seconds).
+
+@item force_discard, f
+Set force to discard or keep freeze frame.
+ 0:  do nothing
+-1:  discard non-freeze frame
+ 1:  discard freeze frame
+
+Default is 0
 @end table
 
 @anchor{frei0r}
diff --git a/libavfilter/vf_freezedetect.c b/libavfilter/vf_freezedetect.c
index cc086afee6..fc08c235f7 100644
--- a/libavfilter/vf_freezedetect.c
+++ b/libavfilter/vf_freezedetect.c
@@ -45,6 +45,9 @@  typedef struct FreezeDetectContext {
 
     double noise;
     int64_t duration;            ///< minimum duration of frozen frame until notification
+
+    int force_discard;           ///< 0: no discard, -1: discard non-freeze frame, 1: discard freeze frame
+    int drop_count;
 } FreezeDetectContext;
 
 #define OFFSET(x) offsetof(FreezeDetectContext, x)
@@ -56,6 +59,8 @@  static const AVOption freezedetect_options[] = {
     { "noise",               "set noise tolerance",                       OFFSET(noise),  AV_OPT_TYPE_DOUBLE,   {.dbl=0.001},     0,       1.0, V|F },
     { "d",                   "set minimum duration in seconds",        OFFSET(duration),  AV_OPT_TYPE_DURATION, {.i64=2000000},   0, INT64_MAX, V|F },
     { "duration",            "set minimum duration in seconds",        OFFSET(duration),  AV_OPT_TYPE_DURATION, {.i64=2000000},   0, INT64_MAX, V|F },
+    { "f",                   "set frame discard",                      OFFSET(force_discard), AV_OPT_TYPE_INT,  {.i64=0},         -1,        1, V|F },
+    { "force_discard",       "set frame discard",                      OFFSET(force_discard), AV_OPT_TYPE_INT,  {.i64=0},         -1,        1, V|F },
 
     {NULL}
 };
@@ -115,6 +120,7 @@  static int config_input(AVFilterLink *inlink)
     if (!s->sad)
         return AVERROR(EINVAL);
 
+    s->drop_count = 0;
     return 0;
 }
 
@@ -184,10 +190,22 @@  static int activate(AVFilterContext *ctx)
                     set_meta(s, frame, "lavfi.freezedetect.freeze_end", av_ts2timestr(frame->pts, &inlink->time_base));
                 }
                 s->frozen = frozen;
+                if ( s->force_discard > 0 && frozen)
+                    s->drop_count++;
+                else if ( s->force_discard < 0 && frozen && s->drop_count < 0) {
+                    s->drop_count = 0;
+                }
+            } else {
+                if ( s->force_discard < 0)
+                    s->drop_count--;
             }
         }
 
         if (!frozen) {
+            if (s->force_discard > 0) {
+                s->drop_count = 0;
+            } else if ( s->force_discard < 0)
+                s->drop_count--;
             av_frame_free(&s->reference_frame);
             s->reference_frame = av_frame_clone(frame);
             s->reference_n = s->n;
@@ -196,7 +214,10 @@  static int activate(AVFilterContext *ctx)
                 return AVERROR(ENOMEM);
             }
         }
-        return ff_filter_frame(outlink, frame);
+        if (s->drop_count > 0 || s->drop_count < 0) {
+            av_frame_free(&frame);
+        } else
+            return ff_filter_frame(outlink, frame);
     }
 
     FF_FILTER_FORWARD_STATUS(inlink, outlink);