| Message ID | 20211124052848.1122682-3-wenbin.chen@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,V2,1/5] hwcontext_vaapi: Use PRIME_2 memory type for modifiers. | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
24 Nov 2021, 06:28 by wenbin.chen@intel.com: > The vaapi can import external frame, but the planes of the external > frames should be in the same drm object. A new option "contiguous_planes" > is added to device. This flag tells device to allocate places in one > memory. When device is derived from vaapi this flag will be enabled. > A new flag frame_flag is also added to AVVulkanFramesContext. User > can use this flag to force enable or disable this behaviour. > A new variable "offset "is added to AVVKFrame. It describe describe the > offset from the memory currently bound to the VkImage. > > Signed-off-by: Wenbin Chen <wenbin.chen@intel.com> > --- > libavutil/hwcontext_vulkan.c | 62 ++++++++++++++++++++++++++++++++++-- > libavutil/hwcontext_vulkan.h | 22 +++++++++++++ > 2 files changed, 82 insertions(+), 2 deletions(-) > > diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c > index f1e750cd3e..4100e8b0a2 100644 > --- a/libavutil/hwcontext_vulkan.c > +++ b/libavutil/hwcontext_vulkan.c > @@ -103,6 +103,9 @@ typedef struct VulkanDevicePriv { > /* Settings */ > int use_linear_images; > > + /* allocate planes in a contiguous memory */ > + int contiguous_planes; > + > /* Nvidia */ > int dev_is_nvidia; > Add a new `int dev_is_intel;` field, and set it in `vulkan_device_init()`, where `dev_is_nvidia` is set. > } VulkanDevicePriv; > @@ -1266,6 +1269,11 @@ static int vulkan_device_create_internal(AVHWDeviceContext *ctx, > if (opt_d) > p->use_linear_images = strtol(opt_d->value, NULL, 10); > > + opt_d = av_dict_get(opts, "contiguous_planes", NULL, 0); > + if (opt_d) > + p->contiguous_planes = strtol(opt_d->value, NULL, 10); > Set `p->contiguous_planes` to -1 if not specified. > hwctx->enabled_dev_extensions = dev_info.ppEnabledExtensionNames; > hwctx->nb_enabled_dev_extensions = dev_info.enabledExtensionCount; > > @@ -1410,8 +1418,10 @@ static int vulkan_device_derive(AVHWDeviceContext *ctx, > return AVERROR_EXTERNAL; > } > > - if (strstr(vendor, "Intel")) > + if (strstr(vendor, "Intel")) { > + av_dict_set_int(&opts, "contiguous_planes", 1, 0); > Don't set this here, it's not needed with the changes I mentioned. > dev_select.vendor_id = 0x8086; > + } > if (strstr(vendor, "AMD")) > dev_select.vendor_id = 0x1002; > > @@ -1634,8 +1644,12 @@ static int alloc_bind_mem(AVHWFramesContext *hwfc, AVVkFrame *f, > AVHWDeviceContext *ctx = hwfc->device_ctx; > VulkanDevicePriv *p = ctx->internal->priv; > FFVulkanFunctions *vk = &p->vkfn; > + AVVulkanFramesContext *hwfctx = hwfc->hwctx; > 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; > > @@ -1663,6 +1677,19 @@ static int alloc_bind_mem(AVHWFramesContext *hwfc, AVVkFrame *f, > req.memoryRequirements.size = FFALIGN(req.memoryRequirements.size, > p->props.properties.limits.minMemoryMapAlignment); > > + if (hwfctx->contiguous_planes == AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY) { > Bitwise &. Not equals. > + 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; > @@ -1684,6 +1711,29 @@ static int alloc_bind_mem(AVHWFramesContext *hwfc, AVVkFrame *f, > bind_info[i].memory = f->mem[i]; > } > > + if (hwfctx->contiguous_planes == AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY) { > Same. > + 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) { > @@ -2046,6 +2096,12 @@ static int vulkan_frames_init(AVHWFramesContext *hwfc) > if (!hwctx->usage) > hwctx->usage = FF_VK_DEFAULT_USAGE_FLAGS; > > + if (!(hwctx->contiguous_planes & 1ULL)) { > + hwctx->contiguous_planes = p->contiguous_planes ? > + AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY : > + AV_VK_FRAME_FLAG_NONE; > + } > The code should look something like this: ``` if (!(hwfc->flags & AV_VK_FRAME_FLAG_NONE) { if (p->contiguous_planes == 1 || ((p->contiguous_planes == -1) && p->dev_is_intel)) hwfc->flags |= AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY; } ``` This way, if the `contiguous_planes` option in `VulkanDevicePriv` is not set (-1), then if the vendor is intel, the flag is added, or if the flag in `VulkanDevicePriv` is explicitly set, the flag is added as well, unless `p->contiguous_planes` is set to 0. > err = create_exec_ctx(hwfc, &fp->conv_ctx, > dev_hwctx->queue_family_comp_index, > dev_hwctx->nb_comp_queues); > @@ -2966,6 +3022,7 @@ static int vulkan_map_to_drm(AVHWFramesContext *hwfc, AVFrame *dst, > FFVulkanFunctions *vk = &p->vkfn; > VulkanFramesPriv *fp = hwfc->internal->priv; > AVVulkanDeviceContext *hwctx = hwfc->device_ctx->hwctx; > + AVVulkanFramesContext *hwfctx = hwfc->hwctx; > const int planes = av_pix_fmt_count_planes(hwfc->sw_format); > VkImageDrmFormatModifierPropertiesEXT drm_mod = { > .sType = VK_STRUCTURE_TYPE_IMAGE_DRM_FORMAT_MODIFIER_PROPERTIES_EXT, > @@ -3034,7 +3091,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 = hwfctx->contiguous_planes == AV_VK_FRAME_FLAG_CONTIGUOUS_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 fdf2a60156..62ea56ecdd 100644 > --- a/libavutil/hwcontext_vulkan.h > +++ b/libavutil/hwcontext_vulkan.h > @@ -35,6 +35,14 @@ > * with the data pointer set to an AVVkFrame. > */ > > +/** > + * Behaviour of frame allocation > + */ > +typedef enum { > + AV_VK_FRAME_FLAG_NONE = (1ULL << 0), > + AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY = (1ULL << 1) | 1ULL > +} AVVkFrameFlags; > + > /** > * Main Vulkan context, allocated as AVHWDeviceContext.hwctx. > * All of these can be set before init to change what the context uses > @@ -157,6 +165,15 @@ typedef struct AVVulkanFramesContext { > */ > void *create_pnext; > > + /** > + * Defines the behaviour of frame allocation > + * Default is 0, this flag will be autamatically set. > + * AV_VK_FRAME_FLAG_NONE, planes will be allocated in separte memory > + * AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY, planes will be allocated in a > + * contiguous memory. > + */ > + AVVkFrameFlags contiguous_planes; > Rename this to `flags`, and move the description of each option to the enum. Mention here that if no flag is set, then the flags are automatically determined based on the device. > /** > * Extension data for memory allocation. Must have as many entries as > * the number of planes of the sw_format. > @@ -198,6 +215,11 @@ typedef struct AVVkFrame { > VkDeviceMemory mem[AV_NUM_DATA_POINTERS]; > size_t size[AV_NUM_DATA_POINTERS]; > > + /** > + * Describe the offset from the memory currently bound to the VkImage. > + */ > + size_t offset[AV_NUM_DATA_POINTERS]; > + > /** > * OR'd flags for all memory allocated > */ > With those changes, it should look good, just resubmit and if it does look good and works fine, I'll apply it.
> 24 Nov 2021, 06:28 by wenbin.chen@intel.com: > > > The vaapi can import external frame, but the planes of the external > > frames should be in the same drm object. A new option > "contiguous_planes" > > is added to device. This flag tells device to allocate places in one > > memory. When device is derived from vaapi this flag will be enabled. > > A new flag frame_flag is also added to AVVulkanFramesContext. User > > can use this flag to force enable or disable this behaviour. > > A new variable "offset "is added to AVVKFrame. It describe describe the > > offset from the memory currently bound to the VkImage. > > > > Signed-off-by: Wenbin Chen <wenbin.chen@intel.com> > > --- > > libavutil/hwcontext_vulkan.c | 62 > ++++++++++++++++++++++++++++++++++-- > > libavutil/hwcontext_vulkan.h | 22 +++++++++++++ > > 2 files changed, 82 insertions(+), 2 deletions(-) > > > > diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c > > index f1e750cd3e..4100e8b0a2 100644 > > --- a/libavutil/hwcontext_vulkan.c > > +++ b/libavutil/hwcontext_vulkan.c > > @@ -103,6 +103,9 @@ typedef struct VulkanDevicePriv { > > /* Settings */ > > int use_linear_images; > > > > + /* allocate planes in a contiguous memory */ > > + int contiguous_planes; > > + > > /* Nvidia */ > > int dev_is_nvidia; > > > > Add a new `int dev_is_intel;` field, and set it > in `vulkan_device_init()`, where `dev_is_nvidia` is set. > > > > } VulkanDevicePriv; > > @@ -1266,6 +1269,11 @@ static int > vulkan_device_create_internal(AVHWDeviceContext *ctx, > > if (opt_d) > > p->use_linear_images = strtol(opt_d->value, NULL, 10); > > > > + opt_d = av_dict_get(opts, "contiguous_planes", NULL, 0); > > + if (opt_d) > > + p->contiguous_planes = strtol(opt_d->value, NULL, 10); > > > > Set `p->contiguous_planes` to -1 if not specified. > > > > hwctx->enabled_dev_extensions = dev_info.ppEnabledExtensionNames; > > hwctx->nb_enabled_dev_extensions = dev_info.enabledExtensionCount; > > > > @@ -1410,8 +1418,10 @@ static int > vulkan_device_derive(AVHWDeviceContext *ctx, > > return AVERROR_EXTERNAL; > > } > > > > - if (strstr(vendor, "Intel")) > > + if (strstr(vendor, "Intel")) { > > + av_dict_set_int(&opts, "contiguous_planes", 1, 0); > > > > Don't set this here, it's not needed with the changes I mentioned. > > > > dev_select.vendor_id = 0x8086; > > + } > > if (strstr(vendor, "AMD")) > > dev_select.vendor_id = 0x1002; > > > > @@ -1634,8 +1644,12 @@ static int > alloc_bind_mem(AVHWFramesContext *hwfc, AVVkFrame *f, > > AVHWDeviceContext *ctx = hwfc->device_ctx; > > VulkanDevicePriv *p = ctx->internal->priv; > > FFVulkanFunctions *vk = &p->vkfn; > > + AVVulkanFramesContext *hwfctx = hwfc->hwctx; > > 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; > > > > @@ -1663,6 +1677,19 @@ static int > alloc_bind_mem(AVHWFramesContext *hwfc, AVVkFrame *f, > > req.memoryRequirements.size = FFALIGN(req.memoryRequirements.size, > > p->props.properties.limits.minMemoryMapAlignment); > > > > + if (hwfctx->contiguous_planes == > AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY) { > > > > Bitwise &. Not equals. > > > > + 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; > > @@ -1684,6 +1711,29 @@ static int > alloc_bind_mem(AVHWFramesContext *hwfc, AVVkFrame *f, > > bind_info[i].memory = f->mem[i]; > > } > > > > + if (hwfctx->contiguous_planes == > AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY) { > > > > Same. > > > > + 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) { > > @@ -2046,6 +2096,12 @@ static int > vulkan_frames_init(AVHWFramesContext *hwfc) > > if (!hwctx->usage) > > hwctx->usage = FF_VK_DEFAULT_USAGE_FLAGS; > > > > + if (!(hwctx->contiguous_planes & 1ULL)) { > > + hwctx->contiguous_planes = p->contiguous_planes ? > > + AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY : > > + AV_VK_FRAME_FLAG_NONE; > > + } > > > > The code should look something like this: > ``` > if (!(hwfc->flags & AV_VK_FRAME_FLAG_NONE) { > if (p->contiguous_planes == 1 || > ((p->contiguous_planes == -1) && p->dev_is_intel)) > hwfc->flags |= AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY; > } > ``` > > This way, if the `contiguous_planes` option in `VulkanDevicePriv` > is not set (-1), then if the vendor is intel, the flag is added, or > if the flag in `VulkanDevicePriv` is explicitly set, the flag is added > as well, unless `p->contiguous_planes` is set to 0. > > > > err = create_exec_ctx(hwfc, &fp->conv_ctx, > > dev_hwctx->queue_family_comp_index, > > dev_hwctx->nb_comp_queues); > > @@ -2966,6 +3022,7 @@ static int > vulkan_map_to_drm(AVHWFramesContext *hwfc, AVFrame *dst, > > FFVulkanFunctions *vk = &p->vkfn; > > VulkanFramesPriv *fp = hwfc->internal->priv; > > AVVulkanDeviceContext *hwctx = hwfc->device_ctx->hwctx; > > + AVVulkanFramesContext *hwfctx = hwfc->hwctx; > > const int planes = av_pix_fmt_count_planes(hwfc->sw_format); > > VkImageDrmFormatModifierPropertiesEXT drm_mod = { > > .sType = > VK_STRUCTURE_TYPE_IMAGE_DRM_FORMAT_MODIFIER_PROPERTIES_EXT, > > @@ -3034,7 +3091,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 = hwfctx->contiguous_planes > == AV_VK_FRAME_FLAG_CONTIGUOUS_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 fdf2a60156..62ea56ecdd 100644 > > --- a/libavutil/hwcontext_vulkan.h > > +++ b/libavutil/hwcontext_vulkan.h > > @@ -35,6 +35,14 @@ > > * with the data pointer set to an AVVkFrame. > > */ > > > > +/** > > + * Behaviour of frame allocation > > + */ > > +typedef enum { > > + AV_VK_FRAME_FLAG_NONE = (1ULL << 0), > > + AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY = (1ULL << 1) | 1ULL > > +} AVVkFrameFlags; > > + > > /** > > * Main Vulkan context, allocated as AVHWDeviceContext.hwctx. > > * All of these can be set before init to change what the context uses > > @@ -157,6 +165,15 @@ typedef struct AVVulkanFramesContext { > > */ > > void *create_pnext; > > > > + /** > > + * Defines the behaviour of frame allocation > > + * Default is 0, this flag will be autamatically set. > > + * AV_VK_FRAME_FLAG_NONE, planes will be allocated in separte > memory > > + * AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY, planes will be > allocated in a > > + * contiguous memory. > > + */ > > + AVVkFrameFlags contiguous_planes; > > > > Rename this to `flags`, and move the description of each > option to the enum. > Mention here that if no flag is set, then the flags are automatically > determined based on the device. > > > > /** > > * Extension data for memory allocation. Must have as many entries as > > * the number of planes of the sw_format. > > @@ -198,6 +215,11 @@ typedef struct AVVkFrame { > > VkDeviceMemory mem[AV_NUM_DATA_POINTERS]; > > size_t size[AV_NUM_DATA_POINTERS]; > > > > + /** > > + * Describe the offset from the memory currently bound to the VkImage. > > + */ > > + size_t offset[AV_NUM_DATA_POINTERS]; > > + > > /** > > * OR'd flags for all memory allocated > > */ > > > > With those changes, it should look good, just resubmit and if it does > look good and works fine, I'll apply it. Thanks for your detailed review. I have one question here. If I use bitwise to check flag, "flags & AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY" will be both true when set flag to AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY (0b11) or to AV_VK_FRAME_FLAG_NONE (0b01), so I need to write code like this: ``` If ((flags & AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY) == AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY) ``` Or like this ``` If (flags & AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY & (~AV_VK_FRAME_FLAG_NONE)) ``` Is this acceptable? Thanks Wenbin > _______________________________________________ > 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".
25 Nov 2021, 03:58 by wenbin.chen@intel.com: >> 24 Nov 2021, 06:28 by wenbin.chen@intel.com: >> >> > The vaapi can import external frame, but the planes of the external >> > frames should be in the same drm object. A new option >> "contiguous_planes" >> > is added to device. This flag tells device to allocate places in one >> > memory. When device is derived from vaapi this flag will be enabled. >> > A new flag frame_flag is also added to AVVulkanFramesContext. User >> > can use this flag to force enable or disable this behaviour. >> > A new variable "offset "is added to AVVKFrame. It describe describe the >> > offset from the memory currently bound to the VkImage. >> > >> > Signed-off-by: Wenbin Chen <wenbin.chen@intel.com> >> > --- >> > libavutil/hwcontext_vulkan.c | 62 >> ++++++++++++++++++++++++++++++++++-- >> > libavutil/hwcontext_vulkan.h | 22 +++++++++++++ >> > 2 files changed, 82 insertions(+), 2 deletions(-) >> > >> > diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c >> > index f1e750cd3e..4100e8b0a2 100644 >> > --- a/libavutil/hwcontext_vulkan.c >> > +++ b/libavutil/hwcontext_vulkan.c >> > @@ -103,6 +103,9 @@ typedef struct VulkanDevicePriv { >> > /* Settings */ >> > int use_linear_images; >> > >> > + /* allocate planes in a contiguous memory */ >> > + int contiguous_planes; >> > + >> > /* Nvidia */ >> > int dev_is_nvidia; >> > >> >> Add a new `int dev_is_intel;` field, and set it >> in `vulkan_device_init()`, where `dev_is_nvidia` is set. >> >> >> > } VulkanDevicePriv; >> > @@ -1266,6 +1269,11 @@ static int >> vulkan_device_create_internal(AVHWDeviceContext *ctx, >> > if (opt_d) >> > p->use_linear_images = strtol(opt_d->value, NULL, 10); >> > >> > + opt_d = av_dict_get(opts, "contiguous_planes", NULL, 0); >> > + if (opt_d) >> > + p->contiguous_planes = strtol(opt_d->value, NULL, 10); >> > >> >> Set `p->contiguous_planes` to -1 if not specified. >> >> >> > hwctx->enabled_dev_extensions = dev_info.ppEnabledExtensionNames; >> > hwctx->nb_enabled_dev_extensions = dev_info.enabledExtensionCount; >> > >> > @@ -1410,8 +1418,10 @@ static int >> vulkan_device_derive(AVHWDeviceContext *ctx, >> > return AVERROR_EXTERNAL; >> > } >> > >> > - if (strstr(vendor, "Intel")) >> > + if (strstr(vendor, "Intel")) { >> > + av_dict_set_int(&opts, "contiguous_planes", 1, 0); >> > >> >> Don't set this here, it's not needed with the changes I mentioned. >> >> >> > dev_select.vendor_id = 0x8086; >> > + } >> > if (strstr(vendor, "AMD")) >> > dev_select.vendor_id = 0x1002; >> > >> > @@ -1634,8 +1644,12 @@ static int >> alloc_bind_mem(AVHWFramesContext *hwfc, AVVkFrame *f, >> > AVHWDeviceContext *ctx = hwfc->device_ctx; >> > VulkanDevicePriv *p = ctx->internal->priv; >> > FFVulkanFunctions *vk = &p->vkfn; >> > + AVVulkanFramesContext *hwfctx = hwfc->hwctx; >> > 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; >> > >> > @@ -1663,6 +1677,19 @@ static int >> alloc_bind_mem(AVHWFramesContext *hwfc, AVVkFrame *f, >> > req.memoryRequirements.size = FFALIGN(req.memoryRequirements.size, >> > p->props.properties.limits.minMemoryMapAlignment); >> > >> > + if (hwfctx->contiguous_planes == >> AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY) { >> > >> >> Bitwise &. Not equals. >> >> >> > + 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; >> > @@ -1684,6 +1711,29 @@ static int >> alloc_bind_mem(AVHWFramesContext *hwfc, AVVkFrame *f, >> > bind_info[i].memory = f->mem[i]; >> > } >> > >> > + if (hwfctx->contiguous_planes == >> AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY) { >> > >> >> Same. >> >> >> > + 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) { >> > @@ -2046,6 +2096,12 @@ static int >> vulkan_frames_init(AVHWFramesContext *hwfc) >> > if (!hwctx->usage) >> > hwctx->usage = FF_VK_DEFAULT_USAGE_FLAGS; >> > >> > + if (!(hwctx->contiguous_planes & 1ULL)) { >> > + hwctx->contiguous_planes = p->contiguous_planes ? >> > + AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY : >> > + AV_VK_FRAME_FLAG_NONE; >> > + } >> > >> >> The code should look something like this: >> ``` >> if (!(hwfc->flags & AV_VK_FRAME_FLAG_NONE) { >> if (p->contiguous_planes == 1 || >> ((p->contiguous_planes == -1) && p->dev_is_intel)) >> hwfc->flags |= AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY; >> } >> ``` >> >> This way, if the `contiguous_planes` option in `VulkanDevicePriv` >> is not set (-1), then if the vendor is intel, the flag is added, or >> if the flag in `VulkanDevicePriv` is explicitly set, the flag is added >> as well, unless `p->contiguous_planes` is set to 0. >> >> >> > err = create_exec_ctx(hwfc, &fp->conv_ctx, >> > dev_hwctx->queue_family_comp_index, >> > dev_hwctx->nb_comp_queues); >> > @@ -2966,6 +3022,7 @@ static int >> vulkan_map_to_drm(AVHWFramesContext *hwfc, AVFrame *dst, >> > FFVulkanFunctions *vk = &p->vkfn; >> > VulkanFramesPriv *fp = hwfc->internal->priv; >> > AVVulkanDeviceContext *hwctx = hwfc->device_ctx->hwctx; >> > + AVVulkanFramesContext *hwfctx = hwfc->hwctx; >> > const int planes = av_pix_fmt_count_planes(hwfc->sw_format); >> > VkImageDrmFormatModifierPropertiesEXT drm_mod = { >> > .sType = >> VK_STRUCTURE_TYPE_IMAGE_DRM_FORMAT_MODIFIER_PROPERTIES_EXT, >> > @@ -3034,7 +3091,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 = hwfctx->contiguous_planes >> == AV_VK_FRAME_FLAG_CONTIGUOUS_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 fdf2a60156..62ea56ecdd 100644 >> > --- a/libavutil/hwcontext_vulkan.h >> > +++ b/libavutil/hwcontext_vulkan.h >> > @@ -35,6 +35,14 @@ >> > * with the data pointer set to an AVVkFrame. >> > */ >> > >> > +/** >> > + * Behaviour of frame allocation >> > + */ >> > +typedef enum { >> > + AV_VK_FRAME_FLAG_NONE = (1ULL << 0), >> > + AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY = (1ULL << 1) | 1ULL >> > +} AVVkFrameFlags; >> > + >> > /** >> > * Main Vulkan context, allocated as AVHWDeviceContext.hwctx. >> > * All of these can be set before init to change what the context uses >> > @@ -157,6 +165,15 @@ typedef struct AVVulkanFramesContext { >> > */ >> > void *create_pnext; >> > >> > + /** >> > + * Defines the behaviour of frame allocation >> > + * Default is 0, this flag will be autamatically set. >> > + * AV_VK_FRAME_FLAG_NONE, planes will be allocated in separte >> memory >> > + * AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY, planes will be >> allocated in a >> > + * contiguous memory. >> > + */ >> > + AVVkFrameFlags contiguous_planes; >> > >> >> Rename this to `flags`, and move the description of each >> option to the enum. >> Mention here that if no flag is set, then the flags are automatically >> determined based on the device. >> >> >> > /** >> > * Extension data for memory allocation. Must have as many entries as >> > * the number of planes of the sw_format. >> > @@ -198,6 +215,11 @@ typedef struct AVVkFrame { >> > VkDeviceMemory mem[AV_NUM_DATA_POINTERS]; >> > size_t size[AV_NUM_DATA_POINTERS]; >> > >> > + /** >> > + * Describe the offset from the memory currently bound to the VkImage. >> > + */ >> > + size_t offset[AV_NUM_DATA_POINTERS]; >> > + >> > /** >> > * OR'd flags for all memory allocated >> > */ >> > >> >> With those changes, it should look good, just resubmit and if it does >> look good and works fine, I'll apply it. >> > > Thanks for your detailed review. > I have one question here. If I use bitwise to check flag, > "flags & AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY" will be both true when > set flag to AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY (0b11) or > to AV_VK_FRAME_FLAG_NONE (0b01), so I need to write code like this: > ``` > If ((flags & AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY) == AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY) > ``` > Or like this > ``` > If (flags & AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY & (~AV_VK_FRAME_FLAG_NONE)) > ``` > Is this acceptable? > Just define ``` #define VKF_FLAG(x, f) (((x) & (~AV_VK_FRAME_FLAG_NONE)) & (f)) ``` somewhere at the start of hwcontext_vulkan.c and use it `if (VKF_FLAG(hwfc->flags, AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY)) {`
diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c index f1e750cd3e..4100e8b0a2 100644 --- a/libavutil/hwcontext_vulkan.c +++ b/libavutil/hwcontext_vulkan.c @@ -103,6 +103,9 @@ typedef struct VulkanDevicePriv { /* Settings */ int use_linear_images; + /* allocate planes in a contiguous memory */ + int contiguous_planes; + /* Nvidia */ int dev_is_nvidia; } VulkanDevicePriv; @@ -1266,6 +1269,11 @@ static int vulkan_device_create_internal(AVHWDeviceContext *ctx, if (opt_d) p->use_linear_images = strtol(opt_d->value, NULL, 10); + opt_d = av_dict_get(opts, "contiguous_planes", NULL, 0); + if (opt_d) + p->contiguous_planes = strtol(opt_d->value, NULL, 10); + + hwctx->enabled_dev_extensions = dev_info.ppEnabledExtensionNames; hwctx->nb_enabled_dev_extensions = dev_info.enabledExtensionCount; @@ -1410,8 +1418,10 @@ static int vulkan_device_derive(AVHWDeviceContext *ctx, return AVERROR_EXTERNAL; } - if (strstr(vendor, "Intel")) + if (strstr(vendor, "Intel")) { + av_dict_set_int(&opts, "contiguous_planes", 1, 0); dev_select.vendor_id = 0x8086; + } if (strstr(vendor, "AMD")) dev_select.vendor_id = 0x1002; @@ -1634,8 +1644,12 @@ static int alloc_bind_mem(AVHWFramesContext *hwfc, AVVkFrame *f, AVHWDeviceContext *ctx = hwfc->device_ctx; VulkanDevicePriv *p = ctx->internal->priv; FFVulkanFunctions *vk = &p->vkfn; + AVVulkanFramesContext *hwfctx = hwfc->hwctx; 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; @@ -1663,6 +1677,19 @@ static int alloc_bind_mem(AVHWFramesContext *hwfc, AVVkFrame *f, req.memoryRequirements.size = FFALIGN(req.memoryRequirements.size, p->props.properties.limits.minMemoryMapAlignment); + if (hwfctx->contiguous_planes == AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY) { + 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; @@ -1684,6 +1711,29 @@ static int alloc_bind_mem(AVHWFramesContext *hwfc, AVVkFrame *f, bind_info[i].memory = f->mem[i]; } + if (hwfctx->contiguous_planes == AV_VK_FRAME_FLAG_CONTIGUOUS_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) { @@ -2046,6 +2096,12 @@ static int vulkan_frames_init(AVHWFramesContext *hwfc) if (!hwctx->usage) hwctx->usage = FF_VK_DEFAULT_USAGE_FLAGS; + if (!(hwctx->contiguous_planes & 1ULL)) { + hwctx->contiguous_planes = p->contiguous_planes ? + AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY : + AV_VK_FRAME_FLAG_NONE; + } + err = create_exec_ctx(hwfc, &fp->conv_ctx, dev_hwctx->queue_family_comp_index, dev_hwctx->nb_comp_queues); @@ -2966,6 +3022,7 @@ static int vulkan_map_to_drm(AVHWFramesContext *hwfc, AVFrame *dst, FFVulkanFunctions *vk = &p->vkfn; VulkanFramesPriv *fp = hwfc->internal->priv; AVVulkanDeviceContext *hwctx = hwfc->device_ctx->hwctx; + AVVulkanFramesContext *hwfctx = hwfc->hwctx; const int planes = av_pix_fmt_count_planes(hwfc->sw_format); VkImageDrmFormatModifierPropertiesEXT drm_mod = { .sType = VK_STRUCTURE_TYPE_IMAGE_DRM_FORMAT_MODIFIER_PROPERTIES_EXT, @@ -3034,7 +3091,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 = hwfctx->contiguous_planes == AV_VK_FRAME_FLAG_CONTIGUOUS_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 fdf2a60156..62ea56ecdd 100644 --- a/libavutil/hwcontext_vulkan.h +++ b/libavutil/hwcontext_vulkan.h @@ -35,6 +35,14 @@ * with the data pointer set to an AVVkFrame. */ +/** + * Behaviour of frame allocation + */ +typedef enum { + AV_VK_FRAME_FLAG_NONE = (1ULL << 0), + AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY = (1ULL << 1) | 1ULL +} AVVkFrameFlags; + /** * Main Vulkan context, allocated as AVHWDeviceContext.hwctx. * All of these can be set before init to change what the context uses @@ -157,6 +165,15 @@ typedef struct AVVulkanFramesContext { */ void *create_pnext; + /** + * Defines the behaviour of frame allocation + * Default is 0, this flag will be autamatically set. + * AV_VK_FRAME_FLAG_NONE, planes will be allocated in separte memory + * AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY, planes will be allocated in a + * contiguous memory. + */ + AVVkFrameFlags contiguous_planes; + /** * Extension data for memory allocation. Must have as many entries as * the number of planes of the sw_format. @@ -198,6 +215,11 @@ typedef struct AVVkFrame { VkDeviceMemory mem[AV_NUM_DATA_POINTERS]; size_t size[AV_NUM_DATA_POINTERS]; + /** + * Describe the offset from the memory currently bound to the VkImage. + */ + size_t offset[AV_NUM_DATA_POINTERS]; + /** * OR'd flags for all memory allocated */
The vaapi can import external frame, but the planes of the external frames should be in the same drm object. A new option "contiguous_planes" is added to device. This flag tells device to allocate places in one memory. When device is derived from vaapi this flag will be enabled. A new flag frame_flag is also added to AVVulkanFramesContext. User can use this flag to force enable or disable this behaviour. A new variable "offset "is added to AVVKFrame. It describe describe the offset from the memory currently bound to the VkImage. Signed-off-by: Wenbin Chen <wenbin.chen@intel.com> --- libavutil/hwcontext_vulkan.c | 62 ++++++++++++++++++++++++++++++++++-- libavutil/hwcontext_vulkan.h | 22 +++++++++++++ 2 files changed, 82 insertions(+), 2 deletions(-)