diff mbox

[FFmpeg-devel,RFC] libswscale: Sanitize isRGBinInt() and isBGRinInt()

Message ID CAB0OVGowy0+1fe__gtJh2bQ-4=3rEjxjCE6u5Ex+bCqO7dZ1JA@mail.gmail.com
State New
Headers show

Commit Message

Carl Eugen Hoyos Aug. 2, 2017, 10:14 p.m. UTC 
2017-07-25 21:27 GMT+02:00 Michael Niedermayer <michael@niedermayer.cc>:
> On Mon, Jul 24, 2017 at 08:00:51PM +0200, Carl Eugen Hoyos wrote:

>> Attached is a PoC for a code simplification that imo makes a logic in
>> libswscale easier to read and also fixes the Big Endian fate failure.
>> If this approach is preferred, I will try to fix the five affected asm
>> functions.
>>
>> Please comment, Carl Eugen
>
> breaks: (blue faces)
> ./ffplay -i matrixbench_mpeg2.mpg
> -vf format=argb,scale=flags=16,format=rgb565

New PoC attached.
This breaks fate, if the change is wanted, 4158fba3
has to be reverted.

Please comment, Carl Eugen
diff mbox

Patch

diff --git a/libswscale/rgb2rgb.c b/libswscale/rgb2rgb.c
index f7f8188..a974d64 100644
--- a/libswscale/rgb2rgb.c
+++ b/libswscale/rgb2rgb.c
@@ -133,8 +133,10 @@  void (*yuyvtoyuv422)(uint8_t *ydst, uint8_t *udst, uint8_t *vdst,
 av_cold void ff_sws_rgb2rgb_init(void)
 {
     rgb2rgb_init_c();
+/*
     if (ARCH_X86)
         rgb2rgb_init_x86();
+*/
 }
 
 void rgb32to24(const uint8_t *src, uint8_t *dst, int src_size)
@@ -142,16 +144,9 @@  void rgb32to24(const uint8_t *src, uint8_t *dst, int src_size)
     int i, num_pixels = src_size >> 2;
 
     for (i = 0; i < num_pixels; i++) {
-#if HAVE_BIGENDIAN
-        /* RGB32 (= A,B,G,R) -> BGR24 (= B,G,R) */
-        dst[3 * i + 0] = src[4 * i + 1];
-        dst[3 * i + 1] = src[4 * i + 2];
-        dst[3 * i + 2] = src[4 * i + 3];
-#else
-        dst[3 * i + 0] = src[4 * i + 2];
+        dst[3 * i + 0] = src[4 * i + 0];
         dst[3 * i + 1] = src[4 * i + 1];
-        dst[3 * i + 2] = src[4 * i + 0];
-#endif
+        dst[3 * i + 2] = src[4 * i + 2];
     }
 }
 
@@ -160,18 +155,10 @@  void rgb24to32(const uint8_t *src, uint8_t *dst, int src_size)
     int i;
 
     for (i = 0; 3 * i < src_size; i++) {
-#if HAVE_BIGENDIAN
-        /* RGB24 (= R, G, B) -> BGR32 (= A, R, G, B) */
-        dst[4 * i + 0] = 255;
-        dst[4 * i + 1] = src[3 * i + 0];
-        dst[4 * i + 2] = src[3 * i + 1];
-        dst[4 * i + 3] = src[3 * i + 2];
-#else
-        dst[4 * i + 0] = src[3 * i + 2];
+        dst[4 * i + 0] = src[3 * i + 0];
         dst[4 * i + 1] = src[3 * i + 1];
-        dst[4 * i + 2] = src[3 * i + 0];
+        dst[4 * i + 2] = src[3 * i + 2];
         dst[4 * i + 3] = 255;
-#endif
     }
 }
 
@@ -183,17 +170,10 @@  void rgb16tobgr32(const uint8_t *src, uint8_t *dst, int src_size)
 
     while (s < end) {
         register uint16_t bgr = *s++;
-#if HAVE_BIGENDIAN
-        *d++ = 255;
         *d++ = ((bgr&0x001F)<<3) | ((bgr&0x001F)>> 2);
         *d++ = ((bgr&0x07E0)>>3) | ((bgr&0x07E0)>> 9);
         *d++ = ((bgr&0xF800)>>8) | ((bgr&0xF800)>>13);
-#else
-        *d++ = ((bgr&0xF800)>>8) | ((bgr&0xF800)>>13);
-        *d++ = ((bgr&0x07E0)>>3) | ((bgr&0x07E0)>> 9);
-        *d++ = ((bgr&0x001F)<<3) | ((bgr&0x001F)>> 2);
         *d++ = 255;
-#endif
     }
 }
 
