[FFmpeg-devel] [PATCH] hwcontext_vulkan: fix exporting multi-plane DRM modifiers
Lynne
dev at lynne.ee
Fri May 2 03:09:46 EEST 2025
On 01/05/2025 07:05, Russell Greene wrote:
> From: Russell Greene <russell at shotover.com>
>
> Previously, it was assumed that `drmFormatModifierPlaneCount` was one
> for all modifiers when exporting, which is not always the case, in
> particular for AMD GPUs and maybe others.
>
> Fetch the number of memory planes and fill the structs appropriately in this situation.
>
> The encoded stream is still bad in the case whre modifers are involved,
> but I think this patch still stands on its own and I suspect that may be a driver bug.
>
> A potential improvement that could be make is to cache the format
> information, so we can avoid the two GetPhysicalDeviceFormatProperties2
> calls for each export, as well as the allocation. I doubt this is very
> expensive, but seemed worth noting.
>
> Signed-off-by: Russell Greene <russellgreene8 at gmail.com>
> ---
> libavutil/hwcontext_vulkan.c | 76 +++++++++++++++++++++++++++++++-----
> 1 file changed, 67 insertions(+), 9 deletions(-)
>
> diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c
> index ade0235ef1..d14fa4655b 100644
> --- a/libavutil/hwcontext_vulkan.c
> +++ b/libavutil/hwcontext_vulkan.c
> @@ -3787,6 +3787,17 @@ static inline uint32_t vulkan_fmt_to_drm(VkFormat vkfmt)
> return DRM_FORMAT_INVALID;
> }
>
> +#define MAX_MEMORY_PLANES 4
> +static VkImageAspectFlags plane_index_to_aspect(int plane) {
> + if (plane == 0) return VK_IMAGE_ASPECT_MEMORY_PLANE_0_BIT_EXT;
> + if (plane == 1) return VK_IMAGE_ASPECT_MEMORY_PLANE_1_BIT_EXT;
> + if (plane == 2) return VK_IMAGE_ASPECT_MEMORY_PLANE_2_BIT_EXT;
> + if (plane == 3) return VK_IMAGE_ASPECT_MEMORY_PLANE_3_BIT_EXT;
> +
> + av_assert2 (false && "Invalid plane index");
> + return VK_IMAGE_ASPECT_MEMORY_PLANE_0_BIT_EXT;
> +}
> +
> static int vulkan_map_to_drm(AVHWFramesContext *hwfc, AVFrame *dst,
> const AVFrame *src, int flags)
> {
> @@ -3855,14 +3866,65 @@ static int vulkan_map_to_drm(AVHWFramesContext *hwfc, AVFrame *dst,
>
> drm_desc->nb_layers = planes;
> for (int i = 0; i < drm_desc->nb_layers; i++) {
> - VkSubresourceLayout layout;
> - VkImageSubresource sub = {
> - .aspectMask = VK_IMAGE_ASPECT_MEMORY_PLANE_0_BIT_EXT,
> + VkDrmFormatModifierPropertiesListEXT modp = {
> + .sType = VK_STRUCTURE_TYPE_DRM_FORMAT_MODIFIER_PROPERTIES_LIST_EXT,
> + };
> + VkFormatProperties2 fmtp = {
> + .sType = VK_STRUCTURE_TYPE_FORMAT_PROPERTIES_2,
> + .pNext = &modp,
> };
> VkFormat plane_vkfmt = av_vkfmt_from_pixfmt(hwfc->sw_format)[i];
>
> - drm_desc->layers[i].format = vulkan_fmt_to_drm(plane_vkfmt);
> - drm_desc->layers[i].nb_planes = 1;
> + drm_desc->layers[i].format = vulkan_fmt_to_drm(plane_vkfmt);
> +
> + /* query drmFormatModifierCount by keeping pDrmFormatModifierProperties NULL */
> + vk->GetPhysicalDeviceFormatProperties2(hwctx->phys_dev, plane_vkfmt, &fmtp);
> +
> + modp.pDrmFormatModifierProperties =
> + av_calloc(modp.drmFormatModifierCount, sizeof(*modp.pDrmFormatModifierProperties));
> + if (!modp.pDrmFormatModifierProperties) {
> + err = AVERROR(ENOMEM);
> + goto end;
> + }
> + vk->GetPhysicalDeviceFormatProperties2(hwctx->phys_dev, plane_vkfmt, &fmtp);
> +
> + VkDrmFormatModifierPropertiesEXT *mod_props = NULL;
> + for (uint32_t i = 0; i < modp.drmFormatModifierCount; ++i) {
> + VkDrmFormatModifierPropertiesEXT *m = &modp.pDrmFormatModifierProperties[i];
> + if (m->drmFormatModifier == drm_mod.drmFormatModifier) {
> + mod_props = m;
> + break;
> + }
> + }
> +
> + if (!mod_props) {
> + av_free(modp.pDrmFormatModifierProperties);
> + av_log(hwfc, AV_LOG_ERROR, "Cannot fetch modifier properties for modifier "PRIu64"!\n",
> + drm_mod.drmFormatModifier);
> + err = AVERROR_EXTERNAL;
> + goto end;
> + }
> + drm_desc->layers[i].nb_planes = mod_props->drmFormatModifierPlaneCount;
> + av_free(modp.pDrmFormatModifierProperties);
> +
> + if (drm_desc->layers[i].nb_planes > MAX_MEMORY_PLANES) {
> + av_log(hwfc, AV_LOG_ERROR, "Too many memory planes for DRM format!\n");
> + err = AVERROR_EXTERNAL;
> + goto end;
> + }
> +
> + for (int j = 0; j < drm_desc->layers[i].nb_planes; j++) {
> + VkSubresourceLayout layout;
> + VkImageSubresource sub = {
> + .aspectMask = plane_index_to_aspect(j),
> + };
> +
> + drm_desc->layers[i].planes[j].object_index = FFMIN(i, drm_desc->nb_objects - 1);
> +
> + vk->GetImageSubresourceLayout(hwctx->act_dev, f->img[i], &sub, &layout);
> + drm_desc->layers[i].planes[j].offset = layout.offset;
> + drm_desc->layers[i].planes[j].pitch = layout.rowPitch;
> + }
>
> if (drm_desc->layers[i].format == DRM_FORMAT_INVALID) {
> av_log(hwfc, AV_LOG_ERROR, "Cannot map to DRM layer, unsupported!\n");
> @@ -3870,14 +3932,10 @@ static int vulkan_map_to_drm(AVHWFramesContext *hwfc, AVFrame *dst,
> goto end;
> }
>
> - drm_desc->layers[i].planes[0].object_index = FFMIN(i, drm_desc->nb_objects - 1);
>
> if (f->tiling == VK_IMAGE_TILING_OPTIMAL)
> continue;
>
> - vk->GetImageSubresourceLayout(hwctx->act_dev, f->img[i], &sub, &layout);
> - drm_desc->layers[i].planes[0].offset = layout.offset;
> - drm_desc->layers[i].planes[0].pitch = layout.rowPitch;
> }
>
> dst->width = src->width;
You don't need allocation for this, you can just use a fixed number
large enough for a few coefficients. From memory, most software uses 16
modifiers.
The dmabuf export code is still very much in a bad shape. I wrote code
which made the decoder output dmabuf-backed VkImages, and ran into this
issue.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xA2FEA5F03F034464.asc
Type: application/pgp-keys
Size: 624 bytes
Desc: OpenPGP public key
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20250502/f5f705e2/attachment.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 236 bytes
Desc: OpenPGP digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20250502/f5f705e2/attachment.sig>
More information about the ffmpeg-devel
mailing list