[Xvid-devel] RGBA output images have their alpha values set to 0 (fully transparent)

[Michael Militzer]

Hi,

thanks for your suggestion and you may be right that 255 would be the  
better default alpha value. However, I'm hesitant to apply this change.

One reason is that changing the c code (like in your patch) is not  
enough. But there are SIMD assembler versions of the RGB conversion  
functions as well, which also all would have to be changed to be  
consistent.

And then second, this patch changes the behavior of the xvidcore  
library towards applications. So there may be existing applications  
that rely on xvid_decore to return RGBA images with alpha value 0. So  
this would be a potentially breaking change and could cause certain  
existing programs to not work correctly anymore when linking against a  
new xvidcore library that has this patch applied.

Best regards,
Michael



Quoting delthas <delthas at dille.cc>:

> Hey,
>
> I noticed that when decoding a frame to the XVID_CSP_RGBA color space,
> the alpha values of all pixels are set to 0. In the RGBA color space an
> alpha value of 0 means fully transparent and 255 means fully opaque, so
> the decoder effectively returns an image that is fully transparent: the
> RGB values are correct but all the pixels are fully transparent.
>
> Setting the alpha to 0 is done in src/image/colorspace.c, in the
> WRITE_RGB macro (at line 389 in the current revision, 2176).
>
> The code is as follows:
> =====
> #define WRITE_RGB(SIZE,ROW,UV_ROW,C1,C2,C3,C4)	\
> 	rgb_y = RGB_Y_tab[ y_ptr[(ROW)*y_stride + 0] ];						\
> 	x_ptr[(ROW)*x_stride+(C3)] = MAX(0, MIN(255, (rgb_y + b_u##UV_ROW) >>
> SCALEBITS_OUT));	\
> 	x_ptr[(ROW)*x_stride+(C2)] = MAX(0, MIN(255, (rgb_y - g_uv##UV_ROW) >>
> SCALEBITS_OUT));	\
> 	x_ptr[(ROW)*x_stride+(C1)] = MAX(0, MIN(255, (rgb_y + r_v##UV_ROW) >>
> SCALEBITS_OUT));	\
> 	if ((SIZE)>3) x_ptr[(ROW)*x_stride+(C4)] = 0;									\
> 	rgb_y = RGB_Y_tab[ y_ptr[(ROW)*y_stride + 1] ];									\
> 	x_ptr[(ROW)*x_stride+(SIZE)+(C3)] = MAX(0, MIN(255, (rgb_y +
> b_u##UV_ROW) >> SCALEBITS_OUT));	\
> 	x_ptr[(ROW)*x_stride+(SIZE)+(C2)] = MAX(0, MIN(255, (rgb_y -
> g_uv##UV_ROW) >> SCALEBITS_OUT));	\
> 	x_ptr[(ROW)*x_stride+(SIZE)+(C1)] = MAX(0, MIN(255, (rgb_y +
> r_v##UV_ROW) >> SCALEBITS_OUT));	\
> 	if ((SIZE)>3) x_ptr[(ROW)*x_stride+(SIZE)+(C4)] = 0;
> =====
>
> The lines responsible for setting the value to 0 are:
> if ((SIZE)>3) x_ptr[(ROW)*x_stride+(C4)] = 0;
> and
> if ((SIZE)>3) x_ptr[(ROW)*x_stride+(SIZE)+(C4)] = 0;
>
> The values 0 could be replaced by 255 to output fully opaque data rather
> than fully transparent data.
>
> It seems to me that outputting fully transparent data is a bug and that
> the alpha values should be set to 255 instead. Is it the case, or is
> this intended?
>
> A simple diff is provided below in case this is actually a bug.
>
> Thanks,
> delthas
>
> Index: xvidcore/src/image/colorspace.c
> ===================================================================
> --- xvidcore/src/image/colorspace.c	(revision 2176)
> +++ xvidcore/src/image/colorspace.c	(working copy)
> @@ -391,12 +391,12 @@
>   	x_ptr[(ROW)*x_stride+(C3)] = MAX(0, MIN(255, (rgb_y + b_u##UV_ROW) >>
> SCALEBITS_OUT));	\
>   	x_ptr[(ROW)*x_stride+(C2)] = MAX(0, MIN(255, (rgb_y - g_uv##UV_ROW) >>
> SCALEBITS_OUT));	\
>   	x_ptr[(ROW)*x_stride+(C1)] = MAX(0, MIN(255, (rgb_y + r_v##UV_ROW) >>
> SCALEBITS_OUT));	\
> -	if ((SIZE)>3) x_ptr[(ROW)*x_stride+(C4)] = 0;									\
> +	if ((SIZE)>3) x_ptr[(ROW)*x_stride+(C4)] = 255;									\
>   	rgb_y = RGB_Y_tab[ y_ptr[(ROW)*y_stride + 1] ];									\
>   	x_ptr[(ROW)*x_stride+(SIZE)+(C3)] = MAX(0, MIN(255, (rgb_y +
> b_u##UV_ROW) >> SCALEBITS_OUT));	\
>   	x_ptr[(ROW)*x_stride+(SIZE)+(C2)] = MAX(0, MIN(255, (rgb_y -
> g_uv##UV_ROW) >> SCALEBITS_OUT));	\
>   	x_ptr[(ROW)*x_stride+(SIZE)+(C1)] = MAX(0, MIN(255, (rgb_y +
> r_v##UV_ROW) >> SCALEBITS_OUT));	\
> -	if ((SIZE)>3) x_ptr[(ROW)*x_stride+(SIZE)+(C4)] = 0;
> +	if ((SIZE)>3) x_ptr[(ROW)*x_stride+(SIZE)+(C4)] = 255;
> _______________________________________________
> Xvid-devel mailing list
> Xvid-devel at xvid.org
> http://list.xvid.org/mailman/listinfo/xvid-devel
>
>