| Message ID | 20210805082650.32639-2-jdek@itanimul.li |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/3] checkasm: collapse hevc pel tests | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | fail | Make fate failed |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
On Thu, 5 Aug 2021, J. Dekker wrote: > Signed-off-by: J. Dekker <jdek@itanimul.li> > --- > tests/checkasm/Makefile | 1 + > tests/checkasm/checkasm.c | 3 + > tests/checkasm/checkasm.h | 1 + > tests/checkasm/h264chroma.c | 109 ++++++++++++++++++++++++++++++++++++ > tests/fate/checkasm.mak | 1 + > 5 files changed, 115 insertions(+) > create mode 100644 tests/checkasm/h264chroma.c > > What should I do for other codecs here, or just ignore non-h264? What do you mean here? > diff --git a/tests/checkasm/Makefile b/tests/checkasm/Makefile > index 4ef5fa87da..41222c3827 100644 > --- a/tests/checkasm/Makefile > +++ b/tests/checkasm/Makefile > @@ -6,6 +6,7 @@ AVCODECOBJS-$(CONFIG_BSWAPDSP) += bswapdsp.o > AVCODECOBJS-$(CONFIG_FLACDSP) += flacdsp.o > AVCODECOBJS-$(CONFIG_FMTCONVERT) += fmtconvert.o > AVCODECOBJS-$(CONFIG_G722DSP) += g722dsp.o > +AVCODECOBJS-$(CONFIG_H264CHROMA) += h264chroma.o > AVCODECOBJS-$(CONFIG_H264DSP) += h264dsp.o > AVCODECOBJS-$(CONFIG_H264PRED) += h264pred.o > AVCODECOBJS-$(CONFIG_H264QPEL) += h264qpel.o > diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c > index b1353f7cbe..154c4a5c01 100644 > --- a/tests/checkasm/checkasm.c > +++ b/tests/checkasm/checkasm.c > @@ -104,6 +104,9 @@ static const struct { > #if CONFIG_G722DSP > { "g722dsp", checkasm_check_g722dsp }, > #endif > + #if CONFIG_H264CHROMA > + { "h264chroma", checkasm_check_h264chroma }, > + #endif > #if CONFIG_H264DSP > { "h264dsp", checkasm_check_h264dsp }, > #endif > diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h > index 68b0697d3e..ac2f22af05 100644 > --- a/tests/checkasm/checkasm.h > +++ b/tests/checkasm/checkasm.h > @@ -56,6 +56,7 @@ void checkasm_check_flacdsp(void); > void checkasm_check_float_dsp(void); > void checkasm_check_fmtconvert(void); > void checkasm_check_g722dsp(void); > +void checkasm_check_h264chroma(void); > void checkasm_check_h264dsp(void); > void checkasm_check_h264pred(void); > void checkasm_check_h264qpel(void); > diff --git a/tests/checkasm/h264chroma.c b/tests/checkasm/h264chroma.c > new file mode 100644 > index 0000000000..0bd333327d > --- /dev/null > +++ b/tests/checkasm/h264chroma.c > @@ -0,0 +1,109 @@ > +/* > + * Copyright (c) 2015 Henrik Gramner > + * Copyright (c) 2021 J. Dekker > + * > + * 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 <string.h> > +#include "checkasm.h" > +#include "libavcodec/avcodec.h" > +#include "libavcodec/h264chroma.h" > +#include "libavutil/common.h" > +#include "libavutil/internal.h" > +#include "libavutil/intreadwrite.h" > +#include "libavutil/mem_internal.h" > + > +// static const int codec_ids[4] = { AV_CODEC_ID_H264, AV_CODEC_ID_VP8, AV_CODEC_ID_RV40, AV_CODEC_ID_SVQ3 }; I guess the comment above refers to this somehow, but I don't see what it does as it's unused and commented out? > +static const uint32_t pixel_mask[3] = { 0xffffffff, 0x01ff01ff, 0x03ff03ff }; > + > +#define SIZEOF_PIXEL ((bit_depth + 7) / 8) > +#define BUF_SIZE (3 * 16 * 17) > + > +#define randomize_buffers() \ > + do { \ > + uint32_t mask = pixel_mask[bit_depth - 8]; \ > + int i; \ > + for (i = 0; i < BUF_SIZE; i += 4) { \ > + uint32_t r = rnd() & mask; \ > + AV_WN32A(buf0 + i, r); \ > + AV_WN32A(buf1 + i, r); \ > + } \ > + } while (0) > + > +#define src0 (buf0 + 4 * 16) /* Offset to allow room for top and left */ > +#define src1 (buf1 + 4 * 16) > + > +static void check_avg() > +{ > + int i, bit_depth, x, y; > + LOCAL_ALIGNED_16(uint8_t, buf0, [BUF_SIZE]); > + LOCAL_ALIGNED_16(uint8_t, buf1, [BUF_SIZE]); > + LOCAL_ALIGNED_16(uint8_t, dst0, [BUF_SIZE]); > + LOCAL_ALIGNED_16(uint8_t, dst1, [BUF_SIZE]); > + H264ChromaContext h; > + > + declare_func_emms(AV_CPU_FLAG_MMX, void, uint8_t *dst, uint8_t *src, ptrdiff_t stride, int h, int x, int y); > + for (bit_depth = 8; bit_depth <= 10; bit_depth++) { > + ff_h264chroma_init(&h, bit_depth); > + for (i = 0; i < 4; i++) { > + if (check_func(h.avg_h264_chroma_pixels_tab[i], "avg_chroma_mc%d_%d", 1 << (3 - i), bit_depth)) { > + randomize_buffers(); > + x = rnd() & 0x7; y = rnd() & 0x7; > + call_ref(dst0, src0, 8, 4, x, y); > + call_new(dst1, src1, 8, 4, x, y); This doesn't modify the source, so both calls could use the same source buffer. But as avg reads the destination buffer, both destination buffers need to be intiailized matchingly. What heights is this called with in practice; if the real decoder calls it with heights other than 4, then the test should too. (And if it doesn't, why is there a parameter?) As a nice to have, it's often good to have a stride wider than the widest output, so there's always some padding between each line, to easier spot overwrites and avoid the possibility of some class of bugs while implementing the asm. > + if (memcmp(buf0, buf1, BUF_SIZE)) > + fail(); You're not comparing the destination buffers, you're comparing the source buffers. > +static void check_put() > +{ As the test code for put is essentially identical to avg, I guess they could share code too? Not that it's much or complicated right now, but it becomes at least a bit more complicated if you actually make it do the right thing ;-) // Martin
diff --git a/tests/checkasm/Makefile b/tests/checkasm/Makefile index 4ef5fa87da..41222c3827 100644 --- a/tests/checkasm/Makefile +++ b/tests/checkasm/Makefile @@ -6,6 +6,7 @@ AVCODECOBJS-$(CONFIG_BSWAPDSP) += bswapdsp.o AVCODECOBJS-$(CONFIG_FLACDSP) += flacdsp.o AVCODECOBJS-$(CONFIG_FMTCONVERT) += fmtconvert.o AVCODECOBJS-$(CONFIG_G722DSP) += g722dsp.o +AVCODECOBJS-$(CONFIG_H264CHROMA) += h264chroma.o AVCODECOBJS-$(CONFIG_H264DSP) += h264dsp.o AVCODECOBJS-$(CONFIG_H264PRED) += h264pred.o AVCODECOBJS-$(CONFIG_H264QPEL) += h264qpel.o diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c index b1353f7cbe..154c4a5c01 100644 --- a/tests/checkasm/checkasm.c +++ b/tests/checkasm/checkasm.c @@ -104,6 +104,9 @@ static const struct { #if CONFIG_G722DSP { "g722dsp", checkasm_check_g722dsp }, #endif + #if CONFIG_H264CHROMA + { "h264chroma", checkasm_check_h264chroma }, + #endif #if CONFIG_H264DSP { "h264dsp", checkasm_check_h264dsp }, #endif diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h index 68b0697d3e..ac2f22af05 100644 --- a/tests/checkasm/checkasm.h +++ b/tests/checkasm/checkasm.h @@ -56,6 +56,7 @@ void checkasm_check_flacdsp(void); void checkasm_check_float_dsp(void); void checkasm_check_fmtconvert(void); void checkasm_check_g722dsp(void); +void checkasm_check_h264chroma(void); void checkasm_check_h264dsp(void); void checkasm_check_h264pred(void); void checkasm_check_h264qpel(void); diff --git a/tests/checkasm/h264chroma.c b/tests/checkasm/h264chroma.c new file mode 100644 index 0000000000..0bd333327d --- /dev/null +++ b/tests/checkasm/h264chroma.c @@ -0,0 +1,109 @@ +/* + * Copyright (c) 2015 Henrik Gramner + * Copyright (c) 2021 J. Dekker + * + * 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 <string.h> +#include "checkasm.h" +#include "libavcodec/avcodec.h" +#include "libavcodec/h264chroma.h" +#include "libavutil/common.h" +#include "libavutil/internal.h" +#include "libavutil/intreadwrite.h" +#include "libavutil/mem_internal.h" + +// static const int codec_ids[4] = { AV_CODEC_ID_H264, AV_CODEC_ID_VP8, AV_CODEC_ID_RV40, AV_CODEC_ID_SVQ3 }; +static const uint32_t pixel_mask[3] = { 0xffffffff, 0x01ff01ff, 0x03ff03ff }; + +#define SIZEOF_PIXEL ((bit_depth + 7) / 8) +#define BUF_SIZE (3 * 16 * 17) + +#define randomize_buffers() \ + do { \ + uint32_t mask = pixel_mask[bit_depth - 8]; \ + int i; \ + for (i = 0; i < BUF_SIZE; i += 4) { \ + uint32_t r = rnd() & mask; \ + AV_WN32A(buf0 + i, r); \ + AV_WN32A(buf1 + i, r); \ + } \ + } while (0) + +#define src0 (buf0 + 4 * 16) /* Offset to allow room for top and left */ +#define src1 (buf1 + 4 * 16) + +static void check_avg() +{ + int i, bit_depth, x, y; + LOCAL_ALIGNED_16(uint8_t, buf0, [BUF_SIZE]); + LOCAL_ALIGNED_16(uint8_t, buf1, [BUF_SIZE]); + LOCAL_ALIGNED_16(uint8_t, dst0, [BUF_SIZE]); + LOCAL_ALIGNED_16(uint8_t, dst1, [BUF_SIZE]); + H264ChromaContext h; + + declare_func_emms(AV_CPU_FLAG_MMX, void, uint8_t *dst, uint8_t *src, ptrdiff_t stride, int h, int x, int y); + for (bit_depth = 8; bit_depth <= 10; bit_depth++) { + ff_h264chroma_init(&h, bit_depth); + for (i = 0; i < 4; i++) { + if (check_func(h.avg_h264_chroma_pixels_tab[i], "avg_chroma_mc%d_%d", 1 << (3 - i), bit_depth)) { + randomize_buffers(); + x = rnd() & 0x7; y = rnd() & 0x7; + call_ref(dst0, src0, 8, 4, x, y); + call_new(dst1, src1, 8, 4, x, y); + if (memcmp(buf0, buf1, BUF_SIZE)) + fail(); + bench_new(dst1, src1, 8, 4, x, y); + } + } + } +} + +static void check_put() +{ + int i, bit_depth, x, y; + LOCAL_ALIGNED_16(uint8_t, buf0, [BUF_SIZE]); + LOCAL_ALIGNED_16(uint8_t, buf1, [BUF_SIZE]); + LOCAL_ALIGNED_16(uint8_t, dst0, [BUF_SIZE]); + LOCAL_ALIGNED_16(uint8_t, dst1, [BUF_SIZE]); + H264ChromaContext h; + + declare_func_emms(AV_CPU_FLAG_MMX, void, uint8_t *dst, uint8_t *src, ptrdiff_t stride, int h, int x, int y); + for (bit_depth = 8; bit_depth <= 10; bit_depth++) { + ff_h264chroma_init(&h, bit_depth); + for (i = 0; i < 4; i++) { + if (check_func(h.put_h264_chroma_pixels_tab[i], "put_chroma_mc%d_%d", 1 << (3 - i), bit_depth)) { + randomize_buffers(); + x = rnd() & 0x7; y = rnd() & 0x7; + call_ref(dst0, src0, 8, 4, x, y); + call_new(dst1, src1, 8, 4, x, y); + if (memcmp(buf0, buf1, BUF_SIZE)) + fail(); + bench_new(dst1, src1, 8, 4, x, y); + } + } + } +} + +void checkasm_check_h264chroma(void) +{ + check_avg(); + report("avg"); + check_put(); + report("put"); +} diff --git a/tests/fate/checkasm.mak b/tests/fate/checkasm.mak index cec6d28286..449f8f89f9 100644 --- a/tests/fate/checkasm.mak +++ b/tests/fate/checkasm.mak @@ -11,6 +11,7 @@ FATE_CHECKASM = fate-checkasm-aacpsdsp \ fate-checkasm-float_dsp \ fate-checkasm-fmtconvert \ fate-checkasm-g722dsp \ + fate-checkasm-h264chroma \ fate-checkasm-h264dsp \ fate-checkasm-h264pred \ fate-checkasm-h264qpel \
Signed-off-by: J. Dekker <jdek@itanimul.li> --- tests/checkasm/Makefile | 1 + tests/checkasm/checkasm.c | 3 + tests/checkasm/checkasm.h | 1 + tests/checkasm/h264chroma.c | 109 ++++++++++++++++++++++++++++++++++++ tests/fate/checkasm.mak | 1 + 5 files changed, 115 insertions(+) create mode 100644 tests/checkasm/h264chroma.c What should I do for other codecs here, or just ignore non-h264?