[FFmpeg-devel] [PATCH] swscale alpha channel support
Cédric Schieli
cschieli
Sat Mar 14 10:00:48 CET 2009
2009/3/12 Michael Niedermayer <michaelni at gmx.at>:
> On Wed, Mar 11, 2009 at 03:44:52PM +0100, C?dric Schieli wrote:
>> 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:
> [...]
>> >
>> >
>> > [...]
>> >> @@ -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];
>> >> + ? ? ? ? ? ? ? ?}
>> >> + ? ? ? ? ? ?}
> [...]
>> >> + ? ?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
>
> above
ah, ok for a separate function
patch updated
>
>
> [...]
>>
>>
>> #1 : sws_parametrized_yscaleyuv2rgb.patch
>
> ok if striped obj are the same
applied
>
>
>> #3 : sws_yuva2rgb.patch
>
> see below
>
>
>> #4 : sws_scale_alpha.patch
>
> see below
>
>
>> #5 : sws_output_yuva420p.patch
>
> see below
>
>> #6 : swscale-example_use_alpha.patch
>
> see below
>
> [...]
>
> #3
> [...]
>> @@ -432,7 +508,16 @@
>> ?#if (HAVE_MMX2 || HAVE_MMX) && CONFIG_GPL
>> ? ? ?if (c->flags & SWS_CPU_CAPS_MMX2) {
>> ? ? ? ? ?switch (c->dstFormat) {
>> +#if CONFIG_SWSCALE_ALPHA
>> + ? ? ? ?case PIX_FMT_RGB32:
>> +#if HAVE_7REGS
>> + ? ? ? ? ? ?return (c->srcFormat == PIX_FMT_YUVA420P) ? yuva420_rgb32_MMX2 : yuv420_rgb32_MMX2;
>> +#else
>> + ? ? ? ? ? ?if (c->srcFormat != PIX_FMT_YUVA420P) return yuv420_rgb32_MMX2;
>> +#endif /* HAVE_7REGS */
>> +#else
>> ? ? ? ? ?case PIX_FMT_RGB32: ?return yuv420_rgb32_MMX2;
>> +#endif /* CONFIG_SWSCALE_ALPHA */
>> ? ? ? ? ?case PIX_FMT_BGR24: ?return yuv420_rgb24_MMX2;
>> ? ? ? ? ?case PIX_FMT_RGB565: return yuv420_rgb16_MMX2;
>> ? ? ? ? ?case PIX_FMT_RGB555: return yuv420_rgb15_MMX2;
>> @@ -440,7 +525,16 @@
>> ? ? ?}
>> ? ? ?if (c->flags & SWS_CPU_CAPS_MMX) {
>> ? ? ? ? ?switch (c->dstFormat) {
>> +#if CONFIG_SWSCALE_ALPHA
>> + ? ? ? ?case PIX_FMT_RGB32:
>> +#if HAVE_7REGS
>> + ? ? ? ? ? ?return (c->srcFormat == PIX_FMT_YUVA420P) ? yuva420_rgb32_MMX : yuv420_rgb32_MMX;
>> +#else
>> + ? ? ? ? ? ?if (c->srcFormat != PIX_FMT_YUVA420P) return yuv420_rgb32_MMX;
>> +#endif /* HAVE_7REGS */
>> +#else
>> ? ? ? ? ?case PIX_FMT_RGB32: ?return yuv420_rgb32_MMX;
>> +#endif /* CONFIG_SWSCALE_ALPHA */
>> ? ? ? ? ?case PIX_FMT_BGR24: ?return yuv420_rgb24_MMX;
>> ? ? ? ? ?case PIX_FMT_RGB565: return yuv420_rgb16_MMX;
>> ? ? ? ? ?case PIX_FMT_RGB555: return yuv420_rgb15_MMX;
>
> these look more complex than needed
patch updated with no more #if
>
>
>> @@ -468,6 +562,16 @@
>>
>> ? ? ?av_log(c, AV_LOG_WARNING, "No accelerated colorspace conversion found.\n");
>>
>> +#if CONFIG_SWSCALE_ALPHA
>> + ? ?if (c->srcFormat == PIX_FMT_YUVA420P)
>> + ? ? ? ?switch(c->dstFormat){
>> + ? ? ? ?case PIX_FMT_ARGB:
>> + ? ? ? ?case PIX_FMT_ABGR: return yuva2argb_c;
>> + ? ? ? ?case PIX_FMT_RGBA:
>> + ? ? ? ?case PIX_FMT_BGRA: return yuva2rgba_c;
>> + ? ? ? ?}
>> +#endif
>> +
>> ? ? ?switch (c->dstFormat) {
>> ? ? ?case PIX_FMT_BGR32_1:
>> ? ? ?case PIX_FMT_RGB32_1:
>
> same
> could you btw show us the error messages from the compilation failure?
nothing too worry about anymore (without the #if) ;-)
/home/cedric/work/ffmpeg/ffmpeg/libswscale/yuv2rgb.c: In function
'sws_yuv2rgb_get_func_ptr':
/home/cedric/work/ffmpeg/ffmpeg/libswscale/yuv2rgb.c:513: error:
'yuva420_rgb32_MMX2' undeclared (first use in this function)
/home/cedric/work/ffmpeg/ffmpeg/libswscale/yuv2rgb.c:513: error: (Each
undeclared identifier is reported only once
/home/cedric/work/ffmpeg/ffmpeg/libswscale/yuv2rgb.c:513: error: for
each function it appears in.)
/home/cedric/work/ffmpeg/ffmpeg/libswscale/yuv2rgb.c:526: error:
'yuva420_rgb32_MMX' undeclared (first use in this function)
>
> [...]
>
>
>> @@ -644,6 +644,14 @@
>>
>> ?#define YSCALEYUV2RGB1b(index, c) ?REAL_YSCALEYUV2RGB1b(index, c)
>>
>> +#define REAL_YSCALEYUV2RGB1_ALPHA(index) \
>> + ? ?"movq ?(%1, "#index", 2), %%mm7 ? ? \n\t" /* abuf0[eax] */\
>> + ? ?"movq 8(%1, "#index", 2), %%mm1 ? ? \n\t" /* abuf0[eax] */\
>> + ? ?"psraw ? ? ? ? ? ? ? ?$7, %%mm7 ? ? \n\t" /* abuf0[eax] >>4*/\
>> + ? ?"psraw ? ? ? ? ? ? ? ?$7, %%mm1 ? ? \n\t" /* abuf0[eax] >>4*/\
>> + ? ?"packuswb ? ? ? ? ?%%mm1, %%mm7 ? ? \n\t"
>> +#define YSCALEYUV2RGB1_ALPHA(index) REAL_YSCALEYUV2RGB1_ALPHA(index)
>> +
>
> the comments are not matching the code
patch updated
>
>
> [...]
>> @@ -1642,6 +1769,13 @@
>> ?BGR2Y(uint16_t, rgb16ToY, 0, 0, 0, 0xF800, 0x07E0, 0x001F, RY ? ?, GY<<5, BY<<11, RGB2YUV_SHIFT+8)
>> ?BGR2Y(uint16_t, rgb15ToY, 0, 0, 0, 0x7C00, 0x03E0, 0x001F, RY ? ?, GY<<5, BY<<10, RGB2YUV_SHIFT+7)
>>
>> +static inline void RENAME(bgr32ToA)(uint8_t *dst, uint8_t *src, long width, uint32_t *unused){
>
> the name should be argb/ or abgrToA
patch updated
>
>
> [...]
>> @@ -2053,16 +2054,21 @@
>> ? ? ? ? ? ? ? ? ? ? ? ?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 ((isGray(c->srcFormat) || isGray(c->dstFormat)) && plane>0)
>> - ? ? ? ?{
>> - ? ? ? ? ? ?if (!isGray(c->dstFormat))
>> - ? ? ? ? ? ? ? ?memset(dst[plane], 128, dstStride[plane]*height);
>> + ? ? ? ?if (((isGray(c->srcFormat) || isGray(c->dstFormat)) && plane>0) || (!(isALPHA(c->srcFormat) && isALPHA(c->dstFormat)) && plane==3)){
>> + ? ? ? ? ? ?if (!isGray(c->dstFormat) || isALPHA(c->dstFormat)){
>
> what about something like if(dst && !src) ?
yes, if dst[1..3] and src[1..3] are guaranteed to be NULL when not used
patch updated
>
>
> [...]
>> Index: ffmpeg/libswscale/swscale-example.c
>> ===================================================================
>> --- ffmpeg.orig/libswscale/swscale-example.c ?2009-03-11 12:22:50.000000000 +0100
>> +++ ffmpeg/libswscale/swscale-example.c ? ? ? 2009-03-11 14:53:58.000000000 +0100
>> @@ -48,19 +48,20 @@
>>
>> ?// test by ref -> src -> dst -> out & compare out against ref
>> ?// ref & out are YV12
>> -static int doTest(uint8_t *ref[3], int refStride[3], int w, int h, int srcFormat, int dstFormat,
>> +static int doTest(uint8_t *ref[4], int refStride[4], int w, int h, int srcFormat, int dstFormat,
>> ? ? ? ? ? ? ? ? ? ?int srcW, int srcH, int dstW, int dstH, int flags){
>> - ? ?uint8_t *src[3];
>> - ? ?uint8_t *dst[3];
>> - ? ?uint8_t *out[3];
>> - ? ?int srcStride[3], dstStride[3];
>> + ? ?uint8_t *src[4];
>> + ? ?uint8_t *dst[4];
>> + ? ?uint8_t *out[4];
>> + ? ?int srcStride[4], dstStride[4];
>> ? ? ?int i;
>> - ? ?uint64_t ssdY, ssdU, ssdV;
>> + ? ?uint64_t ssdY, ssdU, ssdV, ssdA;
>
>> + ? ?int needAlpha = (isALPHA(srcFormat) && isALPHA(dstFormat));
>
> useless ()
>
>
> [...]
>> @@ -121,6 +122,10 @@
>> ? ? ?ssdY= getSSD(ref[0], out[0], refStride[0], refStride[0], w, h);
>> ? ? ?ssdU= getSSD(ref[1], out[1], refStride[1], refStride[1], (w+1)>>1, (h+1)>>1);
>> ? ? ?ssdV= getSSD(ref[2], out[2], refStride[2], refStride[2], (w+1)>>1, (h+1)>>1);
>> + ? ?if (needAlpha){
>> + ? ? ? ?ssdA= getSSD(ref[3], out[3], refStride[3], refStride[3], w, h);
>> + ? ? ? ?ssdA/= w*h;
>> + ? ?}
>
> why is the /wh under the if?
>
>
>>
>> ? ? ?if (srcFormat == PIX_FMT_GRAY8 || dstFormat==PIX_FMT_GRAY8) ssdU=ssdV=0; //FIXME check that output is really gray
>>
>
>> @@ -128,10 +133,13 @@
>> ? ? ?ssdU/= w*h/4;
>> ? ? ?ssdV/= w*h/4;
>>
>> - ? ?printf(" %s %dx%d -> %s %4dx%4d flags=%2d SSD=%5lld,%5lld,%5lld\n",
>> + ? ?printf(" %s %dx%d -> %s %4dx%4d flags=%2d SSD=%5lld,%5lld,%5lld",
>> ? ? ? ? ? ? sws_format_name(srcFormat), srcW, srcH,
>> ? ? ? ? ? ? sws_format_name(dstFormat), dstW, dstH,
>> ? ? ? ? ? ? flags, ssdY, ssdU, ssdV);
>> + ? ?if (needAlpha)
>> + ? ? ? ?printf(",%5lld", ssdA);
>> + ? ?printf("\n");
>
> always print it and init it to 0
ok, I wanted to shorten the diff in the results to easily check
everything was correct
patch updated
Regards,
C?dric Schieli
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sws_scale_alpha.patch
Type: text/x-patch
Size: 51832 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090314/02e3f189/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sws_output_yuva420p.patch
Type: text/x-patch
Size: 4413 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090314/02e3f189/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: swscale-example_use_alpha.patch
Type: text/x-patch
Size: 4681 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090314/02e3f189/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sws_yuva2rgb.patch
Type: text/x-patch
Size: 12416 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090314/02e3f189/attachment-0003.bin>
More information about the ffmpeg-devel
mailing list