Bug 194899 - [WPE] Add support for holepunch using an external video player
Summary: [WPE] Add support for holepunch using an external video player
Alias: None
Product: WebKit
Classification: Unclassified
Component: WPE WebKit (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Miguel Gomez
Depends on:
Reported: 2019-02-21 07:34 PST by Miguel Gomez
Modified: 2019-02-25 01:20 PST (History)
4 users (show)

See Also:

Patch (26.58 KB, patch)
2019-02-21 08:18 PST, Miguel Gomez
no flags Details | Formatted Diff | Diff
Patch (21.07 KB, patch)
2019-02-22 07:27 PST, Miguel Gomez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Miguel Gomez 2019-02-21 07:34:51 PST
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.
Comment 1 Miguel Gomez 2019-02-21 08:18:03 PST
Created attachment 362608 [details]
Comment 2 EWS Watchlist 2019-02-21 08:21:18 PST
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.
Comment 3 Xabier Rodríguez Calvar 2019-02-22 03:10:30 PST
Comment on attachment 362608 [details]

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.
Comment 4 Miguel Gomez 2019-02-22 07:27:55 PST
Created attachment 362721 [details]
Comment 5 WebKit Commit Bot 2019-02-25 01:20:08 PST
Comment on attachment 362721 [details]

Clearing flags on attachment: 362721

Committed r242033: <https://trac.webkit.org/changeset/242033>
Comment 6 WebKit Commit Bot 2019-02-25 01:20:09 PST
All reviewed patches have been landed.  Closing bug.