[FFmpeg-devel] tests/checkasm: add a test for VorbisDSPContext

Signed-off-by: James Almer <jamrial@gmail.com>
---
 tests/checkasm/Makefile    |  1 +
 tests/checkasm/checkasm.c  |  3 ++
 tests/checkasm/checkasm.h  |  1 +
 tests/checkasm/vorbisdsp.c | 88 ++++++++++++++++++++++++++++++++++++++
 tests/fate/checkasm.mak    |  1 +
 5 files changed, 94 insertions(+)
 create mode 100644 tests/checkasm/vorbisdsp.c

Hi,

On Mon, Sep 19, 2022 at 1:05 PM James Almer <jamrial@gmail.com> wrote:

> --- a/tests/checkasm/checkasm.c
> +++ b/tests/checkasm/checkasm.c
> @@ -165,6 +165,9 @@ static const struct {
>      #if CONFIG_VIDEODSP
>          { "videodsp", checkasm_check_videodsp },
>      #endif
> +    #if CONFIG_VIDEODSP
> +        { "vorbisdsp", checkasm_check_vorbisdsp },
> +    #endif
>

Why under config_videodsp?

Ronald

Le maanantaina 19. syyskuuta 2022, 20.04.41 EEST James Almer a écrit :
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  tests/checkasm/Makefile    |  1 +
>  tests/checkasm/checkasm.c  |  3 ++
>  tests/checkasm/checkasm.h  |  1 +
>  tests/checkasm/vorbisdsp.c | 88 ++++++++++++++++++++++++++++++++++++++
>  tests/fate/checkasm.mak    |  1 +
>  5 files changed, 94 insertions(+)
>  create mode 100644 tests/checkasm/vorbisdsp.c
> 
> diff --git a/tests/checkasm/Makefile b/tests/checkasm/Makefile
> index 1ac170491b..ac02670e64 100644
> --- a/tests/checkasm/Makefile
> +++ b/tests/checkasm/Makefile
> @@ -31,6 +31,7 @@ AVCODECOBJS-$(CONFIG_HEVC_DECODER)      += hevc_add_res.o
> hevc_idct.o hevc_sao.o AVCODECOBJS-$(CONFIG_UTVIDEO_DECODER)   +=
> utvideodsp.o
>  AVCODECOBJS-$(CONFIG_V210_DECODER)      += v210dec.o
>  AVCODECOBJS-$(CONFIG_V210_ENCODER)      += v210enc.o
> +AVCODECOBJS-$(CONFIG_VORBIS_DECODER)    += vorbisdsp.o
>  AVCODECOBJS-$(CONFIG_VP9_DECODER)       += vp9dsp.o
> 
>  CHECKASMOBJS-$(CONFIG_AVCODEC)          += $(AVCODECOBJS-yes)
> diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
> index e56fd3850e..f1080f2a06 100644
> --- a/tests/checkasm/checkasm.c
> +++ b/tests/checkasm/checkasm.c
> @@ -165,6 +165,9 @@ static const struct {
>      #if CONFIG_VIDEODSP
>          { "videodsp", checkasm_check_videodsp },
>      #endif
> +    #if CONFIG_VIDEODSP
> +        { "vorbisdsp", checkasm_check_vorbisdsp },
> +    #endif
>  #endif
>  #if CONFIG_AVFILTER
>      #if CONFIG_AFIR_FILTER
> diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h
> index d7645d3730..171dd06b47 100644
> --- a/tests/checkasm/checkasm.h
> +++ b/tests/checkasm/checkasm.h
> @@ -88,6 +88,7 @@ void checkasm_check_vf_threshold(void);
>  void checkasm_check_vp8dsp(void);
>  void checkasm_check_vp9dsp(void);
>  void checkasm_check_videodsp(void);
> +void checkasm_check_vorbisdsp(void);
> 
>  struct CheckasmPerf;
> 
> diff --git a/tests/checkasm/vorbisdsp.c b/tests/checkasm/vorbisdsp.c
> new file mode 100644
> index 0000000000..d28f08eace
> --- /dev/null
> +++ b/tests/checkasm/vorbisdsp.c
> @@ -0,0 +1,88 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with FFmpeg; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +#include "config.h"
> +
> +#include <float.h>
> +
> +#include "libavutil/mem_internal.h"
> +
> +#include "libavcodec/vorbisdsp.h"
> +
> +#include "checkasm.h"
> +
> +#define LEN 512
> +
> +#define randomize_buffer(buf)                 \
> +do {                                          \
> +    int i;                                    \
> +    double bmg[2], stddev = 10.0, mean = 0.0; \
> +                                              \
> +    for (i = 0; i < LEN; i += 2) {            \
> +        av_bmg_get(&checkasm_lfg, bmg);       \
> +        buf[i]     = bmg[0] * stddev + mean;  \
> +        buf[i + 1] = bmg[1] * stddev + mean;  \
> +    }                                         \
> +} while(0);
> +
> +static void test_inverse_coupling(void)
> +{
> +    LOCAL_ALIGNED_16(float, src0,  [LEN]);
> +    LOCAL_ALIGNED_16(float, src1,  [LEN]);
> +    LOCAL_ALIGNED_16(float, cdst,  [LEN]);
> +    LOCAL_ALIGNED_16(float, odst,  [LEN]);
> +    LOCAL_ALIGNED_16(float, cdst1, [LEN]);
> +    LOCAL_ALIGNED_16(float, odst1, [LEN]);
> +
> +    declare_func(void, float *av_restrict mag, float *av_restrict ang,
> +                 ptrdiff_t blocksize);
> +
> +    randomize_buffer(src0);
> +    randomize_buffer(src1);
> +
> +    memcpy(cdst,  src0, LEN * sizeof(*src0));
> +    memcpy(cdst1, src1, LEN * sizeof(*src1));
> +    memcpy(odst,  src0, LEN * sizeof(*src0));
> +    memcpy(odst1, src1, LEN * sizeof(*src1));
> +
> +    call_ref(cdst, cdst1, LEN);
> +    call_new(odst, odst1, LEN);
> +    for (int i = 0; i < LEN; i++) {
> +        if (!float_near_abs_eps(cdst[i],  odst[i],  FLT_EPSILON) ||
> +            !float_near_abs_eps(cdst1[i], odst1[i], FLT_EPSILON)) {
> +            fprintf(stderr, "%d: %- .12f - %- .12f = % .12g\n",
> +                    i, cdst[i], odst[i], cdst[i] - odst[i]);
> +            fprintf(stderr, "%d: %- .12f - %- .12f = % .12g\n",
> +                    i, cdst1[i], odst1[i], cdst1[i] - odst1[i]);
> +            fail();
> +            break;
> +        }
> +    }
> +    bench_new(src0, src1, LEN);
> +}
> +
> +void checkasm_check_vorbisdsp(void)
> +{
> +    VorbisDSPContext dsp;
> +
> +    ff_vorbisdsp_init(&dsp);
> +
> +    if (check_func(dsp.vorbis_inverse_coupling, "vorbis_inverse_coupling"))
> +        test_inverse_coupling();
> +    report("vorbis_inverse_coupling");

Should these not be just "inverse_coupling" seen as there is already a 
"vorbisdsp" prefix in the logs?

Other than that, this is very much welcome for me.

> +}
> diff --git a/tests/fate/checkasm.mak b/tests/fate/checkasm.mak
> index 4d2f321e84..fbba0b5b8f 100644
> --- a/tests/fate/checkasm.mak
> +++ b/tests/fate/checkasm.mak
> @@ -43,6 +43,7 @@ FATE_CHECKASM = fate-checkasm-aacpsdsp                    
>              \ fate-checkasm-vf_nlmeans                                \
> fate-checkasm-vf_threshold                              \
> fate-checkasm-videodsp                                  \ +               
> fate-checkasm-vorbisdsp                                 \
> fate-checkasm-vp8dsp                                    \
> fate-checkasm-vp9dsp                                    \

On 9/19/2022 2:55 PM, Ronald S. Bultje wrote:
> Hi,
> 
> On Mon, Sep 19, 2022 at 1:05 PM James Almer <jamrial@gmail.com> wrote:
> 
>> --- a/tests/checkasm/checkasm.c
>> +++ b/tests/checkasm/checkasm.c
>> @@ -165,6 +165,9 @@ static const struct {
>>       #if CONFIG_VIDEODSP
>>           { "videodsp", checkasm_check_videodsp },
>>       #endif
>> +    #if CONFIG_VIDEODSP
>> +        { "vorbisdsp", checkasm_check_vorbisdsp },
>> +    #endif
>>
> 
> Why under config_videodsp?

Because i copy pasted and forgot to change it :p

Fixed locally.

> 
> Ronald
> _______________________________________________
> 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 9/19/2022 2:57 PM, Rémi Denis-Courmont wrote:
>> +void checkasm_check_vorbisdsp(void)
>> +{
>> +    VorbisDSPContext dsp;
>> +
>> +    ff_vorbisdsp_init(&dsp);
>> +
>> +    if (check_func(dsp.vorbis_inverse_coupling, "vorbis_inverse_coupling"))
>> +        test_inverse_coupling();
>> +    report("vorbis_inverse_coupling");
> 
> Should these not be just "inverse_coupling" seen as there is already a
> "vorbisdsp" prefix in the logs?

It can, if anything so the relevant line is shorter when the report is 
printed. Changed locally.

I just went and used the name of the function pointer as it's in 
VorbisDSPContext. A name that could be changed too, for that matter.

> 
> Other than that, this is very much welcome for me.
> 
>> +}
>> diff --git a/tests/fate/checkasm.mak b/tests/fate/checkasm.mak
>> index 4d2f321e84..fbba0b5b8f 100644
>> --- a/tests/fate/checkasm.mak
>> +++ b/tests/fate/checkasm.mak
>> @@ -43,6 +43,7 @@ FATE_CHECKASM = fate-checkasm-aacpsdsp
>>               \ fate-checkasm-vf_nlmeans                                \
>> fate-checkasm-vf_threshold                              \
>> fate-checkasm-videodsp                                  \ +
>> fate-checkasm-vorbisdsp                                 \
>> fate-checkasm-vp8dsp                                    \
>> fate-checkasm-vp9dsp                                    \
> 
>