[Bug 726193] waylandsink: subsurface & scaling support, plus many other improvements

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=726193
  GStreamer | gst-plugins-bad | git

--- Comment #3 from George Kiagiadakis <kiagiadakis.george at gmail.com> 2014-04-04 14:22:42 UTC ---
(In reply to comment #2)
> Hi George, well done!
> 
> Just trying to catch-up, do you have the log of the discussion somewhere that
> leads to this GstWaylandVideo interface ? :)

I don't think there was a discussion about that. There was only an irc
discussion with Daniel about the requirements of the sink for resizing and
after that I came up with this interface.

> As I have not followed those discussions I wonder if
> gst_video_overlay_set_render_rectangle could have been used instead of a new
> gst_wayland_video_set_surface_size ?
> 
> Also why is it not possible to manage the _pause_rendering / _resume_rendering
> in the waylandsink directly ?
> I'm not against, I just wonder :). Also this GstWaylandVideo could be useful in
> glimagesink for our wayland backend.

I'm glad that you raised this topic.

gst_video_overlay_set_render_rectangle can indeed be used instead of
gst_wayland_video_set_surface_size. The reason I didn't do that is because the
semantics are slightly different I think. set_render_rectangle is to specify a
sub-region inside the given surface, while in our case we want to draw in the
whole given surface, the only problem being that surfaces in wayland have no
specific size. So we need a method to set that size and complement the lack of
such an api in wayland directly. This is no big deal, though, especially if we
document it. But for the moment we need GstWaylandVideo anyway (see below), so
that's why I have kept it like this.

_pause_rendering and _resume_rendering now are another problem. These functions
intend to solve the problem that we need to synchronize the main drawing thread
with waylandsink's drawing thread when a resizing happens.

In general, wayland subsurfaces have an API for synchronization,
wl_subsurface_set_(de)sync (afair). When a subsurface is set in sync mode, all
its redrawing operations are queued to happen when the parent (sub)surface is
redrawn. This is handy here and of course we expect toolkits to use it, but
it's not enough. When the parent (sub)surface is redrawn, we cannot guarantee
that the last drawn frame of the gstreamer subsurface has the correct geometry.
This is why _resume_rendering exists and as you can see, it waits for a new
frame to be drawn (which will be drawn with the new geometry). If we don't do
that, resizing has quite visible artifacts.

Now before I added the wl_scaler support, _pause_rendering was also quite
useful, but I've started to think that it perhaps can go away now. The reason
it was added in the first place was to avoid rendering anything while the
internal state of the sink was being changed, as a resize would trigger a caps
& pool change, which would have to be negotiated before continuing. It also
still serves as an optimization in order to avoid sending buffers to wayland
that will never be shown, as the subsurface is in sync mode and it will be
redrawn in _resume_rendering before the end of the resize operation.

So, perhaps if we drop _pause_rendering and its performance optimization and
replace _set_surface_size with _set_render_rectangle, then maybe we can also
replace _resume_rendering with gst_video_overlay_expose, but again the
semantics are different imho. It's doable if we document it, though.

> Finally I asked gtk maintainer if it could be possible to handle most of the
> subsurface code of gtk itself when using "gdk_window_ensure_native
> (gdkwindow)".
> Could save some lines here
> http://cgit.collabora.com/git/user/gkiagia/gst-wayland-gtk-demo.git/tree/main.c
> and avoid to duplicate same code over and over on each application etc...
> They said it sounds useful. So at this point we should just open a bug and ask
> them to implement the prototypes we need. We can discuss that on IRC.

Yes, gtk definitely needs to implement that stuff. And also fix a lot of
bugs...

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.