[FFmpeg-devel] [PATCH] hwcontext_vulkan: fix exporting multi-plane DRM modifiers

[FFmpeg-devel] [PATCH] hwcontext_vulkan: fix exporting multi-plane DRM modifiers

[Russell Greene]

From: Russell Greene <russell@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@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;
-- 
2.49.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] hwcontext_vulkan: fix exporting multi-plane DRM modifiers

[Lynne]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 6270 bytes --]

On 01/05/2025 07:05, Russell Greene wrote:
> From: Russell Greene <russell@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@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.

[-- Attachment #1.1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 637 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] hwcontext_vulkan: fix exporting multi-plane DRM modifiers

[Russell Greene]

Is this documented anywhere? Should I maybe have a fixed sized stack
buffer and fallback to an allocation if it's just a rule of thumb that
it'll be less than some known small number. I just somehow doubt this
is in the vulkan spec or any sort of guaranteed....

On Thu, May 1, 2025 at 6:10 PM Lynne <dev@lynne.ee> wrote:
>
> On 01/05/2025 07:05, Russell Greene wrote:
> > From: Russell Greene <russell@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@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.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] hwcontext_vulkan: fix exporting multi-plane DRM modifiers

[Lynne]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 965 bytes --]

On 03/05/2025 04:39, Russell Greene wrote:
> Is this documented anywhere? Should I maybe have a fixed sized stack
> buffer and fallback to an allocation if it's just a rule of thumb that
> it'll be less than some known small number. I just somehow doubt this
> is in the vulkan spec or any sort of guaranteed....



vkGetPhysicalDeviceToolPropertiesEXT(dev, nb, array)
If pToolProperties is NULL, then the number of tools currently active on 
physicalDevice is returned in pToolCount. Otherwise, pToolCount must 
point to a variable set by the application to the number of elements in 
the pToolProperties array, and on return the variable is overwritten 
with the number of structures actually written to pToolProperties. If 
pToolCount is less than the number of currently active tools, at most 
pToolCount structures will be written.

It's in the spec. Different function, but all functions which write to 
an array follow the same signature.

[-- Attachment #1.1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 637 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] hwcontext_vulkan: fix exporting multi-plane DRM modifiers

[Russell Greene]

> vkGetPhysicalDeviceToolPropertiesEXT(dev, nb, array)
> If pToolProperties is NULL, then the number of tools currently active on
> physicalDevice is returned in pToolCount. Otherwise, pToolCount must
> point to a variable set by the application to the number of elements in
> the pToolProperties array, and on return the variable is overwritten
> with the number of structures actually written to pToolProperties. If
> pToolCount is less than the number of currently active tools, at most
> pToolCount structures will be written.
>
> It's in the spec. Different function, but all functions which write to
> an array follow the same signature.

Sorry, I guess I wasn't clear about my question.

I understand that you can fetch the size by setting the data pointer
to NULL, etc. The question is "is there way to fetch the number of
memory planes a modifier uses without allocating memory?"

My understanding is:
1. There is no fixed number of modifiers a driver can expose (this may
be wrong, but seems to me it would be a strange limitation, and I
don't see anything in the vk spec or drivers that indicat that this
could be the case)
2. We don't know where in the modifier list the one we want info about is

So therefore, we have to fetch the info for *all* modifiers, which as
I understand it probably requires at least a slow path with an
allocation for drivers that potentially have a lot of modifiers.

If we're OK with a practical "we don't support drivers with more than
X modifiers because we doubt it'll ever exist" then that's a valid
answer as well.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] hwcontext_vulkan: fix exporting multi-plane DRM modifiers

[Lynne]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 1942 bytes --]

On 03/05/2025 17:01, Russell Greene wrote:
>> vkGetPhysicalDeviceToolPropertiesEXT(dev, nb, array)
>> If pToolProperties is NULL, then the number of tools currently active on
>> physicalDevice is returned in pToolCount. Otherwise, pToolCount must
>> point to a variable set by the application to the number of elements in
>> the pToolProperties array, and on return the variable is overwritten
>> with the number of structures actually written to pToolProperties. If
>> pToolCount is less than the number of currently active tools, at most
>> pToolCount structures will be written.
>>
>> It's in the spec. Different function, but all functions which write to
>> an array follow the same signature.
> 
> Sorry, I guess I wasn't clear about my question.
> 
> I understand that you can fetch the size by setting the data pointer
> to NULL, etc. The question is "is there way to fetch the number of
> memory planes a modifier uses without allocating memory?"

