[FFmpeg-devel] [PATCH 4/7] libavutil/hwcontext_vulkan: Allocate vkFrame in one memory
Chen, Wenbin
wenbin.chen at intel.com
Mon Nov 22 06:26:42 EET 2021
> 19 Nov 2021, 19:13 by dev at lynne.ee:
>
> > 19 Nov 2021, 18:59 by dev at lynne.ee:
> >
> >> 15 Nov 2021, 08:25 by wenbin.chen at intel.com:
> >>
> >>>> 9 Nov 2021, 10:18 by wenbin.chen at intel.com:
> >>>>
> >>>> > The vaapi can import external frame, but the planes of the external
> >>>> > frames should be in the same drm object. I add a new function to
> >>>> > allocate vkFrame in one memory and vulkan device will choose a way
> >>>> > to allocate memory according to one_memory flag.
> >>>> > A new variable is added to AVVKFrame to store the offset of each
> plane.
> >>>> >
> >>>> > Signed-off-by: Wenbin Chen <wenbin.chen at intel.com>
> >>>> > ---
> >>>> > libavutil/hwcontext_vulkan.c | 46
> >>>> +++++++++++++++++++++++++++++++++++-
> >>>> > libavutil/hwcontext_vulkan.h | 1 +
> >>>> > 2 files changed, 46 insertions(+), 1 deletion(-)
> >>>> >
> >>>> > diff --git a/libavutil/hwcontext_vulkan.c
> b/libavutil/hwcontext_vulkan.c
> >>>> > index ccf3e58f49..f7878ed9c3 100644
> >>>> > --- a/libavutil/hwcontext_vulkan.c
> >>>> > +++ b/libavutil/hwcontext_vulkan.c
> >>>> > @@ -1600,6 +1600,9 @@ static int
> alloc_bind_mem(AVHWFramesContext
> >>>> *hwfc, AVVkFrame *f,
> >>>> > FFVulkanFunctions *vk = &p->vkfn;
> >>>> > const int planes = av_pix_fmt_count_planes(hwfc->sw_format);
> >>>> > VkBindImageMemoryInfo bind_info[AV_NUM_DATA_POINTERS] =
> { { 0 } };
> >>>> > + VkMemoryRequirements memory_requirements = { 0 };
> >>>> > + int mem_size = 0;
> >>>> > + int mem_size_list[AV_NUM_DATA_POINTERS] = { 0 };
> >>>> >
> >>>> > AVVulkanDeviceContext *hwctx = ctx->hwctx;
> >>>> >
> >>>> > @@ -1627,6 +1630,23 @@ static int
> >>>> alloc_bind_mem(AVHWFramesContext *hwfc, AVVkFrame *f,
> >>>> > req.memoryRequirements.size =
> FFALIGN(req.memoryRequirements.size,
> >>>> > p->props.properties.limits.minMemoryMapAlignment);
> >>>> >
> >>>> > + if (p->use_one_memory) {
> >>>> > + if (ded_req.prefersDedicatedAllocation |
> >>>> ded_req.requiresDedicatedAllocation) {
> >>>> > + av_log(hwfc, AV_LOG_ERROR, "Cannot use dedicated
> allocation
> >>>> for intel vaapi\n");
> >>>> > + return AVERROR(EINVAL);
> >>>> > + }
> >>>> >
> >>>>
> >>>> We don't set the flag unless the driver tells us to, so if the
> >>>> driver asks us to use dedicated memory when it can't handle such
> >>>> images, shouldn't the driver just not set this flag?
> >>>>
> >>>
> >>> I check the dedicatedAllocation flag because I don't know if vaapi driver
> >>> support importing dedicated memory.
> >>> Actually I am not sure if I need to check this flag for vaapi. I can remove it.
> >>>
> >>>>
> >>>>
> >>>> > + if (memory_requirements.size == 0) {
> >>>> > + memory_requirements = req.memoryRequirements;
> >>>> > + } else if (memory_requirements.memoryTypeBits !=
> >>>> req.memoryRequirements.memoryTypeBits) {
> >>>> > + av_log(hwfc, AV_LOG_ERROR, "the param for each planes
> are
> >>>> not the same\n");
> >>>> > + return AVERROR(EINVAL);
> >>>> > + }
> >>>> > +
> >>>> > + mem_size_list[i] = req.memoryRequirements.size;
> >>>> > + mem_size += mem_size_list[i];
> >>>> > + continue;
> >>>> > + }
> >>>> > +
> >>>> > /* In case the implementation prefers/requires dedicated allocation
> */
> >>>> > use_ded_mem = ded_req.prefersDedicatedAllocation |
> >>>> > ded_req.requiresDedicatedAllocation;
> >>>> > @@ -1648,6 +1668,29 @@ static int
> >>>> alloc_bind_mem(AVHWFramesContext *hwfc, AVVkFrame *f,
> >>>> > bind_info[i].memory = f->mem[i];
> >>>> > }
> >>>> >
> >>>> > + if (p->use_one_memory) {
> >>>> > + memory_requirements.size = mem_size;
> >>>> > +
> >>>> > + /* Allocate memory */
> >>>> > + if ((err = alloc_mem(ctx, &memory_requirements,
> >>>> > + f->tiling == VK_IMAGE_TILING_LINEAR ?
> >>>> > + VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT :
> >>>> > + VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT,
> >>>> > + (void *)(((uint8_t *)alloc_pnext)),
> >>>> > + &f->flags, &f->mem[0])))
> >>>> > + return err;
> >>>> > +
> >>>> > + f->size[0] = memory_requirements.size;
> >>>> > +
> >>>> > + for (int i = 0; i < planes; i++) {
> >>>> > + bind_info[i].sType =
> >>>> VK_STRUCTURE_TYPE_BIND_IMAGE_MEMORY_INFO;
> >>>> > + bind_info[i].image = f->img[i];
> >>>> > + bind_info[i].memory = f->mem[0];
> >>>> > + bind_info[i].memoryOffset = i == 0 ? 0 : mem_size_list[i-1];
> >>>> > + f->offset[i] = bind_info[i].memoryOffset;
> >>>> > + }
> >>>> > + }
> >>>> > +
> >>>> > /* Bind the allocated memory to the images */
> >>>> > ret = vk->BindImageMemory2(hwctx->act_dev, planes, bind_info);
> >>>> > if (ret != VK_SUCCESS) {
> >>>> > @@ -2924,7 +2967,8 @@ static int
> >>>> vulkan_map_to_drm(AVHWFramesContext *hwfc, AVFrame *dst,
> >>>> > 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].offset = p->use_one_memory ?
> >>>> > + f->offset[i] : layout.offset;
> >>>> > drm_desc->layers[i].planes[0].pitch = layout.rowPitch;
> >>>> > }
> >>>> >
> >>>> > diff --git a/libavutil/hwcontext_vulkan.h
> b/libavutil/hwcontext_vulkan.h
> >>>> > index 9264f70dbf..efb602ef27 100644
> >>>> > --- a/libavutil/hwcontext_vulkan.h
> >>>> > +++ b/libavutil/hwcontext_vulkan.h
> >>>> > @@ -189,6 +189,7 @@ typedef struct AVVkFrame {
> >>>> > */
> >>>> > VkDeviceMemory mem[AV_NUM_DATA_POINTERS];
> >>>> > size_t size[AV_NUM_DATA_POINTERS];
> >>>> > + size_t offset[AV_NUM_DATA_POINTERS];
> >>>> >
> >>>>
> >>>> Is this necessary? Can't you use vkGetImageSubresourceLayout
> >>>> to retrieve it?
> >>>>
> >>>
> >>> I think it is needed. The offset we get from vkGetImgeSubresourceLayout
> >>> is relative to the start of the image, but the offset drm_descriptor need
> >>> is relative to the start to the this whole memory object. I don't know
> which API can retrieve
> >>> it, so I store it.
> >>>
> >>> Thanks
> >>>
> >>
> >> I thought about it. IMO, you should do the following:
> >> Introduce a new AVVulkanFramesContext entry called
> >> "contiguous_planes", which would enable or disable the behaviour.
> >> Additionally, keep the device option, just rename it to
> "contiguous_planes",
> >> such that those without the right hardware can still test it. Also keep the
> >> way it's set to 1 by default on intel hardware.
> >> Add an offset field to AVVkFrame, and document that it describes
> >> the offset from the memory currently bound to the VkImage.
> >>
> >> As for the modifier field, I'm still unsure. What situation requires that
> >> the modifier is known in advance? Can't the driver simply pick
> >> the best modifier in the exact same way your code does it, by
> >> checking the usage flags?
> >> Images with the drm tiling are limited and have a lot of corner cases
> >> described in the spec, so would be really nice if we could always output
> >> images with the optimal tiling, whilst the driver takes care of the modifier
> >> opaquely from API users.
According to spec: https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/vkGetImageDrmFormatModifierPropertiesEXT.html
"image must have been created with tiling equal to
VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT "
Also according to: https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkImageCreateInfo.html
" If tiling is VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT, then the
pNext chain must include exactly one of VkImageDrmFormatModifierListCreateInfoEXT
or VkImageDrmFormatModifierExplicitCreateInfoEXT structures"
I agree with you. It would be ideal if we can use optimal modifier.
Unfortunately, according to spec if I want to export drm I have to
set modifier in advance.
> >>
> >
> > Actually, add a new `typedef enum AVVkFrameFlags`, and a
> > new entry `AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY = 1ULL << 1`
> > instead of a boolean flag.
> > AVVulkanFramesContext is always initialized as 0, and we really
> > want to let users supply their own context settings, and with a single
> > boolean flag, we wouldn't know if they set it or left it as default.
> >
>
> Hmm, that still wouldn't work, we want to still be able to let users
> disable the option, even on intel hardware. So, do this instead:
> `AV_VK_FRAME_FLAG_NONE = (1ULL << 0)`
> `AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY = (1ULL << 1) | 1ULL`
> This way, if a user sets any option, the bottom-most bit will be 1,
> and autodetection of flags would be disabled during vulkan_frames_init().
> If the bottom bit is not set, however, vulkan_frames_init() will auto-decide
> on the flags, and set the field, as well as the bottom-most bit.
Thanks for your advice! I will resubmit the patchset.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list