| Message ID | 20190930152218.6860-1-lance.lmwang@gmail.com |
|---|---|
| State | New |
| Headers | show |
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
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
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".
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".
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);