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

Michael Niedermayer michaelni
Sun Mar 15 00:13:32 CET 2009


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

[...]


#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

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

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- 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/20090315/3337929a/attachment.pgp>



More information about the ffmpeg-devel mailing list