| Message ID | 20190912102259.16630-1-lance.lmwang@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 12 Sep 2019, lance.lmwang@gmail.com wrote: > From: Limin Wang <lance.lmwang@gmail.com> > > Have been tested by fate-suite/svq3/Vertical400kbit.sorenson3.mov and the > testing results are consistent. > > If it is acceptable, I want to move get_scene_score() to the public function and > make the scene change detection calculation method of the relevant module consistent. > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > --- > doc/filters.texi | 4 ++-- > libavfilter/vf_framerate.c | 6 +++--- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/doc/filters.texi b/doc/filters.texi > index 9d500e44a9..1c2e86b635 100644 > --- a/doc/filters.texi > +++ b/doc/filters.texi > @@ -10626,10 +10626,10 @@ the default is @code{240}. > > @item scene > Specify the level at which a scene change is detected as a value between > -0 and 100 to indicate a new scene; a low value reflects a low > +0 and 1.0 to indicate a new scene; a low value reflects a low You cannot change the scale of a user facing parameter, that would break existing command lines. > probability for the current frame to introduce a new scene, while a higher > value means the current frame is more likely to be one. > -The default is @code{8.2}. > +The default is @code{0.25}. You should not change the default unless you have a very good reason, as it would change existing behaviour. > > @item flags > Specify flags influencing the filter process. > diff --git a/libavfilter/vf_framerate.c b/libavfilter/vf_framerate.c > index 06e463e4d7..def3f19f55 100644 > --- a/libavfilter/vf_framerate.c > +++ b/libavfilter/vf_framerate.c > @@ -51,7 +51,7 @@ static const AVOption framerate_options[] = { > > {"interp_start", "point to start linear interpolation", OFFSET(interp_start), AV_OPT_TYPE_INT, {.i64=15}, 0, 255, V|F }, > {"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 }, > + {"scene", "scene change level", OFFSET(scene_score), AV_OPT_TYPE_DOUBLE, {.dbl=0.25}, 0, 1.0, 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" }, > @@ -77,9 +77,9 @@ static double get_scene_score(AVFilterContext *ctx, AVFrame *crnt, AVFrame *next > ff_dlog(ctx, "get_scene_score() process\n"); > s->sad(crnt->data[0], crnt->linesize[0], next->data[0], next->linesize[0], crnt->width, crnt->height, &sad); > emms_c(); > - mafd = (double)sad * 100.0 / (crnt->width * crnt->height) / (1 << s->bitdepth); > + mafd = (double)sad / (crnt->width * crnt->height) / (1ULL << (s->bitdepth - 8)); No, dividing by bitdepth-8 is completely wrong. It is wrong in f_select, but we could not change it there as that would have changed the scale of a user facing option. Regards, Marton > diff = fabs(mafd - s->prev_mafd); > - ret = av_clipf(FFMIN(mafd, diff), 0, 100.0); > + ret = av_clipf(FFMIN(mafd, diff) / 100., 0, 1); > s->prev_mafd = mafd; > } > ff_dlog(ctx, "get_scene_score() result is:%f\n", ret); > -- > 2.21.0 > > _______________________________________________ > 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 Thu, Sep 12, 2019 at 07:55:30PM +0200, Marton Balint wrote: > > > On Thu, 12 Sep 2019, lance.lmwang@gmail.com wrote: > > >From: Limin Wang <lance.lmwang@gmail.com> > > > >Have been tested by fate-suite/svq3/Vertical400kbit.sorenson3.mov and the > >testing results are consistent. > > > >If it is acceptable, I want to move get_scene_score() to the public function and > >make the scene change detection calculation method of the relevant module consistent. > > > >Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > >--- > >doc/filters.texi | 4 ++-- > >libavfilter/vf_framerate.c | 6 +++--- > >2 files changed, 5 insertions(+), 5 deletions(-) > > > >diff --git a/doc/filters.texi b/doc/filters.texi > >index 9d500e44a9..1c2e86b635 100644 > >--- a/doc/filters.texi > >+++ b/doc/filters.texi > >@@ -10626,10 +10626,10 @@ the default is @code{240}. > > > >@item scene > >Specify the level at which a scene change is detected as a value between > >-0 and 100 to indicate a new scene; a low value reflects a low > >+0 and 1.0 to indicate a new scene; a low value reflects a low > > You cannot change the scale of a user facing parameter, that would break > existing command lines. Yes, I'm afraid of it when I'm change it. Then it's impossible to make consistent. > > >probability for the current frame to introduce a new scene, while a higher > >value means the current frame is more likely to be one. > >-The default is @code{8.2}. > >+The default is @code{0.25}. > > You should not change the default unless you have a very good > reason, as it would change existing behaviour. > > > > >@item flags > >Specify flags influencing the filter process. > >diff --git a/libavfilter/vf_framerate.c b/libavfilter/vf_framerate.c > >index 06e463e4d7..def3f19f55 100644 > >--- a/libavfilter/vf_framerate.c > >+++ b/libavfilter/vf_framerate.c > >@@ -51,7 +51,7 @@ static const AVOption framerate_options[] = { > > > > {"interp_start", "point to start linear interpolation", OFFSET(interp_start), AV_OPT_TYPE_INT, {.i64=15}, 0, 255, V|F }, > > {"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 }, > >+ {"scene", "scene change level", OFFSET(scene_score), AV_OPT_TYPE_DOUBLE, {.dbl=0.25}, 0, 1.0, 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" }, > >@@ -77,9 +77,9 @@ static double get_scene_score(AVFilterContext *ctx, AVFrame *crnt, AVFrame *next > > ff_dlog(ctx, "get_scene_score() process\n"); > > s->sad(crnt->data[0], crnt->linesize[0], next->data[0], next->linesize[0], crnt->width, crnt->height, &sad); > > emms_c(); > >- mafd = (double)sad * 100.0 / (crnt->width * crnt->height) / (1 << s->bitdepth); > >+ mafd = (double)sad / (crnt->width * crnt->height) / (1ULL << (s->bitdepth - 8)); > > No, dividing by bitdepth-8 is completely wrong. It is wrong in > f_select, but we could not change it there as that would have > changed the scale of a user facing option. > I have changed vf_minterpolate filter, it divides (crnt->width * crnt->height * 3) which I think it's wrong, so I consider to make them using consistent method. If we need to keep the user option unchanged, then it's impossible to do it. I'll submit the change for review. > Regards, > Marton > > > > diff = fabs(mafd - s->prev_mafd); > >- ret = av_clipf(FFMIN(mafd, diff), 0, 100.0); > >+ ret = av_clipf(FFMIN(mafd, diff) / 100., 0, 1); > > s->prev_mafd = mafd; > > } > > ff_dlog(ctx, "get_scene_score() result is:%f\n", ret); > >-- > >2.21.0 > > > >_______________________________________________ > >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".
diff --git a/doc/filters.texi b/doc/filters.texi index 9d500e44a9..1c2e86b635 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -10626,10 +10626,10 @@ the default is @code{240}. @item scene Specify the level at which a scene change is detected as a value between -0 and 100 to indicate a new scene; a low value reflects a low +0 and 1.0 to indicate a new scene; a low value reflects a low probability for the current frame to introduce a new scene, while a higher value means the current frame is more likely to be one. -The default is @code{8.2}. +The default is @code{0.25}. @item flags Specify flags influencing the filter process. diff --git a/libavfilter/vf_framerate.c b/libavfilter/vf_framerate.c index 06e463e4d7..def3f19f55 100644 --- a/libavfilter/vf_framerate.c +++ b/libavfilter/vf_framerate.c @@ -51,7 +51,7 @@ static const AVOption framerate_options[] = { {"interp_start", "point to start linear interpolation", OFFSET(interp_start), AV_OPT_TYPE_INT, {.i64=15}, 0, 255, V|F }, {"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 }, + {"scene", "scene change level", OFFSET(scene_score), AV_OPT_TYPE_DOUBLE, {.dbl=0.25}, 0, 1.0, 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" }, @@ -77,9 +77,9 @@ static double get_scene_score(AVFilterContext *ctx, AVFrame *crnt, AVFrame *next ff_dlog(ctx, "get_scene_score() process\n"); s->sad(crnt->data[0], crnt->linesize[0], next->data[0], next->linesize[0], crnt->width, crnt->height, &sad); emms_c(); - mafd = (double)sad * 100.0 / (crnt->width * crnt->height) / (1 << s->bitdepth); + mafd = (double)sad / (crnt->width * crnt->height) / (1ULL << (s->bitdepth - 8)); diff = fabs(mafd - s->prev_mafd); - ret = av_clipf(FFMIN(mafd, diff), 0, 100.0); + ret = av_clipf(FFMIN(mafd, diff) / 100., 0, 1); s->prev_mafd = mafd; } ff_dlog(ctx, "get_scene_score() result is:%f\n", ret);