[Bug 732280] New: proposal: merge GstWaylandVideo in GstVideoOverlay

[GStreamer (bugzilla.gnome.org)]

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

           Summary: proposal: merge GstWaylandVideo in GstVideoOverlay
    Classification: Platform
           Product: GStreamer
           Version: git
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: Normal
         Component: gst-plugins-base
        AssignedTo: gstreamer-bugs at lists.freedesktop.org
        ReportedBy: george.kiagiadakis at collabora.com
         QAContact: gstreamer-bugs at lists.freedesktop.org
     GNOME version: ---


GstWaylandVideo (in gst-plugins-bad/gst-libs) is an interface used by
waylandsink that provides methods for rendering synchronization between the
parent surface and the video surface when the video surface is embedded with
GstVideoOverlay. Namely, these methods are:

  void gst_wayland_video_begin_geometry_change (GstWaylandVideo * video);
  void gst_wayland_video_end_geometry_change (GstWaylandVideo * video);

The use case for these methods is to allow smooth resizing of the video sink
together with its parent surface. This is important because the video
subsurface is in desync mode in general, which means that it draws in its own
thread independently of the parent surface. When it goes in sync mode, then the
parent surface needs to be committed for changes to become visible. If
waylandsink was in sync mode always, it would mean that it would have to signal
the application for every frame and the application would have to trigger a
repaint to make the frame appear. Fortunately, desync mode exists, so the video
surface can draw on its own. However, when resizing, if the video surface
resizes asynchronously on its own, then there is a high chance of the user
seeing a glitch, with the video surface being resized before the parent one.
This gives a sluggish and ugly feeling that we *can* avoid, hence this
proposal.

In X11 and similar platforms, we don't normally see this problem, because
resizing is synchronized in the window manager and the application doesn't even
need to inform the video sink (i.e. no calls to
gst_video_overlay_set_render_rectangle), because the video sink can receive a
signal from the window manager and update its state independently. In wayland,
the overall simplistic approach forces the applications to do more work, but in
the end this turns out to have a better and more robust performance.

In gtk, example usage of this interface is like:

static gboolean
on_video_widget_draw (GtkWidget * widget, cairo_t *cr, gpointer data)
{
  struct AppData *d = data;
  ...
  gst_wayland_video_begin_geometry_change (d->overlay);
  gst_video_overlay_set_render_rectangle (d->overlay, x, y, width, height);
  d->geometry_changing = TRUE;
  ...
}

static void
on_frame_clock_after_paint (GdkFrameClock * clock, gpointer data)
{
  struct AppData *d = data;

  if (d->geometry_changing) {
    gst_wayland_video_end_geometry_change (d->overlay);
    d->geometry_changing = FALSE;
  }
}

This internally gets translated to wayland calls like this:

gst_wayland_video_begin_geometry_change():
  wl_subsurface_set_sync ()      # puts the video surface in sync mode
gst_video_overlay_set_render_rectangle():
  wl_subsurface_set_position ()  # changes x,y
  wl_viewport_set_destination () # changes width, height
  wl_surface_damage()            # informs compositor of changes
  wl_surface_commit()            # queues the changes in the compositor
# nothing happens yet because we are in sync mode
... redraw & commit the parent surface ...
# committed changes of the video surface are now visible too
gst_wayland_video_end_geometry_change():
  wl_subsurface_set_desync()     # puts the video surface in desync mode again


Leaving aside how these two methods are used in wayland, it looks to me that
they would be useful for any other platform paints the video surface
asynchronously and needs to synchronize manually with the parent surface for
any kind of geometry change. Therefore, I think that adding these two methods
in GstVideoOverlay can be justified. These methods would probably be called:

  void gst_video_overlay_begin_geometry_change (GstVideoOverlay * overlay);
  void gst_video_overlay_end_geometry_change (GstVideoOverlay * overlay);

In the default implementation they should simply do nothing. They change
nothing for existing users of GstVideoOverlay and they are also optional for
applications to use even in cases where they are needed (i.e. waylandsink). If
they are not used, the user will simply see resizing glitches.

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