[Webkit-unassigned] [Bug 116042] [GStreamer] [texmap] upload a video buffer into the video texture

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=116042





--- Comment #22 from Martin Robinson <mrobinson at webkit.org>  2013-06-07 14:24:51 PST ---
(From update of attachment 204061)
View in context: https://bugs.webkit.org/attachment.cgi?id=204061&action=review

Is the purpose of this patch to more quickly prepare the texture in the case that the video is already in GPU memory?

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:356
> +    if (meta) {
> +        const BitmapTextureGL* textureGL = static_cast<const BitmapTextureGL*>(m_texture.get());
> +        guint ids[4] = { textureGL->id(), 0, 0, 0 };
> +
> +        ret = gst_video_gl_texture_upload_meta_upload(meta, ids);

What's happening behind the scenes here? Is it just using the current OpenGL context? If not I wonder about the cost of context switching.

In WebKit we do early returns, so you should write:

if (GstVideoGLTextureUploadMeta* meta = gst_buffer_get_video_gl_texture_upload_meta(buffer)) {
    if (gst_video_gl_texture_upload_meta_upload(meta, ids)) {
        client()->setPlatformLayerNeedsDisplay();
        return;
    }
}

The nice benefit being you don't have to touch the code below it.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:362
> +    { // copy the buffer into the texture

Please use full sentences with periods and capital letters in comments.

--- Comment #23 from Martin Robinson <mrobinson at webkit.org>  2013-06-07 14:24:53 PST ---
(From update of attachment 204061)
View in context: https://bugs.webkit.org/attachment.cgi?id=204061&action=review

Is the purpose of this patch to more quickly prepare the texture in the case that the video is already in GPU memory?

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:356
> +    if (meta) {
> +        const BitmapTextureGL* textureGL = static_cast<const BitmapTextureGL*>(m_texture.get());
> +        guint ids[4] = { textureGL->id(), 0, 0, 0 };
> +
> +        ret = gst_video_gl_texture_upload_meta_upload(meta, ids);

What's happening behind the scenes here? Is it just using the current OpenGL context? If not I wonder about the cost of context switching.

In WebKit we do early returns, so you should write:

if (GstVideoGLTextureUploadMeta* meta = gst_buffer_get_video_gl_texture_upload_meta(buffer)) {
    if (gst_video_gl_texture_upload_meta_upload(meta, ids)) {
        client()->setPlatformLayerNeedsDisplay();
        return;
    }
}

The nice benefit being you don't have to touch the code below it.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:362
> +    { // copy the buffer into the texture

Please use full sentences with periods and capital letters in comments.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.