Re: [FFmpeg-devel] [PATCH v2] checkasm/hevc_deblock: add luma and chroma full

["Martin Storsjö" <martin@martin.st>]

On Wed, 24 Jan 2024, J. Dekker wrote:

> Signed-off-by: J. Dekker <jdek@itanimul.li>
> ---
> tests/checkasm/hevc_deblock.c | 225 +++++++++++++++++++++++++++++-----
> 1 file changed, 195 insertions(+), 30 deletions(-)
>
> - added luma 10/12 bit
> - supporting full (*_c) luma & chroma functions
> - dynamically generating all test data
>
> Appears to work for me. Testing on x86, hits the filtering decisions correctly.
> x86 doesn't have the full asm functions though, need to check a platform which
> has them (though the difference is minor, not sure why it wouldn't work).

Looks mostly good, although I didn't test it myself.

A couple of cosmetic comments below:

> +#define RNDDIFF(val, diff) av_clip(((SIZEOF_PIXEL == 1) ? \
> +    *(uint8_t*)(&val) : *(uint16_t*)(&val)) - (diff), 0, \
> +    (1 << (bit_depth)) - 1) + rnd() % FFMAX(2 * (diff), 1)

This macro is quite hard to read - can you indent it semantically based on 
nesting level or something like that?

> +#define TC25(x) ((tc[x] * 5 + 1) >> 1);
> +
> +static void randomize_luma_buffers(int type, int *beta, int32_t tc[2], uint8_t *buf, ptrdiff_t xstride, ptrdiff_t ystride, int bit_depth)
> +{

This one line is quite significantly longer than all the surrounding ones 
- can you wrap it? (Elsewhere, there are also a couple rather long lines, 
but they fit in better, but wrapping them is welcome too.)

// 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".