[MPlayer-dev-eng] Solaris patches

[Dominik 'Rathann' Mierzejewski]

On Sunday, 03 September 2006 at 02:50, Robin KAY wrote:
> Hello,
> 
> I'm the maintainer of blastwave.org's mplayer package. I have made some 
> patches in the course of my work. They are available both against 
> 1.0pre8 and the current SVN head.

Thanks you for the patches. I can't comment on the code itself, but a quick
review reveals several minor problems with your patches. See below.

> http://www.blastwave.org/~komadori/patches/mplayer-1.0pre8-fix-big-endian-colours.diff
> http://www.blastwave.org/~komadori/patches/mplayer-head-20060902-fix-big-endian-colours.diff
> 
> Fixes swapped red and blue channels in video output on SPARC.

--- mplayer-head-20060902-orig/libswscale/yuv2rgb.c	Sat Sep  2 19:17:33 2006
+++ mplayer-head-20060902/libswscale/yuv2rgb.c	Sat Sep  2 20:53:32 2006
@@ -598,7 +598,7 @@
 #endif
 #ifdef HAVE_MLIB
     {
-	SwsFunc t= yuv2rgb_init_mlib(c);
+	SwsFunc t = yuv2rgb_init_mlib(c);
 	if(t) return t;
     }
 #endif

Cosmetics, rejected.

--- mplayer-head-20060902-orig/libvo/vo_x11.c	Sat Sep  2 19:16:18 2006
+++ mplayer-head-20060902/libvo/vo_x11.c	Sat Sep  2 20:53:02 2006
@@ -7,8 +7,8 @@
 #include "config.h"
 #include "video_out.h"
 #include "video_out_internal.h"
+#include "bswap.h"
 
-
 #include <X11/Xlib.h>
 #include <X11/Xutil.h>
 
Mixed cosmetics and functional.


> http://www.blastwave.org/~komadori/patches/mplayer-1.0pre8-fix-solaris8-errors.diff
> http://www.blastwave.org/~komadori/patches/mplayer-head-20060902-fix-solaris8-errors.diff
> 
> Fixes building under Solaris 8.

What errors does this fix?

> http://www.blastwave.org/~komadori/patches/mplayer-1.0pre8-fix-unaligned-read.diff
> http://www.blastwave.org/~komadori/patches/mplayer-head-20060902-fix-unaligned-read.diff
> 
> Fixes a crash on SPARC in the ASF demuxer.

@@ -413,10 +414,11 @@
               streams = malloc(2*stream_count*sizeof(uint32_t));
         mp_msg(MSGT_HEADER,MSGL_V," stream count=[0x%x][%u]\n", stream_count, stream_count );
         for( i=0 ; i<stream_count ; i++ ) {
-          stream_id = le2me_16(*(uint16_t*)ptr);
+          memcpy(&stream_id, ptr, sizeof(uint16_t)); // align value
+          stream_id = le2me_16(stream_count);
           ptr += sizeof(uint16_t);
           if (ptr > &hdr[hdr_len]) goto len_err_out;
-          memcpy(&max_bitrate, ptr, sizeof(uint32_t));// workaround unaligment bug on sparc
+          memcpy(&max_bitrate, ptr, sizeof(uint32_t)); // align value
           max_bitrate = le2me_32(max_bitrate);
           ptr += sizeof(uint32_t);
           if (ptr > &hdr[hdr_len]) goto len_err_out;

Mixed cosmetics with functional. Rewording a comment *is* considered cosmetics.

Regards,
R.

-- 
MPlayer developer and RPMs maintainer: http://rpm.greysector.net/mplayer/
There should be a science of discontent. People need hard times and
oppression to develop psychic muscles.
	-- from "Collected Sayings of Muad'Dib" by the Princess Irulan