[FFmpeg-devel] [PATCH] Added more tests to libswscale/utils.c
Michael Niedermayer
michael at niedermayer.cc
Mon Apr 4 22:38:19 CEST 2016
On Mon, Apr 04, 2016 at 06:41:51PM +0000, Petru Rares Sincraian wrote:
> Hi,
>
> Now the function computes the sum of the values and displays the result.
>
>
> Thanks,
> Petru Rares.
>
> ________________________________________
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> on behalf of Michael Niedermayer <michael at niedermayer.cc>
> Sent: Wednesday, March 30, 2016 4:18 PM
> To: FFmpeg development discussions and patches
> Subject: Re: [FFmpeg-devel] [PATCH] Added more tests to libswscale/utils.c
>
> On Wed, Mar 30, 2016 at 10:12:29AM +0000, Petru Rares Sincraian wrote:
> > Hi,
> >
> > I solved the problems with swscale_license() and alloc_gamma_tbl() functions. I am working to solve alphaless_fmt() function in a different way.
> >
> > ________________________________________
> > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> on behalf of Michael Niedermayer <michael at niedermayer.cc>
> > Sent: Sunday, March 27, 2016 9:43 PM
> > To: FFmpeg development discussions and patches
> > Subject: Re: [FFmpeg-devel] [PATCH] Added more tests to libswscale/utils.c
> >
> > On Sun, Mar 27, 2016 at 04:39:39PM +0000, Petru Rares Sincraian wrote:
> > >
> > > - Added test for: swscale_license()
> > > - Added test for: alphaless_fmt()
> > > - Added test for: alloc_gamma_tbl()
> > [...]
> >
> > > +static void test_alloc_gamma_tbl(void)
> > > +{
> > > + uint16_t *tbl = alloc_gamma_tbl(1.);
> > > + int i;
> > > +
> > > + // print only 32 elements
> > > + printf("e = 1.0\n");
> > > + for (i = 0; i < 65536; i += 2048)
> > > + printf("it: %d\t value: %d\n", i, tbl[i]);
> > > +
> > > + tbl = alloc_gamma_tbl(0.75);
> > > + printf("\ne = 0.75\n");
> > > + for (i = 0; i < 65536; i += 2048)
> > > + printf("it: %d\t value: %d\n", i, tbl[i]);
> > > +
> > > + tbl = alloc_gamma_tbl(2.8);
> > > + printf("\ne = 2.8\n");
> > > + for (i = 0; i < 65536; i += 2048)
> > > + printf("it: %d\t value: %d\n", i, tbl[i]);
> >
> > this leaks memory
> >
> > [...]
> > --
> > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > In a rich man's house there is no place to spit but his face.
> > -- Diogenes of Sinope
>
> > libswscale/Makefile | 1
> > libswscale/utils.c | 49 +++++++++++++++++++++
> > tests/Makefile | 1
> > tests/fate/libswscale.mak | 6 ++
> > tests/ref/fate/utils | 105 ++++++++++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 162 insertions(+)
> > f3e7cf692c31bf2df982eeeb97fc6fef8e3ec736 0001-Added-more-tests-to-libswscale-utils.c.patch
> > From ebefe53b13c878d50f5a388022c894d2b2c5ee96 Mon Sep 17 00:00:00 2001
> > From: Petru Rares Sincraian <psincraian at outlook.com>
> > Date: Thu, 24 Mar 2016 18:46:02 +0100
> > Subject: [PATCH] Added more tests to libswscale/utils.c
> >
> > - Added test for: swscale_license()
> > - Added test for: alloc_gamma_tbl()
> > ---
> > libswscale/Makefile | 1 +
> > libswscale/utils.c | 49 ++++++++++++++++++++++
> > tests/Makefile | 1 +
> > tests/fate/libswscale.mak | 6 +++
> > tests/ref/fate/utils | 105 ++++++++++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 162 insertions(+)
> > create mode 100644 tests/fate/libswscale.mak
> > create mode 100644 tests/ref/fate/utils
> >
> > diff --git a/libswscale/Makefile b/libswscale/Makefile
> > index a9f9e03..a6ae81d 100644
> > --- a/libswscale/Makefile
> > +++ b/libswscale/Makefile
> > @@ -27,3 +27,4 @@ SLIBOBJS-$(HAVE_GNU_WINDRES) += swscaleres.o
> >
> > TESTPROGS = colorspace \
> > swscale \
> > + utils \
> > diff --git a/libswscale/utils.c b/libswscale/utils.c
> > index ba409d6..b572a11 100644
> > --- a/libswscale/utils.c
> > +++ b/libswscale/utils.c
> > @@ -2410,3 +2410,52 @@ struct SwsContext *sws_getCachedContext(struct SwsContext *context, int srcW,
> > }
> > return context;
> > }
> > +
> > +#ifdef TEST
> > +
>
> > +static void test_swscale_license(void)
> > +{
> > + const char *license = swscale_license();
> > + if (!strcmp(FFMPEG_LICENSE, license))
> > + printf("License OK\n");
> > + else
> > + printf("License don't match.\nExpect: %s\nGiven: %s\n",
> > + FFMPEG_LICENSE, license);
> > +}
>
>
> > +
> > +static void test_alloc_gamma_tbl(void)
> > +{
> > + uint16_t *tbl;
> > + int i;
> > +
> > + // print only 32 elements
> > + tbl = alloc_gamma_tbl(1.);
> > + printf("e = 1.0\n");
> > + for (i = 0; i < 65536; i += 2048)
> > + printf("it: %d\t value: %d\n", i, tbl[i]);
> > + av_free(tbl);
> > +
> > + tbl = alloc_gamma_tbl(0.75);
> > + printf("\ne = 0.75\n");
> > + for (i = 0; i < 65536; i += 2048)
> > + printf("it: %d\t value: %d\n", i, tbl[i]);
> > + av_free(tbl);
> > +
> > + tbl = alloc_gamma_tbl(2.8);
> > + printf("\ne = 2.8\n");
> > + for (i = 0; i < 65536; i += 2048)
> > + printf("it: %d\t value: %d\n", i, tbl[i]);
> > + av_free(tbl);
> > +
> > +}
>
> missing malloc failure checks
> also a better way to test matematical tables is to check how close
> they are to the ideal double precission values
> dumping all values to a file would detect changes but not give
> any information about if the changes are better or worse so printing
> the sum of squared differences or something should be more usefull
>
>
> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Observe your enemies, for they first find out your faults. -- Antisthenes
> libswscale/Makefile | 1
> libswscale/utils.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++
> tests/Makefile | 1
> tests/fate/libswscale.mak | 6 ++++
> tests/ref/fate/utils | 12 +++++++++
> 5 files changed, 80 insertions(+)
> 150b3cb8ccffdcb1a7a884dee4833860fc8f3be9 0001-Added-more-tests-to-libswscale-utils.c.patch
> From 257094f0485c7b800c14fd8da86af01c87e671ff Mon Sep 17 00:00:00 2001
> From: Petru Rares Sincraian <psincraian at outlook.com>
> Date: Thu, 24 Mar 2016 18:46:02 +0100
> Subject: [PATCH] Added more tests to libswscale/utils.c
>
> - Added test for: swscale_license()
> - Added test for: alloc_gamma_tbl()
> ---
> libswscale/Makefile | 1 +
> libswscale/utils.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++
> tests/Makefile | 1 +
> tests/fate/libswscale.mak | 6 +++++
> tests/ref/fate/utils | 12 ++++++++++
> 5 files changed, 80 insertions(+)
> create mode 100644 tests/fate/libswscale.mak
> create mode 100644 tests/ref/fate/utils
>
> diff --git a/libswscale/Makefile b/libswscale/Makefile
> index a9f9e03..a6ae81d 100644
> --- a/libswscale/Makefile
> +++ b/libswscale/Makefile
> @@ -27,3 +27,4 @@ SLIBOBJS-$(HAVE_GNU_WINDRES) += swscaleres.o
>
> TESTPROGS = colorspace \
> swscale \
> + utils \
> diff --git a/libswscale/utils.c b/libswscale/utils.c
> index ba409d6..6ddaf4b 100644
> --- a/libswscale/utils.c
> +++ b/libswscale/utils.c
> @@ -2410,3 +2410,63 @@ struct SwsContext *sws_getCachedContext(struct SwsContext *context, int srcW,
> }
> return context;
> }
> +
> +#ifdef TEST
> +
> +static void test_swscale_license(void)
> +{
> + const char *license = swscale_license();
> + if (!strcmp(FFMPEG_LICENSE, license))
> + printf("License OK\n");
> + else
> + printf("License don't match.\nExpect: %s\nGiven: %s\n",
> + FFMPEG_LICENSE, license);
> +}
> +
> +static uint64_t get_sum_tbl_elements(uint16_t *tbl)
> +{
> + uint64_t sum = 0;
> + int i;
> +
> + if (!tbl) {
> + fprintf(stderr, "Error: tbl is null\n");
> + return sum;
> + }
> +
> + for (i = 0; i < 65536; ++i)
> + sum += (double) tbl[i];
> +
> + return sum;
the uint16 values should be compared against the ideal correct
double values and the sum of squared differences be checked against
some threshold for example
just calculatng the sum of values wont tell if one value is by 20
to big and another by 20 too small
that is if you wanted to know if func() is a sine then you could
compare |func(i) - sin(i)| < something_small
also this test fails on x86-32 (due to rounding differences somewhere
i assume)
this could be from a rounding difference in alloc_gamma_tbl()
between x86-32 and x86-64 pow()
implementing alloc_gamma_tbl() with only integers and no float/double
code like pow() would avoid the rouding difference
having a test that tests if the function still matches the ideal one
would allow testing such implementation
(implemeting alloc_gamma_tbl() purely with integers is non trivial
though and not your problem but of course if you want to try it
all contributions are certainly counted for GSoC qualification,
see log16 in tests/tiny_psnr.c for fixed point log/exp if you want
to try)
--- /home/michael/ffmpeg-git/ffmpeg/tests/ref/fate/utils 2016-04-04 21:44:13.325632330 +0200
+++ tests/data/fate/utils 2016-04-04 21:54:38.073645491 +0200
@@ -3,7 +3,7 @@
Testing alloc_gamma_tbl()
e = 1.0
-Sum: 2147450880
+Sum: 2147418105
e = 0.75
Sum: 2454191928
Test utils failed. Look at tests/data/fate/utils.err for details.
make: *** [fate-utils] Error 1
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160404/b4d8cffe/attachment.sig>
More information about the ffmpeg-devel
mailing list