@@ -258,17 +238,10 @@  void rgb15tobgr32(const uint8_t *src, uint8_t *dst, int src_size)
 
     while (s < end) {
         register uint16_t bgr = *s++;
-#if HAVE_BIGENDIAN
-        *d++ = 255;
         *d++ = ((bgr&0x001F)<<3) | ((bgr&0x001F)>> 2);
         *d++ = ((bgr&0x03E0)>>2) | ((bgr&0x03E0)>> 7);
         *d++ = ((bgr&0x7C00)>>7) | ((bgr&0x7C00)>>12);
-#else
-        *d++ = ((bgr&0x7C00)>>7) | ((bgr&0x7C00)>>12);
-        *d++ = ((bgr&0x03E0)>>2) | ((bgr&0x03E0)>> 7);
-        *d++ = ((bgr&0x001F)<<3) | ((bgr&0x001F)>> 2);
         *d++ = 255;
-#endif
     }
 }
 
diff --git a/libswscale/rgb2rgb_template.c b/libswscale/rgb2rgb_template.c
index 499d25b..479474f 100644
--- a/libswscale/rgb2rgb_template.c
+++ b/libswscale/rgb2rgb_template.c
@@ -36,19 +36,11 @@  static inline void rgb24tobgr32_c(const uint8_t *src, uint8_t *dst,
     const uint8_t *end = s + src_size;
 
     while (s < end) {
-#if HAVE_BIGENDIAN
-        /* RGB24 (= R, G, B) -> RGB32 (= A, B, G, R) */
-        *dest++  = 255;
         *dest++  = s[2];
         *dest++  = s[1];
         *dest++  = s[0];
-        s       += 3;
-#else
-        *dest++  = *s++;
-        *dest++  = *s++;
-        *dest++  = *s++;
         *dest++  = 255;
-#endif
+        s       += 3;
     }
 }
 
@@ -60,19 +52,11 @@  static inline void rgb32tobgr24_c(const uint8_t *src, uint8_t *dst,
     const uint8_t *end = s + src_size;
 
     while (s < end) {
-#if HAVE_BIGENDIAN
-        /* RGB32 (= A, B, G, R) -> RGB24 (= R, G, B) */
-        s++;
         dest[2]  = *s++;
         dest[1]  = *s++;
         dest[0]  = *s++;
         dest    += 3;
-#else
-        *dest++  = *s++;
-        *dest++  = *s++;
-        *dest++  = *s++;
         s++;
-#endif
     }
 }
 
@@ -120,14 +104,14 @@  static inline void rgb16to15_c(const uint8_t *src, uint8_t *dst, int src_size)
     }
 }
 
-static inline void rgb32to16_c(const uint8_t *src, uint8_t *dst, int src_size)
+static inline void rgb32tobgr16_c(const uint8_t *src, uint8_t *dst, int src_size)
 {
     uint16_t *d        = (uint16_t *)dst;
     const uint8_t *s   = src;
     const uint8_t *end = s + src_size;
 
     while (s < end) {
-        register int rgb  = *(const uint32_t *)s;
+        register int rgb  = AV_RL32(s);
         s                += 4;
         *d++              = ((rgb & 0xFF)     >> 3) +
                             ((rgb & 0xFC00)   >> 5) +
@@ -135,7 +119,7 @@  static inline void rgb32to16_c(const uint8_t *src, uint8_t *dst, int src_size)
     }
 }
 
