[MPlayer-dev-eng] Solaris patches

Robin KAY komadori at gekkou.co.uk
Sun Sep 3 15:49:27 CEST 2006


Attila Kinali wrote:
[snip]

> I guess you have read DOCS/tech/patches.txt ?
> You have a lot of cosmetics in your patches which should not
> be there.

Do not the revised patches in my reply to 'Dominik Mierzejewski' address 
these? From the time of your post, I would have thought you would have 
received it.

http://www.blastwave.org/~komadori/patches/mplayer-head-20060902-fix-unaligned-read_NOCOSMETICS.diff

The other one has been split into two patches as linked below.

[snip]

>> http://www.blastwave.org/~komadori/patches/mplayer-head-20060902-fix-big-endian-colours.diff
>
[snip]

> You should make this rather in the form of
> if (..)
> { ..}
> else
> {...}
> This would make it clearer what's going on.

Ok.

[snip]

> This is an unrelated change and can be counted
> as cosmetics. It is valid though. Please split
> it into a seperate patch.

http://www.blastwave.org/~komadori/patches/mplayer-head-20060902-simplify-switch.diff

[snip]

> Can you explain why this doesn't work for you?
> It apreantly works for everone else on a big endian machine.
> (including my tests on solaris/sparc)

Sorry, the patch is erroneously named. It's not really a big-endian bug 
at all (although that's where I discovered it), but a bug in the plain C 
versions of the YUV conversion routines (technically) affecting both 
byte orderings.

The real bug was in assigning the wrong (inverted) value to isRgb in 
yuv2rgb.c. This is not apparent if platform-specific versions of the 
conversion routines are used. Neither is it apparent if Xvideo is used. 
Your test builds under Solaris/SPARC are probably compiled with mediaLib.

Since vo_x11.c always used machine word BGR irrespective of the colour 
masks, the bug is also not apparent if the server XImage format matches 
the client's machine word RGB. The changes to vo_x11.c perform a more 
stringent test of the colour masks in order to select machine word RGB 
or BGR.

Modified the if statement as above and removed the change to the switch 
statement:-

http://www.blastwave.org/~komadori/patches/mplayer-head-20060902-fix-broken-colours.diff

>>http://www.blastwave.org/~komadori/patches/mplayer-head-20060902-fix-solaris8-errors.diff
>>    
>>
>What error is fixed here?
>  
>
Answered in my replay to 'Dominik Mierzejewski'.

>>http://www.blastwave.org/~komadori/patches/mplayer-head-20060902-fix-unaligned-read.diff
>>
>>Fixes a crash on SPARC in the ASF demuxer.
>>    
>>
>Using memcpy to fix alignment here looks bogus to me.
>But i'm not sure what the correct way to do it would be.
>I've never dealt with such issues.
>  
>
I initially just used ptr[0] | (ptr[1] << 1), but then I saw the 
memcpy(3C) method was used elsewhere in asfheader.c. I imagine this was 
done to make the code portable to systems with non-octet chars.

-- 
Wishing you good fortune,
Robin KAY (komadori)



More information about the MPlayer-dev-eng mailing list