GNOME Bugzilla – Bug 760502
waylandsink: Preconditon of display handle is wrong, waylandsink need to change form GST_ELEMENT_ERROR to GST_ELEMENT_WARNING in gst_wayland_sink_set_window_handle
Last modified: 2016-09-21 19:36:50 UTC
Waylandsink use "GST_ELEMENT_ERROR" In gst_wayland_sink_set_window_handle() if application did not provide a wayland display handle. So gst_bus_add_watch return GST_MESSAGE_ERROR and Application can be stop. This is a precondition that waylandsink can not use externally-supplied surface without an externally-supplied display handle But this preconditon is wrong !!!. display is only used to communicate with Wayland-server. Waylandsink can use internal display handle which is created waylandsink. And It is working well with externally-supplied surface. So. we need to change LOG level from form GST_ELEMENT_ERROR to GST_ELEMENT_WARNING in gst_wayland_sink_set_window_handle(). thank you. diff --git a/ext/wayland/gstwaylandsink.c b/ext/wayland/gstwaylandsink.c index f4f34a8..e2d77a9 100644 --- a/ext/wayland/gstwaylandsink.c +++ b/ext/wayland/gstwaylandsink.c @@ -742,10 +742,10 @@ gst_wayland_sink_set_window_handle (GstVideoOverlay * overlay, guintptr handle) if (G_LIKELY (gst_wayland_sink_find_display (sink))) { /* we cannot use our own display with an external window handle */ if (G_UNLIKELY (sink->display->own_display)) { - GST_ELEMENT_ERROR (sink, RESOURCE, OPEN_READ_WRITE, + GST_ELEMENT_WARNING (sink, RESOURCE, OPEN_READ_WRITE, ("Application did not provide a wayland display handle"), - ("waylandsink cannot use an externally-supplied surface without " - "an externally-supplied display handle. Consider providing a " + ("Now waylandsink use internal display handle " + "which is created ourselves. Consider providing a " "display handle from your application with GstContext")); } else { sink->window = gst_wl_window_new_in_surface (sink->display, surface);
Created attachment 318833 [details] [review] waylandsink : precondition of display handle is wrong
(In reply to Hyunil Park from comment #0) > This is a precondition that waylandsink can not use externally-supplied > surface without an externally-supplied display handle > > But this preconditon is wrong !!!. > display is only used to communicate with Wayland-server. > Waylandsink can use internal display handle which is created waylandsink. > And It is working well with externally-supplied surface. I don't understand how this can possibly be true. To display a wl_buffer, you must have a handle to a wl_surface. Handles to wl_surfaces cannot be shared between clients - the object ID space in Wayland is per-client, and there are no global objects. So how can the application supply a wl_surface handle for the sink to use, when they have a different connection?
I tried test. The result is that waylandsink is working with internal display handle. I only get wl_surface from external window. I don't set gst_wayland_sink_set_display_from_context() to get display the patch is need to below line .. :) sorry, and thank you. if (handle) { if (G_LIKELY (gst_wayland_sink_find_display (sink))) { /* we cannot use our own display with an external window handle */ if (G_UNLIKELY (sink->display->own_display)) { GST_ELEMENT_WARNING (sink, RESOURCE, OPEN_READ_WRITE, ("Application did not provide a wayland display handle"), ("Now waylandsink use internal display handle " "which is created ourselves. Consider providing a " "display handle from your application with GstContext")); + sink->window = gst_wl_window_new_in_surface (sink->display, surface); } else { sink->window = gst_wl_window_new_in_surface (sink->display, surface); } } else { GST_ERROR_OBJECT (sink, "Failed to find display handle, " "ignoring window handle"); } }
Review of attachment 318833 [details] [review]: Something works or doesn't work, this seems like you want to verbally ignore what you describe as normal.
(In reply to Hyunil Park from comment #3) > I tried test. > The result is that waylandsink is working with internal display handle. > I only get wl_surface from external window. > I don't set gst_wayland_sink_set_display_from_context() to get display > > the patch is need to below line .. :) > sorry, and thank you. > > if (handle) { > if (G_LIKELY (gst_wayland_sink_find_display (sink))) { > /* we cannot use our own display with an external window handle */ > if (G_UNLIKELY (sink->display->own_display)) { > GST_ELEMENT_WARNING (sink, RESOURCE, OPEN_READ_WRITE, > ("Application did not provide a wayland display handle"), > ("Now waylandsink use internal display handle " > "which is created ourselves. Consider providing a " > "display handle from your application with GstContext")); > + sink->window = gst_wl_window_new_in_surface (sink->display, surface); > } else { > sink->window = gst_wl_window_new_in_surface (sink->display, surface); > } > } else { > GST_ERROR_OBJECT (sink, "Failed to find display handle, " > "ignoring window handle"); > } > } That patch is incorrect, if that was correct the proper patch would be to remove own_display path, as it would not matter any-more. As Daniel state, it should matters (unless you have special code in your compositor, but that would not be portable).