-static inline void rgb32tobgr16_c(const uint8_t *src, uint8_t *dst,
+static inline void rgb32to16_c(const uint8_t *src, uint8_t *dst,
                                   int src_size)
 {
     uint16_t *d        = (uint16_t *)dst;
@@ -143,7 +127,7 @@  static inline void rgb32tobgr16_c(const uint8_t *src, uint8_t *dst,
     const uint8_t *end = s + src_size;
 
     while (s < end) {
-        register int rgb  = *(const uint32_t *)s;
+        register int rgb  = AV_RL32(s);
         s                += 4;
         *d++              = ((rgb & 0xF8)     << 8) +
                             ((rgb & 0xFC00)   >> 5) +
@@ -151,14 +135,14 @@  static inline void rgb32tobgr16_c(const uint8_t *src, uint8_t *dst,
     }
 }
 
-static inline void rgb32to15_c(const uint8_t *src, uint8_t *dst, int src_size)
+static inline void rgb32tobgr15_c(const uint8_t *src, uint8_t *dst, int src_size)
 {
     uint16_t *d        = (uint16_t *)dst;
     const uint8_t *s   = src;
     const uint8_t *end = s + src_size;
 
     while (s < end) {
-        register int rgb  = *(const uint32_t *)s;
+        register int rgb  = AV_RL32(s);
         s                += 4;
         *d++              = ((rgb & 0xFF)     >> 3) +
                             ((rgb & 0xF800)   >> 6) +
@@ -166,7 +150,7 @@  static inline void rgb32to15_c(const uint8_t *src, uint8_t *dst, int src_size)
     }
 }
 
-static inline void rgb32tobgr15_c(const uint8_t *src, uint8_t *dst,
+static inline void rgb32to15_c(const uint8_t *src, uint8_t *dst,
                                   int src_size)
 {
     uint16_t *d        = (uint16_t *)dst;
@@ -174,7 +158,7 @@  static inline void rgb32tobgr15_c(const uint8_t *src, uint8_t *dst,
     const uint8_t *end = s + src_size;
 
     while (s < end) {
-        register int rgb  = *(const uint32_t *)s;
+        register int rgb  = AV_RL32(s);
         s                += 4;
         *d++              = ((rgb & 0xF8)     <<  7) +
                             ((rgb & 0xF800)   >>  6) +
@@ -278,17 +262,10 @@  static inline void rgb15to32_c(const uint8_t *src, uint8_t *dst, int src_size)
 
     while (s < end) {
         register uint16_t bgr = *s++;
-#if HAVE_BIGENDIAN
-        *d++ = 255;
         *d++ = ((bgr&0x7C00)>>7) | ((bgr&0x7C00)>>12);
         *d++ = ((bgr&0x03E0)>>2) | ((bgr&0x03E0)>> 7);
         *d++ = ((bgr&0x001F)<<3) | ((bgr&0x001F)>> 2);
-#else
-        *d++ = ((bgr&0x001F)<<3) | ((bgr&0x001F)>> 2);
-        *d++ = ((bgr&0x03E0)>>2) | ((bgr&0x03E0)>> 7);
-        *d++ = ((bgr&0x7C00)>>7) | ((bgr&0x7C00)>>12);
         *d++ = 255;
-#endif
     }
 }
 
@@ -300,17 +277,10 @@  static inline void rgb16to32_c(const uint8_t *src, uint8_t *dst, int src_size)
 
     while (s < end) {
         register uint16_t bgr = *s++;
-#if HAVE_BIGENDIAN
-        *d++ = 255;
         *d++ = ((bgr&0xF800)>>8) | ((bgr&0xF800)>>13);
         *d++ = ((bgr&0x07E0)>>3) | ((bgr&0x07E0)>> 9);
         *d++ = ((bgr&0x001F)<<3) | ((bgr&0x001F)>> 2);
-#else
-        *d++ = ((bgr&0x001F)<<3) | ((bgr&0x001F)>> 2);
-        *d++ = ((bgr&0x07E0)>>3) | ((bgr&0x07E0)>> 9);
-        *d++ = ((bgr&0xF800)>>8) | ((bgr&0xF800)>>13);
         *d++ = 255;
-#endif
     }
 }
 
diff --git a/libswscale/swscale_internal.h b/libswscale/swscale_internal.h
index 0f51df9..16c041b 100644
--- a/libswscale/swscale_internal.h
+++ b/libswscale/swscale_internal.h
@@ -698,8 +698,8 @@  static av_always_inline int isRGBinInt(enum AVPixelFormat pix_fmt)
 {
     return pix_fmt == AV_PIX_FMT_RGB48BE     ||
            pix_fmt == AV_PIX_FMT_RGB48LE     ||
-           pix_fmt == AV_PIX_FMT_RGB32       ||
-           pix_fmt == AV_PIX_FMT_RGB32_1     ||
+           pix_fmt == AV_PIX_FMT_RGBA       ||
+           pix_fmt == AV_PIX_FMT_ARGB     ||
            pix_fmt == AV_PIX_FMT_RGB24       ||
            pix_fmt == AV_PIX_FMT_RGB565BE    ||
            pix_fmt == AV_PIX_FMT_RGB565LE    ||
@@ -720,8 +720,8 @@  static av_always_inline int isBGRinInt(enum AVPixelFormat pix_fmt)
 {
     return pix_fmt == AV_PIX_FMT_BGR48BE     ||
            pix_fmt == AV_PIX_FMT_BGR48LE     ||
-           pix_fmt == AV_PIX_FMT_BGR32       ||
-           pix_fmt == AV_PIX_FMT_BGR32_1     ||
+           pix_fmt == AV_PIX_FMT_BGRA       ||
+           pix_fmt == AV_PIX_FMT_ABGR     ||
            pix_fmt == AV_PIX_FMT_BGR24       ||
            pix_fmt == AV_PIX_FMT_BGR565BE    ||
            pix_fmt == AV_PIX_FMT_BGR565LE    ||