[FFmpeg-devel] [PATCH 1/2] hwcontext_vulkan: add a new mechanism to expose used queue families

[FFmpeg-devel] [PATCH 1/2] hwcontext_vulkan: add a new mechanism to expose used queue families

[Lynne via ffmpeg-devel]

The issue with the old mechanism is that we had to introduce new
API each time we needed a new queue family, and all the queue families
were functionally fixed to a given purpose.

Nvidia's GPUs are able to handle video encoding and compute on the
same queue, which results in a speedup when pre-processing is required.

Also, this enables us to expose optical flow queues for frame interpolation.
---
APIChanges and lavu version will be bumped when comitting.

 libavutil/hwcontext_vulkan.c | 22 ++++++++++++++++++++++
 libavutil/hwcontext_vulkan.h | 22 ++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c
index da377aa1a4..ff5d34d042 100644
--- a/libavutil/hwcontext_vulkan.c
+++ b/libavutil/hwcontext_vulkan.c
@@ -1550,6 +1550,28 @@ static int vulkan_device_init(AVHWDeviceContext *ctx)
 
 #undef CHECK_QUEUE
 
+    /* Update the new queue family fields. If non-zero already,
+     * it means API users have set it. */
+    if (!hwctx->nb_qf) {
+#define ADD_QUEUE(ctx_qf, qc, flag)                                    \
+    do {                                                               \
+        if (ctx_qf != -1) {                                            \
+            hwctx->qf[hwctx->nb_qf++] = (AVVulkanDeviceQueueFamily) {  \
+                .idx = ctx_qf,                                         \
+                .num = qc,                                             \
+                .flags = flag,                                         \
+            };                                                         \
+        }                                                              \
+    } while (0)
+
+        ADD_QUEUE(hwctx->queue_family_index, hwctx->nb_graphics_queues, VK_QUEUE_GRAPHICS_BIT);
+        ADD_QUEUE(hwctx->queue_family_comp_index, hwctx->nb_comp_queues, VK_QUEUE_COMPUTE_BIT);
+        ADD_QUEUE(hwctx->queue_family_tx_index, hwctx->nb_tx_queues, VK_QUEUE_TRANSFER_BIT);
+        ADD_QUEUE(hwctx->queue_family_decode_index, hwctx->nb_decode_queues, VK_QUEUE_VIDEO_DECODE_BIT_KHR);
+        ADD_QUEUE(hwctx->queue_family_encode_index, hwctx->nb_encode_queues, VK_QUEUE_VIDEO_ENCODE_BIT_KHR);
+#undef ADD_QUEUE
+    }
+
     if (!hwctx->lock_queue)
         hwctx->lock_queue = lock_queue;
     if (!hwctx->unlock_queue)
diff --git a/libavutil/hwcontext_vulkan.h b/libavutil/hwcontext_vulkan.h
index cbbd2390c1..28fc2c73ff 100644
--- a/libavutil/hwcontext_vulkan.h
+++ b/libavutil/hwcontext_vulkan.h
@@ -30,6 +30,17 @@
 
 typedef struct AVVkFrame AVVkFrame;
 
+typedef struct AVVulkanDeviceQueueFamily {
+    /* Queue family index */
+    int idx;
+    /* Number of queues in the queue family in use */
+    int num;
+    /* Queue family capabilities. Must be non-zero.
+     * Flags may be removed to indicate the queue family may not be used
+     * for a given purpose. */
+    VkQueueFlagBits flags;
+} AVVulkanDeviceQueueFamily;
+
 /**
  * @file
  * API-specific header for AV_HWDEVICE_TYPE_VULKAN.
@@ -151,6 +162,17 @@ typedef struct AVVulkanDeviceContext {
      * Similar to lock_queue(), unlocks a queue. Must only be called after locking.
      */
     void (*unlock_queue)(struct AVHWDeviceContext *ctx, uint32_t queue_family, uint32_t index);
