[MPlayer-dev-eng] another RGB ordering fix for altivec

Alan Curry pacman at TheWorld.com
Sat Mar 18 09:56:59 CET 2006


Ivan Kalvachev writes the following:
>
>2006/3/16, Alan Curry <pacman at theworld.com>:
>> Ivan Kalvachev writes the following:
>> >
>> >Could there be argb64 ?? (e.g. film gimp / cine paint)
>>
>> At an unspecified future time, it could appear. Then we'd either
>> s/IMGFMT_ARGB/IMGFMT_ARGB32/ throughout mplayer or decide to keep it as it
>> is, with the 32 implied. Either way, why should that stop us from using
>> consistent names for 32-bit formats now?
>
>MPlayer policy is to highly discourage non-functional/cosmetic changes.

And what a wonderful dogma that is! It's probably why mplayer is one of the
few packages for which a recursive, indiscriminate `indent --gnu' would be an
improvement.

>So it is better to do it in a way you will be sure that you won't have
>to do such things.

This did fix a bug (play something -vf format=rgb32,scale with altivec and
the colors were wrong), and the bug was directly related to the confusion
over what these argb32/bgra32 functions actually do. Something had to be
changed.

The one that produced correct output before my patch was this:

  IMGFMT_BGR32 (=IMGFMT_ARGB) -> altivec_yuv2_bgra32 -> out_argb

Now you might call that a cosmetic problem; I call it 2 bugs that happen to
cancel each other out. IMGFMT_BGR32 shouldn't be calling anything with "bgra"
in the name because it's not in that order, and anything "bgra32" should not
be calling "argb" - allowing those 2 things to have the same meaning would be
absolutely nuts.

The other one, which things turn blue on playback, was this:

  IMGFMT_RGB32 (=IMGFMT_ABGR) -> altivec_yuv2_argb32 -> out_argb

If I can't fix the name "altivec_yuv2_argb32" to comply with the official
naming scheme documented in colorspaces.txt, how should the above problem be
fixed? At least one of the calls has to be changed... which one, and to what?

The only answer I can come up with is to fix them all so that it's clear what
they're supposed to do, and that they do it.




More information about the MPlayer-dev-eng mailing list