Using the WPEBackend-FDO video-plane-display-dmabuf protocol, to be shipped in 1.6.
Created attachment 387790 [details] Patch
Comment on attachment 387790 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387790&action=review > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:259 > + wpe_video_plane_display_dmabuf_source_update(videoPlaneDisplayDmaBufSource, m_dmabufFD, rect.x(), rect.y(), m_size.width(), m_size.height(), m_dmabufStride, [](void* data) { > + gst_buffer_unref(GST_BUFFER_CAST(data)); > + }, gst_buffer_ref(m_buffer.get())); Is this lambda a C callback? If not, then it would be nice to capture the smart pointer (by copy!) inside the lambda. > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:487 > +#if USE(WPE_VIDEO_PLANE_DISPLAY_DMABUF) > + if (m_wpeVideoPlaneDisplayDmaBuf) > + wpe_video_plane_display_dmabuf_source_destroy(m_wpeVideoPlaneDisplayDmaBuf); > +#endif > + I think we can use GUniquePtr by defining this as deleter in GUniquePtrGStreamer.h, can't we? > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:3146 > + GStreamerDMABufHolePunchClient(std::unique_ptr<GstVideoFrameHolder> frameHolder, struct wpe_video_plane_display_dmabuf_source* videoPlaneDisplayDmaBufSource) I think we should be taking a std::unique_ptr<GstVideoFrameHolder>&& frameHolder, specially with the way you call this constructor. > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:3181 > + proxy.pushNextBuffer(WTFMove(layerBuffer)); And from what I see, TextureMapperPlatformLayerProxy::pushNextBuffer lacks a && in the parameter as well. Either we add it or we are not taking advantage of all the WTFMove calling it.
Comment on attachment 387790 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387790&action=review >> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:259 >> + }, gst_buffer_ref(m_buffer.get())); > > Is this lambda a C callback? If not, then it would be nice to capture the smart pointer (by copy!) inside the lambda. I think it's what you call a C callback? https://github.com/Igalia/WPEBackend-fdo/blob/master/include/wpe-fdo/extensions/video-plane-display-dmabuf.h#L66
Created attachment 387904 [details] Patch
(In reply to Xabier Rodríguez Calvar from comment #2) > > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:3181 > > + proxy.pushNextBuffer(WTFMove(layerBuffer)); > > And from what I see, TextureMapperPlatformLayerProxy::pushNextBuffer lacks a > && in the parameter as well. Either we add it or we are not taking advantage > of all the WTFMove calling it. Is this coming in a follow up or did you forget?
Committed r254682: <https://trac.webkit.org/changeset/254682>
<rdar://problem/58644258>
Hi Philippe! This broke `//Tools/Scripts/update-webkitwpe-libs` script - the cmake config is complaining for libwpe. I filed the bug upstream to libwpe: https://github.com/WebPlatformForEmbedded/libwpe/issues/63 Meanwhile, is there anything we can do about this change to fix it? Thanks, Andrey
(In reply to Andrey Lushnikov from comment #8) > This broke `//Tools/Scripts/update-webkitwpe-libs` script - the cmake config > is complaining for libwpe. > > I filed the bug upstream to libwpe: > https://github.com/WebPlatformForEmbedded/libwpe/issues/63 > I have a fix for this. I'll push it into libwpe and then update the jhbuild tag.
(In reply to Zan Dobersek from comment #9) > (In reply to Andrey Lushnikov from comment #8) > > This broke `//Tools/Scripts/update-webkitwpe-libs` script - the cmake config > > is complaining for libwpe. > > > > I filed the bug upstream to libwpe: > > https://github.com/WebPlatformForEmbedded/libwpe/issues/63 > > > > I have a fix for this. I'll push it into libwpe and then update the jhbuild > tag. Done in r255062. http://trac.webkit.org/changeset/255062/webkit