Re: [FFmpeg-devel] [PATCH v2] area changed: scdet filter

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 13656 bytes --]

On Tue, Jun 11, 2024 at 10:07:32AM +0300, radu.taraibuta@gmail.com wrote:
> 
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Michael Niedermayer
> > Sent: marți, 4 iunie 2024 01:42
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH] area changed: scdet filter
> > 
> > On Sun, Jun 02, 2024 at 11:17:29PM +0300, radu.taraibuta@gmail.com wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > > Michael Niedermayer
> > > > Sent: vineri, 31 mai 2024 00:32
> > > > To: FFmpeg development discussions and patches
> > > > <ffmpeg-devel@ffmpeg.org>
> > > > Subject: Re: [FFmpeg-devel] [PATCH] area changed: scdet filter
> > > >
> > > > On Mon, May 13, 2024 at 06:52:19PM +0300, radu.taraibuta@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@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@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@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@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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".