[FFmpeg-devel] [PATCH] swscale alpha channel support (was: [RFC] Alpha support)

[Michael Niedermayer]

On Sun, Feb 08, 2009 at 10:33:56AM +0100, C?dric Schieli wrote:
> Hello,
> 
> 
> This is a new (and hopefully final) version of my patch to bring alpha
> channel support into swscale.

patience, it looks already quite good, but patches of such size need a few
revissions.


[...]
> @@ -768,17 +807,41 @@
>  #define YSCALE_YUV_2_ANYRGB_C(func, func2, func_g16, func_monoblack)\
>      switch(c->dstFormat)\
>      {\
> -    case PIX_FMT_RGB32:\
> -    case PIX_FMT_BGR32:\
> -    case PIX_FMT_RGB32_1:\
> -    case PIX_FMT_BGR32_1:\
> -        func(uint32_t)\
> -            ((uint32_t*)dest)[i2+0]= r[Y1] + g[Y1] + b[Y1];\
> -            ((uint32_t*)dest)[i2+1]= r[Y2] + g[Y2] + b[Y2];\
> -        }                \
> +    case PIX_FMT_RGBA:\
> +    case PIX_FMT_BGRA:\

> +        if (!CONFIG_SMALL && c->alpPixBuf)\
> +        {\
> +            func(uint32_t,1)\
> +                ((uint32_t*)dest)[i2+0]= r[Y1] + g[Y1] + b[Y1] + (A1<<24);\
> +                ((uint32_t*)dest)[i2+1]= r[Y2] + g[Y2] + b[Y2] + (A2<<24);\
> +            }\
> +        }else{\
> +            int needAlpha = (int)c->alpPixBuf;\
> +            func(uint32_t,CONFIG_SMALL ? needAlpha : 0)\
> +                ((uint32_t*)dest)[i2+0]= r[Y1] + g[Y1] + b[Y1] + ((CONFIG_SMALL && needAlpha) ? (A1<<24) : 0);\
> +                ((uint32_t*)dest)[i2+1]= r[Y2] + g[Y2] + b[Y2] + ((CONFIG_SMALL && needAlpha) ? (A2<<24) : 0);\
> +            }\
> +        }\
> +        break;\

i think
if(CONFIG_SMALL){
    int needAlpha = CONFIG_ALPHA ? (int)c->alpPixBuf : 0;\
    func(uint32_t, needAlpha)\
        ((uint32_t*)dest)[i2+0]= r[Y1] + g[Y1] + b[Y1] + (needAlpha ? (A1<<24) : 0);\
        ((uint32_t*)dest)[i2+1]= r[Y2] + g[Y2] + b[Y2] + (needAlpha ? (A2<<24) : 0);\
    }\
}else{
    if(CONFIG_ALPHA && c->alpPixBuf){
        func(uint32_t, 1)\
            ((uint32_t*)dest)[i2+0]= r[Y1] + g[Y1] + b[Y1] + (A1<<24) : 0);\
            ((uint32_t*)dest)[i2+1]= r[Y2] + g[Y2] + b[Y2] + (A2<<24) : 0);\
        }\
    }else{
        func(uint32_t, 0)\
            ((uint32_t*)dest)[i2+0]= r[Y1] + g[Y1] + b[Y1]);\
            ((uint32_t*)dest)[i2+1]= r[Y2] + g[Y2] + b[Y2]);\
        }\
    }
}

would be clearer, more flexible and generate the same code, its a few lines
more source sadly but i think the clarity outweights that.


[...]
> @@ -1165,6 +1231,36 @@
>          {
>              //Note 8280 == DSTW_OFFSET but the preprocessor can't handle that there :(
>              case PIX_FMT_RGB32:
> +                if(c->alpPixBuf)
> +                {
> +                    __asm__ volatile(
> +                    "mov %%"REG_b", "ESP_OFFSET"(%5)        \n\t"
> +                    "mov        %6, %%"REG_b"               \n\t"

you write into ebx here

> +                    "mov %%"REG_b", "U_TEMP"(%5)            \n\t"
> +                    "mov        %7, %%"REG_b"               \n\t"

%7 is "m" and could use ebx to address
you can prevent this by putting REG_b on the clobber list though this
will likely not compile with -fPIC

basically once you changed a register that is not on the output and not
clobber list you no longer can access any "m" nor "r"

also keep in mind push/pop change esp, and esp can be used in "m" as well
so again no more "m" after push


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Democracy is the form of government in which you can choose your dictator
-------------- 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/20090208/7a786785/attachment.pgp>