+
+    /**
+     * Queue families used. Must be preferentially ordered. List may contain
+     * duplicates, as long as their capability flags do not match.
+     *
+     * For compatibility reasons, all the enabled queue families listed above
+     * (queue_family_(tx/comp/encode/decode)_index) must also be included in
+     * this list until they're removed after deprecation.
+     */
+    AVVulkanDeviceQueueFamily qf[16];
+    int nb_qf;
 } AVVulkanDeviceContext;
 
 /**
-- 
2.45.1.288.g0e0cd299f1
_______________________________________________
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".

[FFmpeg-devel] [PATCH 2/2] vulkan: use the new queue family mechanism

[Lynne via ffmpeg-devel]

---
 libavutil/vulkan.c | 68 ++++++++++++++--------------------------------
 libavutil/vulkan.h |  2 +-
 2 files changed, 21 insertions(+), 49 deletions(-)

diff --git a/libavutil/vulkan.c b/libavutil/vulkan.c
index e0208c5a7c..d98e863711 100644
--- a/libavutil/vulkan.c
+++ b/libavutil/vulkan.c
@@ -189,37 +189,14 @@ int ff_vk_load_props(FFVulkanContext *s)
 
 static int vk_qf_get_index(FFVulkanContext *s, VkQueueFlagBits dev_family, int *nb)
 {
-    int ret, num;
-
-    switch (dev_family) {
-    case VK_QUEUE_GRAPHICS_BIT:
-        ret = s->hwctx->queue_family_index;
-        num = s->hwctx->nb_graphics_queues;
-        break;
-    case VK_QUEUE_COMPUTE_BIT:
-        ret = s->hwctx->queue_family_comp_index;
-        num = s->hwctx->nb_comp_queues;
-        break;
-    case VK_QUEUE_TRANSFER_BIT:
-        ret = s->hwctx->queue_family_tx_index;
-        num = s->hwctx->nb_tx_queues;
-        break;
-    case VK_QUEUE_VIDEO_ENCODE_BIT_KHR:
-        ret = s->hwctx->queue_family_encode_index;
-        num = s->hwctx->nb_encode_queues;
-        break;
-    case VK_QUEUE_VIDEO_DECODE_BIT_KHR:
-        ret = s->hwctx->queue_family_decode_index;
-        num = s->hwctx->nb_decode_queues;
-        break;
-    default:
-        av_assert0(0); /* Should never happen */
+    for (int i = 0; i < s->hwctx->nb_qf; i++) {
+        if (s->hwctx->qf[i].flags & dev_family) {
+            *nb = s->hwctx->qf[i].num;
+            return s->hwctx->qf[i].idx;
+        }
     }
 
