From ca69230ef227402c3e32b9aa34bb9a8826ebda10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Manuel=20J=C3=A1quez=20Leal?= Date: Thu, 30 Nov 2023 12:00:27 +0100 Subject: [PATCH 1/5] vaallocator: don't fail if drm fourcc are different When exporting a DMABuf from a VASurface the user might tell that the surface was allocated with certain fourcc, but the returned VADRMPRIMESurfaceDescriptor migth tell a different fourcc, as in the case or radeonsi driver, for duplicated fourcc, such as YUY2 and YUYV. Originally it was supposed to be a failed exportation. This patch relax this validation by allowing different fourcc. Part-of: --- .../gst-plugins-bad/gst-libs/gst/va/gstvaallocator.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/subprojects/gst-plugins-bad/gst-libs/gst/va/gstvaallocator.c b/subprojects/gst-plugins-bad/gst-libs/gst/va/gstvaallocator.c index dedf86794f1..c1d88924dd1 100644 --- a/subprojects/gst-plugins-bad/gst-libs/gst/va/gstvaallocator.c +++ b/subprojects/gst-plugins-bad/gst-libs/gst/va/gstvaallocator.c @@ -607,10 +607,13 @@ gst_va_dmabuf_allocator_setup_buffer_full (GstAllocator * allocator, g_assert (GST_VIDEO_INFO_N_PLANES (&self->info) == desc.num_layers); + /* YUY2 and YUYV are the same. radeonsi returns always YUYV. + * There's no reason to fail if the different fourcc if there're dups. + * https://fourcc.org/pixel-format/yuv-yuy2/ */ if (fourcc != desc.fourcc) { - GST_ERROR ("Unsupported fourcc: %" GST_FOURCC_FORMAT, + GST_INFO ("Different fourcc: requested %" GST_FOURCC_FORMAT " - returned %" + GST_FOURCC_FORMAT, GST_FOURCC_ARGS (fourcc), GST_FOURCC_ARGS (desc.fourcc)); - goto failed; } if (desc.num_objects == 0) { -- GitLab From e5642b496f11d8b7500d4e9da48c79a561418407 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Manuel=20J=C3=A1quez=20Leal?= Date: Thu, 30 Nov 2023 16:52:02 +0100 Subject: [PATCH 2/5] va: check surface status before get derive image According with documentation the surface has to be in ready state before getting it derived image. This patch adds that check. Part-of: --- .../gst-libs/gst/va/vasurfaceimage.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/subprojects/gst-plugins-bad/gst-libs/gst/va/vasurfaceimage.c b/subprojects/gst-plugins-bad/gst-libs/gst/va/vasurfaceimage.c index d6d8f4985ae..96d169e500b 100644 --- a/subprojects/gst-plugins-bad/gst-libs/gst/va/vasurfaceimage.c +++ b/subprojects/gst-plugins-bad/gst-libs/gst/va/vasurfaceimage.c @@ -147,6 +147,21 @@ va_get_derive_image (GstVaDisplay * display, VASurfaceID surface, { VADisplay dpy = gst_va_display_get_va_dpy (display); VAStatus status; + VASurfaceStatus state; + + /* When directly accessing a surface special care must be taken to insure sync + * proper synchronization with the graphics hardware. Clients should call + * vaQuerySurfaceStatus to insure that a surface is not the target of + * concurrent rendering or currently being displayed by an overlay. */ + status = vaQuerySurfaceStatus (dpy, surface, &state); + if (status != VA_STATUS_SUCCESS) { + GST_WARNING ("vaQuerySurfaceStatus: %s", vaErrorStr (status)); + return FALSE; + } + if (state != VASurfaceReady) { + GST_INFO ("Surface not ready"); + return FALSE; + } status = vaDeriveImage (dpy, surface, image); if (status != VA_STATUS_SUCCESS) { -- GitLab From eab752c8649bd01d96a89fc0c87f632c1ad78e97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Manuel=20J=C3=A1quez=20Leal?= Date: Mon, 4 Dec 2023 20:05:48 +0100 Subject: [PATCH 3/5] vaallocator: force non-derived for old mesa drivers Mesa <23.3 can't map derived images for P010 format. This patch forces non-derived if this is the case. See: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24381 Part-of: --- .../gst-libs/gst/va/gstvaallocator.c | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/subprojects/gst-plugins-bad/gst-libs/gst/va/gstvaallocator.c b/subprojects/gst-plugins-bad/gst-libs/gst/va/gstvaallocator.c index c1d88924dd1..faf8c63a6fb 100644 --- a/subprojects/gst-plugins-bad/gst-libs/gst/va/gstvaallocator.c +++ b/subprojects/gst-plugins-bad/gst-libs/gst/va/gstvaallocator.c @@ -40,6 +40,8 @@ #include #include +#include /* sscanf */ + #include "gstvasurfacecopy.h" #include "gstvavideoformat.h" #include "vasurfaceimage.h" @@ -1212,6 +1214,50 @@ _reset_mem (GstVaMemory * mem, GstAllocator * allocator, gsize size) 0 /* align */ , 0 /* offset */ , size); } +/* + * HACK: + * + * This method should be defined as a public method of GstVaDisplay. But in + * order to backport this fix, it's kept locally. + */ +static gboolean +_gst_va_display_get_vendor_version (GstVaDisplay * display, guint * major, + guint * minor) +{ + VADisplay dpy; + guint maj, min; + const char *vendor; + + dpy = gst_va_display_get_va_dpy (display); + vendor = vaQueryVendorString (dpy); + if (vendor && sscanf (vendor, "Mesa Gallium driver %d.%d.", &maj, &min) == 2) { + *major = maj; + *minor = min; + return TRUE; + } + + return FALSE; +} + +static gboolean +_is_old_mesa (GstVaAllocator * va_allocator) +{ + guint major, minor; + + if (!GST_VA_DISPLAY_IS_IMPLEMENTATION (va_allocator->display, MESA_GALLIUM)) + return FALSE; + if (!_gst_va_display_get_vendor_version (va_allocator->display, &major, + &minor)) { + GST_WARNING ("Could not parse version from Mesa vendor string"); + return FALSE; + } + if (major > 23) + return FALSE; + if (major == 23 && minor > 2) + return FALSE; + return TRUE; +} + static inline void _update_info (GstVideoInfo * info, const VAImage * image) { @@ -1244,6 +1290,18 @@ _update_image_info (GstVaAllocator * va_allocator) GST_VIDEO_INFO_WIDTH (&va_allocator->info), GST_VIDEO_INFO_HEIGHT (&va_allocator->info)); + /* XXX: Derived in Mesa <23.3 can't use derived images for P010 format + * https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24381 + */ + if (va_allocator->img_format == GST_VIDEO_FORMAT_P010_10LE + && _is_old_mesa (va_allocator)) { + if (va_allocator->feat_use_derived != GST_VA_FEATURE_DISABLED) { + GST_INFO_OBJECT (va_allocator, "Disable image derive on old Mesa."); + va_allocator->feat_use_derived = GST_VA_FEATURE_DISABLED; + } + va_allocator->use_derived = FALSE; + } + /* Try derived first, but different formats can never derive */ if (va_allocator->feat_use_derived != GST_VA_FEATURE_DISABLED && va_allocator->surface_format == va_allocator->img_format) { -- GitLab From 28bf83ca3d89845c57ad8fb4c4caf6eed07ae5d4 Mon Sep 17 00:00:00 2001 From: Mengkejiergeli Ba Date: Thu, 13 Jul 2023 16:56:56 +0800 Subject: [PATCH 4/5] vaallocator: let pool alloc_info be consitent with the test order in gst_va_allocator_try In gst_va_allocator_try, the first try is to use derive_image, if it succeeds, we should use info from derived image to create bufferpool. If derive fails, then try create_image and give created image info to the pool. Part-of: --- .../gst-libs/gst/va/gstvaallocator.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/subprojects/gst-plugins-bad/gst-libs/gst/va/gstvaallocator.c b/subprojects/gst-plugins-bad/gst-libs/gst/va/gstvaallocator.c index faf8c63a6fb..e418e6be07d 100644 --- a/subprojects/gst-plugins-bad/gst-libs/gst/va/gstvaallocator.c +++ b/subprojects/gst-plugins-bad/gst-libs/gst/va/gstvaallocator.c @@ -1091,7 +1091,6 @@ struct _GstVaAllocator guint32 fourcc; guint32 rt_format; - GstVideoInfo derived_info; GstVideoInfo info; guint usage_hint; @@ -1307,9 +1306,7 @@ _update_image_info (GstVaAllocator * va_allocator) && va_allocator->surface_format == va_allocator->img_format) { if (va_get_derive_image (va_allocator->display, surface, &image)) { va_allocator->use_derived = TRUE; - va_allocator->derived_info = va_allocator->info; - _update_info (&va_allocator->derived_info, &image); - va_destroy_image (va_allocator->display, image.image_id); + goto done; } image.image_id = VA_INVALID_ID; /* reset it */ } @@ -1328,6 +1325,7 @@ _update_image_info (GstVaAllocator * va_allocator) return FALSE; } +done: _update_info (&va_allocator->info, &image); va_destroy_image (va_allocator->display, image.image_id); va_destroy_surfaces (va_allocator->display, &surface, 1); @@ -1386,23 +1384,21 @@ _va_map_unlocked (GstVaMemory * mem, GstMapFlags flags) * problematic */ use_derived = va_allocator->use_derived && !((flags & GST_MAP_READ) || ((flags & GST_MAP_WRITE) - && GST_VIDEO_INFO_IS_YUV (&va_allocator->derived_info))); + && GST_VIDEO_INFO_IS_YUV (&va_allocator->info))); break; case GST_VA_IMPLEMENTATION_MESA_GALLIUM: /* Reading RGB derived images, with non-standard resolutions, * looks like tiled too. TODO(victor): fill a bug in Mesa. */ use_derived = va_allocator->use_derived && !((flags & GST_MAP_READ) - && GST_VIDEO_INFO_IS_RGB (&va_allocator->derived_info)); + && GST_VIDEO_INFO_IS_RGB (&va_allocator->info)); break; default: use_derived = va_allocator->use_derived; break; } } - if (use_derived) - info = &va_allocator->derived_info; - else - info = &va_allocator->info; + + info = &va_allocator->info; if (!va_ensure_image (display, mem->surface, info, &mem->image, use_derived)) return NULL; -- GitLab From 4f294570d2fdc17511bb9dccb401a483246f3d03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Manuel=20J=C3=A1quez=20Leal?= Date: Mon, 11 Dec 2023 15:26:11 +0100 Subject: [PATCH 5/5] vaallocator: only i965 can switch derived/non-derived at mapping Since newer drivers change the strides and offset, and they have to be defined at allocation time because those parameters are stored in the GstVideoMeta in the buffer pool. Thinks patch is based on commit 6b1fba14 and commit 809a984b --- .../gst-libs/gst/va/gstvaallocator.c | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/subprojects/gst-plugins-bad/gst-libs/gst/va/gstvaallocator.c b/subprojects/gst-plugins-bad/gst-libs/gst/va/gstvaallocator.c index e418e6be07d..b9a38684c6f 100644 --- a/subprojects/gst-plugins-bad/gst-libs/gst/va/gstvaallocator.c +++ b/subprojects/gst-plugins-bad/gst-libs/gst/va/gstvaallocator.c @@ -1372,13 +1372,6 @@ _va_map_unlocked (GstVaMemory * mem, GstMapFlags flags) use_derived = FALSE; } else { switch (gst_va_display_get_implementation (display)) { - case GST_VA_IMPLEMENTATION_INTEL_IHD: - /* On Gen7+ Intel graphics the memory is mappable but not - * cached, so normal memcpy() access is very slow to read, but - * it's ok for writing. So let's assume that users won't prefer - * direct-mapped memory if they request read access. */ - use_derived = va_allocator->use_derived && !(flags & GST_MAP_READ); - break; case GST_VA_IMPLEMENTATION_INTEL_I965: /* YUV derived images are tiled, so writing them is also * problematic */ @@ -1386,12 +1379,6 @@ _va_map_unlocked (GstVaMemory * mem, GstMapFlags flags) || ((flags & GST_MAP_WRITE) && GST_VIDEO_INFO_IS_YUV (&va_allocator->info))); break; - case GST_VA_IMPLEMENTATION_MESA_GALLIUM: - /* Reading RGB derived images, with non-standard resolutions, - * looks like tiled too. TODO(victor): fill a bug in Mesa. */ - use_derived = va_allocator->use_derived && !((flags & GST_MAP_READ) - && GST_VIDEO_INFO_IS_RGB (&va_allocator->info)); - break; default: use_derived = va_allocator->use_derived; break; -- GitLab