| Message ID | pull.32.v2.ffstaging.FFmpeg.1653733190354.ffmpegagent@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v2] tests/fate/vcodec: Limit mem alignment for vsynth..mpeg2-422 tests | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
On Sat, 28 May 2022, 11:20 softworkz, <ffmpegagent@gmail.com> wrote: > From: softworkz <softworkz@hotmail.com> > > These 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@hotmail.com> > --- > tests/fate/vcodec: Limit mem alignment for vsynth..mpeg2-422 tests > > Fix FATE tests which are failing on newer x86 CPUs. > > v2: Add the flag on x86 platforms only > > Published-As: > https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-32%2Fsoftworkz%2Fsubmit_alignment-v2 > Fetch-It-Via > <https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-32%2Fsoftworkz%2Fsubmit_alignment-v2Fetch-It-Via>: > git fetch https://github.com/ffstaging/FFmpeg > pr-ffstaging-32/softworkz/submit_alignment-v2 > Pull-Request: https://github.com/ffstaging/FFmpeg/pull/32 > > Range-diff vs v1: > > 1: c36271eb6b ! 1: 1586956890 tests/fate/vcodec: Limit mem alignment > for vsynth..mpeg2-422 tests > @@ Commit message > > ## tests/fate/vcodec.mak ## > @@ tests/fate/vcodec.mak: fate-vsynth%-mpeg2-422: ENCOPTS = > -b:v 1000k \ > - -trellis 1 > \ > - -flags +ildct+ilme > \ > -mpv_flags +qp_rd+mv0 > \ > -+ -cpuflags -avx512 > \ > -intra_vlc 1 > \ > -mbd rd > \ > - -pix_fmt yuv422p > +- -pix_fmt yuv422p > ++ -pix_fmt yuv422p > \ > ++ > $(if $(HAVE_MMX), -cpuflags -avx512 ) > ++ > > + fate-vsynth%-mpeg2-idct-int: ENCOPTS = -qscale 10 -idct int > -dct int > + fate-vsynth%-mpeg2-ilace: ENCOPTS = -qscale 10 -flags > +ildct+ilme > + fate-vsynth%-mpeg2-ivlc-qprd: ENCOPTS = -b:v 500k > \ > > > tests/fate/vcodec.mak | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/tests/fate/vcodec.mak b/tests/fate/vcodec.mak > index 8ca17950ea..91228237bd 100644 > --- a/tests/fate/vcodec.mak > +++ b/tests/fate/vcodec.mak > @@ -264,7 +264,9 @@ fate-vsynth%-mpeg2-422: ENCOPTS = -b:v 1000k > \ > -mpv_flags +qp_rd+mv0 \ > -intra_vlc 1 \ > -mbd rd \ > - -pix_fmt yuv422p > + -pix_fmt yuv422p \ > + > $(if $(HAVE_MMX), -cpuflags -avx512 ) > + > > fate-vsynth%-mpeg2-idct-int: ENCOPTS = -qscale 10 -idct int -dct int > fate-vsynth%-mpeg2-ilace: ENCOPTS = -qscale 10 -flags +ildct+ilme > fate-vsynth%-mpeg2-ivlc-qprd: ENCOPTS = -b:v 500k \ > > base-commit: 9fba0b8a8c754a012fc74c90ffb7c26a56be8ca0 > -- > ffmpeg-codebot > _______________________________________________ > Huh, this is horrible. Kieran >
From: Kieran Kunhya <kierank@obe.tv> Sent: Saturday, May 28, 2022 3:11 PM To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> Cc: softworkz <softworkz@hotmail.com> Subject: Re: [FFmpeg-devel] [PATCH v2] tests/fate/vcodec: Limit mem alignment for vsynth..mpeg2-422 tests On Sat, 28 May 2022, 11:20 softworkz, <ffmpegagent@gmail.com<mailto:ffmpegagent@gmail.com>> wrote: From: softworkz <softworkz@hotmail.com<mailto:softworkz@hotmail.com>> These 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@hotmail.com<mailto:softworkz@hotmail.com>> --- tests/fate/vcodec: Limit mem alignment for vsynth..mpeg2-422 tests Fix FATE tests which are failing on newer x86 CPUs. v2: Add the flag on x86 platforms only Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-32%2Fsoftworkz%2Fsubmit_alignment-v2 Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-32/softworkz/submit_alignment-v2 Pull-Request: https://github.com/ffstaging/FFmpeg/pull/32 Range-diff vs v1: 1: c36271eb6b ! 1: 1586956890 tests/fate/vcodec: Limit mem alignment for vsynth..mpeg2-422 tests @@ Commit message ## tests/fate/vcodec.mak ## @@ tests/fate/vcodec.mak: fate-vsynth%-mpeg2-422: ENCOPTS = -b:v 1000k \ - -trellis 1 \ - -flags +ildct+ilme \ -mpv_flags +qp_rd+mv0 \ -+ -cpuflags -avx512 \ -intra_vlc 1 \ -mbd rd \ - -pix_fmt yuv422p +- -pix_fmt yuv422p ++ -pix_fmt yuv422p \ ++ $(if $(HAVE_MMX), -cpuflags -avx512 ) ++ + fate-vsynth%-mpeg2-idct-int: ENCOPTS = -qscale 10 -idct int -dct int + fate-vsynth%-mpeg2-ilace: ENCOPTS = -qscale 10 -flags +ildct+ilme + fate-vsynth%-mpeg2-ivlc-qprd: ENCOPTS = -b:v 500k \ tests/fate/vcodec.mak | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/fate/vcodec.mak b/tests/fate/vcodec.mak index 8ca17950ea..91228237bd 100644 --- a/tests/fate/vcodec.mak +++ b/tests/fate/vcodec.mak @@ -264,7 +264,9 @@ fate-vsynth%-mpeg2-422: ENCOPTS = -b:v 1000k \ -mpv_flags +qp_rd+mv0 \ -intra_vlc 1 \ -mbd rd \ - -pix_fmt yuv422p + -pix_fmt yuv422p \ + $(if $(HAVE_MMX), -cpuflags -avx512 ) + fate-vsynth%-mpeg2-idct-int: ENCOPTS = -qscale 10 -idct int -dct int fate-vsynth%-mpeg2-ilace: ENCOPTS = -qscale 10 -flags +ildct+ilme fate-vsynth%-mpeg2-ivlc-qprd: ENCOPTS = -b:v 500k \ base-commit: 9fba0b8a8c754a012fc74c90ffb7c26a56be8ca0 -- ffmpeg-codebot _______________________________________________ Huh, this is horrible. 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. Suggestions welcome :-) Thanks, sw
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.
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Anton Khirnov > Sent: Monday, May 30, 2022 9:35 AM > To: FFmpeg development discussions and patches <ffmpeg- > devel@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. There was a longer conversation ("FATE Errors") with a number of people, I'm not sure whether you followed. I submitted this as a possible way to work around the issue. If you find that patch acceptable, then I'll gladly adjust the commit message with a detailed explanation. It's just that I learned that it's not very effective to spend a lot of time on things that are likely to get rejected or ignored. Thanks for not ignoring (at least ;-) softworkz
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Soft Works > Sent: Monday, May 30, 2022 10:08 AM > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH v2] tests/fate/vcodec: Limit mem > alignment for vsynth..mpeg2-422 tests > > > > > -----Original Message----- > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > > Anton Khirnov > > Sent: Monday, May 30, 2022 9:35 AM > > To: FFmpeg development discussions and patches <ffmpeg- > > devel@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. > > > There was a longer conversation ("FATE Errors") with a number of > people, I'm not sure whether you followed. > I submitted this as a possible way to work around the issue. If you > find that patch acceptable, then I'll gladly adjust the commit > message > with a detailed explanation. > It's just that I learned that it's not very effective to spend a lot > of time on things that are likely to get rejected or ignored. > > Thanks for not ignoring (at least ;-) > > softworkz I should have conclude with a question: Would you consider this kind of workaround as reasonable and possibly acceptable? Thanks, sw
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Anton Khirnov > Sent: Monday, May 30, 2022 9:35 AM > To: FFmpeg development discussions and patches <ffmpeg- > devel@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: ------------------------ 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@hotmail.com> ------------------------ What do you want me to add to it? sw
On Mon, May 30, 2022 at 10:35 AM Soft Works <softworkz@hotmail.com> wrote: > > > > -----Original Message----- > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > > Anton Khirnov > > Sent: Monday, May 30, 2022 9:35 AM > > To: FFmpeg development discussions and patches <ffmpeg- > > devel@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: > > ------------------------ > > 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@hotmail.com> > > ------------------------ > > What do you want me to add to it? > > There appears a very large language barrier present here. In that FATE thread I explicitly wrote that this or similar solutions are considered the hack. sw > > _______________________________________________ > 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". >
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Paul B Mahol > Sent: Monday, May 30, 2022 11:07 AM > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH v2] tests/fate/vcodec: Limit mem > alignment for vsynth..mpeg2-422 tests > > On Mon, May 30, 2022 at 10:35 AM Soft Works <softworkz@hotmail.com> > wrote: > > > > > > > > -----Original Message----- > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > > > Anton Khirnov > > > Sent: Monday, May 30, 2022 9:35 AM > > > To: FFmpeg development discussions and patches <ffmpeg- > > > devel@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: > > > > ------------------------ > > > > 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@hotmail.com> > > > > ------------------------ > > > > What do you want me to add to it? > > > > > There appears a very large language barrier present here. > > In that FATE thread I explicitly wrote that this or similar solutions It's not similar but quite different to the one you commented on. > are > considered the hack. Well, the test .mak files are full of hacks then, with the many super- special flags in place that do not reflect any real world use at all. That flag does nothing more than to assure that the tests are run under the same conditions under which the ref files were generated. Many other flags are used in tests for the same reason. Anyway, this is about regression testing, not about annoying developers (with later CPU models) by failing tests for months or years due to a well-known problem. Thanks, sw
Quoting Soft Works (2022-05-30 10:08:11) > > > > -----Original Message----- > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > > Anton Khirnov > > Sent: Monday, May 30, 2022 9:35 AM > > To: FFmpeg development discussions and patches <ffmpeg- > > devel@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. > > > There was a longer conversation ("FATE Errors") with a number of > people, I'm not sure whether you followed. I saw the thread, I did not read it in enough detail to understand the cause of the errors. > I submitted this as a possible way to work around the issue. If you > find that patch acceptable, then I'll gladly adjust the commit message > with a detailed explanation. > It's just that I learned that it's not very effective to spend a lot > of time on things that are likely to get rejected or ignored. From a first glance, this looks very ad-hoc (i.e. a hack). But ugly hacks may sometimes be temporarily acceptable if a proper solution requires too much work and the problem needs to be fixed ASAP. So whether the patch is acceptable or not really depends on what the problem is and why are you fixing it in this specific way.
Quoting Soft Works (2022-05-30 10:35:26) > > > > -----Original Message----- > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > > Anton Khirnov > > Sent: Monday, May 30, 2022 9:35 AM > > To: FFmpeg development discussions and patches <ffmpeg- > > devel@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@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.
Quoting Soft Works (2022-05-30 11:26:56) > Well, the test .mak files are full of hacks then, with the many super- > special flags in place that do not reflect any real world use at all. There are AFAIK zero tests that override cpuflags. > > That flag does nothing more than to assure that the tests are run > under the same conditions under which the ref files were generated. Most of the code is supposed to be arch-independent. A test producing different results on different architectures should be considered a bug.
Anton Khirnov: > Quoting Soft Works (2022-05-30 10:35:26) >> >> >>> -----Original Message----- >>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of >>> Anton Khirnov >>> Sent: Monday, May 30, 2022 9:35 AM >>> To: FFmpeg development discussions and patches <ffmpeg- >>> devel@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@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
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Anton Khirnov > Sent: Monday, May 30, 2022 12:24 PM > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH v2] tests/fate/vcodec: Limit mem > alignment for vsynth..mpeg2-422 tests > > Quoting Soft Works (2022-05-30 10:08:11) > > > > > > > -----Original Message----- > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > > > Anton Khirnov > > > Sent: Monday, May 30, 2022 9:35 AM > > > To: FFmpeg development discussions and patches <ffmpeg- > > > devel@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. > > > > > > There was a longer conversation ("FATE Errors") with a number of > > people, I'm not sure whether you followed. > > I saw the thread, I did not read it in enough detail to understand > the > cause of the errors. > > > I submitted this as a possible way to work around the issue. If you > > find that patch acceptable, then I'll gladly adjust the commit > message > > with a detailed explanation. > > It's just that I learned that it's not very effective to spend a > lot > > of time on things that are likely to get rejected or ignored. > > Well, the test .mak files are full of hacks then, with the many > super- > > special flags in place that do not reflect any real world use at > all. > > There are AFAIK zero tests that override cpuflags. I meant other flags, but well, that's not getting us anywhere.. > From a first glance, this looks very ad-hoc (i.e. a hack). But ugly > hacks may sometimes be temporarily acceptable if a proper solution > requires too much work and the problem needs to be fixed ASAP. Yes, that's how I see the situation. > So whether the patch is acceptable or not really depends on what the > problem is and why are you fixing it in this specific way. From what I gathered from James' and Paul's responses, there is no easy fix for this. Personally, I am not familiar with the code nor do I have the capacity to resolve the issue at its core. > Most of the code is supposed to be arch-independent. A test producing > different results on different architectures should be considered a > bug. Yes, I fully agree to that. But those FATE tests have already fulfilled their duty: the problem is now a known problem It can be put on a list, a trac entry can be made and somebody can work on a fix for it at some time. It just doesn't make sense to keep those tests failing up until that point in time. Being unable to get clean FATE runs locally is affecting productivity. When you or one of the others in this conversation would be affected, I wonder whether such discussion about a trivial issue would have been necessary. (trivial: the tests should not fail; probably non-trivial: the actual fix) Thanks, softworkz
diff --git a/tests/fate/vcodec.mak b/tests/fate/vcodec.mak index 8ca17950ea..91228237bd 100644 --- a/tests/fate/vcodec.mak +++ b/tests/fate/vcodec.mak @@ -264,7 +264,9 @@ fate-vsynth%-mpeg2-422: ENCOPTS = -b:v 1000k \ -mpv_flags +qp_rd+mv0 \ -intra_vlc 1 \ -mbd rd \ - -pix_fmt yuv422p + -pix_fmt yuv422p \ + $(if $(HAVE_MMX), -cpuflags -avx512 ) + fate-vsynth%-mpeg2-idct-int: ENCOPTS = -qscale 10 -idct int -dct int fate-vsynth%-mpeg2-ilace: ENCOPTS = -qscale 10 -flags +ildct+ilme fate-vsynth%-mpeg2-ivlc-qprd: ENCOPTS = -b:v 500k \