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

Sat Feb 28 09:18:46 CET 2009

2009/2/28 Michael Niedermayer <michaelni at gmx.at>:
> On Fri, Feb 27, 2009 at 11:30:25PM +0100, C?dric Schieli wrote:
>> 2009/2/27 Michael Niedermayer <michaelni at gmx.at>:
>> > On Fri, Feb 27, 2009 at 06:28:38PM +0100, C?dric Schieli wrote:
>> >> Hi all,
[...]
>> >>
>> >> #4 : sws_default_alpha_value.patch
>> >> When converting from a non alpha format to an alpha format, defaults
>> >> to all ones rather than all zeroes
>> >> This patch introduces some (small) differences in swscale-exemple
>> >> output for RGB32_1 and BGR32_1 (see swscale-example.log.diff) that I
>> >> can't reproduce manually
>> >
>> > these look rather harmless, maybe some uninitialized var or missing emms,
>> > i doubt there is a bug in your code
>> > anyway comments below
>> >
>>
>> It can't be missing emms as the same differences occurs when built
>> with --disable-mmx
>
> does valgrind say something?

Didn't look at it for the moment. Will do.

[...]
>> Index: ffmpeg/libavcodec/imgconvert.c
>> ===================================================================
>> --- ffmpeg.orig/libavcodec/imgconvert.c ? ? ? 2009-02-27 23:03:59.311598606 +0100
>> +++ ffmpeg/libavcodec/imgconvert.c ? ?2009-02-27 23:27:11.755632484 +0100
>> @@ -721,7 +721,7 @@
>> ? ? ? ? ? ? ? ? ? ? ? unsigned char *dest, int dest_size)
>> ?{
>> ? ? ?const PixFmtInfo* pf = &pix_fmt_info[pix_fmt];
>> - ? ?int i, j, w, h, data_planes;
>> + ? ?int i, j, w, ow, h, oh, data_planes;
>> ? ? ?const unsigned char* s;
>> ? ? ?int size = avpicture_get_size(pix_fmt, width, height);
>>
>> @@ -751,10 +751,16 @@
>> ? ? ? ? ?h = height;
>> ? ? ?}
>>
>> + ? ?ow = w;
>> + ? ?oh = h;
>> +
>> ? ? ?for (i=0; i<data_planes; i++) {
>> ? ? ? ? ? if (i == 1) {
>> ? ? ? ? ? ? ? w = width >> pf->x_chroma_shift;
>> ? ? ? ? ? ? ? h = height >> pf->y_chroma_shift;
>> + ? ? ? ? } else if (i == 3) {
>> + ? ? ? ? ? ? w = ow;
>> + ? ? ? ? ? ? h = oh;
>> ? ? ? ? ? }
>> ? ? ? ? ? s = src->data[i];
>> ? ? ? ? ? for(j=0; j<h; j++) {
>
> patch ok

Applied

>> Index: ffmpeg/libswscale/swscale_template.c
>> ===================================================================
>> --- ffmpeg.orig/libswscale/swscale_template.c 2009-02-27 23:01:40.355609018 +0100
>> +++ ffmpeg/libswscale/swscale_template.c ? ? ?2009-02-27 23:27:15.219633216 +0100
>> @@ -1031,7 +1031,7 @@
[...]
>> @@ -126,7 +126,7 @@
>> ? ? ? ? ?*dest++ = *s++;
>> ? ? ? ? ?*dest++ = *s++;
>> ? ? ? ? ?*dest++ = *s++;
>> - ? ? ? ?*dest++ = 0;
>> + ? ? ? ?*dest++ = 255;
>> ? ? ?#endif
>> ? ? ?}
>> ?}
>
> hunks from start of patch to here ok

Applied

>
>
>> @@ -1213,6 +1213,7 @@
>> ? ? ?end = s + src_size/2;
>> ?#if HAVE_MMX
>> ? ? ?__asm__ volatile(PREFETCH" ? ?%0"::"m"(*s):"memory");
>> + ? ?__asm__ volatile("movq ?%0,%%mm6"::"m"(mask32a):"memory");
>> ? ? ?__asm__ volatile("pxor ? ?%%mm7,%%mm7 ? ?\n\t":::"memory");
>> ? ? ?mm_end = end - 3;
>> ? ? ?while (s < mm_end)
>> @@ -1245,6 +1246,8 @@
>> ? ? ? ? ?"psllq ? ? ? ?$16, %%mm5 ? ?\n\t"
>> ? ? ? ? ?"por ? ? ? ?%%mm4, %%mm3 ? ?\n\t"
>> ? ? ? ? ?"por ? ? ? ?%%mm5, %%mm3 ? ?\n\t"
>> + ? ? ? ?"por ? ? ? ?%%mm6, %%mm0 ? ?\n\t"
>> + ? ? ? ?"por ? ? ? ?%%mm6, %%mm3 ? ?\n\t"
>> ? ? ? ? ?MOVNTQ" ? ? %%mm0, ?%0 ? ? ?\n\t"
>> ? ? ? ? ?MOVNTQ" ? ? %%mm3, 8%0 ? ? ?\n\t"
>> ? ? ? ? ?:"=m"(*d)
>
> the code can be implemented with
> significantly fewer instructions (as shows in my previous review)
> and at the same time supporting alpha = 0xFF

