diff mbox series

[FFmpeg-devel,V3,3/5] libavutil/hwcontext_vulkan: Allocate vkFrame in one memory

Message ID 20211126025443.22499-3-wenbin.chen@intel.com
State New
Headers show
Series [FFmpeg-devel,V3,1/5] hwcontext_vaapi: Use PRIME_2 memory type for modifiers. | expand

Checks

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

Commit Message

Wenbin Chen Nov. 26, 2021, 2:54 a.m. UTC 
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 | 67 +++++++++++++++++++++++++++++++++++-
 libavutil/hwcontext_vulkan.h | 24 +++++++++++++
 2 files changed, 90 insertions(+), 1 deletion(-)

Comments

Lynne Nov. 26, 2021, 10:54 a.m. UTC | #1 
26 Nov 2021, 03:54 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>
>

Why is a new offset variable needed?
vkGetImageSubresourceLayout is valid for DRM tiled images.
According to the specs,
"If the image’s tiling is VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT, then vkGetImageSubresourceLayout describes one memory plane of the image. If the image’s tiling is VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT and the image is non-linear, then the returned layout has an implementation-dependent meaning; the vendor of the image’s DRM format modifier may provide documentation that explains how to interpret the returned layout.".

Isn't this what you already have in the offset field?
Wenbin Chen Nov. 29, 2021, 4:07 a.m. UTC | #2 
> 26 Nov 2021, 03:54 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>
> >
> 
> Why is a new offset variable needed?
> vkGetImageSubresourceLayout is valid for DRM tiled images.
> According to the specs,
> "If the image’s tiling is VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT,
> then vkGetImageSubresourceLayout describes one memory plane of the
> image. If the image’s tiling is
> VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT and the image is non-
> linear, then the returned layout has an implementation-dependent meaning;
> the vendor of the image’s DRM format modifier may provide documentation
> that explains how to interpret the returned layout.".
> 
> Isn't this what you already have in the offset field?

The offset we get from vkGetImageSubresourceLayout is from the start of the image or plane.
The offset drm_object need is from the start of the memory, and vkGetImageSubresourceLayout
cannot get this information. I add a new offset variable because I allocate all planes in one memory 
not because I use VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT.

> _______________________________________________
> 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".
Lynne Nov. 29, 2021, 6:41 a.m. UTC | #3 
29 Nov 2021, 05:07 by wenbin.chen@intel.com:

>> 26 Nov 2021, 03:54 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>
>> >
>>
>> Why is a new offset variable needed?
>> vkGetImageSubresourceLayout is valid for DRM tiled images.
>> According to the specs,
>> "If the image’s tiling is VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT,
>> then vkGetImageSubresourceLayout describes one memory plane of the
>> image. If the image’s tiling is
>> VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT and the image is non-
>> linear, then the returned layout has an implementation-dependent meaning;
>> the vendor of the image’s DRM format modifier may provide documentation
>> that explains how to interpret the returned layout.".
>>
>> Isn't this what you already have in the offset field?
>>
>
> The offset we get from vkGetImageSubresourceLayout is from the start of the image or plane.
> The offset drm_object need is from the start of the memory, and vkGetImageSubresourceLayout
> cannot get this information. I add a new offset variable because I allocate all planes in one memory 
> not because I use VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT.
>

What do you mean, from the start of the plane? I don't recall it
being a thing.
The spec says it's driver-dependent, so wouldn't it make sense for
the driver to return the offset from the start of the memory?
diff mbox series

Patch

diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c
index 644ed947f8..b8076fb425 100644
--- a/libavutil/hwcontext_vulkan.c
+++ b/libavutil/hwcontext_vulkan.c
@@ -103,8 +103,14 @@  typedef struct VulkanDevicePriv {
     /* Settings */
     int use_linear_images;
 
+    /* allocate planes in a contiguous memory */
+    int contiguous_planes;
+
     /* Nvidia */
     int dev_is_nvidia;
+
+    /* Intel */
+    int dev_is_intel;
 } VulkanDevicePriv;
 
 typedef struct VulkanFramesPriv {
@@ -146,6 +152,8 @@  typedef struct AVVkFrameInternal {
         }                                                                      \
     } while(0)
 
+#define VKF_FLAG(x, f) (((x) & (~AV_VK_FRAME_FLAG_NONE)) & (f))
+
 static const struct {
     enum AVPixelFormat pixfmt;
     const VkFormat vkfmts[4];
@@ -1268,6 +1276,13 @@  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);
+    else
+        p->contiguous_planes = -1;
+
+
     hwctx->enabled_dev_extensions = dev_info.ppEnabledExtensionNames;
     hwctx->nb_enabled_dev_extensions = dev_info.enabledExtensionCount;
 
@@ -1319,6 +1334,8 @@  static int vulkan_device_init(AVHWDeviceContext *ctx)
 
     p->dev_is_nvidia = (p->props.properties.vendorID == 0x10de);
 
+    p->dev_is_intel = (p->props.properties.vendorID == 0x8086);
+
     vk->GetPhysicalDeviceQueueFamilyProperties(hwctx->phys_dev, &queue_num, NULL);
     if (!queue_num) {
         av_log(ctx, AV_LOG_ERROR, "Failed to get queues!\n");
@@ -1636,8 +1653,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;
 
@@ -1665,6 +1686,19 @@  static int alloc_bind_mem(AVHWFramesContext *hwfc, AVVkFrame *f,
             req.memoryRequirements.size = FFALIGN(req.memoryRequirements.size,
                                                   p->props.properties.limits.minMemoryMapAlignment);
 
+        if (VKF_FLAG(hwfctx->flags, 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;
@@ -1686,6 +1720,29 @@  static int alloc_bind_mem(AVHWFramesContext *hwfc, AVVkFrame *f,
         bind_info[i].memory = f->mem[i];
     }
 
+    if (VKF_FLAG(hwfctx->flags, 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) {
@@ -2048,6 +2105,12 @@  static int vulkan_frames_init(AVHWFramesContext *hwfc)
     if (!hwctx->usage)
         hwctx->usage = FF_VK_DEFAULT_USAGE_FLAGS;
 
+    if (!(hwctx->flags & AV_VK_FRAME_FLAG_NONE)) {
+        if (p->contiguous_planes == 1 ||
+           ((p->contiguous_planes == -1) && p->dev_is_intel))
+           hwctx->flags |= AV_VK_FRAME_FLAG_CONTIGUOUS_MEMORY;
+    }
+
     err = create_exec_ctx(hwfc, &fp->conv_ctx,
                           dev_hwctx->queue_family_comp_index,
                           dev_hwctx->nb_comp_queues);
@@ -2968,6 +3031,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,
@@ -3036,7 +3100,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       = VKF_FLAG(hwfctx->flags, 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..c485ee7437 100644
--- a/libavutil/hwcontext_vulkan.h
+++ b/libavutil/hwcontext_vulkan.h
@@ -35,6 +35,17 @@ 
  * with the data pointer set to an AVVkFrame.
  */
 
+/**
+ * Defines the behaviour of frame allocation
+ * 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.
+ */
+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 +168,14 @@  typedef struct AVVulkanFramesContext {
      */
     void *create_pnext;
 
+    /**
+     * Is a combination of AVVkFrameFlags. Defines the behaviour of frame
+     * allocation.
+     * If no flag is set, then the flags are automatically determined
+     * based on the device.
+     */
+    int flags;
+
     /**
      * Extension data for memory allocation. Must have as many entries as
      * the number of planes of the sw_format.
@@ -198,6 +217,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
      */