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 <d...@lynne.ee> wrote:
>
> 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.
> _______________________________________________
> 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".
_______________________________________________
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".
