> 19 Nov 2021, 19:13 by d...@lynne.ee: > > > 19 Nov 2021, 18:59 by d...@lynne.ee: > > > >> 15 Nov 2021, 08:25 by wenbin.c...@intel.com: > >> > >>>> 9 Nov 2021, 10:18 by wenbin.c...@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.c...@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@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".