[Bug 761059] kmssink: add new plugin and element

[GStreamer (GNOME Bugzilla)]

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

Nicolas Dufresne (stormer) <nicolas.dufresne at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #319631|none                        |needs-work
             status|                            |

--- Comment #3 from Nicolas Dufresne (stormer) <nicolas.dufresne at collabora.co.uk> ---
Review of attachment 319631:
 --> (https://bugzilla.gnome.org/review?bug=761059&attachment=319631)

::: sys/kms/gstkmssink.c
@@ +54,3 @@
+    GST_PAD_SINK,
+    GST_PAD_ALWAYS,
+    GST_STATIC_CAPS (GST_VIDEO_CAPS_MAKE (GST_VIDEO_FORMATS_ALL)));

Frome the utility functions I can see that there exist a very limited list of
formats. Maybe we should stick with that here ?

@@ +60,3 @@
+{
+  static const char *drivers[] = { "i915", "radeon", "nouveau", "vmwgfx",
+    "exynos", "amdgpu", "imx-drm", "rockchip", "atmel-hlcdc"

That's not exactly elegant, any better idea ? It's in udev for sure, hence also
found in /sys/class/drm/card0/device/uevent . What if you have two cards on
your system ?

@@ +544,3 @@
+  pool = NULL;
+  if (need_pool) {
+    if (!gst_kms_sink_ensure_pool (self, caps, size))

Caching pools makes certain renegotiation scenario fails. Create a new pool for
each propose_allocation() call here. The allocator can be reused without
problems.

::: sys/kms/gstkmsutils.c
@@ +43,3 @@
+  /* DEF_FMT (XRGB1555, ???), */
+  /* DEF_FMT (XBGR1555, ???), */
+  DEF_FMT (ARGB8888, ARGB),

I believe this is not the correct match. Normally, DRM formats are expressed
from a register point of view, while GStreamer format are defined as seen in
the buffer array. For that reason, in little endian, DRM_FORMAT_ARGB8888
matches GST_VIDEO_FORMAT_BGRA, and for big endiant it matches
GST_VIDEO_FORMAT_ARGB.

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