[FFmpeg-devel] [PATCH] swscale alpha channel support

Cédric Schieli cschieli
Sun Mar 15 11:06:18 CET 2009


2009/3/15 Michael Niedermayer <michaelni at gmx.at>:
> On Sat, Mar 14, 2009 at 10:00:48AM +0100, C?dric Schieli wrote:
>> 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
>
> #1 of 4
>> Index: ffmpeg/libswscale/swscale.c
>> ===================================================================
>> --- ffmpeg.orig/libswscale/swscale.c ?2009-03-11 21:12:48.835896134 +0100
>> +++ ffmpeg/libswscale/swscale.c ? ? ? 2009-03-14 09:24:49.757051709 +0100
> ...
>
> ok
>
>
> #2 of 4
>> Index: ffmpeg/libswscale/swscale.c
>> ===================================================================
>> --- ffmpeg.orig/libswscale/swscale.c ?2009-03-14 09:24:49.757051709 +0100
>> +++ ffmpeg/libswscale/swscale.c ? ? ? 2009-03-14 09:25:06.726052729 +0100
>> @@ -133,6 +133,7 @@ unsigned swscale_version(void)
>> ? ? ?)
>> ?#define isSupportedOut(x) ? ( ? ? ? \
>> ? ? ? ? ? ? (x)==PIX_FMT_YUV420P ? ? \
>> + ? ? ? ?|| (x)==PIX_FMT_YUVA420P ? ?\
>> ? ? ? ? ?|| (x)==PIX_FMT_YUYV422 ? ? \
>> ? ? ? ? ?|| (x)==PIX_FMT_UYVY422 ? ? \
>> ? ? ? ? ?|| (x)==PIX_FMT_YUV444P ? ? \
>> @@ -1080,6 +1081,15 @@ static inline void yuv2rgbXinC_full(SwsC
>> ? ? ?}
>> ?}
>>
>> +static void fillPlane(uint8_t* plane, int stride, int width, int height, int y, uint8_t val){
>> + ? ?int i;
>> + ? ?uint8_t *ptr = plane + stride*y;
>> + ? ?for (i=0; i<height; i++){
>> + ? ? ? ?memset(ptr, val, width);
>> + ? ? ? ?ptr += stride;
>> + ? ?}
>> +}
>> +
>> ?//Note: we have C, X86, MMX, MMX2, 3DNOW versions, there is no 3DNOW+MMX2 one
>> ?//Plain C versions
> [...]
>> - ? ? ? ?if ((isGray(c->srcFormat) || isGray(c->dstFormat)) && plane>0)
>> - ? ? ? ?{
>> - ? ? ? ? ? ?if (!isGray(c->dstFormat))
>> - ? ? ? ? ? ? ? ?memset(dst[plane], 128, dstStride[plane]*height);
>> - ? ? ? ?}
>> + ? ? ? ?if (dst[plane] && !src[plane])
>> + ? ? ? ? ? ?fillPlane(dst[plane], dstStride[plane], length, height, y, (plane==3) ? 255 : 128);
>> ? ? ? ? ?else
>> ? ? ? ? ?{
>> ? ? ? ? ? ? ?if (dstStride[plane]==srcStride[plane] && srcStride[plane] > 0)
>
> factorizing code out should be a seperate patch

patch attached

>
> [...]
>
>
> #3 of 4
>> Index: ffmpeg/libswscale/swscale-example.c
>> ===================================================================
>> --- ffmpeg.orig/libswscale/swscale-example.c ?2009-03-10 07:58:16.757609825 +0100
>> +++ ffmpeg/libswscale/swscale-example.c ? ? ? 2009-03-14 09:25:09.673025808 +0100
> ...
>
> ok
>
>
> #4 of 4
> [...]
>> Index: ffmpeg/libswscale/yuv2rgb_template.c
>> ===================================================================
>> --- ffmpeg.orig/libswscale/yuv2rgb_template.c 2009-03-14 09:39:28.349025270 +0100
>> +++ ffmpeg/libswscale/yuv2rgb_template.c ? ? ?2009-03-14 09:40:06.818059394 +0100
>> @@ -162,7 +162,8 @@
>> ? ? ? ? ?"add $"AV_STRINGIFY(depth*8)", %1 ? ?\n\t" \
>> ? ? ? ? ?"add ? ? ? ? ? ? ? ? ? ? ? $4, %0 ? ?\n\t" \
>> ? ? ? ? ?" js ? ? ? ? ? ? ? ? ? ? ? 1b ? ? ? ?\n\t" \
>> -\
>> +
>> +#define YUV2RGB_OPERANDS \
>> ? ? ? ? ?: "+r" (index), "+r" (image) \
>> ? ? ? ? ?: "r" (pu - index), "r" (pv - index), "r"(&c->redDither), "r" (py - 2*index) \
>> ? ? ? ? ?); \
> [...]
>> @@ -223,6 +232,7 @@ static inline int RENAME(yuv420_rgb16)(S
>> ? ? ? ? ?MOVNTQ " ? %%mm5, 8 (%1);" /* store pixel 4-7 */
>>
>> ? ? ?YUV2RGB_ENDLOOP(2)
>> + ? ?YUV2RGB_OPERANDS
>> ?}
>>
>> ?static inline int RENAME(yuv420_rgb15)(SwsContext *c, uint8_t* src[], int srcStride[], int srcSliceY,
>> @@ -280,6 +290,7 @@ static inline int RENAME(yuv420_rgb15)(S
>> ? ? ? ? ?MOVNTQ " %%mm5, 8 (%1);" /* store pixel 4-7 */
>>
>> ? ? ?YUV2RGB_ENDLOOP(2)
>> + ? ?YUV2RGB_OPERANDS
>> ?}
>>
>> ?static inline int RENAME(yuv420_rgb24)(SwsContext *c, uint8_t* src[], int srcStride[], int srcSliceY,
>
> factorizing code should be a seperate patch

patch attached

#1 : sws_yuv2rgb_operands.patch
#2 : sws_yuva2rgb.patch
#3 : sws_fillPlane.patch
#4 : sws_output_yuva420p.patch


Regards,
C?dric Schieli
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sws_yuv2rgb_operands.patch
Type: text/x-patch
Size: 1513 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090315/39a089d0/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sws_yuva2rgb.patch
Type: text/x-patch
Size: 11245 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090315/39a089d0/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sws_fillPlane.patch
Type: text/x-patch
Size: 1096 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090315/39a089d0/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sws_output_yuva420p.patch
Type: text/x-patch
Size: 3946 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090315/39a089d0/attachment-0003.bin>



More information about the ffmpeg-devel mailing list