[FFmpeg-devel] [PATCH] swscale alpha channel support
Cédric Schieli
cschieli
Wed Mar 11 15:44:52 CET 2009
2009/3/11 Michael Niedermayer <michaelni at gmx.at>:
> On Tue, Mar 10, 2009 at 05:05:53PM +0100, C?dric Schieli wrote:
>> 2009/3/5 Michael Niedermayer <michaelni at gmx.at>:
>> > On Thu, Mar 05, 2009 at 03:09:26PM +0100, C?dric Schieli wrote:
>> >> 2009/3/2 Michael Niedermayer <michaelni at gmx.at>:
>> >> > On Fri, Feb 27, 2009 at 11:30:25PM +0100, C?dric Schieli wrote:
[...]
>> #1 : sws_fix_initMMX2HScaler.patch
>
> ok (assuming tested and improvment confirmed)
yes to both
applied
>
>
>> #2 : swscale-example_yuva.patch
>
> see below
>
>
>> #3 : sws_parametrized_yscaleyuv2packed.patch
>
> ok if no change to striped obj files
yes
applied
>
>
>> #4 : sws_configure_alpha.patch
>
> not maintained by me
>
>
>> #5 : sws_yuva2rgb.patch
>
> ok
this one needs CONFIG_SWSCALE_ALPHA to be defined, so should I commit
#4 or wait for (work on) another way of disabling pixel format
individually (which would force enabling CONFIG_SWSCALE_ALPHA if at
least one alpha srcFormat and one alpha dstFormat is enabled) ?
BTW, this will break build with CONFIG_SWSCALE_ALPHA=0 as it is
attached one has more #if but is more correct
>
>
>> #6 : sws_scale_alpha.patch
>
> see below
>
>
>> #7 : sws_output_yuva420p.patch
>
> see below
>
>
>> #8 : swscale-example_use_alpha.patch
>
> see below
>
> [...]
>
>> ? ? ? ? ?// avoid stride % bpp != 0
>> + ? ? ? ?if (srcFormat==PIX_FMT_YUVA420P)
>> + ? ? ? ? ? ?srcStride[i]= refStride[i];
>> + ? ? ? ?else
>> ? ? ? ? ?if (srcFormat==PIX_FMT_RGB24 || srcFormat==PIX_FMT_BGR24)
>> ? ? ? ? ? ? ?srcStride[i]= srcW*3;
>> ? ? ? ? ?else
>> @@ -72,7 +75,10 @@ static int doTest(uint8_t *ref[3], int r
>> ? ? ? ? ?else
>> ? ? ? ? ? ? ?dstStride[i]= dstW*4;
>>
>> - ? ? ? ?src[i]= (uint8_t*) malloc(srcStride[i]*srcH);
>> + ? ? ? ?if (srcFormat==PIX_FMT_YUVA420P)
>> + ? ? ? ? ? ?src[i]= ref[i];
>> + ? ? ? ?else
>> + ? ? ? ? ? ?src[i]= (uint8_t*) malloc(srcStride[i]*srcH);
>> ? ? ? ? ?dst[i]= (uint8_t*) malloc(dstStride[i]*dstH);
>> ? ? ? ? ?out[i]= (uint8_t*) malloc(refStride[i]*h);
>> ? ? ? ? ?if (!src[i] || !dst[i] || !out[i]) {
>> @@ -84,14 +90,16 @@ static int doTest(uint8_t *ref[3], int r
>> ? ? ?}
>>
>> ? ? ?dstContext = outContext = NULL;
>> - ? ?srcContext= sws_getContext(w, h, PIX_FMT_YUV420P, srcW, srcH, srcFormat, flags, NULL, NULL, NULL);
>> - ? ?if (!srcContext) {
>> - ? ? ? ?fprintf(stderr, "Failed to get %s ---> %s\n",
>> - ? ? ? ? ? ? ? ?sws_format_name(PIX_FMT_YUV420P),
>> - ? ? ? ? ? ? ? ?sws_format_name(srcFormat));
>> - ? ? ? ?res = -1;
>> + ? ?if (srcFormat!=PIX_FMT_YUVA420P){
>> + ? ? ? ?srcContext= sws_getContext(w, h, PIX_FMT_YUVA420P, srcW, srcH, srcFormat, flags, NULL, NULL, NULL);
>> + ? ? ? ?if (!srcContext) {
>> + ? ? ? ? ? ?fprintf(stderr, "Failed to get %s ---> %s\n",
>> + ? ? ? ? ? ? ? ? ? ?sws_format_name(PIX_FMT_YUVA420P),
>> + ? ? ? ? ? ? ? ? ? ?sws_format_name(srcFormat));
>> + ? ? ? ? ? ?res = -1;
>
> why?
this patch is an intermediate step which I shouldn't have published
it let me produce YUVA420P src buffers without having YUVA420P as a
supported output format
>
>
>>
>> - ? ? ? ?goto end;
>> + ? ? ? ? ? ?goto end;
>
> cosmetic, doesnt belong in functional patches ...
>
>
>> + ? ? ? ?}
>> ? ? ?}
>> ? ? ?dstContext= sws_getContext(srcW, srcH, srcFormat, dstW, dstH, dstFormat, flags, NULL, NULL, NULL);
>> ? ? ?if (!dstContext) {
>> @@ -114,7 +122,8 @@ static int doTest(uint8_t *ref[3], int r
>> ?// ? ?printf("test %X %X %X -> %X %X %X\n", (int)ref[0], (int)ref[1], (int)ref[2],
>> ?// ? ? ? ?(int)src[0], (int)src[1], (int)src[2]);
>>
>> + ? ?if (srcFormat!=PIX_FMT_YUVA420P)
>
>> - ? ?sws_scale(srcContext, ref, refStride, 0, h ? , src, srcStride);
>> + ? ? ? ?sws_scale(srcContext, ref, refStride, 0, h ? , src, srcStride);
>
> same
>
>
>> ? ? ?sws_scale(dstContext, src, srcStride, 0, srcH, dst, dstStride);
>> ? ? ?sws_scale(outContext, dst, dstStride, 0, dstH, out, refStride);
>>
>> @@ -136,12 +145,14 @@ static int doTest(uint8_t *ref[3], int r
>>
>> ? ? ?end:
>>
>> + ? ?if (srcFormat!=PIX_FMT_YUVA420P)
>
>> - ? ?sws_freeContext(srcContext);
>> + ? ? ? ?sws_freeContext(srcContext);
>
> same
>
>
> [...]
>> @@ -203,6 +214,11 @@ int main(int argc, char **argv){
>> ? ? ? ? ?}
>> ? ? ?}
>> ? ? ?sws_scale(sws, rgb_src, rgb_stride, 0, H, src, stride);
>> + ? ?for (y=0; y<H; y++){
>> + ? ? ? ?for (x=0; x<W; x++){
>> + ? ? ? ? ? ?src[3][ x + y*W]= random();
>> + ? ? ? ?}
>> + ? ?}
>>
>> ? ? ?selfTest(src, stride, W, H);
>>
>
> more "why?" code
>
> [...]
>> + ? ? ? ? ? ?A = 0;\
>> + ? ? ? ? ? ?for (j=0; j<lumFilterSize; j++)\
>> + ? ? ? ? ? ? ? ?A += alpSrc[j][i ? ? ] * lumFilter[j];\
>> + ? ? ? ? ? ?A >>=10;\
>> + ? ? ? ? ? ?A += rnd;\
>> + ? ? ? ? ? ?A >>=9;\
>
> this contains a few useless operations
> a single shift should be enough,so should be chnaging the initial value of A
yes
patch updated
>
>
> [...]
>> Index: ffmpeg/libswscale/swscale_template.c
>> ===================================================================
>> --- ffmpeg.orig/libswscale/swscale_template.c 2009-03-10 16:35:58.000000000 +0100
>> +++ ffmpeg/libswscale/swscale_template.c ? ? ?2009-03-10 16:37:12.000000000 +0100
>> @@ -458,11 +458,11 @@
>> ? ? ?"pmulhw "VG_COEFF"("#c"), %%mm4 ? ? \n\t"\
>> ? ? ?/* mm2=(U-128)8, mm3=ug, mm4=vg mm5=(V-128)8 */\
>>
>> -#define REAL_YSCALEYUV2RGB_YA(index, c) \
>> - ? ?"movq ?(%0, "#index", 2), %%mm0 ? ? \n\t" /*buf0[eax]*/\
>> - ? ?"movq ?(%1, "#index", 2), %%mm1 ? ? \n\t" /*buf1[eax]*/\
>> - ? ?"movq 8(%0, "#index", 2), %%mm6 ? ? \n\t" /*buf0[eax]*/\
>> - ? ?"movq 8(%1, "#index", 2), %%mm7 ? ? \n\t" /*buf1[eax]*/\
>> +#define REAL_YSCALEYUV2RGB_YA(index, c, b1, b2) \
>> + ? ?"movq ?("#b1", "#index", 2), %%mm0 ? ? \n\t" /*buf0[eax]*/\
>> + ? ?"movq ?("#b2", "#index", 2), %%mm1 ? ? \n\t" /*buf1[eax]*/\
>> + ? ?"movq 8("#b1", "#index", 2), %%mm6 ? ? \n\t" /*buf0[eax]*/\
>> + ? ?"movq 8("#b2", "#index", 2), %%mm7 ? ? \n\t" /*buf1[eax]*/\
>> ? ? ?"psubw ? ? ? ? ? ? %%mm1, %%mm0 ? ? \n\t" /* buf0[eax] - buf1[eax]*/\
>> ? ? ?"psubw ? ? ? ? ? ? %%mm7, %%mm6 ? ? \n\t" /* buf0[eax] - buf1[eax]*/\
>> ? ? ?"pmulhw "LUM_MMX_FILTER_OFFSET"+8("#c"), %%mm0 ?\n\t" /* (buf0[eax] - buf1[eax])yalpha1>>16*/\
>> @@ -501,11 +501,11 @@
>> ? ? ?"packuswb ? ? ? ? ?%%mm6, %%mm5 ? ? \n\t"\
>> ? ? ?"packuswb ? ? ? ? ?%%mm3, %%mm4 ? ? \n\t"\
>>
>> -#define YSCALEYUV2RGB_YA(index, c) REAL_YSCALEYUV2RGB_YA(index, c)
>> +#define YSCALEYUV2RGB_YA(index, c, b1, b2) REAL_YSCALEYUV2RGB_YA(index, c, b1, b2)
>>
>> ?#define YSCALEYUV2RGB(index, c) \
>> ? ? ?REAL_YSCALEYUV2RGB_UV(index, c) \
>> - ? ?REAL_YSCALEYUV2RGB_YA(index, c) \
>> + ? ?REAL_YSCALEYUV2RGB_YA(index, c, %0, %1) \
>> ? ? ?REAL_YSCALEYUV2RGB_COEFF(c)
>>
>> ?#define REAL_YSCALEYUV2PACKED1(index, c) \
>
> this looks like it should be a seperate patch
yes
patch attached
>
>
> [...]
>> @@ -952,16 +966,32 @@
>> ? ? ? ? ? ? ? dest, uDest, dstW, chrDstW, dstFormat);
>> ?}
>>
>> -static inline void RENAME(yuv2yuv1)(SwsContext *c, int16_t *lumSrc, int16_t *chrSrc,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?uint8_t *dest, uint8_t *uDest, uint8_t *vDest, long dstW, long chrDstW)
>> +static inline void RENAME(yuv2yuv1)(SwsContext *c, int16_t *lumSrc, int16_t *chrSrc, int16_t *alpSrc,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?uint8_t *dest, uint8_t *uDest, uint8_t *vDest, uint8_t *aDest, long dstW, long chrDstW)
>> ?{
>> ? ? ?int i;
>> ?#if HAVE_MMX
>> ? ? ?if(!(c->flags & SWS_BITEXACT)){
>> +#if CONFIG_SWSCALE_ALPHA
>> + ? ? ? ?long p= uDest ? 4 : 2;
>> + ? ? ? ?uint8_t *src2[4]= {alpSrc + dstW, lumSrc + dstW, chrSrc + chrDstW, chrSrc + VOFW + chrDstW};
>> + ? ? ? ?uint8_t *dst2[4]= {aDest, dest, uDest, vDest};
>> + ? ? ? ?long counter2[4] = {dstW, dstW, chrDstW, chrDstW};
>> + ? ? ? ?uint8_t **src = src2, **dst = dst2;
>> + ? ? ? ?long *counter = counter2;
>> +
>> + ? ? ? ?if (!aDest){
>> + ? ? ? ? ? ?src++;
>> + ? ? ? ? ? ?dst++;
>> + ? ? ? ? ? ?counter++;
>> + ? ? ? ? ? ?p--;
>> + ? ? ? ?}
>
> hmm
> looks like a mess
>
> for()
> ? ?if(dest[i])
> ? ? ? ?do filter
patch updated
>
>
> [...]
>> @@ -2055,12 +2056,22 @@
>> ? ? ? ? ? ? ? ? ? ? ? ?int srcSliceH, uint8_t* dst[], int dstStride[])
>> ?{
>> ? ? ?int plane;
>> - ? ?for (plane=0; plane<3; plane++)
>> + ? ?for (plane=0; plane<4; plane++)
>> ? ? ?{
>> - ? ? ? ?int length= plane==0 ? c->srcW ?: -((-c->srcW ?)>>c->chrDstHSubSample);
>> - ? ? ? ?int y= ? ? ?plane==0 ? srcSliceY: -((-srcSliceY)>>c->chrDstVSubSample);
>> - ? ? ? ?int height= plane==0 ? srcSliceH: -((-srcSliceH)>>c->chrDstVSubSample);
>> + ? ? ? ?int length= (plane==0 || plane==3) ? c->srcW ?: -((-c->srcW ?)>>c->chrDstHSubSample);
>> + ? ? ? ?int y= ? ? ?(plane==0 || plane==3) ? srcSliceY: -((-srcSliceY)>>c->chrDstVSubSample);
>> + ? ? ? ?int height= (plane==0 || plane==3) ? srcSliceH: -((-srcSliceH)>>c->chrDstVSubSample);
>>
>> + ? ? ? ?if (!(isALPHA(c->srcFormat) && isALPHA(c->dstFormat)) && plane == 3){
>> + ? ? ? ? ? ?if (isALPHA(c->dstFormat)){
>> + ? ? ? ? ? ? ? ?int i;
>> + ? ? ? ? ? ? ? ?uint8_t *dstPtr= dst[3] + dstStride[3]*y;
>> + ? ? ? ? ? ? ? ?for (i=0; i<height; i++){
>> + ? ? ? ? ? ? ? ? ? ?memset(dstPtr, 255, length);
>> + ? ? ? ? ? ? ? ? ? ?dstPtr+= dstStride[3];
>> + ? ? ? ? ? ? ? ?}
>> + ? ? ? ? ? ?}
>> + ? ? ? ?} else
>> ? ? ? ? ?if ((isGray(c->srcFormat) || isGray(c->dstFormat)) && plane>0)
>> ? ? ? ? ?{
>> ? ? ? ? ? ? ?if (!isGray(c->dstFormat))
>
> the memset fill code is duplicated relative to the gray case
>
>
patch updated
> [...]
>> Index: ffmpeg/libswscale/swscale_template.c
>> ===================================================================
>> --- ffmpeg.orig/libswscale/swscale_template.c 2009-03-10 16:37:12.000000000 +0100
>> +++ ffmpeg/libswscale/swscale_template.c ? ? ?2009-03-10 16:38:05.000000000 +0100
>> @@ -3218,6 +3218,15 @@
>> ? ? ? ? ?}
>> ? ? ?}
>>
>> + ? ?if (CONFIG_SWSCALE_ALPHA && (dstFormat == PIX_FMT_YUVA420P) && !alpPixBuf){
>> + ? ? ? ?int i;
>> + ? ? ? ?uint8_t *dstPtr= dst[3] + dstStride[3]*lastDstY;
>> + ? ? ? ?for (i=lastDstY; i<dstY; i++){
>> + ? ? ? ? ? ?memset(dstPtr, 255, dstW);
>> + ? ? ? ? ? ?dstPtr+= dstStride[3];
>> + ? ? ? ?}
>> + ? ?}
>> +
>
> another duplicate
duplicated from where ?
BTW, checking CONFIG_SWSCALE_ALPHA is not needed here
>
>
> [...]
>
>> @@ -55,16 +61,14 @@
>> ? ? ?uint8_t *out[4];
>> ? ? ?int srcStride[4], dstStride[4];
>> ? ? ?int i;
>> - ? ?uint64_t ssdY, ssdU, ssdV;
>> + ? ?uint64_t ssdY, ssdU, ssdV, ssdA;
>> + ? ?int needAlpha = (CONFIG_SWSCALE_ALPHA && isALPHA(srcFormat) && isALPHA(dstFormat));
>> ? ? ?struct SwsContext *srcContext, *dstContext, *outContext;
>> ? ? ?int res;
>
> i dont see what sense CONFIG_SWSCALE_ALPHA checks have in the example code.
yes, this is not needed
>
>
>
>>
>> ? ? ?res = 0;
>> ? ? ?for (i=0; i<4; i++){
>> ? ? ? ? ?// avoid stride % bpp != 0
>> - ? ? ? ?if (srcFormat==PIX_FMT_YUVA420P)
>> - ? ? ? ? ? ?srcStride[i]= refStride[i];
>> - ? ? ? ?else
>> ? ? ? ? ?if (srcFormat==PIX_FMT_RGB24 || srcFormat==PIX_FMT_BGR24)
>> ? ? ? ? ? ? ?srcStride[i]= srcW*3;
>> ? ? ? ? ?else
>
> a patch adding code and another removing it again means both are rejected.
full patch attached
#1 : sws_parametrized_yscaleyuv2rgb.patch
#2 : sws_configure_alpha.patch (omitted)
#3 : sws_yuva2rgb.patch
#4 : sws_scale_alpha.patch
#5 : sws_output_yuva420p.patch
#6 : swscale-example_use_alpha.patch
Regards,
C?dric Schieli
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sws_parametrized_yscaleyuv2rgb.patch
Type: text/x-patch
Size: 1761 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090311/c4820a53/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sws_yuva2rgb.patch
Type: text/x-patch
Size: 11689 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090311/c4820a53/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sws_scale_alpha.patch
Type: text/x-patch
Size: 49179 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090311/c4820a53/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sws_output_yuva420p.patch
Type: text/x-patch
Size: 4106 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090311/c4820a53/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: swscale-example_use_alpha.patch
Type: text/x-patch
Size: 4470 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090311/c4820a53/attachment-0004.bin>
More information about the ffmpeg-devel
mailing list