Bug 200914

Summary: [GStreamer] Add support to copy YUV video textures into platform textures
Product: WebKit Reporter: Chris Lord <clord>
Component: WPE WebKitAssignee: Chris Lord <clord>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, calvaris, commit-queue, ggaren, magomez, sabouhallawa, simon.fraser, ysuzuki, zan
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=201184
https://bugs.webkit.org/show_bug.cgi?id=201422
Bug Depends on: 132869    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Chris Lord
Reported 2019-08-20 01:24:01 PDT
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).
Attachments
Patch (18.06 KB, patch)
2019-08-20 01:42 PDT, Chris Lord 		no flags
DetailsFormatted DiffDiff
Patch (19.59 KB, patch)
2019-08-21 03:17 PDT, Chris Lord 		no flags
DetailsFormatted DiffDiff
Patch (19.17 KB, patch)
2019-08-27 03:49 PDT, Chris Lord 		no flags
DetailsFormatted DiffDiff
Patch (19.21 KB, patch)
2019-09-03 03:36 PDT, Chris Lord 		no flags
DetailsFormatted DiffDiff
Patch (19.28 KB, patch)
2019-09-03 05:35 PDT, Chris Lord 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Chris Lord
Comment 1 2019-08-20 01:42:50 PDT
Created attachment 376753 [details] Patch
Chris Lord
Comment 2 2019-08-21 03:17:45 PDT
Created attachment 376858 [details] Patch
Xabier Rodríguez Calvar
Comment 3 2019-08-26 07:42:51 PDT
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
Comment 4 2019-08-27 03:49:52 PDT
Created attachment 377334 [details] Patch
Xabier Rodríguez Calvar
Comment 5 2019-08-27 08:15:44 PDT
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
Comment 6 2019-09-02 06:56:19 PDT
(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
Comment 7 2019-09-03 03:36:01 PDT
Created attachment 377883 [details] Patch
Chris Lord
Comment 8 2019-09-03 05:35:41 PDT
Created attachment 377891 [details] Patch
WebKit Commit Bot
Comment 9 2019-09-03 07:15:04 PDT
Comment on attachment 377891 [details] Patch Clearing flags on attachment: 377891 Committed r249428: <https://trac.webkit.org/changeset/249428>
WebKit Commit Bot
Comment 10 2019-09-03 07:15:06 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.