All of lore.kernel.org
 help / color / mirror / Atom feed
From: Adrian Perez de Castro <aperez@igalia.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH 4/4] webkitgtk: explicitly set USE_GSTREAMER_GL build option
Date: Fri, 19 Oct 2018 20:25:44 +0200	[thread overview]
Message-ID: <20181019202544.GC941@momiji> (raw)
In-Reply-To: <b93c6915-8190-c39a-9251-d2530d8e136d@mind.be>

Hi everybody,

I have some comments more about this topic, please read below...

On Thu, 11 Oct 2018 23:56:36 +0200, Arnout Vandecappelle <arnout@mind.be> wrote:
> 
> 
> On 11/10/18 21:23, Yann E. MORIN wrote:
> > Peter, Arnout, All,
> > 
> > On 2018-10-11 20:16 +0200, Peter Korsgaard spake thusly:
> >>>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:
> >>  >  'imply' is syntactic sugar for 'default y if ...'. The *only* thing it changes
> >>  > is the place where you put the imply. (To be exact, it is the same as 'default
> >>  > ...' because it propagates the m/y/n state to the default, not just y/n. But
> >>  > since we don't use m, it doesn't matter.)
> >>
> >>  >  For this specific case, it is pretty obvious that putting a 'default y if
> >>  > BR2_PACKAGE_WEBKITGTK_MULTIMEDIA' on BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_GL
> > 
> > As the submitter said "in general it is preferred due to better
> > performance", why don't we just have, in webkitgtk:
> > 
> >     select BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_GL if BR2_PACKAGE_GST1_PLUGINS_BAD
> > 
> > Or even further:
> > 
> >     select BR2_PACKAGE_GST1_PLUGINS_BAD
> >     select BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_GL
> > 
> > Of course, that requires propagating the required dependencies...
> > But since webkitgtk already depends on libgtk3, which itself depends on
> > LIBEGL_WAYLAND or LIBGL, we're not too far off...
> 
>  For this specific case, that's a very good idea indeed. Something like (to be
> tested!):

While something like this would probably work, there can be cases in which
GStreamer-GL cannot be built (for example when there is no windowing system
supported by GStreamer-GL), but WebKitGTK+ may still build and work fine with
GL support enabled and GStreamer-GL support disabled. The GL code in WebKit
has a few different methods used at runtime as fall-back to create the GL
context [1].  On the other hand GStreamer-GL has a few more requirements on
the GL implementation than WebKit for building [2] ? so I do see value in
allowing to build without GStreamer-GL ?\_(?)_/? 

> config BR2_PACKAGE_WEBKITGTK_MULTIMEDIA
>         bool "multimedia support"
>         select BR2_PACKAGE_GSTREAMER1
>         select BR2_PACKAGE_GST1_PLUGINS_BAD
>         select BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_GL if
> BR2_PACKAGE_GST1_PLUGINS_BASE_HAS_LIB_OPENGL
>         ...
> 
>  Ideally we would also select the plugins that make sure HAS_LIB_OPENGL becomes
> true, but unfortunately that's really complicated. Selecting
> BR2_PACKAGE_GST1_PLUGINS_BASE_LIB_OPENGL would help, but that has the
> disadvantage that it might not actually make the GL plugin available (depending
> on the state of X11/wayland/...

I am a bit reluctant of doing this, due to the complications in making sure
that ?BR2_PACKAGE_GST1_PLUGINS_BASE_HAS_LIB_OPENGL? gets selected for all
configuration where it's supported, and the also complicated alternative of
using ?BR2_PACKAGE_GST1_PLUGINS_BASE_LIB_OPENGL?.

There's one more issue, though: I can't remember right now the precise case
(driver, GPU, GStreamer versions, etc), but some work mates from Igalia found
a couple of cases where using GStreamer-GL in WebKit results in garbled and/or
tinted video output, so it can actually be handy if we allow to manually
disable using it. So I'm leaning towards:

  if BR2_PACKAGE_WEBKITGTK_MULTIMEDIA

  config BR2_PACKAGE_WEBKITGTK_USE_GSTREAMER_GL
      bool "use gstreamer-gl"
	  default y
	  depends on BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_GL

  comment "using gstreamer-gl needs gst1-plugins-bad with the gl element enabled"
      depends on !BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_GL

  endif

WDYT?

> >>  > would be crazy. On the other hand, adding the imply in webkitgtk looks nice,
> >>  > concise and clear.
> > 
> > Sorry, I am still not convinced that using 'imply' in Buildroot is a
> > good idea overall... :-(
> > 
> >> Exactly, which is why I suggested it to Adrian.
> 
>  Apparently our BDFL disagrees :-)

I guess we will stay away from using ?imply? for now :)

Best regards,


-Adri?n


---
[1] In some cases it can even be enough that the driver supports Pbuffer
    contexts which with many drivers do not even require any windowing
	system running.
[2] Now that it's going to be Halloween, I could tell you some horror stories
    about how we have managed to run WebKit on some GL/EGL ?frankendriver?
	implementations :D
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20181019/23f9ca25/attachment.asc>

  reply	other threads:[~2018-10-19 18:25 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-09 22:08 [Buildroot] [PATCH 0/4] webkitgtk: assorted fixes and improvements Adrian Perez de Castro
2018-10-09 22:08 ` [Buildroot] [PATCH 1/4] webkitgtk: move JSC JIT selection logic to kconfig Adrian Perez de Castro
2018-10-09 22:08 ` [Buildroot] [PATCH 2/4] webkitgtk: enable package for aarch64 Adrian Perez de Castro
2018-10-09 22:08 ` [Buildroot] [PATCH 3/4] webkitgtk: enable JIT support on 32-bit MIPS Adrian Perez de Castro
2018-10-09 22:08 ` [Buildroot] [PATCH 4/4] webkitgtk: explicitly set USE_GSTREAMER_GL build option Adrian Perez de Castro
2018-10-10 19:26   ` Thomas Petazzoni
2018-10-10 20:24     ` Yann E. MORIN
2018-10-11 12:39       ` Arnout Vandecappelle
2018-10-11 18:16         ` Peter Korsgaard
2018-10-11 19:23           ` Yann E. MORIN
2018-10-11 21:56             ` Arnout Vandecappelle
2018-10-19 18:25               ` Adrian Perez de Castro [this message]
2018-10-20 12:26                 ` Arnout Vandecappelle
2018-10-25  0:39                   ` Adrian Perez de Castro
2018-10-10 22:04     ` Arnout Vandecappelle
2018-10-11  6:43       ` Thomas Petazzoni
2018-10-11 13:48         ` Adrian Perez de Castro
2018-10-10 19:23 ` [Buildroot] [PATCH 0/4] webkitgtk: assorted fixes and improvements Thomas Petazzoni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181019202544.GC941@momiji \
    --to=aperez@igalia.com \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.