| Message ID | a9b8f0e3-14f7-d985-2747-9430a43c334a@googlemail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, Nov 01, 2016 at 12:27:00AM +0100, Andreas Cadhalpun wrote: > The checkasm test fails, see trac ticket 5508. > > Also, the following tests fail due to this: > fate-vsynth1-dnxhd-2k-hr-hq fate-vsynth1-dnxhd-edge1-hr > fate-vsynth1-dnxhd-edge2-hr fate-vsynth1-dnxhd-edge3-hr > fate-vsynth1-dnxhd-hr-sq-mov fate-vsynth1-dnxhd-hr-hq-mov > fate-vsynth2-dnxhd-2k-hr-hq fate-vsynth2-dnxhd-edge1-hr > fate-vsynth2-dnxhd-edge2-hr fate-vsynth2-dnxhd-edge3-hr > fate-vsynth2-dnxhd-hr-sq-mov fate-vsynth2-dnxhd-hr-hq-mov > fate-vsynth3-dnxhd-2k-hr-hq fate-vsynth3-dnxhd-edge1-hr > fate-vsynth3-dnxhd-edge2-hr fate-vsynth3-dnxhd-edge3-hr > fate-vsynth3-dnxhd-hr-sq-mov fate-vsynth3-dnxhd-hr-hq-mov > > Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> > --- > > Just disabling the checkasm_check_pixblockdsp test for ppc64be, > as was done in commit e5d434 for release/3.1, does not make much sense, > as the altivec functions actually don't work... has this been tested with actual hw or an emulator ? is this a regression ? if so since when ? [...]
On 01.11.2016 02:04, Michael Niedermayer wrote: > On Tue, Nov 01, 2016 at 12:27:00AM +0100, Andreas Cadhalpun wrote: >> The checkasm test fails, see trac ticket 5508. >> >> Also, the following tests fail due to this: >> fate-vsynth1-dnxhd-2k-hr-hq fate-vsynth1-dnxhd-edge1-hr >> fate-vsynth1-dnxhd-edge2-hr fate-vsynth1-dnxhd-edge3-hr >> fate-vsynth1-dnxhd-hr-sq-mov fate-vsynth1-dnxhd-hr-hq-mov >> fate-vsynth2-dnxhd-2k-hr-hq fate-vsynth2-dnxhd-edge1-hr >> fate-vsynth2-dnxhd-edge2-hr fate-vsynth2-dnxhd-edge3-hr >> fate-vsynth2-dnxhd-hr-sq-mov fate-vsynth2-dnxhd-hr-hq-mov >> fate-vsynth3-dnxhd-2k-hr-hq fate-vsynth3-dnxhd-edge1-hr >> fate-vsynth3-dnxhd-edge2-hr fate-vsynth3-dnxhd-edge3-hr >> fate-vsynth3-dnxhd-hr-sq-mov fate-vsynth3-dnxhd-hr-hq-mov >> >> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> >> --- >> >> Just disabling the checkasm_check_pixblockdsp test for ppc64be, >> as was done in commit e5d434 for release/3.1, does not make much sense, >> as the altivec functions actually don't work... > > has this been tested with actual hw or an emulator ? The tests failed on the Debian buildd [1] and I could reproduce the exact same failures with qemu. The patch is only tested with qemu. > is this a regression ? if so since when ? Well, the commit e5d434 was never on master, but in release/3.1. And the failing dnxhd tests are new: eb5f4b1 tests/fate/vcodec: add dnxhr mov tests 44ac2b9 tests/fate/vcodec: add dnxhr edge tests 6108cb2 tests/fate: add dnxhr encoding tests So this altivec code probably never worked on ppc64be, it was just not tested. Best regards, Andreas 1: https://buildd.debian.org/status/fetch.php?pkg=ffmpeg&arch=ppc64&ver=7%3A3.2-1&stamp=1477927610
2016-11-01 0:27 GMT+01:00 Andreas Cadhalpun <andreas.cadhalpun@googlemail.com>: > -#if HAVE_ALTIVEC > +#if HAVE_ALTIVEC && !(ARCH_PPC64 && HAVE_BIGENDIAN) > if (!PPC_ALTIVEC(av_get_cpu_flags())) Do the dnxhd tests work for you on 32bit BE powerpc? (It fails here afaict) Carl Eugen
On Wed, Nov 02, 2016 at 01:40:01AM +0100, Carl Eugen Hoyos wrote: > 2016-11-01 0:27 GMT+01:00 Andreas Cadhalpun <andreas.cadhalpun@googlemail.com>: > > > -#if HAVE_ALTIVEC > > +#if HAVE_ALTIVEC && !(ARCH_PPC64 && HAVE_BIGENDIAN) > > if (!PPC_ALTIVEC(av_get_cpu_flags())) > > Do the dnxhd tests work for you on 32bit BE powerpc? > (It fails here afaict) i find it strange to belive that the code never worked on either HW if we assume it did once work for some case, what is differnt now ? the testcase? the compiler? the code? something else ? or this really never worked and we didnt notice ? [...]
On 02.11.2016 11:31, Michael Niedermayer wrote: > On Wed, Nov 02, 2016 at 01:40:01AM +0100, Carl Eugen Hoyos wrote: >> 2016-11-01 0:27 GMT+01:00 Andreas Cadhalpun <andreas.cadhalpun@googlemail.com>: >> >>> -#if HAVE_ALTIVEC >>> +#if HAVE_ALTIVEC && !(ARCH_PPC64 && HAVE_BIGENDIAN) >>> if (!PPC_ALTIVEC(av_get_cpu_flags())) >> >> Do the dnxhd tests work for you on 32bit BE powerpc? Not when using the altivec functions. (It's not tested on the Debian buildd, since the baseline of Debian's powerpc architecture does not include altivec.) >> (It fails here afaict) > > i find it strange to belive that the code never worked on either HW Your intuition is correct. ;-) > if we assume it did once work for some case, what is differnt now ? > the testcase? It wasn't tested with FATE, so it wasn't noticed, when it broke. > the compiler? No. > the code? Yes, by the following Libav commit: 4c387c7 ppc: dsputil: do unaligned block accesses correctly LOL! I'll send a patch fixing this in a moment. Best regards, Andreas
diff --git a/libavcodec/ppc/pixblockdsp.c b/libavcodec/ppc/pixblockdsp.c index 84aa562..7822eb0 100644 --- a/libavcodec/ppc/pixblockdsp.c +++ b/libavcodec/ppc/pixblockdsp.c @@ -266,7 +266,7 @@ av_cold void ff_pixblockdsp_init_ppc(PixblockDSPContext *c, AVCodecContext *avctx, unsigned high_bit_depth) { -#if HAVE_ALTIVEC +#if HAVE_ALTIVEC && !(ARCH_PPC64 && HAVE_BIGENDIAN) if (!PPC_ALTIVEC(av_get_cpu_flags())) return;
The checkasm test fails, see trac ticket 5508. Also, the following tests fail due to this: fate-vsynth1-dnxhd-2k-hr-hq fate-vsynth1-dnxhd-edge1-hr fate-vsynth1-dnxhd-edge2-hr fate-vsynth1-dnxhd-edge3-hr fate-vsynth1-dnxhd-hr-sq-mov fate-vsynth1-dnxhd-hr-hq-mov fate-vsynth2-dnxhd-2k-hr-hq fate-vsynth2-dnxhd-edge1-hr fate-vsynth2-dnxhd-edge2-hr fate-vsynth2-dnxhd-edge3-hr fate-vsynth2-dnxhd-hr-sq-mov fate-vsynth2-dnxhd-hr-hq-mov fate-vsynth3-dnxhd-2k-hr-hq fate-vsynth3-dnxhd-edge1-hr fate-vsynth3-dnxhd-edge2-hr fate-vsynth3-dnxhd-edge3-hr fate-vsynth3-dnxhd-hr-sq-mov fate-vsynth3-dnxhd-hr-hq-mov Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> --- Just disabling the checkasm_check_pixblockdsp test for ppc64be, as was done in commit e5d434 for release/3.1, does not make much sense, as the altivec functions actually don't work... --- libavcodec/ppc/pixblockdsp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)