[vlc-devel] [PATCH 2/2] wayland: add background surface and center video
Rémi Denis-Courmont
remi at remlab.net
Tue Feb 5 17:09:07 CET 2019
Hi,
Le tiistaina 5. helmikuuta 2019, 15.51.11 EET Alexandre Janniaux a écrit :
> Add a subsurface which display the video, allowing to change its
> position from the vout display to center it, and use the vout
> window surface to render a black area of the size of the display area.
It is generally not a good idea to implement a fix for a bug that is assigned
to somebody else.
> The black surface is needed so that the parent surface is correctly
> mapped, and will be needed with new UI needing transparency. It's also a
> way to let the interface control the layering of the video, instead of
> setting it in the video output.
> ---
> modules/video_output/wayland/shm.c | 142 +++++++++++++++++++++++++++--
> 1 file changed, 134 insertions(+), 8 deletions(-)
>
> diff --git a/modules/video_output/wayland/shm.c
> b/modules/video_output/wayland/shm.c index 5d772269be..127664f39b 100644
> --- a/modules/video_output/wayland/shm.c
> +++ b/modules/video_output/wayland/shm.c
> @@ -50,6 +50,18 @@ struct vout_display_sys_t
> struct wl_shm *shm;
> struct wp_viewporter *viewporter;
> struct wp_viewport *viewport;
> + struct wl_compositor *compositor;
> + struct wl_subcompositor *subcompositor;
> +
> + struct wl_surface *surface;
> + struct wl_subsurface *subsurface;
> +
> + struct
> + {
> + int fd;
I don't exactly see why you'd need to keep the descriptor around.
> + struct wl_buffer *buffer;
> + struct wp_viewport *viewport;
Scaling black seems rather weird, and AFAIK you cannot assume that viewport is
supported anyway.
> + } back_bg;
>
> size_t active_buffers;
>
> @@ -88,7 +100,7 @@ static void Prepare(vout_display_t *vd, picture_t *pic,
> subpicture_t *subpic, VLC_UNUSED(date);
> vout_display_sys_t *sys = vd->sys;
> struct wl_display *display = sys->embed->display.wl;
> - struct wl_surface *surface = sys->embed->handle.wl;
> + struct wl_surface *surface = sys->surface;
> struct picture_buffer_t *picbuf = pic->p_sys;
>
> if (picbuf->fd == -1)
> @@ -133,6 +145,9 @@ static void Prepare(vout_display_t *vd, picture_t *pic,
> subpicture_t *subpic, wl_buffer_add_listener(buf, &buffer_cbs, d);
> wl_surface_attach(surface, buf, 0, 0);
> wl_surface_damage(surface, 0, 0, sys->display_width,
> sys->display_height); +
> + wl_surface_attach(sys->embed->handle.wl, sys->back_bg.buffer, 0, 0);
> + wl_surface_damage(sys->embed->handle.wl, 0, 0, sys->display_width,
> sys->display_height);
> wl_display_flush(display);
It' seems a bit dubious that you need to attach and damage both surfaces. Also
seems to contradict use of unsynchronized mode.
> sys->active_buffers++;
> @@ -144,9 +159,9 @@ static void Display(vout_display_t *vd, picture_t *pic)
> {
> vout_display_sys_t *sys = vd->sys;
> struct wl_display *display = sys->embed->display.wl;
> - struct wl_surface *surface = sys->embed->handle.wl;
>
> - wl_surface_commit(surface);
> + wl_surface_commit(sys->surface);
> + wl_surface_commit(sys->embed->handle.wl);
> wl_display_roundtrip_queue(display, sys->eventq);
Likewise.
>
> (void) pic;
> @@ -207,6 +222,17 @@ static int Control(vout_display_t *vd, int query,
> va_list ap) wl_fixed_from_int(fmt.i_visible_height));
> wp_viewport_set_destination(sys->viewport,
> place.width, place.height);
> +
> + wl_subsurface_set_position(sys->subsurface, place.x,
> place.y); +
> + wp_viewport_set_source(sys->back_bg.viewport,
> + wl_fixed_from_int(0),
> + wl_fixed_from_int(0),
> + wl_fixed_from_int(1),
> + wl_fixed_from_int(1));
> + wp_viewport_set_destination(sys->back_bg.viewport,
> + cfg->display.width,
> + cfg->display.height);
> }
> else
> return VLC_EGENERIC;
Missing handling here.
> @@ -254,7 +280,16 @@ static void registry_global_cb(void *data, struct
> wl_registry *registry, &wp_viewporter_interface, 1); else
> if (!strcmp(iface, "wl_compositor"))
> + {
> sys->use_buffer_transform = vers >= 2;
> + sys->compositor = wl_registry_bind(registry, name,
> + &wl_compositor_interface,
> + __MIN(2, vers));
> + }
> + else
> + if (!strcmp(iface, "wl_subcompositor"))
> + sys->subcompositor = wl_registry_bind(registry, name,
> + &wl_subcompositor_interface,
> 1); }
>
> static void registry_global_remove_cb(void *data, struct wl_registry
> *registry, @@ -287,10 +322,18 @@ static int Open(vout_display_t *vd, const
> vout_display_cfg_t *cfg, sys->eventq = NULL;
> sys->shm = NULL;
> sys->viewporter = NULL;
> + sys->compositor = NULL;
> + sys->subcompositor = NULL;
> sys->active_buffers = 0;
> sys->display_width = cfg->display.width;
> sys->display_height = cfg->display.height;
> sys->use_buffer_transform = false;
> + sys->surface = NULL;
> + sys->subsurface = NULL;
> +
> + sys->back_bg.fd = -1;
> + sys->back_bg.buffer = NULL;
> + sys->back_bg.viewport = NULL;
>
> /* Get window */
> sys->embed = cfg->window;
> @@ -311,15 +354,39 @@ static int Open(vout_display_t *vd, const
> vout_display_cfg_t *cfg, wl_display_roundtrip_queue(display, sys->eventq);
> wl_registry_destroy(registry);
>
> - if (sys->shm == NULL)
> + if (sys->shm == NULL || sys->compositor == NULL || sys->subcompositor
> == NULL) goto error;
>
> wl_shm_add_listener(sys->shm, &shm_cbs, vd);
> wl_display_roundtrip_queue(display, sys->eventq);
>
> - struct wl_surface *surface = sys->embed->handle.wl;
> + /* Create the subsurface containing the video */
> + sys->surface = wl_compositor_create_surface(sys->compositor);
> + if (!sys->surface)
> + goto error;
> +
> + sys->subsurface = wl_subcompositor_get_subsurface(sys->subcompositor,
> + sys->surface,
> +
> sys->embed->handle.wl); + if (!sys->subsurface)
> + goto error;
> +
> + /* UI surface is updated less frequently than the video
> + * so we need to be in a desync mode */
> + wl_subsurface_set_desync(sys->subsurface);
> + wl_subsurface_place_above(sys->subsurface, sys->embed->handle.wl);
> +
> + /* Clients like Qt can expect every surface to be their surface, and
> get
> + * events from pointer object,
No it can't. This module use a separate event queue exactly to prevent that
sort of problems.
> so we avoid these event by
> disabling them on + * this surface */
> + struct wl_region *input_region =
> wl_compositor_create_region(sys->compositor); +
> wl_region_add(input_region, 0, 0, 0, 0);
> + wl_surface_set_input_region(sys->surface, input_region);
> + wl_region_destroy(input_region);
> +
> if (sys->viewporter != NULL)
> - sys->viewport = wp_viewporter_get_viewport(sys->viewporter,
> surface); + sys->viewport =
> wp_viewporter_get_viewport(sys->viewporter, +
> sys->surface); else
> sys->viewport = NULL;
>
> @@ -337,7 +404,7 @@ static int Open(vout_display_t *vd, const
> vout_display_cfg_t *cfg,
>
> if (sys->use_buffer_transform)
> {
> - wl_surface_set_buffer_transform(surface,
> + wl_surface_set_buffer_transform(sys->surface,
> transforms[fmtp->orientation]);
> }
> else
> @@ -346,6 +413,37 @@ static int Open(vout_display_t *vd, const
> vout_display_cfg_t *cfg, video_format_ApplyRotation(fmtp, &fmt);
> }
>
> + /* Initialize background wl_buffer */
> +
> + sys->back_bg.fd = vlc_memfd();
Might as well use /dev/zero AFAICT.
> + if (sys->back_bg.fd < 0)
> + goto error;
> +
> + /* After ftruncate, the pixel value appears as if it was zeroed */
> + if (ftruncate(sys->back_bg.fd, 4) != 0)
> + goto error;
> +
> + struct wl_shm_pool *pool = wl_shm_create_pool(sys->shm,
> sys->back_bg.fd, 4);
> + if (pool == NULL)
> + goto error;
> +
> + sys->back_bg.buffer =
> + wl_shm_pool_create_buffer(pool, 0, 1, 1, 1,
> WL_SHM_FORMAT_XRGB8888); +
> + wl_shm_pool_destroy(pool);
> +
> + if (sys->back_bg.buffer == NULL)
> + goto error;
> +
> + /* TODO: what should we do if we can't have viewport for background?
> + * + have a buffer of the size of the screen, and just change buffer
> size? + * + attach directly on the surface given by vout window?
> + * + attach the 1x1 pixel either way? */
> + if (sys->viewporter != NULL)
> + sys->back_bg.viewport = wp_viewporter_get_viewport(sys->viewporter,
> sys->embed->handle.wl); + else
> + sys->back_bg.viewport = NULL;
> +
> fmtp->i_chroma = VLC_CODEC_RGB32;
>
> vd->info.has_pictures_invalid = sys->viewport == NULL;
> @@ -358,9 +456,25 @@ static int Open(vout_display_t *vd, const
> vout_display_cfg_t *cfg, return VLC_SUCCESS;
>
> error:
> +
> + if (sys->back_bg.fd >= 0)
> + close(sys->back_bg.fd);
> +
> + if (sys->subsurface != NULL)
> + wl_subsurface_destroy(sys->subsurface);
> +
> + if (sys->surface != NULL)
> + wl_surface_destroy(sys->surface);
> +
> if (sys->viewporter != NULL)
> wp_viewporter_destroy(sys->viewporter);
>
> + if (sys->subcompositor != NULL)
> + wl_subcompositor_destroy(sys->subcompositor);
> +
> + if (sys->compositor != NULL)
> + wl_compositor_destroy(sys->compositor);
> +
> if (sys->shm != NULL)
> wl_shm_destroy(sys->shm);
>
> @@ -374,7 +488,7 @@ static void Close(vout_display_t *vd)
> {
> vout_display_sys_t *sys = vd->sys;
> struct wl_display *display = sys->embed->display.wl;
> - struct wl_surface *surface = sys->embed->handle.wl;
> + struct wl_surface *surface = sys->surface;
>
> wl_surface_attach(surface, NULL, 0, 0);
> wl_surface_commit(surface);
> @@ -388,9 +502,21 @@ static void Close(vout_display_t *vd)
>
> if (sys->viewport != NULL)
> wp_viewport_destroy(sys->viewport);
> +
> + if (sys->back_bg.viewport)
> + wp_viewport_destroy(sys->back_bg.viewport);
> + wl_buffer_destroy(sys->back_bg.buffer);
> + close(sys->back_bg.fd);
> +
> if (sys->viewporter != NULL)
> wp_viewporter_destroy(sys->viewporter);
> +
> + wl_subsurface_destroy(sys->subsurface);
> + wl_surface_destroy(sys->surface);
> + wl_subcompositor_destroy(sys->subcompositor);
> + wl_compositor_destroy(sys->compositor);
> wl_shm_destroy(sys->shm);
> +
> wl_display_flush(display);
> wl_event_queue_destroy(sys->eventq);
> free(sys);
--
Rémi Denis-Courmont
http://www.remlab.net/
More information about the vlc-devel
mailing list