[Bug 793997] GBM backend fixes

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=793997

--- Comment #1 from Daniel Stone <daniel at fooishbar.org> ---
Daniel Stone 2018-03-02 16:30:35 UTC
Sorry for chiming in so late here.

Firstly, get_{depth,bpp}_from_format are wrong. There is a very very small and
fixed depth/bpp -> format mapping inside the kernel, which is pretty much:
  * 8/8 -> C8
  * 15/16 -> XRGB1555
  * 16/16 -> RGB565
  * 24/24 -> RGB888
  * 24/32 -> XRGB8888
  * 30/32 -> XRGB2101010
  * 32/32 -> ARGB8888

Anything else will silently be assumed to be XRGB8888.

So you should delete all but those formats, but _also_ preferentially use
drmModeAddFB2, for which you can just pass a format directly. (GBM_FORMAT_* ==
DRM_FORMAT_*, so no need to add a mapping; you can rely on this and it is both
API and ABI guaranteed.) Then you can just use drmModeAddFB as a fallback for
the above formats only.

You can also delete the GBM_BO_FORMAT_* handling inside those two functions, as
gbm_bo_get_format() only ever returns GBM_FORMAT_* enums. Very intuitive, I
know.

I don't quite follow what the comment in
gst_gl_window_gbm_egl_set_window_handle() means (wrt external objects and DRM
pageflips), but I'm happy to try to answer if you can elaborate.

As a final nitpick, GBM is not Mesa-specific: it's also implemented by the
Imagination, ARM, and Vivante proprietary drivers. Those tend to advertise
EGL_KHR_platform_gbm rather than EGL_MESA_platform_gbm in the extension string.
[reply] [−] Comment 23 Nicolas Dufresne (stormer) [developer] 2018-03-02
16:53:53 UTC
Hi Daniel, as this code is merged, can you file a separate bug for these
errors, and specify which files are affect ?
[reply] [−] Comment 24 Carlos Rafael Giani [reporter] 2018-03-02 17:08:16 UTC
Daniel: set_window_handle is part of the GstVideoOverlay functionality. This is
how you can embed a GStreamer video sink inside, say, a Gtk interface.
set_window_handle() is then called to supply the code with an existing window
handle. What this handle is, is platform specific. In X, it is a Window id.

Since I do not see why anybody would want to embed GBM buffer objects anywhere,
I added that comment. I wrote the GBM support with a single fullscreen video
output in mind, just like the Vivante FB support.

As for the rest, thanks for the info! I have one remaining question: do you see
anything wrong with how the BOs and the page flipping are done in draw_cb() ? I
think this is proper triple buffering. Do you?

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.