On 01/05/2025 07:05, Russell Greene wrote:
From: Russell Greene <russ...@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 <russellgree...@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.
OpenPGP_0xA2FEA5F03F034464.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
