[MPlayer-dev-eng] [PATCH] RGB/BGR big-endian fixes

[Alan Curry]

Diego Biurrun writes the following:
>
>>   mplayer -vo x11 -vf format=bgr24,scale movie.avi
>>   mplayer -vo x11 -vf format=rgb24,scale movie.avi
>> 
>> both produce wrong colors while vo_xv is fine.  This problem is not
>> caused by this patch, though, it was present before.  Disabling AltiVec
>> does not make a difference.
>
>I tested some more with vo_png, vo_jpeg, vo_pnm and vo_tga.  vo_tga and
>rgb24 create a blue picture, but this occurs with and without the patch.
>Everything else is fine.

There are several non-altivec-related big-endian bugs in rgb2rgb.c and
rgb2rgb_template.c, which probably explains these color problems. Red and
blue can be reversed, or in a 32 bit format the 8 unused/alpha bits can be on
the wrong side of the other 24 bits. Exactly which kind of color permutation
you get depends on what depth your X server is running in.

Here's a patch that will hopefully fix those problems. I haven't tested any
of the 16bpp-or-less formats, but the 24bpp and 32bpp ones are Definitely
Good (ready the brown paper bag...)

vo_tga is still broken. I still haven't taken a serious look at it.

-------------- next part --------------
Index: postproc/rgb2rgb.c
===================================================================
RCS file: /cvsroot/mplayer/main/postproc/rgb2rgb.c,v
retrieving revision 1.65
diff -u -r1.65 rgb2rgb.c
--- postproc/rgb2rgb.c	14 Nov 2005 00:30:37 -0000	1.65
+++ postproc/rgb2rgb.c	9 Feb 2006 06:58:57 -0000
@@ -446,9 +446,16 @@
 	long num_pixels = src_size >> 2;
 	for(i=0; i<num_pixels; i++)
 	{
+		#ifdef WORDS_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 + 1] = src[4*i + 1];
 		dst[3*i + 2] = src[4*i + 0];
+		#endif
 	}
 }
 
@@ -457,10 +464,18 @@
 	long i;
 	for(i=0; 3*i<src_size; i++)
 	{
+		#ifdef WORDS_BIGENDIAN
+		/* RGB24 (= R,G,B) -> BGR32 (= A,R,G,B) */
+		dst[4*i + 0] = 0;
+		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 + 1] = src[3*i + 1];
 		dst[4*i + 2] = src[3*i + 0];
 		dst[4*i + 3] = 0;
+		#endif
 	}
 }
 
@@ -474,10 +489,17 @@
 	{
 		register uint16_t bgr;
 		bgr = *s++;
+		#ifdef WORDS_BIGENDIAN
+		*d++ = 0;
+		*d++ = (bgr&0x1F)<<3;
+		*d++ = (bgr&0x7E0)>>3;
+		*d++ = (bgr&0xF800)>>8;
+		#else
 		*d++ = (bgr&0xF800)>>8;
 		*d++ = (bgr&0x7E0)>>3;
 		*d++ = (bgr&0x1F)<<3;
 		*d++ = 0;
+		#endif
 	}
 }
 
@@ -541,10 +563,17 @@
 	{
 		register uint16_t bgr;
 		bgr = *s++;
+		#ifdef WORDS_BIGENDIAN
+		*d++ = 0;
+		*d++ = (bgr&0x1F)<<3;
+		*d++ = (bgr&0x3E0)>>2;
+		*d++ = (bgr&0x7C00)>>7;
+		#else
 		*d++ = (bgr&0x7C00)>>7;
 		*d++ = (bgr&0x3E0)>>2;
 		*d++ = (bgr&0x1F)<<3;
 		*d++ = 0;
+		#endif
 	}
 }
 
Index: postproc/rgb2rgb_template.c
===================================================================
RCS file: /cvsroot/mplayer/main/postproc/rgb2rgb_template.c,v
retrieving revision 1.74
diff -u -r1.74 rgb2rgb_template.c
--- postproc/rgb2rgb_template.c	12 Oct 2005 12:11:27 -0000	1.74
+++ postproc/rgb2rgb_template.c	9 Feb 2006 06:59:02 -0000
@@ -104,10 +104,12 @@
   while(s < end)
   {
 #ifdef WORDS_BIGENDIAN
+    /* RGB24 (= R,G,B) -> RGB32 (= A,B,G,R) */
     *dest++ = 0;
-    *dest++ = *s++;
-    *dest++ = *s++;
-    *dest++ = *s++;
+    *dest++ = s[2];
+    *dest++ = s[1];
+    *dest++ = s[0];
+    s+=3;
 #else
     *dest++ = *s++;
     *dest++ = *s++;
@@ -188,10 +190,12 @@
   while(s < end)
   {
 #ifdef WORDS_BIGENDIAN
+    /* RGB32 (= A,B,G,R) -> RGB24 (= R,G,B) */
     s++;
-    *dest++ = *s++;
-    *dest++ = *s++;
-    *dest++ = *s++;
+    dest[2] = *s++;
+    dest[1] = *s++;
+    dest[0] = *s++;
+    dest += 3;
 #else
     *dest++ = *s++;
     *dest++ = *s++;
@@ -465,6 +469,7 @@
 	while(s < end)
 	{
 		// FIXME on bigendian
+		/* Looks bigendian-OK to me. --Pac. */
 		const int src= *s; s += 4;
 		*d++ = ((src&0xF8)<<8) + ((src&0xFC00)>>5) + ((src&0xF80000)>>19);
 	}
@@ -562,6 +567,7 @@
 	while(s < end)
 	{
 		// FIXME on bigendian
+		/* Looks bigendian-OK to me. --Pac. */
 		const int src= *s; s += 4;
 		*d++ = ((src&0xFF)>>3) + ((src&0xF800)>>6) + ((src&0xF80000)>>9);
 	}
@@ -624,6 +630,7 @@
 	while(s < end)
 	{
 		// FIXME on bigendian
+		/* Looks bigendian-OK to me. --Pac. */
 		const int src= *s; s += 4;
 		*d++ = ((src&0xF8)<<7) + ((src&0xF800)>>6) + ((src&0xF80000)>>19);
 	}
@@ -1247,14 +1254,13 @@
 		int bgr= *s++;
 		*((uint32_t*)d)++ = ((bgr&0x1F)<<3) + ((bgr&0x3E0)<<6) + ((bgr&0x7C00)<<9);
 #else
-//FIXME this is very likely wrong for bigendian (and the following converters too)
 		register uint16_t bgr;
 		bgr = *s++;
 #ifdef WORDS_BIGENDIAN
 		*d++ = 0;
-		*d++ = (bgr&0x1F)<<3;
-		*d++ = (bgr&0x3E0)>>2;
 		*d++ = (bgr&0x7C00)>>7;
+		*d++ = (bgr&0x3E0)>>2;
+		*d++ = (bgr&0x1F)<<3;
 #else
 		*d++ = (bgr&0x1F)<<3;
 		*d++ = (bgr&0x3E0)>>2;
@@ -1326,9 +1332,9 @@
 		bgr = *s++;
 #ifdef WORDS_BIGENDIAN
 		*d++ = 0;
-		*d++ = (bgr&0x1F)<<3;
-		*d++ = (bgr&0x7E0)>>3;
 		*d++ = (bgr&0xF800)>>8;
+		*d++ = (bgr&0x7E0)>>3;
+		*d++ = (bgr&0x1F)<<3;
 #else
 		*d++ = (bgr&0x1F)<<3;
 		*d++ = (bgr&0x7E0)>>3;