[webkit-reviews] review denied: [Bug 30308] [GStreamer] Add direct support for ARGB videos : [Attachment 41048] 0002-Add-support-for-ARGB-videos.patch

[bugzilla-daemon at webkit.org]

Gustavo Noronha (kov) <gns at gnome.org> has denied Sebastian Dröge
<slomo at circular-chaos.org>'s request for review:
Bug 30308: [GStreamer] Add direct support for ARGB videos
https://bugs.webkit.org/show_bug.cgi?id=30308

Attachment 41048: 0002-Add-support-for-ARGB-videos.patch
https://bugs.webkit.org/attachment.cgi?id=41048&action=review

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
> +	   m_buffer = gst_buffer_make_writable (m_buffer);

Space.

> +	   int x, y;

You should declare these inside the for loops initialization.

> +	   unsigned short a;

Can we make this alpha, for better readability? =)

> +#if G_BYTE_ORDER == G_LITTLE_ENDIAN
> +		   a = data[3];
> +		   data[0] = (a > 0) ? MIN ((data[0] * 255 + a / 2) / a, 255) :
0;
> +		   data[1] = (a > 0) ? MIN ((data[1] * 255 + a / 2) / a, 255) :
0;
> +		   data[2] = (a > 0) ? MIN ((data[2] * 255 + a / 2) / a, 255) :
0;
> +		   data += 4;
> +#else
> +		   a = data[0];
> +		   data[1] = (a > 0) ? MIN ((data[1] * 255 + a / 2) / a, 255) :
0;
> +		   data[2] = (a > 0) ? MIN ((data[2] * 255 + a / 2) / a, 255) :
0;
> +		   data[3] = (a > 0) ? MIN ((data[3] * 255 + a / 2) / a, 255) :
0;
> +		   data += 4;
> +#endif

I was asking krit about this. He pointed me to code in
WebCore/platform/graphics/Color.cpp that does this and is apparently faster. We
are wondering if that code could be reused?

unsigned premultipliedARGBFromColor(const Color& color)
{
    unsigned pixelColor;

    if (unsigned alpha = color.alpha()) {
	pixelColor = alpha << 24 |
	     ((color.red() * alpha  + 254) / 255) << 16 | 
	     ((color.green() * alpha  + 254) / 255) << 8 | 
	     ((color.blue() * alpha  + 254) / 255);
    } else
	 pixelColor = color.rgb();

    return pixelColor;
}