[FFmpeg-devel] [RFC] RGB32 / BGR32 ethernal bug

Michael Niedermayer michaelni
Tue Jan 26 09:17:12 CET 2010


On Tue, Jan 26, 2010 at 01:06:18AM +0100, Stefano Sabatini wrote:
> I, currently we have:
> isRGB(RGB32   = LE:BGRA) == 1
> isRGB(RGB32_1 = LE:ABGR) == 1
> 
> and similarly for isBGR(), this is the cause of an ethernal confusion
> and frustration for who dares to takle the understanding of rgba
> conversion code, my proposal is to change how the RGB stuff is dealt
> and to use in the isRGB()/isBGRA() definitions something like:
> 
> #define isRGB(x)        (           \
>            (x)==PIX_FMT_RGBA        \
>         || (x)==PIX_FMT_ARGB        \
>         ...
> 
> #define isBGR(x)        (           \
>            (x)==PIX_FMT_BGRA        \
>         || (x)==PIX_FMT_ABGR        \
>         ...
> 
> which is also what the random sane person would expect, and to do the
> necessary changes in the swscale code.

ive cleaned up the macros somewhat, which should also make their meaning
clearer and ive added 2 so you can add your byte based shuffling code.

I think it goes without saying that adding new macros and using them
is fine but giving a new and completely fliped meaning to existing macros
is not fine. At least if people expect me to maintain this in the future.
also your mail sounds only consistent because you skiped telling us what
you do with the majority of rgb formats that arent byte based. like RGB15
they dont fit in your sheme at all


>  rgb2rgb.c |   25 +++++++++++++++++++++++++
>  rgb2rgb.h |    7 +++++++
>  swscale.c |   29 ++++++++++++++++++++++++++---
>  3 files changed, 58 insertions(+), 3 deletions(-)
> 71929befe054ce487ac7ca8be14acd67d7e496fb  fix-rgba-scaling.patch
> Index: ffmpeg/libswscale/rgb2rgb.c
> ===================================================================
> --- ffmpeg.orig/libswscale/rgb2rgb.c	2010-01-20 23:06:57.000000000 +0100
> +++ ffmpeg/libswscale/rgb2rgb.c	2010-01-20 23:07:01.000000000 +0100
> @@ -442,3 +442,28 @@
>          dst[i] = ((b<<1)&0x07) | ((g&0x07)<<3) | ((r&0x03)<<6);
>      }
>  }
> +
> +void rgb32torgb32_0123(const uint8_t *src, uint8_t *dst, long src_size)
> +{
> +    memcpy(dst, src, src_size);
> +}
> +
> +#define RGB32_TO_RGB32_TRANSFORM_CONV(a, b, c, d)                       \
> +void rgb32torgb32_##a##b##c##d(const uint8_t *src, uint8_t *dst, long src_size) \
> +{                                                                       \
> +    long i;                                                             \
> +                                                                        \
> +    for (i = 0; i < src_size; i+=4) {                                   \
> +        dst[i + 0] = src[i + (a)];                                      \
> +        dst[i + 1] = src[i + (b)];                                      \
> +        dst[i + 2] = src[i + (c)];                                      \
> +        dst[i + 3] = src[i + (d)];                                      \
> +    }                                                                   \
> +}
> +
> +RGB32_TO_RGB32_TRANSFORM_CONV(0, 3, 2, 1);
> +RGB32_TO_RGB32_TRANSFORM_CONV(1, 2, 3, 0);
> +RGB32_TO_RGB32_TRANSFORM_CONV(2, 1, 0, 3);
> +RGB32_TO_RGB32_TRANSFORM_CONV(3, 0, 1, 2);
> +RGB32_TO_RGB32_TRANSFORM_CONV(3, 2, 1, 0);

you write byte shuffling code, id suggest to call it accordingly
theres nothing rgb specific on this, it could be usefull to do packed yuv
shuffling as well.

like maybe:
byte_shuffle_0123_to_0321
but before you convert your code to this, what is the speed gain from
a/b/c/d being constants instead of just passed into the function?
there should be enough registers for this in theory
also
dst+=4; src+=4
for(dst<dstend
would reduce the register need


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100126/c220df16/attachment.pgp>



More information about the ffmpeg-devel mailing list