[webkit-reviews] review denied: [Bug 117118] [CoordGraphics][GStreamer] Composited Video support : [Attachment 203544] patchv3

[bugzilla-daemon at webkit.org]

Noam Rosenthal <noam at webkit.org> has denied Kalyan
<kalyan.kondapally at intel.com>'s request for review:
Bug 117118: [CoordGraphics][GStreamer] Composited Video support
https://bugs.webkit.org/show_bug.cgi?id=117118

Attachment 203544: patchv3
https://bugs.webkit.org/attachment.cgi?id=203544&action=review

------- Additional Comments from Noam Rosenthal <noam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=203544&action=review


Looks great, though I would ask to find a way to not introduce a new
duplication of RGBA->BGRA swizzle.

>
Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
388
> +    for (int i = 0; i < height / 2; ++i) {
> +	   for (int j = 0; j < width; ++j) {
> +	       unsigned tmp = buf[i * width + j];
> +	       buf[i * width + j] = buf[(height - i - 1) * width + j];
> +	       buf[(height - i - 1) * width + j] = tmp;
> +	   }
> +    }

Is this the 10th RGBA->BGRA swizzle code path we have in WebCore? Maybe we can
do some reusing?

>
Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
661
> +    return m_context3D ? true : false;

The phrase ? true : false doesn't add anything.

>
Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cp
p:426
> +    m_canvasPlatformLayer->setClient(this);

Maybe this member should be renamed?