Bug 194899 – [WPE] Add support for holepunch using an external video player

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

[Miguel Gomez]

Add a holepunch feature that allows to perform video playback through an external platform-dependent player. Instead of drawing the frames, WebKit draws a transparent rectangle on the position where the video should be. The rendering of the frames is performed by a platform dependent video player that puts the video on a layer below the browser, so it's visible through the transparent rectangle.

As we cannot provide WebKit implementations that bind to every existent platform-dependent player, what we do is to create a dummy player that just draws the transparent rectangle and doesn't do anything else. This new player only accepts the mymeType video/holepunch. This allows the normal GStreamer player to be used together with the holepunch one. When the type video/holepunch is set to a video source, the dummy player is selected and it draws the transparent rectangle.

[Miguel Gomez]

Created attachment 362608 [details]
Patch

[EWS Watchlist]

Attachment 362608 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/holepunch/MediaPlayerPrivateHolePunch.cpp:153:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Xabier Rodríguez Calvar]

Comment on attachment 362608 [details]
Patch

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

Anyway, considering that you added a custom mime type for this, I don't think we need to mess up with controls, at least for now.

> Source/WebCore/ChangeLog:11
> +        This can be used to allow a player placed on a lower plane than the broser to become visible.

broser -> browser

> Source/WebCore/platform/graphics/MediaPlayer.h:246
> +    virtual bool mediaPlayerControlsEnabled() const { return false; }

As save value I would return true?

> Source/WebCore/platform/graphics/MediaPlayer.h:585
> +    bool controlsEnabled() const { return client().mediaPlayerControlsEnabled(); }
> +    void setControlsEnabled(bool enabled) { client().mediaPlayerSetControlsEnabled(enabled); }

areControlsEnabled and setAreControlsEnabled all the way up and down the onion, please.

> Source/WebCore/platform/graphics/holepunch/MediaPlayerPrivateHolePunch.cpp:30
> +using namespace std;

Let's remove this and qualify all names just in case we could class with any WTF member.

> Source/WebCore/platform/graphics/holepunch/MediaPlayerPrivateHolePunch.cpp:69
> +    return const_cast<MediaPlayerPrivateGStreamerBase*>(this);

I think it is wrong. I don't know why you do a const_cast and why you try to cast a class "this" does not derive from... Does it even build?

> Source/WebCore/platform/graphics/holepunch/MediaPlayerPrivateHolePunch.h:54
> +    void load(const String&) override { };

You can probably make all override final in this file.

[Miguel Gomez]

Created attachment 362721 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 362721 [details]
Patch

Clearing flags on attachment: 362721

Committed r242033: <https://trac.webkit.org/changeset/242033>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.