-    if (nb)
-        *nb = num;
-
-    return ret;
+    av_assert0(0); /* Should never happen */
 }
 
 int ff_vk_qf_init(FFVulkanContext *s, FFVkQueueFamilyCtx *qf,
@@ -229,25 +206,20 @@ int ff_vk_qf_init(FFVulkanContext *s, FFVkQueueFamilyCtx *qf,
     if (!s->nb_qfs) {
         s->nb_qfs = 0;
 
-        /* Simply fills in all unique queues into s->qfs */
-        if (s->hwctx->queue_family_index >= 0)
-            s->qfs[s->nb_qfs++] = s->hwctx->queue_family_index;
-        if (!s->nb_qfs || s->qfs[0] != s->hwctx->queue_family_tx_index)
-            s->qfs[s->nb_qfs++] = s->hwctx->queue_family_tx_index;
-        if (!s->nb_qfs || (s->qfs[0] != s->hwctx->queue_family_comp_index &&
-                           s->qfs[1] != s->hwctx->queue_family_comp_index))
-            s->qfs[s->nb_qfs++] = s->hwctx->queue_family_comp_index;
-        if (s->hwctx->queue_family_decode_index >= 0 &&
-             (s->qfs[0] != s->hwctx->queue_family_decode_index &&
-              s->qfs[1] != s->hwctx->queue_family_decode_index &&
-              s->qfs[2] != s->hwctx->queue_family_decode_index))
-            s->qfs[s->nb_qfs++] = s->hwctx->queue_family_decode_index;
-        if (s->hwctx->queue_family_encode_index >= 0 &&
-             (s->qfs[0] != s->hwctx->queue_family_encode_index &&
-              s->qfs[1] != s->hwctx->queue_family_encode_index &&
-              s->qfs[2] != s->hwctx->queue_family_encode_index &&
-              s->qfs[3] != s->hwctx->queue_family_encode_index))
-            s->qfs[s->nb_qfs++] = s->hwctx->queue_family_encode_index;
+        for (int i = 0; i < s->hwctx->nb_qf; i++) {
+            /* Skip duplicates */
+            int skip = 0;
+            for (int j = 0; j < s->nb_qfs; j++) {
+                if (s->qfs[j] == s->hwctx->qf[i].idx) {
+                    skip = 1;
+                    break;
+                }
+            }
+            if (skip)
+                continue;
+
+            s->qfs[s->nb_qfs++] = s->hwctx->qf[i].idx;
+        }
     }
 
     return (qf->queue_family = vk_qf_get_index(s, dev_family, &qf->nb_queues));
diff --git a/libavutil/vulkan.h b/libavutil/vulkan.h
index 15d954fcb8..bedadedde6 100644
--- a/libavutil/vulkan.h
+++ b/libavutil/vulkan.h
@@ -257,7 +257,7 @@ typedef struct FFVulkanContext {
     AVHWFramesContext     *frames;
     AVVulkanFramesContext *hwfc;
 
-    uint32_t               qfs[5];
+    uint32_t               qfs[16];
     int                    nb_qfs;
 
     /* Properties */
-- 
2.45.1.288.g0e0cd299f1
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2] hwcontext_vulkan: add a new mechanism to expose used queue families

[Anton Khirnov]

Quoting Lynne via ffmpeg-devel (2024-07-09 03:07:12)
> @@ -151,6 +162,17 @@ typedef struct AVVulkanDeviceContext {
>       * Similar to lock_queue(), unlocks a queue. Must only be called after locking.
>       */
>      void (*unlock_queue)(struct AVHWDeviceContext *ctx, uint32_t queue_family, uint32_t index);
> +
> +    /**
> +     * Queue families used. Must be preferentially ordered. List may contain
> +     * duplicates, as long as their capability flags do not match.
> +     *
> +     * For compatibility reasons, all the enabled queue families listed above
> +     * (queue_family_(tx/comp/encode/decode)_index) must also be included in
> +     * this list until they're removed after deprecation.
> +     */
> +    AVVulkanDeviceQueueFamily qf[16];

Why 16? And are we really really sure sizeof(AVVulkanDeviceQueueFamily)
should be a part of the ABI?

-- 
Anton Khirnov
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2] hwcontext_vulkan: add a new mechanism to expose used queue families

[Lynne via ffmpeg-devel]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 1423 bytes --]

On 09/07/2024 08:57, Anton Khirnov wrote:
> Quoting Lynne via ffmpeg-devel (2024-07-09 03:07:12)
>> @@ -151,6 +162,17 @@ typedef struct AVVulkanDeviceContext {
>>        * Similar to lock_queue(), unlocks a queue. Must only be called after locking.
>>        */
>>       void (*unlock_queue)(struct AVHWDeviceContext *ctx, uint32_t queue_family, uint32_t index);
>> +
>> +    /**
>> +     * Queue families used. Must be preferentially ordered. List may contain
>> +     * duplicates, as long as their capability flags do not match.
>> +     *
>> +     * For compatibility reasons, all the enabled queue families listed above
>> +     * (queue_family_(tx/comp/encode/decode)_index) must also be included in
>> +     * this list until they're removed after deprecation.
>> +     */
>> +    AVVulkanDeviceQueueFamily qf[16];
> 
> Why 16? And are we really really sure sizeof(AVVulkanDeviceQueueFamily)
> should be a part of the ABI?

16 is just an arbitrary limit. I don't expect to need more than this 
ever, but if we do, its not something that we can't wait until a bump 
occurs.
I can increase it to 32 if you're concerned about it.

There are 6 total queue family types, and 6 more currently supported 
encode and decode operations for each queue -> 12.

I'd like to avoid making this not a part of the ABI, particularly as its 
a context that users should be able to easily set themselves.

[-- Attachment #1.1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 637 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2] hwcontext_vulkan: add a new mechanism to expose used queue families

[Anton Khirnov]

Quoting Lynne via ffmpeg-devel (2024-07-10 01:56:57)
> On 09/07/2024 08:57, Anton Khirnov wrote:
> > Quoting Lynne via ffmpeg-devel (2024-07-09 03:07:12)
> >> @@ -151,6 +162,17 @@ typedef struct AVVulkanDeviceContext {
> >>        * Similar to lock_queue(), unlocks a queue. Must only be called after locking.
> >>        */
> >>       void (*unlock_queue)(struct AVHWDeviceContext *ctx, uint32_t queue_family, uint32_t index);
> >> +
> >> +    /**
> >> +     * Queue families used. Must be preferentially ordered. List may contain
> >> +     * duplicates, as long as their capability flags do not match.
> >> +     *
> >> +     * For compatibility reasons, all the enabled queue families listed above
> >> +     * (queue_family_(tx/comp/encode/decode)_index) must also be included in
> >> +     * this list until they're removed after deprecation.
> >> +     */
> >> +    AVVulkanDeviceQueueFamily qf[16];
> > 
> > Why 16? And are we really really sure sizeof(AVVulkanDeviceQueueFamily)
> > should be a part of the ABI?
> 
> 16 is just an arbitrary limit. I don't expect to need more than this 
> ever, but if we do, its not something that we can't wait until a bump 
> occurs.
> I can increase it to 32 if you're concerned about it.
> 
> There are 6 total queue family types, and 6 more currently supported 
> encode and decode operations for each queue -> 12.
> 
> I'd like to avoid making this not a part of the ABI, particularly as its 
> a context that users should be able to easily set themselves.

I'm more concerned about adding new fields to AVVulkanDeviceQueueFamily.
Can't you just make qf an array of pointers, with a new function that
adds a new queue family to it?

-- 
Anton Khirnov
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2] hwcontext_vulkan: add a new mechanism to expose used queue families

[Lynne via ffmpeg-devel]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 1882 bytes --]

On 10/07/2024 10:18, Anton Khirnov wrote:
> Quoting Lynne via ffmpeg-devel (2024-07-10 01:56:57)
>> On 09/07/2024 08:57, Anton Khirnov wrote:
>>> Quoting Lynne via ffmpeg-devel (2024-07-09 03:07:12)
>>>> @@ -151,6 +162,17 @@ typedef struct AVVulkanDeviceContext {
>>>>         * Similar to lock_queue(), unlocks a queue. Must only be called after locking.
>>>>         */
>>>>        void (*unlock_queue)(struct AVHWDeviceContext *ctx, uint32_t queue_family, uint32_t index);
>>>> +
>>>> +    /**
>>>> +     * Queue families used. Must be preferentially ordered. List may contain
>>>> +     * duplicates, as long as their capability flags do not match.
>>>> +     *
>>>> +     * For compatibility reasons, all the enabled queue families listed above
>>>> +     * (queue_family_(tx/comp/encode/decode)_index) must also be included in
>>>> +     * this list until they're removed after deprecation.
>>>> +     */
>>>> +    AVVulkanDeviceQueueFamily qf[16];
>>>
>>> Why 16? And are we really really sure sizeof(AVVulkanDeviceQueueFamily)
>>> should be a part of the ABI?
>>
>> 16 is just an arbitrary limit. I don't expect to need more than this
>> ever, but if we do, its not something that we can't wait until a bump
>> occurs.
>> I can increase it to 32 if you're concerned about it.
>>
>> There are 6 total queue family types, and 6 more currently supported
>> encode and decode operations for each queue -> 12.
>>
>> I'd like to avoid making this not a part of the ABI, particularly as its
>> a context that users should be able to easily set themselves.
> 
> I'm more concerned about adding new fields to AVVulkanDeviceQueueFamily.
> Can't you just make qf an array of pointers, with a new function that
> adds a new queue family to it?

I don't foresee needing to add any more fields to the struct, so I'd 
rather not have the complexity.

[-- Attachment #1.1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 637 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".