On Fri, 13 Jun 2025, Tristan Matthews wrote:
On Fri, Jun 13, 2025 at 2:08 AM Martin Storsjö <mar...@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-requ...@ffmpeg.org with subject "unsubscribe".