...that's exactly what I posted.
Set array to non-NULL, set nb to the array size you're giving it, and 
you'll get UP TO nb, no more, even if the driver has more.
> My understanding is:
> 1. There is no fixed number of modifiers a driver can expose (this may
> be wrong, but seems to me it would be a strange limitation, and I
> don't see anything in the vk spec or drivers that indicat that this
> could be the case)
> 2. We don't know where in the modifier list the one we want info about is
> 
> So therefore, we have to fetch the info for *all* modifiers, which as
> I understand it probably requires at least a slow path with an
> allocation for drivers that potentially have a lot of modifiers.
> 
> If we're OK with a practical "we don't support drivers with more than
> X modifiers because we doubt it'll ever exist" then that's a valid
> answer as well.
If that's your concern, just fetch all info for all modifiers in the 
init function.

[-- Attachment #1.1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 637 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] hwcontext_vulkan: fix exporting multi-plane DRM modifiers

[Russell Greene]

> > If we're OK with a practical "we don't support drivers with more than
> > X modifiers because we doubt it'll ever exist" then that's a valid
> > answer as well.
> If that's your concern, just fetch all info for all modifiers in the
> init function.

Works for me. I'm new to ffmpeg devel, I assume I should write a v2
and send it to this list?

Also, apologies for my mailing list sins (top posting, failing to
reply to the correct message)

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] hwcontext_vulkan: fix exporting multi-plane DRM modifiers

[Russell Greene]

Now realizing that there's not really a way to just query all
modifiers, you'd actually need to query all formats for their
modifiers...

Options as I see it are:
1. in init(), create a table of all (vk_format, modifier) pairs and
the `VkDrmFormatModifierPropertiesEXT` information about the pair for
fast lookup during the map code
2. the patch i've already suggested
3. don't support drivers with more than X modifiers per format
4. have a slow path for driver with more than that X modifiers per format

Or maybe some other options I'm not seeing?

> ...that's exactly what I posted.
> Set array to non-NULL, set nb to the array size you're giving it, and
> you'll get UP TO nb, no more, even if the driver has more.

Right, but what if the modifier you need info on is after nb?

On Sat, May 3, 2025 at 10:11 AM Russell Greene <russellgreene8@gmail.com> wrote:
>
> > > If we're OK with a practical "we don't support drivers with more than
> > > X modifiers because we doubt it'll ever exist" then that's a valid
> > > answer as well.
> > If that's your concern, just fetch all info for all modifiers in the
> > init function.
>
> Works for me. I'm new to ffmpeg devel, I assume I should write a v2
> and send it to this list?
>
> Also, apologies for my mailing list sins (top posting, failing to
> reply to the correct message)
>
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] hwcontext_vulkan: fix exporting multi-plane DRM modifiers

[Russell Greene]

Lynne,

Any thoughts on this?
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] hwcontext_vulkan: fix exporting multi-plane DRM modifiers

[Russell Greene]

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 where modifiers are
involved, but I think this patch still stands on its own and I suspect
that may be a driver bug.

---
 libavutil/hwcontext_vulkan.c | 78 +++++++++++++++++++++++++++++++-----
 1 file changed, 68 insertions(+), 10 deletions(-)

diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c
index ade0235ef1..1af4dca1d4 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,
-        };
         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;
+
+        VkDrmFormatModifierPropertiesListEXT modp = {
+            .sType = VK_STRUCTURE_TYPE_DRM_FORMAT_MODIFIER_PROPERTIES_LIST_EXT,
+        };
+        VkFormatProperties2 fmtp = {
+            .sType = VK_STRUCTURE_TYPE_FORMAT_PROPERTIES_2,
+            .pNext = &modp,
+        };
+
+        /* 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;
--
2.49.0
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".