Bug 200914 – [GStreamer] Add support to copy YUV video textures into platform textures

• Status: RESOLVED
• Resolution: FIXED
• Component: WPE WebKit

[Chris Lord]

Bug 132869 adds shaders to render YUV video textures to avoid having to render to an extra off-screen surface. The patch on that bug only implements the playback portion and YUV videos become broken/non-working on accelerated and unaccelerated canvases. This bug tracks the work to enable YUV video textures in accelerated canvases (so WebGL and accelerated 2D canvas).

[Chris Lord]

Created attachment 376753 [details]
Patch

[Chris Lord]

Created attachment 376858 [details]
Patch

[Xabier Rodríguez Calvar]

Comment on attachment 376858 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=376858&action=review

Besides my review, I guess this should be checked by Miguel or Zan as well.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:219
> +    std::unique_ptr<TextureMapperPlatformLayerBuffer> getPlatformLayerBuffer(bool treatAsRGB)

platformLayerBuffer and shouldBeTreatedAsRGB

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:229
> +                Buffer::TextureVariant { Buffer::RGBTexture { *static_cast<GLuint*>(m_videoFrame.data[0]) } },
> +                m_size, m_flags, GraphicsContext3D::RGBA);

Nit: this can be just one line.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:236
> +            int nPlanes = GST_VIDEO_INFO_N_PLANES(&m_videoFrame.info);

numberOfPlanes

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:239
> +            std::array<int, 3> yuvPOffset;

yuvPlaneOffset?

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:255
> +                // Default to bt601. This is the same behaviour as GStreamer's glcolorconvert element

Period at the end.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:266
> +            return makeUnique<Buffer>(
> +                Buffer::TextureVariant {
> +                    Buffer::YUVTexture { nPlanes, planes, yuvPlane, yuvPOffset, yuvToRgb } },
> +                m_size, m_flags, GraphicsContext3D::RGBA);

Line lengths need to be reworked here. I think all could fit it one.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:998
> +    frameHolder->SyncWaitCPU();

Where is this defined? besides the name should be a bit more explanatory (I guess adding more words will help)

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:1030
> +    frameHolder->SyncWaitCPU();

ditto

> Source/WebCore/platform/graphics/gstreamer/VideoTextureCopierGStreamer.cpp:164
> +    WTF::switchOn(inputTexture.getTextureVariant(),
> +        [&](const Buffer::RGBTexture&)
> +        {

These three lines could be collapsed into one. Of course we'd need to reindent the function.

> Source/WebCore/platform/graphics/gstreamer/VideoTextureCopierGStreamer.cpp:167
> +        },
> +        [&](const Buffer::YUVTexture& texture)

Collapse.

> Source/WebCore/platform/graphics/gstreamer/VideoTextureCopierGStreamer.cpp:226
> +        [&](const Buffer::RGBTexture& texture)
> +        {

lines

> Source/WebCore/platform/graphics/gstreamer/VideoTextureCopierGStreamer.cpp:236
> +        [&](const Buffer::YUVTexture& texture)
> +        {

lines

> Source/WebCore/platform/graphics/texmap/TextureMapperPlatformLayerBuffer.h:94
> +    const TextureVariant& getTextureVariant() { return m_variant; }

textureVariant

[Chris Lord]

Created attachment 377334 [details]
Patch

[Xabier Rodríguez Calvar]

Comment on attachment 377334 [details]
Patch

Beautiful! Let's see what Miguel or Zan have to say about this cause they know more of the subject.

[Miguel Gomez]

(In reply to Xabier Rodríguez Calvar from comment #5)
> Comment on attachment 377334 [details]
> Patch
> 
> Beautiful! Let's see what Miguel or Zan have to say about this cause they
> know more of the subject.

LGTM. Awesome work Chris!

[Chris Lord]

Created attachment 377883 [details]
Patch

[Chris Lord]

Created attachment 377891 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 377891 [details]
Patch

Clearing flags on attachment: 377891

Committed r249428: <https://trac.webkit.org/changeset/249428>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.