[webkit-reviews] review granted: [Bug 193715] [WPE] Implement GStreamer based holepunch : [Attachment 361294] Patch

[bugzilla-daemon at webkit.org]

Xabier Rodríguez Calvar <calvaris at igalia.com> has granted Miguel Gomez
<magomez at igalia.com>'s request for review:
Bug 193715: [WPE] Implement GStreamer based holepunch
https://bugs.webkit.org/show_bug.cgi?id=193715

Attachment 361294: Patch

https://bugs.webkit.org/attachment.cgi?id=361294&action=review




--- Comment #14 from Xabier Rodríguez Calvar <calvaris at igalia.com> ---
Comment on attachment 361294
  --> https://bugs.webkit.org/attachment.cgi?id=361294
Patch

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

> Source/WebCore/ChangeLog:11
> +	   Implement GStreamer based holepunch functionality. Instead of
getting the video frames from the
> +	   video sink and drawing then, the player just draws a transparent
rectangle on the position where
> +	   the video should be. A platform dependent video sink will be used to
render the video frames on
> +	   a plane below the browser.

Please, explain the arch a bit more.

> Source/WebCore/platform/graphics/MediaPlayer.cpp:1558
> +bool MediaPlayer::shouldIgnoreIntrinsicSize()
> +{
> +    return m_private->shouldIgnoreIntrinsicSize();
> +}

Please, inline this in the .h

> Source/WebCore/platform/graphics/MediaPlayer.h:579
> +    bool shouldIgnoreIntrinsicSize();

As I said above, let's inline here.

>
Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
137
> +static FloatSize s_holepunchDefaultFrameSize(1280, 720);

static const FloatSize

>
Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
1099
> +static void setRectangleToVideoSink(GstElement* videoSink, IntRect& rect)

const IntRect&

>
Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
1131
> +	       std::unique_ptr<GStreamerHolePunchClient> holepunchClient =
std::make_unique<GStreamerHolePunchClient>(m_videoSink.get());

holePunchClient, please check if there are other appearances of this.

> Source/WebCore/platform/graphics/texmap/TextureMapperPlatformLayerBuffer.h:70
> +	   virtual void setVideoRectangle(IntRect) = 0;

Let optimize this by making it (const IntRect&) all the way down the onion.