[FFmpeg-devel] [PATCH 4/7] libavutil/hwcontext_vulkan: Allocate vkFrame in one memory
Lynne
dev at lynne.ee
Fri Nov 19 20:13:14 EET 2021
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.
>
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.
More information about the ffmpeg-devel
mailing list