diff mbox series

[FFmpeg-devel] swscale/rgb2rgb_template: use shuffle macro on BE arches

Message ID 20201130054651.1168490-1-andriy.gelman@gmail.com
State Accepted
Commit 1200264fc4122dbb58bd08861f926cd6448593eb
Headers show
Series [FFmpeg-devel] swscale/rgb2rgb_template: use shuffle macro on BE arches | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Andriy Gelman Nov. 30, 2020, 5:46 a.m. UTC 
From: Andriy Gelman <andriy.gelman@gmail.com>

Fixes fate-qtrle-32bit on PPC64 qemu

The macro does a simple byte swap on uint8 array without any casts, so
it's valid on big endian arches.

The mentioned test was failing because the byteswap function
shuffle_bytes_3210_c() is used in the pixel format conversion
(argb->bgra).

Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
---
 libswscale/rgb2rgb_template.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Carl Eugen Hoyos Nov. 30, 2020, 11:45 p.m. UTC | #1 
Am Mo., 30. Nov. 2020 um 06:48 Uhr schrieb Andriy Gelman
<andriy.gelman@gmail.com>:
>
> From: Andriy Gelman <andriy.gelman@gmail.com>
>
> Fixes fate-qtrle-32bit on PPC64 qemu

I suspect your patch fixes colourspace conversion (and the fate
test) on big-endian.

Carl Eugen
Michael Niedermayer Dec. 2, 2020, 9:09 p.m. UTC | #2 
On Mon, Nov 30, 2020 at 12:46:51AM -0500, Andriy Gelman wrote:
> From: Andriy Gelman <andriy.gelman@gmail.com>
> 
> Fixes fate-qtrle-32bit on PPC64 qemu

also fixes this on mips-qemu

LGTM

thx

[...]
Andriy Gelman Dec. 4, 2020, 4:14 a.m. UTC | #3 
On Tue, 01. Dec 00:45, Carl Eugen Hoyos wrote:
> Am Mo., 30. Nov. 2020 um 06:48 Uhr schrieb Andriy Gelman
> <andriy.gelman@gmail.com>:
> >
> > From: Andriy Gelman <andriy.gelman@gmail.com>
> >
> > Fixes fate-qtrle-32bit on PPC64 qemu
> 
> I suspect your patch fixes colourspace conversion (and the fate
> test) on big-endian.

For this fate test it's currently doing the argb->bgra conversion in swscale()
(libswscale/swscale.c). I saved some of the output images to .bmp and they seem
fine.  Looking at swscale() I think the conversion is done via argb->yuv->bgra.
After the patch it's a permutation of the bytes.

--
Andriy
Andriy Gelman Dec. 12, 2020, 10:39 p.m. UTC | #4 
On Wed, 02. Dec 22:09, Michael Niedermayer wrote:
> On Mon, Nov 30, 2020 at 12:46:51AM -0500, Andriy Gelman wrote:
> > From: Andriy Gelman <andriy.gelman@gmail.com>
> > 
> > Fixes fate-qtrle-32bit on PPC64 qemu
> 
> also fixes this on mips-qemu

> 
> LGTM
> 
> thx
> 

thanks, will apply and enable the test.

--
Andriy
Carl Eugen Hoyos Dec. 12, 2020, 11:22 p.m. UTC | #5 
Am Sa., 12. Dez. 2020 um 23:41 Uhr schrieb Andriy Gelman
<andriy.gelman@gmail.com>:
>
> On Wed, 02. Dec 22:09, Michael Niedermayer wrote:
> > On Mon, Nov 30, 2020 at 12:46:51AM -0500, Andriy Gelman wrote:
> > > From: Andriy Gelman <andriy.gelman@gmail.com>
> > >
> > > Fixes fate-qtrle-32bit on PPC64 qemu
> >
> > also fixes this on mips-qemu
>
> >
> > LGTM
> >
> > thx
> >
>
> thanks, will apply and enable the test.

Please mention big-endian (instead of ppc64) in the commit message.

Carl Eugen
Andriy Gelman Dec. 13, 2020, 4:22 a.m. UTC | #6 
On Sun, 13. Dec 00:22, Carl Eugen Hoyos wrote:
> Am Sa., 12. Dez. 2020 um 23:41 Uhr schrieb Andriy Gelman
> <andriy.gelman@gmail.com>:
> >
> > On Wed, 02. Dec 22:09, Michael Niedermayer wrote:
> > > On Mon, Nov 30, 2020 at 12:46:51AM -0500, Andriy Gelman wrote:
> > > > From: Andriy Gelman <andriy.gelman@gmail.com>
> > > >
> > > > Fixes fate-qtrle-32bit on PPC64 qemu
> > >
> > > also fixes this on mips-qemu
> >
> > >
> > > LGTM
> > >
> > > thx
> > >
> >
> > thanks, will apply and enable the test.

> 
> Please mention big-endian (instead of ppc64) in the commit message.

Updated and pushed.

--
Andriy
diff mbox series

Patch

diff --git a/libswscale/rgb2rgb_template.c b/libswscale/rgb2rgb_template.c
index f7385e3722..42c69801ba 100644
--- a/libswscale/rgb2rgb_template.c
+++ b/libswscale/rgb2rgb_template.c
@@ -342,7 +342,6 @@  static inline void shuffle_bytes_0321_c(const uint8_t *src, uint8_t *dst,
     }
 }
 
-#if !HAVE_BIGENDIAN
 #define DEFINE_SHUFFLE_BYTES(name, a, b, c, d)                          \
 static void shuffle_bytes_##name (const uint8_t *src,                   \
                                         uint8_t *dst, int src_size)     \
@@ -360,7 +359,6 @@  static void shuffle_bytes_##name (const uint8_t *src,                   \
 DEFINE_SHUFFLE_BYTES(1230_c, 1, 2, 3, 0)
 DEFINE_SHUFFLE_BYTES(3012_c, 3, 0, 1, 2)
 DEFINE_SHUFFLE_BYTES(3210_c, 3, 2, 1, 0)
-#endif
 
 static inline void rgb24tobgr24_c(const uint8_t *src, uint8_t *dst, int src_size)
 {
@@ -969,10 +967,10 @@  static av_cold void rgb2rgb_init_c(void)
 #else
     shuffle_bytes_0321 = shuffle_bytes_0321_c;
     shuffle_bytes_2103 = shuffle_bytes_2103_c;
+#endif
     shuffle_bytes_1230 = shuffle_bytes_1230_c;
     shuffle_bytes_3012 = shuffle_bytes_3012_c;
     shuffle_bytes_3210 = shuffle_bytes_3210_c;
-#endif
     rgb32tobgr16       = rgb32tobgr16_c;
     rgb32tobgr15       = rgb32tobgr15_c;
     yv12toyuy2         = yv12toyuy2_c;