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

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

[softworkz]

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

Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-32%2Fsoftworkz%2Fsubmit_alignment-v1
Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-32/softworkz/submit_alignment-v1
Pull-Request: https://github.com/ffstaging/FFmpeg/pull/32

 tests/fate/vcodec.mak | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/fate/vcodec.mak b/tests/fate/vcodec.mak
index 8ca17950ea..2746b0b955 100644
--- a/tests/fate/vcodec.mak
+++ b/tests/fate/vcodec.mak
@@ -262,6 +262,7 @@ 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

base-commit: 9fba0b8a8c754a012fc74c90ffb7c26a56be8ca0
-- 
ffmpeg-codebot
_______________________________________________
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".

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

[softworkz]

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: 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
_______________________________________________
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".

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

[Kieran Kunhya]

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

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

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

[Soft Works]

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

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

[Anton Khirnov]

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.

-- 
Anton Khirnov
_______________________________________________
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".

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

[Soft Works]

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

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

[Soft Works]

> -----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


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

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

[Soft Works]

> -----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

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

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

[Paul B Mahol]

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

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

[Soft Works]

> -----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



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

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

[Anton Khirnov]

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.

-- 
Anton Khirnov
_______________________________________________
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".

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

[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.

-- 
Anton Khirnov
_______________________________________________
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".

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

[Anton Khirnov]

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

Re: [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@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
_______________________________________________
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".

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

[Soft Works]

> -----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













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