[Bug 797025] kmssink: Add "restore-crtc" property

[GStreamer (GNOME Bugzilla)]

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

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

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

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

::: sys/kms/gstkmssink.c
@@ +682,3 @@
   if (pres)
     drmModeFreePlaneResources (pres);
+  if (crtc && !self->restore_crtc)

We should clear crtc pointer to NULL whenever we transfer ownership to
self->saved_crtc instead of adding this condition.

@@ +793,3 @@
+          ("Failed to restore previous CRTC mode"), (NULL));
+    else
+      drmModeFreeCrtc ((drmModeCrtc *) self->saved_crtc);

First read, this seems to leak something on error. Can you comment on how the
ownership of drmModeSetCrtc works in a way that code isn't leaking any DRM
object ? You should set saved_crtc to NULL here.

@@ +794,3 @@
+    else
+      drmModeFreeCrtc ((drmModeCrtc *) self->saved_crtc);
+  }

If restore_crtc is FALSE, there is still a change, due to the API, that
self->saved_crtc is set. You should free it, and then set that pointer to NULL.

@@ +1794,3 @@
+   * Restore previous crtc setting if new crtc mode was set forcefully.
+   * By default this is enabled if user set crtc with a new mode on an already
active
+   * crtc wchich was having a valid mode.

Typo, wchich -> which

@@ +1799,3 @@
+  g_properties[PROP_RESTORE_CRTC] =
+      g_param_spec_boolean ("restore-crtc", "Restore CRTC mode",
+      "When enabled and crtc was set with a new mode, previous crtc mode will
be restored on exit",

Capital CRTC please. Instead of on exit, can you document the transition ?

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