I will work on this now

>> @@ -1265,7 +1268,7 @@
>> ? ? ? ? ?register uint16_t bgr;
>> ? ? ? ? ?bgr = *s++;
>> ?#ifdef WORDS_BIGENDIAN
>> - ? ? ? ?*d++ = 0;
>> + ? ? ? ?*d++ = 255;
>> ? ? ? ? ?*d++ = (bgr&0x7C00)>>7;
>> ? ? ? ? ?*d++ = (bgr&0x3E0)>>2;
>> ? ? ? ? ?*d++ = (bgr&0x1F)<<3;
>> @@ -1273,7 +1276,7 @@
>> ? ? ? ? ?*d++ = (bgr&0x1F)<<3;
>> ? ? ? ? ?*d++ = (bgr&0x3E0)>>2;
>> ? ? ? ? ?*d++ = (bgr&0x7C00)>>7;
>> - ? ? ? ?*d++ = 0;
>> + ? ? ? ?*d++ = 255;
>> ?#endif
>>
>> ?#endif
>
> 2 hunks ok

Applied

>
>> @@ -1291,6 +1294,7 @@
>> ? ? ?end = s + src_size/2;
>> ?#if HAVE_MMX
>> ? ? ?__asm__ volatile(PREFETCH" ? ?%0"::"m"(*s):"memory");
>> + ? ?__asm__ volatile("movq ?%0,%%mm6"::"m"(mask32a):"memory");
>> ? ? ?__asm__ volatile("pxor ? ?%%mm7,%%mm7 ? ?\n\t":::"memory");
>> ? ? ?mm_end = end - 3;
>> ? ? ?while (s < mm_end)
>> @@ -1323,6 +1327,8 @@
>> ? ? ? ? ?"psllq ? ? ? ?$16, %%mm5 ? ?\n\t"
>> ? ? ? ? ?"por ? ? ? ?%%mm4, %%mm3 ? ?\n\t"
>> ? ? ? ? ?"por ? ? ? ?%%mm5, %%mm3 ? ?\n\t"
>> + ? ? ? ?"por ? ? ? ?%%mm6, %%mm0 ? ?\n\t"
>> + ? ? ? ?"por ? ? ? ?%%mm6, %%mm3 ? ?\n\t"
>> ? ? ? ? ?MOVNTQ" ? ? %%mm0, %0 ? ? ? \n\t"
>> ? ? ? ? ?MOVNTQ" ? ? %%mm3, 8%0 ? ? ?\n\t"
>> ? ? ? ? ?:"=m"(*d)
>
> same issue
>
>
>> @@ -1339,7 +1345,7 @@
>> ? ? ? ? ?register uint16_t bgr;
>> ? ? ? ? ?bgr = *s++;
>> ?#ifdef WORDS_BIGENDIAN
>> - ? ? ? ?*d++ = 0;
>> + ? ? ? ?*d++ = 255;
>> ? ? ? ? ?*d++ = (bgr&0xF800)>>8;
>> ? ? ? ? ?*d++ = (bgr&0x7E0)>>3;
>> ? ? ? ? ?*d++ = (bgr&0x1F)<<3;
>> @@ -1347,7 +1353,7 @@
>> ? ? ? ? ?*d++ = (bgr&0x1F)<<3;
>> ? ? ? ? ?*d++ = (bgr&0x7E0)>>3;
>> ? ? ? ? ?*d++ = (bgr&0xF800)>>8;
>> - ? ? ? ?*d++ = 0;
>> + ? ? ? ?*d++ = 255;
>> ?#endif
>> ? ? ?}
>> ?}
>> Index: ffmpeg/libswscale/yuv2rgb_template.c
>> ===================================================================
>> --- ffmpeg.orig/libswscale/yuv2rgb_template.c 2009-02-27 23:01:40.379598031 +0100
>> +++ ffmpeg/libswscale/yuv2rgb_template.c ? ? ?2009-02-27 23:27:15.227630601 +0100
>> @@ -446,7 +446,7 @@
>>
>> ? ? ? ? ?YUV2RGB_INIT
>> ? ? ? ? ?YUV2RGB
>> - ? ? ? ?"pxor ? ? ?%%mm3, %%mm3;" ? /* zero mm3 */
>> + ? ? ? ?"pcmpeqd ? %%mm3, %%mm3;" ? /* fill mm3 */
>> ? ? ? ? ?RGB_PLANAR2PACKED32
>>
>> ? ? ?YUV2RGB_ENDLOOP(4)
>> Index: ffmpeg/libswscale/yuv2rgb.c
>> ===================================================================
>> --- ffmpeg.orig/libswscale/yuv2rgb.c ?2009-02-27 23:01:40.387598140 +0100
>> +++ ffmpeg/libswscale/yuv2rgb.c ? ? ? 2009-02-27 23:27:15.227630601 +0100
>> @@ -533,7 +533,7 @@
>> ? ? ?uint8_t *y_table;
>> ? ? ?uint16_t *y_table16;
>> ? ? ?uint32_t *y_table32;
>> - ? ?int i, base, rbase, gbase, bbase;
>> + ? ?int i, base, rbase, gbase, bbase, abase;
>> ? ? ?const int yoffs = fullRange ? 384 : 326;
>>
>> ? ? ?int64_t crv = ?inv_table[0];
>> @@ -659,12 +659,13 @@
>> ? ? ? ? ?rbase = base + (isRgb ? 16 : 0);
>> ? ? ? ? ?gbase = base + 8;
>> ? ? ? ? ?bbase = base + (isRgb ? 0 : 16);
>> + ? ? ? ?abase = (c->dstFormat == PIX_FMT_RGBA || c->dstFormat == PIX_FMT_BGRA) ? 24 : 0;
>> ? ? ? ? ?c->yuvTable = av_malloc(1024*3*4);
>> ? ? ? ? ?y_table32 = c->yuvTable;
>> ? ? ? ? ?yb = -(384<<16) - oy;
>> ? ? ? ? ?for (i = 0; i < 1024; i++) {
>> ? ? ? ? ? ? ?uint8_t yval = av_clip_uint8((yb + 0x8000) >> 16);
>> - ? ? ? ? ? ?y_table32[i ? ? ] = yval << rbase;
>> + ? ? ? ? ? ?y_table32[i ? ? ] = (yval << rbase) + (255 << abase);
>> ? ? ? ? ? ? ?y_table32[i+1024] = yval << gbase;
>> ? ? ? ? ? ? ?y_table32[i+2048] = yval << bbase;
>> ? ? ? ? ? ? ?yb += cy;
>
> 5 hunks ok

Applied

> [...]
>
>> Index: ffmpeg/libswscale/yuv2rgb.c
>> ===================================================================
>> --- ffmpeg.orig/libswscale/yuv2rgb.c ?2009-02-27 23:27:15.227630601 +0100
>> +++ ffmpeg/libswscale/yuv2rgb.c ? ? ? 2009-02-27 23:27:23.171601482 +0100
>> @@ -101,12 +101,18 @@
[...]
>> @@ -450,4 +462,23 @@
>> ? ? ? ? ?RGB_PLANAR2PACKED32
>>
>> ? ? ?YUV2RGB_ENDLOOP(4)
>> + ? ?YUV2RGB_OPERANDS
>> +}
>> +
>> +#if CONFIG_SWSCALE_ALPHA
>> +static inline int RENAME(yuva420_rgb32)(SwsContext *c, uint8_t* src[], int srcStride[], int srcSliceY,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int srcSliceH, uint8_t* dst[], int dstStride[]){
>> + ? ?int y, h_size;
>> +
>> + ? ?YUV2RGB_LOOP(4)
>> +
>> + ? ? ? ?uint8_t *pa = src[3] + y*srcStride[3];
>> + ? ? ? ?YUV2RGB_INIT
>> + ? ? ? ?YUV2RGB
>> + ? ? ? ?"movq ? ? (%6, %0, 2), %%mm3;" ? ? ? ? ? ?/* Load 8 A A7 A6 A5 A4 A3 A2 A1 A0 */
>> + ? ? ? ?RGB_PLANAR2PACKED32
>> +
>> + ? ?YUV2RGB_ENDLOOP(4)
>> + ? ?YUV2RGB_OPERANDS_ALPHA
>> ?}
>> +#endif
>
> patch ok

Will break build without the configure hunk so I'll wait for this one

> will review las patch later

Regards,
C?dric Schieli