[Bug 796516] kms: add support for kms atomic api

[GStreamer (GNOME Bugzilla)]

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

Nicolas Dufresne (ndufresne) <nicolas at ndufresne.ca> changed:

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

--- Comment #17 from Nicolas Dufresne (ndufresne) <nicolas at ndufresne.ca> ---
Review of attachment 372945:
 --> (https://bugzilla.gnome.org/review?bug=796516&attachment=372945)

::: configure.ac
@@ +1537,3 @@
 AG_GST_CHECK_FEATURE(KMS, [drm/kms libraries], kms, [
   AG_GST_PKG_CHECK_MODULES(GST_ALLOCATORS, gstreamer-allocators-1.0)
+  PKG_CHECK_MODULES([KMS_DRM], [libdrm >= 2.4.78], HAVE_KMS=yes, HAVE_KMS=no)

Update the meson file too please.

::: sys/kms/gstkmssink.c
@@ +62,3 @@
+#define GST_PLUGIN_DESC "Video sink using the Linux kernel drm atomic API"
+
+#define cache_drm_properties(TYPE, filed, self)                         \

This macro and the following, including all the following helpers Should be
move to their own C file. This macro should be turned into function.

@@ +219,3 @@

+static void
+destroy_drm_prop (gpointer data)

drm_prop_free ?

@@ +225,3 @@
+
+static gboolean
+cache_conn_properities (GstKMSSink * self)

cache_connector_properties(), full name + typo.

@@ +229,3 @@
+  gboolean ret = TRUE;
+  cache_drm_properties (CONNECTOR, conn, self);
+  return ret;

What's the point of ret here ?

@@ +237,3 @@
+  gboolean ret = TRUE;
+  cache_drm_properties (CRTC, crtc, self);
+  return ret;

Same.

@@ +244,3 @@
+{
+  gboolean ret = TRUE;
+  cache_drm_properties (PLANE, plane, self);

You have a wrapper, so it's not much to access self->pane_* once.

@@ +253,3 @@
+{
+  gint ret = 0;
+  add_drm_property (conn, self, req, name, value);

Same for conn.

@@ +262,3 @@
+{
+  gint ret = 0;
+  add_drm_property (crtc, self, req, name, value);

Same for crtc.

@@ +271,3 @@
+{
+  gint ret = 0;
+  add_drm_property (plane, self, req, name, value);

Same for plane.

@@ +547,3 @@
     goto mode_failed;

+  err = drmModeCreatePropertyBlob (self->fd, mode, sizeof (*mode), &blob_id);

Please don't drop legacy APIs, remember if you are atomic or not and choose the
right path.

@@ +713,3 @@
+
+again:
+  ret = gst_poll_wait (self->poll, 3 * GST_SECOND);

If this poll is properly unlocked by unlock() virtual, no need for this
timeout.

@@ +1077,3 @@

+static gboolean
+gst_kms_sink_check_scale (GstKMSSink * self, GstVideoInfo * const vinfo)

All this is not new code, you should consider refactoring patches prior to this
one.

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