GNOME Bugzilla – Bug 736734
vaapisink: wayland: add support for wl_viewporter extension
Last modified: 2018-11-03 15:45:56 UTC
The wl_scaler extension is to be useful for presenting VA surfaces with additional scaling, and possibly cropping information, thus avoiding an ad-hoc VPP pipeline. It should be worth trying this out. Though, we would still be missing basic support for bob-deinterlacing.
This feature requires wayland version >= 1.4.0 . I wonder whether we need to support the older versions (more #if checks in source code!!) or just raise the required version to 1.4.0 ??? I prefer to raise the dependency to 1.4.0.
Created attachment 287850 [details] [review] wayland: Build scaler protocol extension source files
Created attachment 287851 [details] [review] wayland: Use wl_scaler extension for surface scaling and cropping
If anyone wants to test, some sample code is here: https://github.com/sreerenjb/playground/blob/master/gst-wayland.c
Review of attachment 287850 [details] [review]: The scaler.xml file should be moved to some ext/wayland/protocol directory. Then the Makefile.am could be updated to point to $(top_srcdir)/ext/wayland/protocol/scaler.xml or probably make the prefix some wayland_protocol_root = $(top_srcdir)/... Besides, there might be some extra arrangements to be made for make dist in all situations where wayland is available or not, but those could be fixed at a later time.
Review of attachment 287851 [details] [review]: ::: gst-libs/gst/vaapi/gstvaapiwindow_wayland.c @@ +459,3 @@ + GST_VAAPI_OBJECT_UNLOCK_DISPLAY (window); + } + need_vpp = TRUE is still needed if dst_rect->x != 0 || dst_rect->y != 0, even if the wl_scaler extension is available. I suggest you do the other way around, i.e. keep the original block to check for need_vpp, and reset that to FALSE only if wayland_check_version(1,4,0) && (no_cropping_needed || wl_scaler >= 2); @@ +526,3 @@ /* XXX: attach to the specified target rectangle */ GST_VAAPI_OBJECT_LOCK_DISPLAY (window); + wl_surface_attach (priv->surface, buffer, dst_rect->x, dst_rect->y); My understanding is that (x,y) is relative to the previous buffer position. This means that if we originally have no source buffer and crop is (2,2), the new buffer would be put at (2,2). OK. However, if we further need to update the buffer with a new crop (2,2), that'd be relative to the current/previous, which is (2,2), thus making it (4,4). Isn't it?
(In reply to comment #6) > Review of attachment 287851 [details] [review]: > > ::: gst-libs/gst/vaapi/gstvaapiwindow_wayland.c > @@ +459,3 @@ > + GST_VAAPI_OBJECT_UNLOCK_DISPLAY (window); > + } > + > > need_vpp = TRUE is still needed if dst_rect->x != 0 || dst_rect->y != 0, even > if the wl_scaler extension is available. Why? see the following comment... > I suggest you do the other way around, i.e. keep the original block to check > for need_vpp, and reset that to FALSE only if wayland_check_version(1,4,0) && > (no_cropping_needed || wl_scaler >= 2); > > @@ +526,3 @@ > /* XXX: attach to the specified target rectangle */ > GST_VAAPI_OBJECT_LOCK_DISPLAY (window); > + wl_surface_attach (priv->surface, buffer, dst_rect->x, dst_rect->y); > > My understanding is that (x,y) is relative to the previous buffer position. > This means that if we originally have no source buffer and crop is (2,2), the > new buffer would be put at (2,2). OK. However, if we further need to update the > buffer with a new crop (2,2), that'd be relative to the current/previous, which > is (2,2), thus making it (4,4). Isn't it? This is what scaler protocol for setting the destination rectangle is saying "Arguments x and y do not exist here, use the x and y arguments to wl_surface.attach. The x, y, width, and height define the surface-local coordinate system irrespective of the attached wl_buffer size."
Created attachment 291634 [details] [review] wayland: Build scaler protocol extension source files
I will be pushing this unless there is no further comments?..
Moving to next release...
I understand the vpp has a higher performance scaler than just using opengl, could that be done compositor-side, has anyone tried to implement that into weston?
Moving to Product:GStreamer, Component:gstreamer-vaapi
Comment on attachment 287851 [details] [review] wayland: Use wl_scaler extension for surface scaling and cropping The scaler protocol from weston got moved to wayland-protcols and renamed to viewporter.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gstreamer-vaapi/issues/23.