[FFmpeg-devel] [PATCH v2] tests/fate/vcodec: Limit mem alignment for vsynth..mpeg2-422 tests

[Andreas Rheinhardt]

Anton Khirnov:
> Quoting Soft Works (2022-05-30 10:35:26)
>>
>>
>>> -----Original Message-----
>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>>> Anton Khirnov
>>> Sent: Monday, May 30, 2022 9:35 AM
>>> To: FFmpeg development discussions and patches <ffmpeg-
>>> devel at ffmpeg.org>
>>> Subject: Re: [FFmpeg-devel] [PATCH v2] tests/fate/vcodec: Limit mem
>>> alignment for vsynth..mpeg2-422 tests
>>>
>>> Quoting Soft Works (2022-05-28 15:17:54)
>>>> Do you have a better idea?
>>>>
>>>> The one advantage of this method is that you don’t need to change
>>> compilation parameters
>>>> nor  any source code. It’s only a runtime flag being set only for
>>> this specific family of tests.
>>>
>>> At the very least, I would expect the commit message to explain what
>>> exactly the problem is, and why is it fixed in this seemingly ad-hoc
>>> manner.
>>>
>>> "limit mem alignment to fix failing tests" explains nothing.
>>
>> BTW, that's not the actual commit message.
>> What I have submitted is this:
> 
> It's effectively equivalent to my summary.
> 
>> ------------------------
>>
>> tests/fate/vcodec: Limit mem alignment for vsynth..mpeg2-422 tests
>>
>> The tests:
>>
>> - vsynth2-mpeg2-422
>> - vsynth1-mpeg2-422
>> - vsynth_lena-mpeg2-422
>>
>> were failing on newer CPUs where av_cpu_max_align() returns
>> values > 32. This patch sets cpuflags to disable avx512
>> extensions for those tests only.
>>
>> Signed-off-by: softworkz <softworkz at hotmail.com>
>>
>> ------------------------
>>
>> What do you want me to add to it?
> 
> Why are those specific tests failing?
> 
> Why is the failure fixed in this specific way.
> 

The only thing known is that always using direct = 0 in
load_input_picture in mpegvideo_enc.c causes the bug to disappear.
Looking at the code shows that the code that is run when direct == 0
sometimes expects a vertical alignment of 32, whereas the check for
whether direct should be set to zero always checks for a vertical
alignment of 16. Yet fixing this discrepancy by setting vpad earlier and
using this at both places does not fix the issue.
To be honest, everything that has to do with whether or not to reuse the
picture (and the padding/edges of the pictures the code can rely upon)
feels like black magic to me.

- Andreas