[FFmpeg-devel] [PATCH v3] area changed: scdet filter
Michael Niedermayer
michael at niedermayer.cc
Thu Jun 13 02:52:17 EEST 2024
On Wed, Jun 12, 2024 at 10:51:47PM +0300, radu.taraibuta at gmail.com wrote:
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> > Michael Niedermayer
> > Sent: marți, 11 iunie 2024 20:18
> > To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH v2] area changed: scdet filter
> >
> > On Tue, Jun 11, 2024 at 10:07:32AM +0300, radu.taraibuta at gmail.com wrote:
> > >
> > > > -----Original Message-----
> > > > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> > > > Michael Niedermayer
> > > > Sent: marți, 4 iunie 2024 01:42
> > > > To: FFmpeg development discussions and patches
> > > > <ffmpeg-devel at ffmpeg.org>
> > > > Subject: Re: [FFmpeg-devel] [PATCH] area changed: scdet filter
> > > >
> > > > On Sun, Jun 02, 2024 at 11:17:29PM +0300, radu.taraibuta at gmail.com
> > wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf
> > > > > > Of Michael Niedermayer
> > > > > > Sent: vineri, 31 mai 2024 00:32
> > > > > > To: FFmpeg development discussions and patches
> > > > > > <ffmpeg-devel at ffmpeg.org>
> > > > > > Subject: Re: [FFmpeg-devel] [PATCH] area changed: scdet filter
> > > > > >
> > > > > > On Mon, May 13, 2024 at 06:52:19PM +0300,
> > > > > > radu.taraibuta at gmail.com
> > > > > > wrote:
> > > > > > > Previous observations:
> > > > > > >
> > > > > > > - Inconsistent code style with other filters. (Mostly using
> > > > > > > AVFilterLink* link instead of AVFilterLink *link).
> > > > > > > I hope it's fine now.
> > > > > > >
> > > > > > > - Unrelated changes, please split trivial unrelated changes
> > > > > > > into separate patches.
> > > > > > > Removed trivial changes from this patch.
> > > > > > >
> > > > > > > - Can't tables be generated at .init/.config_props time? No
> > > > > > > point in storing them into binary.
> > > > > > > Done.
> > > > > > >
> > > > > > > - Adding extra delay is not backward compatible change, it
> > > > > > > should be implemented properly by adding option for users to select
> > mode:
> > > > > > > next & prev frame or just next or prev frame.
> > > > > > > Added legacy option to the mode parameter.
> > > > > > >
> > > > > > > - Could split frame clone change into earlier separate patch.
> > > > > > > Cannot be done. It's either frame clone or 1 frame delay.
> > > > > > >
> > > > > > > - Where are results of improvements with accuracy so it can
> > > > > > > be
> > > > > confirmed?
> > > > > > > Here are my test results with manual labeling of scene changes:
> > > > > > > 2379 Full length movie
> > > > > > >
> > > > > > > Method Threshold TP FP FN
> > Precision
> > > > > > > Recall F
> > > > > > > Cubic 7 2357 423 22 0.847841727
> > > > > 0.990752417
> > > > > > > 0.913742973
> > > > > > > Cubic 10 2297 200 82 0.919903885
> > > > > 0.965531736
> > > > > > > 0.94216571
> > > > > > > Cubic 12 2217 146 162 0.938214135
> > > > > 0.931904161
> > > > > > > 0.935048503
> > > > > > > Cubic 15 2049 101 330 0.953023256
> > > > > 0.861286255
> > > > > > > 0.904835505
> > > > > > > Linear 2.8 2357 1060 22 0.689786362
> > > > > 0.990752417
> > > > > > > 0.813319531
> > > > > > > Linear 8 2099 236 280 0.898929336
> > > > > 0.882303489
> > > > > > > 0.890538821
> > > > > > > Linear 10 1886 173 493 0.91597863
> > > > > 0.792770071
> > > > > > > 0.849932402
> > > > > > > Legacy 5 2235 1260 144 0.639484979
> > > > > > 0.939470366
> > > > > > > 0.760980592
> > > > > > > Legacy 8 1998 414 381 0.828358209
> > > > > > 0.839848676
> > > > > > > 0.83406387
> > > > > > > Legacy 10 1743 193 636 0.900309917
> > > > > > 0.732660782
> > > > > > > 0.80787949
> > > > > > >
> > > > > > > 15 HDR10Plus_PB_EAC3JOC
> > > > > > > https://mega.nz/file/nehDka6Z#C5_OPbSZkONdOp1jRmc09C9-
> > > > > > viDc3zMj8ZHruHcW
> > > > > > > KyA
> > > > > > >
> > > > > > > Method Threshold TP FP FN
> > Precision
> > > > > > > Recall F
> > > > > > > Cubic 10 15 0 0 1 1 1
> > > > > > > Linear 5 13 1 2 0.928571429
> > > > > 0.866666667
> > > > > > > 0.896551724
> > > > > > > Legacy 5 12 2 3 0.857142857 0.8
> > > > > > > 0.827586207
> > > > > > >
> > > > > > > 21 (HDR HEVC 10-bit BT.2020 24fps) Exodus Sample
> > > > > > >
> > > > > >
> > > >
> > https://mega.nz/file/Sfw1hDpK#ErxCOpQDVjcI1gq6ZbX3vIfdtXZompkFe0jq47
> > > > > > E
> > > > > > h
> > > > > > > R2o
> > > > > > >
> > > > > > > Method Threshold TP FP FN
> > Precision
> > > > > > > Recall F
> > > > > > > Cubic 10 21 0 0 1 1 1
> > > > > > > Linear 4 20 0 1 1 0.952380952
> > > > > > > 0.975609756
> > > > > > > Legacy 4 19 0 2 1 0.904761905
> > > > > > 0.95
> > > > > > >
> > > > > > > 94 Bieber Grammys
> > > > > > > https://mega.nz/#!c9dhAaKA!MG5Yi-
> > > > > > MJNATE2_KqcnNJZCRKtTWvdjJP1NwG8Ggdw3E
> > > > > > >
> > > > > > > Method Threshold TP FP FN
> > Precision
> > > > > > > Recall F
> > > > > > > Cubic 15 91 23 3 0.798245614
> > > > > 0.968085106
> > > > > > > 0.875
> > > > > > > Cubic 18 85 9 9 0.904255319
> > > > > 0.904255319
> > > > > > > 0.904255319
> > > > > > > Linear 7 79 49 15 0.6171875
> > > > > 0.840425532
> > > > > > > 0.711711712
> > > > > > > Linear 8 74 28 20 0.725490196
> > > > > 0.787234043
> > > > > > > 0.755102041
> > > > > > > Legacy 7 74 40 20 0.649122807
> > > > > > 0.787234043
> > > > > > > 0.711538462
> > > > > > > Legacy 8 71 26 23 0.731958763
> > > > > > 0.755319149
> > > > > > > 0.743455497
> > > > > > >
> > > > > > >
> > > > > > > Improve scene detection accuracy by comparing frame with both
> > > > > > > previous and next frame (creates one frame delay).
> > > > > > > Add new mode parameter and new method to compute the frame
> > > > > > > difference using cubic square to increase the weight of small
> > > > > > > changes and new mean
> > > > > > formula.
> > > > > > > This improves accuracy significantly. Slightly improve
> > > > > > > performance by not using frame clone.
> > > > > > > Add legacy mode for backward compatibility.
> > > > > > >
> > > > > > > Signed-off-by: raduct <radu.taraibuta at gmail.com>
> > > > > > > ---
> > > > > > > doc/filters.texi | 16 ++++
> > > > > > > libavfilter/scene_sad.c | 151
> > ++++++++++++++++++++++++++++++++++
> > > > > > > libavfilter/scene_sad.h | 6 ++
> > > > > > > libavfilter/vf_scdet.c | 156 +++++++++++++++++++++++++-----------
> > > > > > > tests/fate/filter-video.mak | 3 +
> > > > > > > 5 files changed, 284 insertions(+), 48 deletions(-)
> > > > > > >
> > > > > > > diff --git a/doc/filters.texi b/doc/filters.texi index
> > > > > > > bfa8ccec8b..53814e003b 100644
> > > > > > > --- a/doc/filters.texi
> > > > > > > +++ b/doc/filters.texi
> > > > > > > @@ -21797,6 +21797,22 @@ Default value is @code{10.}.
> > > > > > > @item sc_pass, s
> > > > > > > Set the flag to pass scene change frames to the next filter.
> > > > > > > Default value is @code{0}
> > > > > >
> > > > > > The patch is corrupted by linebreaks:
> > > > > >
> > > > > > Applying: area changed: scdet filter
> > > > > > error: corrupt patch at line 16
> > > > > > Patch failed at 0001 area changed: scdet filter
> > > > > >
> > > > > > please check the linebreak settings or attach the patch or use
> > > > > > git
> > > > > send-email
> > > > > >
> > > > > > thx
> > > > > >
> > > > > > [...]
> > > > > > --
> > > > > > Michael GnuPG fingerprint:
> > > > > > 9FF2128B147EF6730BADF133611EC787040B0FAB
> > > > > >
> > > > > > Homeopathy is like voting while filling the ballot out with
> > > > > > transparent
> > > > > ink.
> > > > > > Sometimes the outcome one wanted occurs. Rarely its worse than
> > > > > > filling out
> > > > > a
> > > > > > ballot properly.
> > > > >
> > > > > Please find attached the patch.
> > > > >
> > > >
> > > > > doc/filters.texi | 16 ++++
> > > > > libavfilter/scene_sad.c | 151
> > > > ++++++++++++++++++++++++++++++++++++++++++
> > > > > libavfilter/scene_sad.h | 6 +
> > > > > libavfilter/vf_scdet.c | 156 ++++++++++++++++++++++++++++++---------
> > -----
> > > > > tests/fate/filter-video.mak | 3
> > > > > 5 files changed, 284 insertions(+), 48 deletions(-)
> > > > > 8f29f2e1c202ab283a9ca0f5d9599de6ab534d7a
> > > > > 0001-area-changed-scdet-filter.patch
> > > > > From 6d55c65d92376b0ab6e3bb2439af30fbcc430d0b Mon Sep 17
> > 00:00:00
> > > > 2001
> > > > > From: raduct <radu.taraibuta at gmail.com>
> > > > > Date: Wed, 8 May 2024 08:24:46 +0300
> > > > > Subject: [PATCH] area changed: scdet filter
> > > > >
> > > > > Improve scene detection accuracy by comparing frame with both
> > > > > previous
> > > > and next frame (creates one frame delay).
> > > > > Add new mode parameter and new method to compute the frame
> > > > > difference
> > > > using cubic square to increase the weight of small changes and new
> > > > mean formula. This improves accuracy significantly. Slightly improve
> > > > performance by not using frame clone.
> > > > > Add legacy mode for backward compatibility.
> > > > >
> > > > > Signed-off-by: raduct <radu.taraibuta at gmail.com>
> > > > > ---
> > > > > doc/filters.texi | 16 ++++
> > > > > libavfilter/scene_sad.c | 151 ++++++++++++++++++++++++++++++++++
> > > > > libavfilter/scene_sad.h | 6 ++
> > > > > libavfilter/vf_scdet.c | 156 +++++++++++++++++++++++++-----------
> > > > > tests/fate/filter-video.mak | 3 +
> > > > > 5 files changed, 284 insertions(+), 48 deletions(-)
> > > >
> > > > fails to build
> > > >
> > > > libavfilter/scene_sad.c: In function ‘ff_init_cbrt’:
> > > > libavfilter/scene_sad.c:86:5: warning: ISO C90 forbids mixed
> > > > declarations and code [-Wdeclaration-after-statement]
> > > > 86 | uint8_t *table = cbrt_table[bitdepth];
> > > > | ^~~~~~~
> > > > libavfilter/scene_sad.c:92:13: error: implicit declaration of
> > > > function ‘av_malloc’; did you mean ‘malloc’? [-Werror=implicit-function-
> > declaration]
> > > > 92 | table = av_malloc((1 << bitdepth) * (bitdepth > 8 ? 2 : 1));
> > > > | ^~~~~~~~~
> > > > | malloc
> > > > libavfilter/scene_sad.c:92:11: warning: assignment to ‘uint8_t *’
> > > > {aka ‘unsigned char *’} from ‘int’ makes pointer from integer without a cast
> > [-Wint-conversion]
> > > > 92 | table = av_malloc((1 << bitdepth) * (bitdepth > 8 ? 2 : 1));
> > > > | ^
> > > > libavfilter/scene_sad.c:98:5: warning: ISO C90 forbids mixed
> > > > declarations and code [-Wdeclaration-after-statement]
> > > > 98 | int size = 1 << bitdepth;
> > > > | ^~~
> > > > libavfilter/scene_sad.c: In function ‘ff_uninit_cbrt’:
> > > > libavfilter/scene_sad.c:120:9: error: implicit declaration of
> > > > function ‘av_free’; did you mean ‘free’? [-Werror=implicit-function-
> > declaration]
> > > > 120 | av_free(cbrt_table[bitdepth]);
> > > > | ^~~~~~~
> > > > | free
> > > > libavfilter/scene_sad.c: At top level:
> > > > libavfilter/scene_sad.c:126:6: error: no previous prototype for
> > > > ‘ff_scene_scrd_c’ [-Werror=missing-prototypes]
> > > > 126 | void ff_scene_scrd_c(SCENE_SAD_PARAMS)
> > > > | ^~~~~~~~~~~~~~~
> > > > libavfilter/scene_sad.c: In function ‘ff_scene_scrd_c’:
> > > > libavfilter/scene_sad.c:148:5: warning: ISO C90 forbids mixed
> > > > declarations and code [-Wdeclaration-after-statement]
> > > > 148 | double mean = (sqrt(scrdPlus) + sqrt(scrdMinus)) / 2.0;
> > > > | ^~~~~~
> > > > libavfilter/scene_sad.c: At top level:
> > > > libavfilter/scene_sad.c:152:6: error: no previous prototype for
> > > > ‘ff_scene_scrd2B_c’ [-Werror=missing-prototypes]
> > > > 152 | void ff_scene_scrd2B_c(SCENE_SAD_PARAMS, int bitdepth)
> > > > | ^~~~~~~~~~~~~~~~~
> > > > libavfilter/scene_sad.c: In function ‘ff_scene_scrd2B_c’:
> > > > libavfilter/scene_sad.c:179:5: warning: ISO C90 forbids mixed
> > > > declarations and code [-Wdeclaration-after-statement]
> > > > 179 | double mean = (sqrt(scrdPlus) + sqrt(scrdMinus)) / 2.0;
> > > > | ^~~~~~
> > > > libavfilter/scene_sad.c: At top level:
> > > > libavfilter/scene_sad.c:183:6: error: no previous prototype for
> > > > ‘ff_scene_scrd9_c’ [-Werror=missing-prototypes]
> > > > 183 | void ff_scene_scrd9_c(SCENE_SAD_PARAMS)
> > > > | ^~~~~~~~~~~~~~~~
> > > > libavfilter/scene_sad.c:188:6: error: no previous prototype for
> > > > ‘ff_scene_scrd10_c’ [-Werror=missing-prototypes]
> > > > 188 | void ff_scene_scrd10_c(SCENE_SAD_PARAMS)
> > > > | ^~~~~~~~~~~~~~~~~
> > > > libavfilter/scene_sad.c:193:6: error: no previous prototype for
> > > > ‘ff_scene_scrd12_c’ [-Werror=missing-prototypes]
> > > > 193 | void ff_scene_scrd12_c(SCENE_SAD_PARAMS)
> > > > | ^~~~~~~~~~~~~~~~~
> > > > libavfilter/scene_sad.c:198:6: error: no previous prototype for
> > > > ‘ff_scene_scrd14_c’ [-Werror=missing-prototypes]
> > > > 198 | void ff_scene_scrd14_c(SCENE_SAD_PARAMS)
> > > > | ^~~~~~~~~~~~~~~~~
> > > > libavfilter/scene_sad.c:203:6: error: no previous prototype for
> > > > ‘ff_scene_scrd16_c’ [-Werror=missing-prototypes]
> > > > 203 | void ff_scene_scrd16_c(SCENE_SAD_PARAMS)
> > > > | ^~~~~~~~~~~~~~~~~
> > > > cc1: some warnings being treated as errors
> > > > make: *** [ffbuild/common.mak:81: libavfilter/scene_sad.o] Error 1
> > > > make: *** Waiting for unfinished jobs....
> > > >
> > > >
> > > > [...]
> > > >
> > > > --
> > > > Michael GnuPG fingerprint:
> > > > 9FF2128B147EF6730BADF133611EC787040B0FAB
> > > >
> > > > Frequently ignored answer#1 FFmpeg bugs should be sent to our
> > bugtracker.
> > > > User questions about the command line tools should be sent to the
> > > > ffmpeg- user ML.
> > > > And questions about how to use libav* should be sent to the libav-user ML.
> > >
> > > Please find attached a new version of the patch.
> > > I'm building using msvc and I don't get these warnings.
> >
> > > doc/filters.texi | 16 ++++
> > > libavfilter/scene_sad.c | 157
> > ++++++++++++++++++++++++++++++++++++++++++
> > > libavfilter/scene_sad.h | 20 +++++
> > > libavfilter/vf_scdet.c | 161 ++++++++++++++++++++++++++++++--------------
> > > tests/fate/filter-video.mak | 3
> > > 5 files changed, 309 insertions(+), 48 deletions(-)
> > > 20737181fba9670a258cd6345edf36eae954bfa2
> > > 0001-area-changed-scdet-filter.patch
> > > From 0a6963360076213d30b70f8297eae3d44a638dab Mon Sep 17 00:00:00
> > 2001
> > > From: raduct <radu.taraibuta at gmail.com>
> > > Date: Wed, 8 May 2024 08:24:46 +0300
> > > Subject: [PATCH] area changed: scdet filter
> >
> > breaks fate (infinite loops here on ubuntu)
> > TEST filter-metadata-scdet
> > ^Cmake: *** [tests/Makefile:311: fate-filter-metadata-scdet] Interrupt
> >
> > it seems to never exit avformat_find_stream_info()
> >
> > thx
> >
> > [...]
> >
> > --
> > Michael GnuPG fingerprint:
> > 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
>
> V3 - fate should work now.
yes
[...]
> @@ -71,3 +73,158 @@ ff_scene_sad_fn ff_scene_sad_get_fn(int depth)
> return sad;
> }
>
> +static AVMutex cbrt_mutex = AV_MUTEX_INITIALIZER;
> +static uint8_t *cbrt_table[16] = { NULL };
> +static int cbrt_table_ref[16] = { 0 };
> +
> +int ff_init_cbrt(int bitdepth)
> +{
> + uint8_t *table;
> + int size;
> +
> + if (bitdepth < 4 || bitdepth > 16)
> + return AVERROR(EINVAL);
> +
> + ff_mutex_lock(&cbrt_mutex);
> +
> + table = cbrt_table[bitdepth];
> + if (table) {
> + cbrt_table_ref[bitdepth]++;
> + goto end;
> + }
> +
> + table = av_malloc((1 << bitdepth) * (bitdepth > 8 ? 2 : 1));
> + if (!table)
> + goto end;
> + cbrt_table[bitdepth] = table;
> + cbrt_table_ref[bitdepth] = 1;
> +
> + size = 1 << bitdepth;
> + double factor = pow(size - 1, 2. / 3.);
> + if (bitdepth <= 8) {
> + for (int i = 0; i < size; i++)
> + table[i] = round(factor * pow(i, 1. / 3.));
> + } else {
> + uint16_t *tablew = (uint16_t*)table;
> + for (int i = 0; i < size; i++)
> + tablew[i] = round(factor * pow(i, 1. / 3.));
> + }
> +
> +end:
> + ff_mutex_unlock(&cbrt_mutex);
> + return table != NULL;
> +}
> +
> +void ff_uninit_cbrt(int bitdepth)
> +{
> + if (bitdepth < 4 || bitdepth > 16)
> + return;
> + ff_mutex_lock(&cbrt_mutex);
> + if (!--cbrt_table_ref[bitdepth]) {
> + av_free(cbrt_table[bitdepth]);
> + cbrt_table[bitdepth] = NULL;
> + }
> + ff_mutex_unlock(&cbrt_mutex);
> +}
I honestly dont see the value of refcounting the global cbrt tables
either they can be static and initialized by a single AV_ONCE_INIT or
just allocate and reinint for each filter.
> +
> +void ff_scene_scrd_c(SCENE_SAD_PARAMS)
> +{
> + uint64_t scrdPlus = 0;
> + uint64_t scrdMinus = 0;
> + int x, y;
> + double mean;
> + uint8_t *table = cbrt_table[8];
> +
> + if (!table) {
> + *sum = 0;
> + return;
> + }
> +
> + for (y = 0; y < height; y++) {
> + for (x = 0; x < width; x++)
> + if (src1[x] > src2[x])
> + scrdMinus += table[src1[x] - src2[x]];
> + else
> + scrdPlus += table[src2[x] - src1[x]];
> + src1 += stride1;
> + src2 += stride2;
> + }
> +
> + mean = (sqrt(scrdPlus) + sqrt(scrdMinus)) / 2.0;
> + *sum = 2.0 * mean * mean;
one sqrt less:
0.5*(scrdPlus + scrdMinus) + sqrt(scrdPlus * (double)scrdMinus)
> +}
> +
> +void ff_scene_scrd2B_c(SCENE_SAD_PARAMS, int bitdepth)
> +{
> + uint64_t scrdPlus = 0;
> + uint64_t scrdMinus = 0;
> + const uint16_t *src1w = (const uint16_t*)src1;
> + const uint16_t *src2w = (const uint16_t*)src2;
> + int x, y;
> + double mean;
> + uint16_t *table = (uint16_t*)cbrt_table[bitdepth];
> +
> + if (!table) {
> + *sum = 0;
> + return;
> + }
> +
> + stride1 /= 2;
> + stride2 /= 2;
> +
> + for (y = 0; y < height; y++) {
> + for (x = 0; x < width; x++)
> + if (src1w[x] > src2w[x])
> + scrdMinus += table[src1w[x] - src2w[x]];
> + else
> + scrdPlus += table[src2w[x] - src1w[x]];
> + src1w += stride1;
> + src2w += stride2;
> + }
> +
> + mean = (sqrt(scrdPlus) + sqrt(scrdMinus)) / 2.0;
> + *sum = 2.0 * mean * mean;
> +}
> +
> +void ff_scene_scrd9_c(SCENE_SAD_PARAMS)
> +{
> + ff_scene_scrd2B_c(src1, stride1, src2, stride2, width, height, sum, 9);
> +}
> +
> +void ff_scene_scrd10_c(SCENE_SAD_PARAMS)
> +{
> + ff_scene_scrd2B_c(src1, stride1, src2, stride2, width, height, sum, 10);
> +}
> +
> +void ff_scene_scrd12_c(SCENE_SAD_PARAMS)
> +{
> + ff_scene_scrd2B_c(src1, stride1, src2, stride2, width, height, sum, 12);
> +}
> +
> +void ff_scene_scrd14_c(SCENE_SAD_PARAMS)
> +{
> + ff_scene_scrd2B_c(src1, stride1, src2, stride2, width, height, sum, 14);
> +}
> +
> +void ff_scene_scrd16_c(SCENE_SAD_PARAMS)
> +{
> + ff_scene_scrd2B_c(src1, stride1, src2, stride2, width, height, sum, 16);
> +}
duplicating this 5 times just for a different LUT pointer is ugly
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Nations do behave wisely once they have exhausted all other alternatives.
-- Abba Eban
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20240613/6feab0fd/attachment.sig>
More information about the ffmpeg-devel
mailing list