| Message ID | 20190921144932.32241-1-lance.lmwang@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sat, 21 Sep 2019, lance.lmwang@gmail.com wrote: > From: Limin Wang <lance.lmwang@gmail.com> > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > --- > doc/filters.texi | 2 ++ > libavfilter/vf_framerate.c | 17 +++++++++++------ > 2 files changed, 13 insertions(+), 6 deletions(-) > > diff --git a/doc/filters.texi b/doc/filters.texi > index bbfdad4..fce4ef4 100644 > --- a/doc/filters.texi > +++ b/doc/filters.texi > @@ -10637,6 +10637,8 @@ Specify flags influencing the filter process. > Available value for @var{flags} is: > > @table @option > +@item none > +Disable scene change detection This is only true if no additional flags are added later. The reason of using a flags option is future extensibility, so adding such a constant kind of defeats that purpose. It is also uneeded, "none" is a a named constant which you can always use to explicitly set no flags. So this patch seems uneeded. Regards, Marton > @item scene_change_detect, scd > Enable scene change detection using the value of the option @var{scene}. > This flag is enabled by default. > diff --git a/libavfilter/vf_framerate.c b/libavfilter/vf_framerate.c > index 06e463e..99faf75 100644 > --- a/libavfilter/vf_framerate.c > +++ b/libavfilter/vf_framerate.c > @@ -44,7 +44,9 @@ > #define OFFSET(x) offsetof(FrameRateContext, x) > #define V AV_OPT_FLAG_VIDEO_PARAM > #define F AV_OPT_FLAG_FILTERING_PARAM > +#define FRAMERATE_FLAG_NONE 00 > #define FRAMERATE_FLAG_SCD 01 > +#define CONST(name, help, val, unit) { name, help, 0, AV_OPT_TYPE_CONST, {.i64=val}, 0, 0, V|F, unit } > > static const AVOption framerate_options[] = { > {"fps", "required output frames per second rate", OFFSET(dest_frame_rate), AV_OPT_TYPE_VIDEO_RATE, {.str="50"}, 0, INT_MAX, V|F }, > @@ -53,9 +55,10 @@ static const AVOption framerate_options[] = { > {"interp_end", "point to end linear interpolation", OFFSET(interp_end), AV_OPT_TYPE_INT, {.i64=240}, 0, 255, V|F }, > {"scene", "scene change level", OFFSET(scene_score), AV_OPT_TYPE_DOUBLE, {.dbl=8.2}, 0, INT_MAX, V|F }, > > - {"flags", "set flags", OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64=1}, 0, INT_MAX, V|F, "flags" }, > - {"scene_change_detect", "enable scene change detection", 0, AV_OPT_TYPE_CONST, {.i64=FRAMERATE_FLAG_SCD}, INT_MIN, INT_MAX, V|F, "flags" }, > - {"scd", "enable scene change detection", 0, AV_OPT_TYPE_CONST, {.i64=FRAMERATE_FLAG_SCD}, INT_MIN, INT_MAX, V|F, "flags" }, > + {"flags", "set flags", OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64=FRAMERATE_FLAG_SCD}, FRAMERATE_FLAG_NONE, FRAMERATE_FLAG_SCD, V|F, "flags" }, > + CONST("none", "disable scene change detection", FRAMERATE_FLAG_NONE, "flags"), > + CONST("scene_change_detect", "enable scene change detection", FRAMERATE_FLAG_SCD, "flags"), > + CONST("scd", "enable scene change detection", FRAMERATE_FLAG_SCD, "flags"), > > {NULL} > }; > @@ -301,9 +304,11 @@ static int config_input(AVFilterLink *inlink) > s->bitdepth = pix_desc->comp[0].depth; > s->vsub = pix_desc->log2_chroma_h; > > - s->sad = ff_scene_sad_get_fn(s->bitdepth == 8 ? 8 : 16); > - if (!s->sad) > - return AVERROR(EINVAL); > + if ((s->flags & FRAMERATE_FLAG_SCD)) { > + s->sad = ff_scene_sad_get_fn(s->bitdepth == 8 ? 8 : 16); > + if (!s->sad) > + return AVERROR(EINVAL); > + } > > s->srce_time_base = inlink->time_base; > > -- > 2.6.4 > > _______________________________________________ > 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 Sun, Sep 22, 2019 at 08:54:36PM +0200, Marton Balint wrote: > > > On Sat, 21 Sep 2019, lance.lmwang@gmail.com wrote: > > >From: Limin Wang <lance.lmwang@gmail.com> > > > >Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > >--- > >doc/filters.texi | 2 ++ > >libavfilter/vf_framerate.c | 17 +++++++++++------ > >2 files changed, 13 insertions(+), 6 deletions(-) > > > >diff --git a/doc/filters.texi b/doc/filters.texi > >index bbfdad4..fce4ef4 100644 > >--- a/doc/filters.texi > >+++ b/doc/filters.texi > >@@ -10637,6 +10637,8 @@ Specify flags influencing the filter process. > >Available value for @var{flags} is: > > > >@table @option > >+@item none > >+Disable scene change detection > > This is only true if no additional flags are added later. The reason > of using a flags option is future extensibility, so adding such a > constant kind of defeats that purpose. It is also uneeded, "none" is > a a named constant which you can always use to explicitly set no > flags. > > So this patch seems uneeded. Marton, thanks for your review, the none flags is for debug purpose, without sceencut detection, the result will consistent. So it's necessary to have none flags to turn it off. > > Regards, > Marton > > >@item scene_change_detect, scd > >Enable scene change detection using the value of the option @var{scene}. > >This flag is enabled by default. > >diff --git a/libavfilter/vf_framerate.c b/libavfilter/vf_framerate.c > >index 06e463e..99faf75 100644 > >--- a/libavfilter/vf_framerate.c > >+++ b/libavfilter/vf_framerate.c > >@@ -44,7 +44,9 @@ > >#define OFFSET(x) offsetof(FrameRateContext, x) > >#define V AV_OPT_FLAG_VIDEO_PARAM > >#define F AV_OPT_FLAG_FILTERING_PARAM > >+#define FRAMERATE_FLAG_NONE 00 > >#define FRAMERATE_FLAG_SCD 01 > >+#define CONST(name, help, val, unit) { name, help, 0, AV_OPT_TYPE_CONST, {.i64=val}, 0, 0, V|F, unit } > > > >static const AVOption framerate_options[] = { > > {"fps", "required output frames per second rate", OFFSET(dest_frame_rate), AV_OPT_TYPE_VIDEO_RATE, {.str="50"}, 0, INT_MAX, V|F }, > >@@ -53,9 +55,10 @@ static const AVOption framerate_options[] = { > > {"interp_end", "point to end linear interpolation", OFFSET(interp_end), AV_OPT_TYPE_INT, {.i64=240}, 0, 255, V|F }, > > {"scene", "scene change level", OFFSET(scene_score), AV_OPT_TYPE_DOUBLE, {.dbl=8.2}, 0, INT_MAX, V|F }, > > > >- {"flags", "set flags", OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64=1}, 0, INT_MAX, V|F, "flags" }, > >- {"scene_change_detect", "enable scene change detection", 0, AV_OPT_TYPE_CONST, {.i64=FRAMERATE_FLAG_SCD}, INT_MIN, INT_MAX, V|F, "flags" }, > >- {"scd", "enable scene change detection", 0, AV_OPT_TYPE_CONST, {.i64=FRAMERATE_FLAG_SCD}, INT_MIN, INT_MAX, V|F, "flags" }, > >+ {"flags", "set flags", OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64=FRAMERATE_FLAG_SCD}, FRAMERATE_FLAG_NONE, FRAMERATE_FLAG_SCD, V|F, "flags" }, > >+ CONST("none", "disable scene change detection", FRAMERATE_FLAG_NONE, "flags"), > >+ CONST("scene_change_detect", "enable scene change detection", FRAMERATE_FLAG_SCD, "flags"), > >+ CONST("scd", "enable scene change detection", FRAMERATE_FLAG_SCD, "flags"), > > > > {NULL} > >}; > >@@ -301,9 +304,11 @@ static int config_input(AVFilterLink *inlink) > > s->bitdepth = pix_desc->comp[0].depth; > > s->vsub = pix_desc->log2_chroma_h; > > > >- s->sad = ff_scene_sad_get_fn(s->bitdepth == 8 ? 8 : 16); > >- if (!s->sad) > >- return AVERROR(EINVAL); > >+ if ((s->flags & FRAMERATE_FLAG_SCD)) { > >+ s->sad = ff_scene_sad_get_fn(s->bitdepth == 8 ? 8 : 16); > >+ if (!s->sad) > >+ return AVERROR(EINVAL); > >+ } > > > > s->srce_time_base = inlink->time_base; > > > >-- > >2.6.4 > > > >_______________________________________________ > >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". > _______________________________________________ > 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 Mon, 23 Sep 2019, Limin Wang wrote: > On Sun, Sep 22, 2019 at 08:54:36PM +0200, Marton Balint wrote: >> >> >> On Sat, 21 Sep 2019, lance.lmwang@gmail.com wrote: >> >> >From: Limin Wang <lance.lmwang@gmail.com> >> > >> >Signed-off-by: Limin Wang <lance.lmwang@gmail.com> >> >--- >> >doc/filters.texi | 2 ++ >> >libavfilter/vf_framerate.c | 17 +++++++++++------ >> >2 files changed, 13 insertions(+), 6 deletions(-) >> > >> >diff --git a/doc/filters.texi b/doc/filters.texi >> >index bbfdad4..fce4ef4 100644 >> >--- a/doc/filters.texi >> >+++ b/doc/filters.texi >> >@@ -10637,6 +10637,8 @@ Specify flags influencing the filter process. >> >Available value for @var{flags} is: >> > >> >@table @option >> >+@item none >> >+Disable scene change detection >> >> This is only true if no additional flags are added later. The reason >> of using a flags option is future extensibility, so adding such a >> constant kind of defeats that purpose. It is also uneeded, "none" is >> a a named constant which you can always use to explicitly set no >> flags. >> >> So this patch seems uneeded. > > Marton, thanks for your review, the none flags is for debug purpose, > without sceencut detection, the result will consistent. So it's > necessary to have none flags to turn it off. You can change flags using the +flag or -flag syntax. You don't need a separate constant for this. Regards, Marton
On Mon, Sep 23, 2019 at 11:18:01PM +0200, Marton Balint wrote: > > > On Mon, 23 Sep 2019, Limin Wang wrote: > > >On Sun, Sep 22, 2019 at 08:54:36PM +0200, Marton Balint wrote: > >> > >> > >>On Sat, 21 Sep 2019, lance.lmwang@gmail.com wrote: > >> > >>>From: Limin Wang <lance.lmwang@gmail.com> > >>> > >>>Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > >>>--- > >>>doc/filters.texi | 2 ++ > >>>libavfilter/vf_framerate.c | 17 +++++++++++------ > >>>2 files changed, 13 insertions(+), 6 deletions(-) > >>> > >>>diff --git a/doc/filters.texi b/doc/filters.texi > >>>index bbfdad4..fce4ef4 100644 > >>>--- a/doc/filters.texi > >>>+++ b/doc/filters.texi > >>>@@ -10637,6 +10637,8 @@ Specify flags influencing the filter process. > >>>Available value for @var{flags} is: > >>> > >>>@table @option > >>>+@item none > >>>+Disable scene change detection > >> > >>This is only true if no additional flags are added later. The reason > >>of using a flags option is future extensibility, so adding such a > >>constant kind of defeats that purpose. It is also uneeded, "none" is > >>a a named constant which you can always use to explicitly set no > >>flags. > >> > >>So this patch seems uneeded. > > > >Marton, thanks for your review, the none flags is for debug > >purpose, without sceencut detection, the result will consistent. > >So it's > >necessary to have none flags to turn it off. > > You can change flags using the +flag or -flag syntax. You don't need > a separate constant for this. OK, thank for the hint, I don't know it's supporot -flag with one only although I have used flag+flag before. Any comments for the other patches, if ok, I'll update the patch and remove this one. > > 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 bbfdad4..fce4ef4 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -10637,6 +10637,8 @@ Specify flags influencing the filter process. Available value for @var{flags} is: @table @option +@item none +Disable scene change detection @item scene_change_detect, scd Enable scene change detection using the value of the option @var{scene}. This flag is enabled by default. diff --git a/libavfilter/vf_framerate.c b/libavfilter/vf_framerate.c index 06e463e..99faf75 100644 --- a/libavfilter/vf_framerate.c +++ b/libavfilter/vf_framerate.c @@ -44,7 +44,9 @@ #define OFFSET(x) offsetof(FrameRateContext, x) #define V AV_OPT_FLAG_VIDEO_PARAM #define F AV_OPT_FLAG_FILTERING_PARAM +#define FRAMERATE_FLAG_NONE 00 #define FRAMERATE_FLAG_SCD 01 +#define CONST(name, help, val, unit) { name, help, 0, AV_OPT_TYPE_CONST, {.i64=val}, 0, 0, V|F, unit } static const AVOption framerate_options[] = { {"fps", "required output frames per second rate", OFFSET(dest_frame_rate), AV_OPT_TYPE_VIDEO_RATE, {.str="50"}, 0, INT_MAX, V|F }, @@ -53,9 +55,10 @@ static const AVOption framerate_options[] = { {"interp_end", "point to end linear interpolation", OFFSET(interp_end), AV_OPT_TYPE_INT, {.i64=240}, 0, 255, V|F }, {"scene", "scene change level", OFFSET(scene_score), AV_OPT_TYPE_DOUBLE, {.dbl=8.2}, 0, INT_MAX, V|F }, - {"flags", "set flags", OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64=1}, 0, INT_MAX, V|F, "flags" }, - {"scene_change_detect", "enable scene change detection", 0, AV_OPT_TYPE_CONST, {.i64=FRAMERATE_FLAG_SCD}, INT_MIN, INT_MAX, V|F, "flags" }, - {"scd", "enable scene change detection", 0, AV_OPT_TYPE_CONST, {.i64=FRAMERATE_FLAG_SCD}, INT_MIN, INT_MAX, V|F, "flags" }, + {"flags", "set flags", OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64=FRAMERATE_FLAG_SCD}, FRAMERATE_FLAG_NONE, FRAMERATE_FLAG_SCD, V|F, "flags" }, + CONST("none", "disable scene change detection", FRAMERATE_FLAG_NONE, "flags"), + CONST("scene_change_detect", "enable scene change detection", FRAMERATE_FLAG_SCD, "flags"), + CONST("scd", "enable scene change detection", FRAMERATE_FLAG_SCD, "flags"), {NULL} }; @@ -301,9 +304,11 @@ static int config_input(AVFilterLink *inlink) s->bitdepth = pix_desc->comp[0].depth; s->vsub = pix_desc->log2_chroma_h; - s->sad = ff_scene_sad_get_fn(s->bitdepth == 8 ? 8 : 16); - if (!s->sad) - return AVERROR(EINVAL); + if ((s->flags & FRAMERATE_FLAG_SCD)) { + s->sad = ff_scene_sad_get_fn(s->bitdepth == 8 ? 8 : 16); + if (!s->sad) + return AVERROR(EINVAL); + } s->srce_time_base = inlink->time_base;