[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