vaapi/wayland: add support for external wl_surface/wl_display
Submitted by Zhao, Halley
Link to original bug (#705821)
Description
in typical video usage, rendering surface is created by media app, not vaapisink.
add such feature for wayland platform.
when rendering to external surface, vaapsink should care wl_compositor/wl_display interface only.
Blocking
See also
Activity
Patch 251328, "0001-add-foreign-wl_display-wl_surface-support":
0001-add-foreign-wl_display-wl_surface-support.patchPatch 251329, "0002-seems-wl_buffer_destroy-is-not-required":
0002-seems-wl_buffer_destroy-is-not-required.patchPatch 251330, "0003-fixes-when-vappsink-renders-to-simple-wayland-compos":
0003-fixes-when-vappsink-renders-to-simple-wayland-compos.patch
@gbsaid:(In reply to comment 0)
in typical video usage, rendering surface is created by media app, not
vaapisink.
add such feature for wayland platform.when rendering to external surface, vaapsink should care
wl_compositor/wl_display interface only.How is this performed in vaapisink? The only supported ways to render to user-provided storage or windows are (i) in vaapisink through the GstXOverlay interface, and I believe we could also use it for Wayland purposes, and (ii) after vaapidecode by having the user handle the GstSurfaceMeta interfaces.
@gbreviewed patch 251328:Please also provide a tests/ patch for test-display for example to exercise this "external" display / window mechanism.
::: gst-libs/gst/vaapi/gstvaapidisplay_wayland.c @@ +434,1 @@This change is not needed. If this fixes something for you, then this means the problem lies in somewhere else.
::: gst-libs/gst/vaapi/gstvaapiwindow_wayland.c @@ +446,3 @@ + struct wl_display *wl_dpy = gst_vaapi_display_wayland_get_display((GstVaapiDisplayWayland*)display); + g_return_val_if_fail(display, NULL); + g_return_val_if_fail(id > 0, NULL);g_return_val_if_fail() are to be considered like asserts, i.e. they can be disabled.
So, this means that if you want id != 0 to be a hard constraint to be always evaluated, then please make if if (!id) return NULL. g_return_val_if_fail(GST_VAAPI_IS_DISPLAY_WAYLAND(display), NULL) can lives in since this is a clear programming error otherwise, in the upper "controlled" layers like plug-in elements.
::: gst-libs/gst/vaapi/gstvaapiwindow_wayland.h @@ +34,3 @@ gst_vaapi_window_wayland_new(GstVaapiDisplay *display, guint width, guint height); +GstVaapiWindow * +gst_vaapi_window_wayland_new_with_id(GstVaapiDisplay *display, guint id);I think that, in Wayland context, the external id could actually represent a pointer (wl_surface object or whatever). So, this needs to be guintptr.
Patch 255005, "add foreign wl_display/wl_surface support in vaapisink":
0001-add-foreign-wl_display-wl_surface-support.patchPatch 255006, "0002-add-frame-rendered-signal-for-wayland-platform":
0002-add-frame-rendered-signal-for-wayland-platform.patchPatch 255007, "0003-wayland-retain-additional-3-gst-buffer-after-video-r":
0003-wayland-retain-additional-3-gst-buffer-after-video-r.patchPatch 255008, "0004-destroy-wl_buffer-by-register-to-wl_buffer_listener":
0004-destroy-wl_buffer-by-register-to-wl_buffer_listener.patchPatch 255892, "test: add gst_vaapi_display_wayland_new_with_display in test-display":
0005-test-add-gst_vaapi_display_wayland_new_with_display-.patch(In reply to comment 4)
How is this performed in vaapisink? The only supported ways to render to
user-provided storage or windows are (i) in vaapisink through the GstXOverlay
interface, and I believe we could also use it for Wayland purposes, and (ii)
after vaapidecode by having the user handle the GstSurfaceMeta interfaces.
yes, GstXOverlay interface is used.
since wl_buffer data is in client side, there is no difference for on-screen or off-screen rendering in gst-vaapi.(In reply to comment 5)
Review of attachment 251328 [details]:
Please also provide a tests/ patch for test-display for example to exercise
this "external" display / window mechanism.
added in test-display
::: gst-libs/gst/vaapi/gstvaapidisplay_wayland.c
@@ +434,1 @@This change is not needed. If this fixes something for you, then this means the
problem lies in somewhere else.removed
::: gst-libs/gst/vaapi/gstvaapiwindow_wayland.c
@@ +446,3 @@- struct wl_display wl_dpy =
gst_vaapi_display_wayland_get_display((GstVaapiDisplayWayland)display); - g_return_val_if_fail(display, NULL);
- g_return_val_if_fail(id > 0, NULL);
g_return_val_if_fail() are to be considered like asserts, i.e. they can be
disabled.So, this means that if you want id != 0 to be a hard constraint to be always
evaluated, then please make if if (!id) return NULL.
g_return_val_if_fail(GST_VAAPI_IS_DISPLAY_WAYLAND(display), NULL) can lives in
since this is a clear programming error otherwise, in the upper "controlled"
layers like plug-in elements.updated.
::: gst-libs/gst/vaapi/gstvaapiwindow_wayland.h
@@ +34,3 @@
gst_vaapi_window_wayland_new(GstVaapiDisplay *display, guint width, guint
height);
+GstVaapiWindow *
+gst_vaapi_window_wayland_new_with_id(GstVaapiDisplay *display, guint id);I think that, in Wayland context, the external id could actually represent a
pointer (wl_surface object or whatever). So, this needs to be guintptr.updated with gpointer. (guintptr leads to some warning).
- struct wl_display wl_dpy =
(In reply to comment 6)
Review of attachment 251329 [details]:
Why wouldn't wl_buffer_destroy() be needed? Aren't those internal reference
counted objects, with the "unreference" from client perspective be only the
wl_buffer_destroy().it is not correct to wl_buffer_destroy upon frame redraw callback.
I add new patch to destory wl_buffer by wl_buffer_listener.(In reply to comment 7)
Review of attachment 251330 [details]:
Can't you just implement the GstVaapiPixmap interfaces?
since wl_buffer data is in client side, there is few/no difference of off-screen and on-screen rendering in gst-vaapi.
so seems GstVaapiPixmap is not necessary.this patch still addresses the difference between external and self-screated wl_surface.
(In reply to comment 7)
Review of attachment 251330 [details]:
Can't you just implement the GstVaapiPixmap interfaces?
since wl_buffer data is in client side, there is few/no difference of off-screen and on-screen rendering in gst-vaapi.
so seems GstVaapiPixmap is not necessary.this patch still addresses the difference between external and self-screated wl_surface.
@sreesaid:Some comments:
-- ASFAIK, the way to set native display pointer to gstreamer pipeline(with h/w acceleration) is via context message from the application(eg: clutter-gst). And there is no need for a new display property. What do you think?
-- wl_display_dispatch_queue_pending() is available from wayland version 1.0.2 on wards. So we need some version checking of wayland to maintain the backward compatibility.
-- If we really need the "frame-rendered" signal as provided in patch2, then i prefer to add a signal instead of sending custom “frame-rendered” message, and which should happen irrespective of the native display server.
Patch 259635, "0001-add-foreign-wl_display-wl_surface-support":
0001-add-foreign-wl_display-wl_surface-support.patchPatch 259636, "0002-add-frame-rendered-signal":
0002-add-frame-rendered-signal.patch
