* [FFmpeg-devel] [PATCH] checkasm: h264dsp: test luma_dc_dequant @ 2025-06-12 17:25 Tristan Matthews 2025-06-12 20:13 ` Martin Storsjö 0 siblings, 1 reply; 13+ messages in thread From: Tristan Matthews @ 2025-06-12 17:25 UTC (permalink / raw) To: ffmpeg-devel; +Cc: Tristan Matthews --- tests/checkasm/h264dsp.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/checkasm/h264dsp.c b/tests/checkasm/h264dsp.c index d1228ed985..5fba31cf69 100644 --- a/tests/checkasm/h264dsp.c +++ b/tests/checkasm/h264dsp.c @@ -22,6 +22,7 @@ #include "checkasm.h" #include "libavcodec/h264dsp.h" #include "libavcodec/h264data.h" +#include "libavcodec/h264idct.h" #include "libavcodec/h264_parse.h" #include "libavutil/common.h" #include "libavutil/intreadwrite.h" @@ -324,6 +325,41 @@ static void check_idct_multiple(void) } } +static void check_idct_dequant(void) +{ + static const int depths[5] = { 8, 9, 10, 12, 14 }; + LOCAL_ALIGNED_16(int16_t, src, [16]); + LOCAL_ALIGNED_16(int16_t, dst0, [16 * 16]); + LOCAL_ALIGNED_16(int16_t, dst1, [16 * 16]); + H264DSPContext h; + int bit_depth, i, qmul; + declare_func_emms(AV_CPU_FLAG_MMX | AV_CPU_FLAG_SSE2, void, int16_t *output, int16_t *input, int qmul); + + for (int j = 0; j < 16; j++) + src[j] = (rnd() % 512) - 256; + + qmul = rnd() % 4096; + + memset(dst0, 0, 16 * 16 * sizeof(dst0[0])); + memset(dst1, 0, 16 * 16 * sizeof(dst1[0])); + + for (i = 0; i < FF_ARRAY_ELEMS(depths); i++) { + bit_depth = depths[i]; + ff_h264dsp_init(&h, bit_depth, 1); + + if (check_func(h.h264_luma_dc_dequant_idct, "h264_luma_dc_dequant_idct_%d", bit_depth)) { + + call_ref(dst0, src, qmul); + call_new(dst1, src, qmul); + + if (memcmp(dst0, dst1, 16 * 16 * sizeof(*dst0))) + fail(); + + bench_new(dst1, src, qmul); + } + } +} + static void check_loop_filter(void) { @@ -453,6 +489,7 @@ void checkasm_check_h264dsp(void) { check_idct(); check_idct_multiple(); + check_idct_dequant(); report("idct"); check_loop_filter(); -- 2.48.1 _______________________________________________ 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". ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH] checkasm: h264dsp: test luma_dc_dequant 2025-06-12 17:25 [FFmpeg-devel] [PATCH] checkasm: h264dsp: test luma_dc_dequant Tristan Matthews @ 2025-06-12 20:13 ` Martin Storsjö 2025-06-13 4:11 ` Tristan Matthews 0 siblings, 1 reply; 13+ messages in thread From: Martin Storsjö @ 2025-06-12 20:13 UTC (permalink / raw) To: FFmpeg development discussions and patches; +Cc: Tristan Matthews On Thu, 12 Jun 2025, Tristan Matthews wrote: > --- > tests/checkasm/h264dsp.c | 37 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > > diff --git a/tests/checkasm/h264dsp.c b/tests/checkasm/h264dsp.c > index d1228ed985..5fba31cf69 100644 > --- a/tests/checkasm/h264dsp.c > +++ b/tests/checkasm/h264dsp.c > @@ -22,6 +22,7 @@ > #include "checkasm.h" > #include "libavcodec/h264dsp.h" > #include "libavcodec/h264data.h" > +#include "libavcodec/h264idct.h" > #include "libavcodec/h264_parse.h" > #include "libavutil/common.h" > #include "libavutil/intreadwrite.h" > @@ -324,6 +325,41 @@ static void check_idct_multiple(void) > } > } > > +static void check_idct_dequant(void) > +{ > + static const int depths[5] = { 8, 9, 10, 12, 14 }; > + LOCAL_ALIGNED_16(int16_t, src, [16]); > + LOCAL_ALIGNED_16(int16_t, dst0, [16 * 16]); > + LOCAL_ALIGNED_16(int16_t, dst1, [16 * 16]); > + H264DSPContext h; > + int bit_depth, i, qmul; > + declare_func_emms(AV_CPU_FLAG_MMX | AV_CPU_FLAG_SSE2, void, int16_t *output, int16_t *input, int qmul); > + > + for (int j = 0; j < 16; j++) > + src[j] = (rnd() % 512) - 256; > + > + qmul = rnd() % 4096; > + > + memset(dst0, 0, 16 * 16 * sizeof(dst0[0])); > + memset(dst1, 0, 16 * 16 * sizeof(dst1[0])); > + > + for (i = 0; i < FF_ARRAY_ELEMS(depths); i++) { > + bit_depth = depths[i]; > + ff_h264dsp_init(&h, bit_depth, 1); > + > + if (check_func(h.h264_luma_dc_dequant_idct, "h264_luma_dc_dequant_idct_%d", bit_depth)) { > + > + call_ref(dst0, src, qmul); > + call_new(dst1, src, qmul); > + > + if (memcmp(dst0, dst1, 16 * 16 * sizeof(*dst0))) > + fail(); If possible, use the checkasm_check_*() helpers for validation for new code; this gives you printout of the differing values if you run "checkasm -v" and more. In this case, I think checkasm_check(int16_t, dst0, 16*sizeof(int16_t), dst1, 16*sizeof(int16_t), 16, 16, "dst") would be suitable one. // Martin _______________________________________________ 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". ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH] checkasm: h264dsp: test luma_dc_dequant 2025-06-12 20:13 ` Martin Storsjö @ 2025-06-13 4:11 ` Tristan Matthews 2025-06-13 4:12 ` [FFmpeg-devel] [PATCH 1/1] " Tristan Matthews 2025-06-13 6:08 ` [FFmpeg-devel] [PATCH] " Martin Storsjö 0 siblings, 2 replies; 13+ messages in thread From: Tristan Matthews @ 2025-06-13 4:11 UTC (permalink / raw) To: Martin Storsjö; +Cc: FFmpeg development discussions and patches On Thu, Jun 12, 2025 at 4:14 PM Martin Storsjö <martin@martin.st> wrote: > > On Thu, 12 Jun 2025, Tristan Matthews wrote: > > > --- > > tests/checkasm/h264dsp.c | 37 +++++++++++++++++++++++++++++++++++++ > > 1 file changed, 37 insertions(+) > > > > diff --git a/tests/checkasm/h264dsp.c b/tests/checkasm/h264dsp.c > > index d1228ed985..5fba31cf69 100644 > > --- a/tests/checkasm/h264dsp.c > > +++ b/tests/checkasm/h264dsp.c > > @@ -22,6 +22,7 @@ > > #include "checkasm.h" > > #include "libavcodec/h264dsp.h" > > #include "libavcodec/h264data.h" > > +#include "libavcodec/h264idct.h" > > #include "libavcodec/h264_parse.h" > > #include "libavutil/common.h" > > #include "libavutil/intreadwrite.h" > > @@ -324,6 +325,41 @@ static void check_idct_multiple(void) > > } > > } > > > > +static void check_idct_dequant(void) > > +{ > > + static const int depths[5] = { 8, 9, 10, 12, 14 }; > > + LOCAL_ALIGNED_16(int16_t, src, [16]); > > + LOCAL_ALIGNED_16(int16_t, dst0, [16 * 16]); > > + LOCAL_ALIGNED_16(int16_t, dst1, [16 * 16]); > > + H264DSPContext h; > > + int bit_depth, i, qmul; > > + declare_func_emms(AV_CPU_FLAG_MMX | AV_CPU_FLAG_SSE2, void, int16_t *output, int16_t *input, int qmul); > > + > > + for (int j = 0; j < 16; j++) > > + src[j] = (rnd() % 512) - 256; > > + > > + qmul = rnd() % 4096; > > + > > + memset(dst0, 0, 16 * 16 * sizeof(dst0[0])); > > + memset(dst1, 0, 16 * 16 * sizeof(dst1[0])); > > + > > + for (i = 0; i < FF_ARRAY_ELEMS(depths); i++) { > > + bit_depth = depths[i]; > > + ff_h264dsp_init(&h, bit_depth, 1); > > + > > + if (check_func(h.h264_luma_dc_dequant_idct, "h264_luma_dc_dequant_idct_%d", bit_depth)) { > > + > > + call_ref(dst0, src, qmul); > > + call_new(dst1, src, qmul); > > + > > + if (memcmp(dst0, dst1, 16 * 16 * sizeof(*dst0))) > > + fail(); > > If possible, use the checkasm_check_*() helpers for validation for new > code; this gives you printout of the differing values if you run "checkasm > -v" and more. In this case, I think checkasm_check(int16_t, dst0, > 16*sizeof(int16_t), dst1, 16*sizeof(int16_t), 16, 16, "dst") would be > suitable one. Good catch, also I realized that the output buffers were too small, will be fixed in the next version. _______________________________________________ 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". ^ permalink raw reply [flat|nested] 13+ messages in thread
* [FFmpeg-devel] [PATCH 1/1] checkasm: h264dsp: test luma_dc_dequant 2025-06-13 4:11 ` Tristan Matthews @ 2025-06-13 4:12 ` Tristan Matthews 2025-06-13 6:08 ` [FFmpeg-devel] [PATCH] " Martin Storsjö 1 sibling, 0 replies; 13+ messages in thread From: Tristan Matthews @ 2025-06-13 4:12 UTC (permalink / raw) To: ffmpeg-devel; +Cc: Tristan Matthews --- tests/checkasm/h264dsp.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/checkasm/h264dsp.c b/tests/checkasm/h264dsp.c index d1228ed985..96540565af 100644 --- a/tests/checkasm/h264dsp.c +++ b/tests/checkasm/h264dsp.c @@ -22,6 +22,7 @@ #include "checkasm.h" #include "libavcodec/h264dsp.h" #include "libavcodec/h264data.h" +#include "libavcodec/h264idct.h" #include "libavcodec/h264_parse.h" #include "libavutil/common.h" #include "libavutil/intreadwrite.h" @@ -324,6 +325,38 @@ static void check_idct_multiple(void) } } +static void check_idct_dequant(void) +{ + static const int depths[5] = { 8, 9, 10, 12, 14 }; + LOCAL_ALIGNED_16(int16_t, src, [16]); + LOCAL_ALIGNED_16(int16_t, dst0, [16 * 16 * 2]); + LOCAL_ALIGNED_16(int16_t, dst1, [16 * 16 * 2]); + H264DSPContext h; + int bit_depth, i, qmul; + declare_func_emms(AV_CPU_FLAG_MMX | AV_CPU_FLAG_SSE2, void, int16_t *output, int16_t *input, int qmul); + + for (int j = 0; j < 16; j++) + src[j] = (rnd() % 512) - 256; + + qmul = rnd() % 4096; + + for (i = 0; i < FF_ARRAY_ELEMS(depths); i++) { + bit_depth = depths[i]; + ff_h264dsp_init(&h, bit_depth, 1); + + memset(dst0, 0, 16 * 16 * SIZEOF_COEF); + memset(dst1, 0, 16 * 16 * SIZEOF_COEF); + + if (check_func(h.h264_luma_dc_dequant_idct, "h264_luma_dc_dequant_idct_%d", bit_depth)) { + + call_ref(dst0, src, qmul); + call_new(dst1, src, qmul); + checkasm_check(int16_t, dst0, 16*sizeof(int16_t), dst1, 16*sizeof(int16_t), 16, 16, "dst"); + bench_new(dst1, src, qmul); + } + } +} + static void check_loop_filter(void) { @@ -453,6 +486,7 @@ void checkasm_check_h264dsp(void) { check_idct(); check_idct_multiple(); + check_idct_dequant(); report("idct"); check_loop_filter(); -- 2.48.1 _______________________________________________ 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". ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH] checkasm: h264dsp: test luma_dc_dequant 2025-06-13 4:11 ` Tristan Matthews 2025-06-13 4:12 ` [FFmpeg-devel] [PATCH 1/1] " Tristan Matthews @ 2025-06-13 6:08 ` Martin Storsjö 2025-06-13 13:21 ` Tristan Matthews 1 sibling, 1 reply; 13+ messages in thread From: Martin Storsjö @ 2025-06-13 6:08 UTC (permalink / raw) To: Tristan Matthews; +Cc: FFmpeg development discussions and patches On Fri, 13 Jun 2025, Tristan Matthews wrote: > On Thu, Jun 12, 2025 at 4:14 PM Martin Storsjö <martin@martin.st> wrote: >> >> On Thu, 12 Jun 2025, Tristan Matthews wrote: >> >>> --- >>> tests/checkasm/h264dsp.c | 37 +++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 37 insertions(+) >>> >>> diff --git a/tests/checkasm/h264dsp.c b/tests/checkasm/h264dsp.c >>> index d1228ed985..5fba31cf69 100644 >>> --- a/tests/checkasm/h264dsp.c >>> +++ b/tests/checkasm/h264dsp.c >>> @@ -22,6 +22,7 @@ >>> #include "checkasm.h" >>> #include "libavcodec/h264dsp.h" >>> #include "libavcodec/h264data.h" >>> +#include "libavcodec/h264idct.h" >>> #include "libavcodec/h264_parse.h" >>> #include "libavutil/common.h" >>> #include "libavutil/intreadwrite.h" >>> @@ -324,6 +325,41 @@ static void check_idct_multiple(void) >>> } >>> } >>> >>> +static void check_idct_dequant(void) >>> +{ >>> + static const int depths[5] = { 8, 9, 10, 12, 14 }; >>> + LOCAL_ALIGNED_16(int16_t, src, [16]); >>> + LOCAL_ALIGNED_16(int16_t, dst0, [16 * 16]); >>> + LOCAL_ALIGNED_16(int16_t, dst1, [16 * 16]); >>> + H264DSPContext h; >>> + int bit_depth, i, qmul; >>> + declare_func_emms(AV_CPU_FLAG_MMX | AV_CPU_FLAG_SSE2, void, int16_t *output, int16_t *input, int qmul); >>> + >>> + for (int j = 0; j < 16; j++) >>> + src[j] = (rnd() % 512) - 256; >>> + >>> + qmul = rnd() % 4096; >>> + >>> + memset(dst0, 0, 16 * 16 * sizeof(dst0[0])); >>> + memset(dst1, 0, 16 * 16 * sizeof(dst1[0])); >>> + >>> + for (i = 0; i < FF_ARRAY_ELEMS(depths); i++) { >>> + bit_depth = depths[i]; >>> + ff_h264dsp_init(&h, bit_depth, 1); >>> + >>> + if (check_func(h.h264_luma_dc_dequant_idct, "h264_luma_dc_dequant_idct_%d", bit_depth)) { >>> + >>> + call_ref(dst0, src, qmul); >>> + call_new(dst1, src, qmul); >>> + >>> + if (memcmp(dst0, dst1, 16 * 16 * sizeof(*dst0))) >>> + fail(); >> >> If possible, use the checkasm_check_*() helpers for validation for new >> code; this gives you printout of the differing values if you run "checkasm >> -v" and more. In this case, I think checkasm_check(int16_t, dst0, >> 16*sizeof(int16_t), dst1, 16*sizeof(int16_t), 16, 16, "dst") would be >> suitable one. > > Good catch, also I realized that the output buffers were too small, > will be fixed in the next version. Why was that too small? If we write (and check) 16x16 int16_t elements, the previous allocation of LOCAL_ALIGNED_16(int16_t, dst0, [16 * 16]) sounds just right? Or does the function use the [16*16,2*16*16) area of the destination as scratch space? // Martin _______________________________________________ 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". ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH] checkasm: h264dsp: test luma_dc_dequant 2025-06-13 6:08 ` [FFmpeg-devel] [PATCH] " Martin Storsjö @ 2025-06-13 13:21 ` Tristan Matthews 2025-06-13 13:26 ` Martin Storsjö 0 siblings, 1 reply; 13+ messages in thread From: Tristan Matthews @ 2025-06-13 13:21 UTC (permalink / raw) To: Martin Storsjö; +Cc: FFmpeg development discussions and patches On Fri, Jun 13, 2025 at 2:08 AM Martin Storsjö <martin@martin.st> wrote: > > On Fri, 13 Jun 2025, Tristan Matthews wrote: > > > On Thu, Jun 12, 2025 at 4:14 PM Martin Storsjö <martin@martin.st> wrote: > >> > >> On Thu, 12 Jun 2025, Tristan Matthews wrote: > >> > >>> --- > >>> tests/checkasm/h264dsp.c | 37 +++++++++++++++++++++++++++++++++++++ > >>> 1 file changed, 37 insertions(+) > >>> > >>> diff --git a/tests/checkasm/h264dsp.c b/tests/checkasm/h264dsp.c > >>> index d1228ed985..5fba31cf69 100644 > >>> --- a/tests/checkasm/h264dsp.c > >>> +++ b/tests/checkasm/h264dsp.c > >>> @@ -22,6 +22,7 @@ > >>> #include "checkasm.h" > >>> #include "libavcodec/h264dsp.h" > >>> #include "libavcodec/h264data.h" > >>> +#include "libavcodec/h264idct.h" > >>> #include "libavcodec/h264_parse.h" > >>> #include "libavutil/common.h" > >>> #include "libavutil/intreadwrite.h" > >>> @@ -324,6 +325,41 @@ static void check_idct_multiple(void) > >>> } > >>> } > >>> > >>> +static void check_idct_dequant(void) > >>> +{ > >>> + static const int depths[5] = { 8, 9, 10, 12, 14 }; > >>> + LOCAL_ALIGNED_16(int16_t, src, [16]); > >>> + LOCAL_ALIGNED_16(int16_t, dst0, [16 * 16]); > >>> + LOCAL_ALIGNED_16(int16_t, dst1, [16 * 16]); > >>> + H264DSPContext h; > >>> + int bit_depth, i, qmul; > >>> + declare_func_emms(AV_CPU_FLAG_MMX | AV_CPU_FLAG_SSE2, void, int16_t *output, int16_t *input, int qmul); > >>> + > >>> + for (int j = 0; j < 16; j++) > >>> + src[j] = (rnd() % 512) - 256; > >>> + > >>> + qmul = rnd() % 4096; > >>> + > >>> + memset(dst0, 0, 16 * 16 * sizeof(dst0[0])); > >>> + memset(dst1, 0, 16 * 16 * sizeof(dst1[0])); > >>> + > >>> + for (i = 0; i < FF_ARRAY_ELEMS(depths); i++) { > >>> + bit_depth = depths[i]; > >>> + ff_h264dsp_init(&h, bit_depth, 1); > >>> + > >>> + if (check_func(h.h264_luma_dc_dequant_idct, "h264_luma_dc_dequant_idct_%d", bit_depth)) { > >>> + > >>> + call_ref(dst0, src, qmul); > >>> + call_new(dst1, src, qmul); > >>> + > >>> + if (memcmp(dst0, dst1, 16 * 16 * sizeof(*dst0))) > >>> + fail(); > >> > >> If possible, use the checkasm_check_*() helpers for validation for new > >> code; this gives you printout of the differing values if you run "checkasm > >> -v" and more. In this case, I think checkasm_check(int16_t, dst0, > >> 16*sizeof(int16_t), dst1, 16*sizeof(int16_t), 16, 16, "dst") would be > >> suitable one. > > > > Good catch, also I realized that the output buffers were too small, > > will be fixed in the next version. > > Why was that too small? If we write (and check) 16x16 int16_t elements, > the previous allocation of LOCAL_ALIGNED_16(int16_t, dst0, [16 * 16]) > sounds just right? Or does the function use the [16*16,2*16*16) area of > the destination as scratch space? That's what I thought too until I noticed the FATE failures (e.g. https://patchwork.ffmpeg.org/check/124147/), and on further digging realized that dctcoef (used for dst here: https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/fb65ecbc9b805571e5ff707b935c343803137e54:/libavcodec/h264idct_template.c#l256 ) will be either 2 or 4 bytes depending on bit-depth IIUC (see https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/fb65ecbc9b805571e5ff707b935c343803137e54:/libavcodec/bit_depth_template.c#l54 ) Best, Tristan _______________________________________________ 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". ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH] checkasm: h264dsp: test luma_dc_dequant 2025-06-13 13:21 ` Tristan Matthews @ 2025-06-13 13:26 ` Martin Storsjö 2025-06-13 14:04 ` [FFmpeg-devel] [PATCH 1/2] checkasm: add checkasm_check_dctcoef Tristan Matthews 0 siblings, 1 reply; 13+ messages in thread From: Martin Storsjö @ 2025-06-13 13:26 UTC (permalink / raw) To: Tristan Matthews; +Cc: FFmpeg development discussions and patches On Fri, 13 Jun 2025, Tristan Matthews wrote: > On Fri, Jun 13, 2025 at 2:08 AM Martin Storsjö <martin@martin.st> wrote: >> >> On Fri, 13 Jun 2025, Tristan Matthews wrote: >> >>> Good catch, also I realized that the output buffers were too small, >>> will be fixed in the next version. >> >> Why was that too small? If we write (and check) 16x16 int16_t elements, >> the previous allocation of LOCAL_ALIGNED_16(int16_t, dst0, [16 * 16]) >> sounds just right? Or does the function use the [16*16,2*16*16) area of >> the destination as scratch space? > > That's what I thought too until I noticed the FATE failures (e.g. > https://patchwork.ffmpeg.org/check/124147/), and on further digging > realized that dctcoef (used for dst here: > https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/fb65ecbc9b805571e5ff707b935c343803137e54:/libavcodec/h264idct_template.c#l256 > ) will be either 2 or 4 bytes depending on bit-depth IIUC (see > https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/fb65ecbc9b805571e5ff707b935c343803137e54:/libavcodec/bit_depth_template.c#l54 > ) Oh, I see. Well in that case, I think that using int16_t and *2 feels quite confusing; I think I'd rather have it be uint8 and *sizeof(int32_t) or something like that, to clarify what's going on. I see that other preexisting tests, like vp9dsp.c, do use int16_t and an extra magical *2, but I think going plain uint8_t is clearer when it isn't always specifically int16_t. In that case, using checkasm_check(int16_t) also is going to be wrong; we have similar cases for pixels, see the checkasm_check_pixel() macros in checkasm.h. Perhaps we need a similar checkasm_check_dctcoef() macro, which checks int16_t or int32_t depending on bit_depth? // Martin _______________________________________________ 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". ^ permalink raw reply [flat|nested] 13+ messages in thread
* [FFmpeg-devel] [PATCH 1/2] checkasm: add checkasm_check_dctcoef 2025-06-13 13:26 ` Martin Storsjö @ 2025-06-13 14:04 ` Tristan Matthews 2025-06-13 14:04 ` [FFmpeg-devel] [PATCH 2/2] checkasm: h264dsp: test luma_dc_dequant Tristan Matthews 0 siblings, 1 reply; 13+ messages in thread From: Tristan Matthews @ 2025-06-13 14:04 UTC (permalink / raw) To: ffmpeg-devel; +Cc: Tristan Matthews This is useful for tests that compare dctcoefs which will be either 2 bytes or 4 bytes, depending on bitdepth. --- tests/checkasm/checkasm.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h index 146bfdec35..e829942d58 100644 --- a/tests/checkasm/checkasm.h +++ b/tests/checkasm/checkasm.h @@ -443,5 +443,16 @@ DECL_CHECKASM_CHECK_FUNC(int32_t); #define checkasm_check_pixel_padded_align(...) \ checkasm_check_pixel2(__VA_ARGS__, 8) +/* This assumes that there is a local variable named "bit_depth". + * For tests that don't have that and only operate on a single + * bitdepth, just call checkasm_check(uint8_t, ...) directly. */ +#define checkasm_check_dctcoef(buf1, stride1, buf2, stride2, ...) \ + ((bit_depth > 8) ? \ + checkasm_check(int32_t, (const int32_t*)buf1, stride1, \ + (const int32_t*)buf2, stride2, \ + __VA_ARGS__) : \ + checkasm_check(int16_t, (const int16_t*)buf1, stride1, \ + (const int16_t*)buf2, stride2, \ + __VA_ARGS__)) #endif /* TESTS_CHECKASM_CHECKASM_H */ -- 2.48.1 _______________________________________________ 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". ^ permalink raw reply [flat|nested] 13+ messages in thread
* [FFmpeg-devel] [PATCH 2/2] checkasm: h264dsp: test luma_dc_dequant 2025-06-13 14:04 ` [FFmpeg-devel] [PATCH 1/2] checkasm: add checkasm_check_dctcoef Tristan Matthews @ 2025-06-13 14:04 ` Tristan Matthews 2025-06-13 14:08 ` Martin Storsjö 0 siblings, 1 reply; 13+ messages in thread From: Tristan Matthews @ 2025-06-13 14:04 UTC (permalink / raw) To: ffmpeg-devel; +Cc: Tristan Matthews --- tests/checkasm/h264dsp.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/checkasm/h264dsp.c b/tests/checkasm/h264dsp.c index d1228ed985..f5f9650224 100644 --- a/tests/checkasm/h264dsp.c +++ b/tests/checkasm/h264dsp.c @@ -22,6 +22,7 @@ #include "checkasm.h" #include "libavcodec/h264dsp.h" #include "libavcodec/h264data.h" +#include "libavcodec/h264idct.h" #include "libavcodec/h264_parse.h" #include "libavutil/common.h" #include "libavutil/intreadwrite.h" @@ -324,6 +325,41 @@ static void check_idct_multiple(void) } } +static void check_idct_dequant(void) +{ + static const int depths[5] = { 8, 9, 10, 12, 14 }; + LOCAL_ALIGNED_16(int16_t, src, [16]); + /* Ensure dst buffers are large enough to hold dctcoefs of all bit-depths. */ + LOCAL_ALIGNED_16(uint8_t, dst0, [16 * 16 * sizeof(int32_t)]); + LOCAL_ALIGNED_16(uint8_t, dst1, [16 * 16 * sizeof(int32_t)]); + int16_t *dst_ref = (int16_t *)dst0; + int16_t *dst_new = (int16_t *)dst1; + H264DSPContext h; + int bit_depth, i, qmul; + declare_func_emms(AV_CPU_FLAG_MMX | AV_CPU_FLAG_SSE2, void, int16_t *output, int16_t *input, int qmul); + + for (int j = 0; j < 16; j++) + src[j] = (rnd() % 512) - 256; + + qmul = rnd() % 4096; + + for (i = 0; i < FF_ARRAY_ELEMS(depths); i++) { + bit_depth = depths[i]; + ff_h264dsp_init(&h, bit_depth, 1); + + memset(dst0, 0, 16 * 16 * SIZEOF_COEF); + memset(dst1, 0, 16 * 16 * SIZEOF_COEF); + + if (check_func(h.h264_luma_dc_dequant_idct, "h264_luma_dc_dequant_idct_%d", bit_depth)) { + + call_ref(dst_ref, src, qmul); + call_new(dst_new, src, qmul); + checkasm_check_dctcoef(dst_ref, 16*SIZEOF_COEF, dst_new, 16*SIZEOF_COEF, 16, 16, "dst"); + bench_new(dst_new, src, qmul); + } + } +} + static void check_loop_filter(void) { @@ -453,6 +489,7 @@ void checkasm_check_h264dsp(void) { check_idct(); check_idct_multiple(); + check_idct_dequant(); report("idct"); check_loop_filter(); -- 2.48.1 _______________________________________________ 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". ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/2] checkasm: h264dsp: test luma_dc_dequant 2025-06-13 14:04 ` [FFmpeg-devel] [PATCH 2/2] checkasm: h264dsp: test luma_dc_dequant Tristan Matthews @ 2025-06-13 14:08 ` Martin Storsjö 2025-06-15 23:39 ` Michael Niedermayer 0 siblings, 1 reply; 13+ messages in thread From: Martin Storsjö @ 2025-06-13 14:08 UTC (permalink / raw) To: FFmpeg development discussions and patches; +Cc: Tristan Matthews On Fri, 13 Jun 2025, Tristan Matthews wrote: > --- > tests/checkasm/h264dsp.c | 37 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) Both patches LGTM, thank you! // Martin _______________________________________________ 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". ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/2] checkasm: h264dsp: test luma_dc_dequant 2025-06-13 14:08 ` Martin Storsjö @ 2025-06-15 23:39 ` Michael Niedermayer 2025-06-16 6:53 ` Zhao Zhili 0 siblings, 1 reply; 13+ messages in thread From: Michael Niedermayer @ 2025-06-15 23:39 UTC (permalink / raw) To: FFmpeg development discussions and patches [-- Attachment #1.1: Type: text/plain, Size: 507 bytes --] On Fri, Jun 13, 2025 at 05:08:30PM +0300, Martin Storsjö wrote: > On Fri, 13 Jun 2025, Tristan Matthews wrote: > > > --- > > tests/checkasm/h264dsp.c | 37 +++++++++++++++++++++++++++++++++++++ > > 1 file changed, 37 insertions(+) > > Both patches LGTM, thank you! will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Good people do not need laws to tell them to act responsibly, while bad people will find a way around the laws. -- Plato [-- 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". ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/2] checkasm: h264dsp: test luma_dc_dequant 2025-06-15 23:39 ` Michael Niedermayer @ 2025-06-16 6:53 ` Zhao Zhili 2025-06-16 13:58 ` Michael Niedermayer 0 siblings, 1 reply; 13+ messages in thread From: Zhao Zhili @ 2025-06-16 6:53 UTC (permalink / raw) To: FFmpeg development discussions and patches > On Jun 16, 2025, at 07:39, Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Fri, Jun 13, 2025 at 05:08:30PM +0300, Martin Storsjö wrote: >> On Fri, 13 Jun 2025, Tristan Matthews wrote: >> >>> --- >>> tests/checkasm/h264dsp.c | 37 +++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 37 insertions(+) >> >> Both patches LGTM, thank you! > > will apply This has a stack overflow. https://ffmpeg.org/pipermail/ffmpeg-devel/2025-June/345305.html > > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Good people do not need laws to tell them to act responsibly, while bad > people will find a way around the laws. -- Plato > > _______________________________________________ > 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". > _______________________________________________ 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". ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/2] checkasm: h264dsp: test luma_dc_dequant 2025-06-16 6:53 ` Zhao Zhili @ 2025-06-16 13:58 ` Michael Niedermayer 0 siblings, 0 replies; 13+ messages in thread From: Michael Niedermayer @ 2025-06-16 13:58 UTC (permalink / raw) To: FFmpeg development discussions and patches [-- Attachment #1.1: Type: text/plain, Size: 953 bytes --] Hi Zhao On Mon, Jun 16, 2025 at 02:53:59PM +0800, Zhao Zhili wrote: > > > > On Jun 16, 2025, at 07:39, Michael Niedermayer <michael@niedermayer.cc> wrote: > > > > On Fri, Jun 13, 2025 at 05:08:30PM +0300, Martin Storsjö wrote: > >> On Fri, 13 Jun 2025, Tristan Matthews wrote: > >> > >>> --- > >>> tests/checkasm/h264dsp.c | 37 +++++++++++++++++++++++++++++++++++++ > >>> 1 file changed, 37 insertions(+) > >> > >> Both patches LGTM, thank you! > > > > will apply > > This has a stack overflow. I think this should have been a reply to the author or reviewer i just pushed it after it was reviewed, I did not look at the code at all thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The real ebay dictionary, page 2 "100% positive feedback" - "All either got their money back or didnt complain" "Best seller ever, very honest" - "Seller refunded buyer after failed scam" [-- 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". ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-06-16 13:59 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-06-12 17:25 [FFmpeg-devel] [PATCH] checkasm: h264dsp: test luma_dc_dequant Tristan Matthews 2025-06-12 20:13 ` Martin Storsjö 2025-06-13 4:11 ` Tristan Matthews 2025-06-13 4:12 ` [FFmpeg-devel] [PATCH 1/1] " Tristan Matthews 2025-06-13 6:08 ` [FFmpeg-devel] [PATCH] " Martin Storsjö 2025-06-13 13:21 ` Tristan Matthews 2025-06-13 13:26 ` Martin Storsjö 2025-06-13 14:04 ` [FFmpeg-devel] [PATCH 1/2] checkasm: add checkasm_check_dctcoef Tristan Matthews 2025-06-13 14:04 ` [FFmpeg-devel] [PATCH 2/2] checkasm: h264dsp: test luma_dc_dequant Tristan Matthews 2025-06-13 14:08 ` Martin Storsjö 2025-06-15 23:39 ` Michael Niedermayer 2025-06-16 6:53 ` Zhao Zhili 2025-06-16 13:58 ` Michael Niedermayer
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git