gstwaylandsink.c:480:14: error: comparison of constant -1 with expression of
type 'enum wl_shm_format' is always false
[-Werror,-Wtautological-constant-out-of-range-compare]
if (format == -1)
~~~~~~ ^ ~~
This allows waylandsink to fail gracefully before going to READY
in case one of the required interfaces does not exist. Not all
interfaces are necessary for all modes of operation, but it is
better imho to fail before going to READY if at least one feature
is not supported, than to fail and/or crash at some later point.
In the future we may want to relax this restriction and allow certain
interfaces not to be present under certain circumstances, for example
if there is an alternative similar interface available (for instance,
xdg_shell instead of wl_shell), but for now let's require them all.
Weston supports them all, which is enough for us now. Other compositors
should really implement them if they don't already. I don't like the
idea of supporting many different compositors with different sets of
interfaces implemented. wl_subcompositor, wl_shm and wl_scaler are
really essential for having a nice video sink. Enough said.
This essentially hides the video and allows the application to
potentially draw a black background or whatever else it wants.
This allows to differentiate the "paused" and "stopped" modes
from the user's point of view.
Also reworded a comment there to make my thinking more clear,
since the "reason for keeping the display around" is not really
the exposed() calls, as there is no buffer shown in READY/NULL
anymore.
1) We know that gst_wayland_sink_render() will commit the surface
in the same thread a little later, as gst_wl_window_set_video_info()
is always called from there, so we can save the compositor from
some extra calculations.
2) We should not commit a resize with the new video info while we are still
showing the buffer of the previous video, with the old caps, as that
would probably be a visible resize glitch.
Previously, in order to change the surface size we had to let the pipeline
redraw it, which at first also involved re-negotiating caps, etc, so a
synchronization with the pipeline was absolutely necessary.
At the moment, we are using wl_viewport, which separates the surface size
from the buffer size and it also allows us to commit a surface resize without
attaching a new buffer, so it is enough to just do:
gst_wayland_video_pause_rendering():
wl_subsurface_set_sync()
gst_video_overlay_set_render_rectangle():
wl_subsurface_set_position()
wl_viewport_set_destination()
wl_surface_damage()
wl_surface_commit()
... commit the parent surface ...
gst_wayland_video_resume_rendering():
wl_subsurface_set_desync()
This is enough to synchronize a surface resize and the pipeline can continue
drawing independently. Now of course, the names pause/resume_rendering are
bad. I will rename them in another commit.
Access is protected only for setting/creating/destroying the display
handle. set_caps() for example is not protected because it cannot be
called before changing state to READY, at which point there will be
a display handle available and which cannot change by any thread at
that point
This is because:
* GST_ELEMENT_WARNING/ERROR do lock the OBJECT_LOCK and we deadlock instantly
* In future commits I want to make use of GstBaseSink functions that also
lock the OBJECT_LOCK inside this code
