[FFmpeg-devel] [PATCH 3/4] checkasm/h263dsp: test dct_unquantize_{intra, inter}

[James Almer]

On 6/12/2024 4:15 PM, Rémi Denis-Courmont wrote:
> Le keskiviikkona 12. kesäkuuta 2024, 21.39.12 EEST James Almer a écrit :
>> On 6/12/2024 1:47 AM, Rémi Denis-Courmont wrote:
>>> ---
>>>
>>>    tests/checkasm/h263dsp.c | 47 +++++++++++++++++++++++++++++++++++++++-
>>>    1 file changed, 46 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/checkasm/h263dsp.c b/tests/checkasm/h263dsp.c
>>> index 2d0957a90b..8a2cdb34df 100644
>>> --- a/tests/checkasm/h263dsp.c
>>> +++ b/tests/checkasm/h263dsp.c
>>> @@ -18,13 +18,55 @@
>>>
>>>     * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>>>     */
>>>
>>> +#include <stdbool.h>
>>>
>>>    #include <string.h>
>>>    
>>>    #include "checkasm.h"
>>>
>>> -#include "libavcodec/h263dsp.h"
>>> +#include "libavutil/avassert.h"
>>>
>>>    #include "libavutil/mem.h"
>>>    #include "libavutil/mem_internal.h"
>>>
>>> +#include "libavcodec/h263dsp.h"
>>> +#include "libavcodec/mpegvideodata.h"
>>> +
>>> +static uint_fast8_t mpeg_qscale_rnd(void)
>>> +{
>>> +    int n = rnd(), q = (n >> 1) & 31;
>>> +
>>> +    if (n & 1)
>>> +        return ff_mpeg2_non_linear_qscale[q];
>>> +    else
>>> +        return q << 1;
>>> +}
>>> +
>>> +typedef void (*unquantizer)(int16_t *, size_t, int, int);
>>> +
>>> +static void check_dct_unquantize(unquantizer func, const char *name)
>>> +{
>>> +#define LEN 64
>>> +    LOCAL_ALIGNED_16(int16_t, block0, [LEN]);
>>> +    LOCAL_ALIGNED_16(int16_t, block1, [LEN]);
>>
>> These are not padded, and unless I'm reading this wrong, an asm
>> implementation loading say 16 bytes at a time will overread/write in
>> dct_unquantize_intra (which offsets block by 1).
> 
> AFAIU, there is no padding per se, but the block buffer size is always exactly
> 64 elements, regardless of the number of coeffs, hence this code. The old NEON
> intrinsic code seems to assume the block is a multiple of 8 elements, and the
> tail can be overwritten safely (hence not checking in memcmp()).
> 
> I have a feeling that I am not grasping the implications of you comment here.

An asm function loading 16 bytes at a time from block[1] onwards for 
intra may end up reading two bytes more than available at the end of the 
128 byte wide buffer.