[Bug 784599] kmssink: support videooverlay interface

[GStreamer (GNOME Bugzilla)]

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

--- Comment #32 from Víctor Manuel Jáquez Leal <vjaquez at igalia.com> ---
Review of attachment 356413:
 --> (https://bugzilla.gnome.org/review?bug=784599&attachment=356413)

::: sys/kms/gstkmssink.c
@@ +1254,3 @@
+
+  if (width == 0 || height == 0)
+    return;

We might have a problem here. According to the documentation [1], having -1 in
width and height is valid:

"To unset the region pass -1 for the width and height parameters."

It's not clear to me what it means in our case, shall we reset the
preferrect_rect? 

For me, it would be more natural, if w/h are -1, use the original w/h

1.
https://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-libs/html/GstVideoOverlay.html#gst-video-overlay-set-render-rectangle

@@ +1384,3 @@
+
+  if (src.w <= 0 || src.h <= 0) {
+    GST_DEBUG_OBJECT (self, "video totally out of range");

this should be a GST_WARNING_OBJECT(), without the "totally" adjective ;)

@@ +1524,3 @@
   sink->conn_id = -1;
   sink->plane_id = -1;
+  sink->reconfigure = FALSE;

this is not required, since the instance structure is zero-ed (less code is
good)

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