[Bug 784599] kmssink: support videooverlay interface

[GStreamer (GNOME Bugzilla)]

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

--- Comment #33 from Haihua Hu <jared.hu at nxp.com> ---
(In reply to Víctor Manuel Jáquez Leal from comment #32)
> Review of attachment 356413 [details] [review]:
> 
> ::: sys/kms/gstkmssink.c
> @@ +1254,3 @@
> +
> +  if (width == 0 || height == 0)
> +    return;

I'm also confused about this, in glimagesink, if (x < 0 || y < 0 || width <= 0
|| height <= 0) it will return also. don't do anthing. Then how to unset this
region, do we need record the previous size?

Hi Nicolas，
what's your opinion？

> 
> 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)

Do I need update the patch? :) or you are doing this? just avoid duplicate
working.

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