[FFmpeg-devel] [PATCH 1/2] vulkan: synchronize access to execution pool fences

[FFmpeg-devel] [PATCH 1/2] vulkan: synchronize access to execution pool fences

[Lynne]

[-- Attachment #1: Type: text/plain, Size: 99 bytes --]

vkResetFences is specified as being user-synchronized
(yet vkWaitFences, is not).

Patch attached.

[-- Attachment #2: 0001-vulkan-synchronize-access-to-execution-pool-fences.patch --]
[-- Type: text/x-diff, Size: 3269 bytes --]

From c79aa3ed01033f515cbb21251e83cb5bafdf83d7 Mon Sep 17 00:00:00 2001
From: Lynne <dev@lynne.ee>
Date: Wed, 7 Jun 2023 00:24:43 +0200
Subject: [PATCH 1/2] vulkan: synchronize access to execution pool fences

vkResetFences is specified as being user-synchronized
(yet vkWaitFences, is not).
---
 libavcodec/vulkan_decode.c |  2 +-
 libavutil/vulkan.c         | 12 ++++++++++--
 libavutil/vulkan.h         |  2 ++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/libavcodec/vulkan_decode.c b/libavcodec/vulkan_decode.c
index 889c67a15f..9e3ebf6770 100644
--- a/libavcodec/vulkan_decode.c
+++ b/libavcodec/vulkan_decode.c
@@ -1106,7 +1106,7 @@ int ff_vk_decode_init(AVCodecContext *avctx)
 
     /* Create decode exec context.
      * 4 async contexts per thread seems like a good number. */
-    err = ff_vk_exec_pool_init(s, &qf_dec, &ctx->exec_pool, 4*avctx->thread_count,
+    err = ff_vk_exec_pool_init(s, &qf_dec, &ctx->exec_pool, 1,
                                nb_q, VK_QUERY_TYPE_RESULT_STATUS_ONLY_KHR, 0,
                                session_create.pVideoProfile);
     if (err < 0)
diff --git a/libavutil/vulkan.c b/libavutil/vulkan.c
index bc4466e6c9..4b96c0c200 100644
--- a/libavutil/vulkan.c
+++ b/libavutil/vulkan.c
@@ -241,6 +241,7 @@ void ff_vk_exec_pool_free(FFVulkanContext *s, FFVkExecPool *pool)
             vk->WaitForFences(s->hwctx->act_dev, 1, &e->fence, VK_TRUE, UINT64_MAX);
             vk->DestroyFence(s->hwctx->act_dev, e->fence, s->hwctx->alloc);
         }
+        pthread_mutex_destroy(&e->lock);
 
         ff_vk_exec_discard_deps(s, e);
 
@@ -379,12 +380,17 @@ int ff_vk_exec_pool_init(FFVulkanContext *s, FFVkQueueFamilyCtx *qf,
     /* Init contexts */
     for (int i = 0; i < pool->pool_size; i++) {
         FFVkExecContext *e = &pool->contexts[i];
-
-        /* Fence */
         VkFenceCreateInfo fence_create = {
             .sType = VK_STRUCTURE_TYPE_FENCE_CREATE_INFO,
             .flags = VK_FENCE_CREATE_SIGNALED_BIT,
         };
+
+        /* Mutex */
+        err = pthread_mutex_init(&e->lock, NULL);
+        if (err != 0)
+            return AVERROR(err);
+
+        /* Fence */
         ret = vk->CreateFence(s->hwctx->act_dev, &fence_create, s->hwctx->alloc,
                               &e->fence);
         if (ret != VK_SUCCESS) {
@@ -489,8 +495,10 @@ int ff_vk_exec_start(FFVulkanContext *s, FFVkExecContext *e)
     };
 
     /* Create the fence and don't wait for it initially */
+    pthread_mutex_lock(&e->lock);
     vk->WaitForFences(s->hwctx->act_dev, 1, &e->fence, VK_TRUE, UINT64_MAX);
     vk->ResetFences(s->hwctx->act_dev, 1, &e->fence);
+    pthread_mutex_unlock(&e->lock);
 
     /* Discard queue dependencies */
     ff_vk_exec_discard_deps(s, e);
diff --git a/libavutil/vulkan.h b/libavutil/vulkan.h
index 58da720a1c..bbbc9374ae 100644
--- a/libavutil/vulkan.h
+++ b/libavutil/vulkan.h
@@ -23,6 +23,7 @@
 
 #include <stdatomic.h>
 
+#include "thread.h"
 #include "pixdesc.h"
 #include "bprint.h"
 #include "hwcontext.h"
@@ -152,6 +153,7 @@ typedef struct FFVulkanPipeline {
 typedef struct FFVkExecContext {
     int idx;
     const struct FFVkExecPool *parent;
+    pthread_mutex_t lock;
 
     /* Queue for the execution context */
     VkQueue queue;
-- 
2.40.1


[-- Attachment #3: 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".

[FFmpeg-devel] [PATCH 2/2] vulkan: discard dependencies when explicitly waiting for execution

[Lynne]

[-- Attachment #1: Type: text/plain, Size: 390 bytes --]

This reduces memory needed dramatically, as unneeded resources
can be immediately returned to the pool.
Although waitforfences is threadsafe, we add a mutex wait around
it, as the mutex fence in combination with waitforfences assures
us that no other thread will reset the fence in the meanwhile
whilst the mutex is locked. This allows is to call
ff_vk_exec_discard_deps.

Patch attached.


[-- Attachment #2: 0002-vulkan-discard-dependencies-when-explicitly-waiting-.patch --]
[-- Type: text/x-diff, Size: 1244 bytes --]

From eb74297de8662c9fa66cd719c6315567966afe56 Mon Sep 17 00:00:00 2001
From: Lynne <dev@lynne.ee>
Date: Wed, 7 Jun 2023 01:16:29 +0200
Subject: [PATCH 2/2] vulkan: discard dependencies when explicitly waiting for
 execution

This reduces memory needed dramatically, as unneeded resources
can be immediately returned to the pool.
Although waitforfences is threadsafe, we add a mutex wait around
it, as the mutex fence in combination with waitforfences assures
us that no other thread will reset the fence in the meanwhile
whilst the mutex is locked. This allows is to call
ff_vk_exec_discard_deps.
---
 libavutil/vulkan.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavutil/vulkan.c b/libavutil/vulkan.c
index 4b96c0c200..0cfe334db0 100644
--- a/libavutil/vulkan.c
+++ b/libavutil/vulkan.c
@@ -480,7 +480,10 @@ FFVkExecContext *ff_vk_exec_get(FFVkExecPool *pool)
 void ff_vk_exec_wait(FFVulkanContext *s, FFVkExecContext *e)
 {
     FFVulkanFunctions *vk = &s->vkfn;
+    pthread_mutex_lock(&e->lock);
     vk->WaitForFences(s->hwctx->act_dev, 1, &e->fence, VK_TRUE, UINT64_MAX);
+    ff_vk_exec_discard_deps(s, e);
+    pthread_mutex_unlock(&e->lock);
 }
 
 int ff_vk_exec_start(FFVulkanContext *s, FFVkExecContext *e)
-- 
2.40.1


[-- Attachment #3: 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] vulkan: synchronize access to execution pool fences

[Lynne]

Jun 7, 2023, 01:22 by dev@lynne.ee:

> vkResetFences is specified as being user-synchronized
> (yet vkWaitFences, is not).
>
> Patch attached.
>

Stray change in vulkan_decode.c removed locally.
_______________________________________________
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] vulkan: synchronize access to execution pool fences

[Lynne]

[-- Attachment #1: Type: text/plain, Size: 377 bytes --]

Jun 7, 2023, 01:45 by dev@lynne.ee:

> Jun 7, 2023, 01:22 by dev@lynne.ee:
>
>> vkResetFences is specified as being user-synchronized
>> (yet vkWaitFences, is not).
>>
>> Patch attached.
>>
>
> Stray change in vulkan_decode.c removed locally.
>

Also removed the vkWaitForFences call during
the _start function from the mutex lock, as
it's safe to do so, and added a comment.


[-- Attachment #2: v2-0001-vulkan-synchronize-access-to-execution-pool-fence.patch --]
[-- Type: text/x-diff, Size: 2749 bytes --]

From ef13ab7974ea3833e1063dfd14e92550ad27b53b Mon Sep 17 00:00:00 2001
From: Lynne <dev@lynne.ee>
Date: Wed, 7 Jun 2023 00:24:43 +0200
Subject: [PATCH v2 1/2] vulkan: synchronize access to execution pool fences

vkResetFences is specified as being user-synchronized
(yet vkWaitFences, is not).
---
 libavutil/vulkan.c | 16 +++++++++++++---
 libavutil/vulkan.h |  2 ++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/libavutil/vulkan.c b/libavutil/vulkan.c
index bc4466e6c9..b1c585292e 100644
--- a/libavutil/vulkan.c
+++ b/libavutil/vulkan.c
@@ -241,6 +241,7 @@ void ff_vk_exec_pool_free(FFVulkanContext *s, FFVkExecPool *pool)
             vk->WaitForFences(s->hwctx->act_dev, 1, &e->fence, VK_TRUE, UINT64_MAX);
             vk->DestroyFence(s->hwctx->act_dev, e->fence, s->hwctx->alloc);
         }
+        pthread_mutex_destroy(&e->lock);
 
         ff_vk_exec_discard_deps(s, e);
 
@@ -379,12 +380,17 @@ int ff_vk_exec_pool_init(FFVulkanContext *s, FFVkQueueFamilyCtx *qf,
     /* Init contexts */
     for (int i = 0; i < pool->pool_size; i++) {
         FFVkExecContext *e = &pool->contexts[i];
-
-        /* Fence */
         VkFenceCreateInfo fence_create = {
             .sType = VK_STRUCTURE_TYPE_FENCE_CREATE_INFO,
             .flags = VK_FENCE_CREATE_SIGNALED_BIT,
         };
+
+        /* Mutex */
+        err = pthread_mutex_init(&e->lock, NULL);
+        if (err != 0)
+            return AVERROR(err);
+
+        /* Fence */
         ret = vk->CreateFence(s->hwctx->act_dev, &fence_create, s->hwctx->alloc,
                               &e->fence);
         if (ret != VK_SUCCESS) {
@@ -488,9 +494,13 @@ int ff_vk_exec_start(FFVulkanContext *s, FFVkExecContext *e)
         .flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT,
     };
 
-    /* Create the fence and don't wait for it initially */
+    /* Wait for the fence to be signalled */
     vk->WaitForFences(s->hwctx->act_dev, 1, &e->fence, VK_TRUE, UINT64_MAX);
+
+    /* vkResetFences is defined as being host-synchronized */
+    pthread_mutex_lock(&e->lock);
     vk->ResetFences(s->hwctx->act_dev, 1, &e->fence);
+    pthread_mutex_unlock(&e->lock);
 
     /* Discard queue dependencies */
     ff_vk_exec_discard_deps(s, e);
diff --git a/libavutil/vulkan.h b/libavutil/vulkan.h
index 58da720a1c..bbbc9374ae 100644
--- a/libavutil/vulkan.h
+++ b/libavutil/vulkan.h
@@ -23,6 +23,7 @@
 
 #include <stdatomic.h>
 
+#include "thread.h"
 #include "pixdesc.h"
 #include "bprint.h"
 #include "hwcontext.h"
@@ -152,6 +153,7 @@ typedef struct FFVulkanPipeline {
 typedef struct FFVkExecContext {
     int idx;
     const struct FFVkExecPool *parent;
+    pthread_mutex_t lock;
 
     /* Queue for the execution context */
     VkQueue queue;
-- 
2.40.1


[-- Attachment #3: 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] vulkan: synchronize access to execution pool fences

[Philip Langdale]

On Wed, 7 Jun 2023 01:22:25 +0200 (CEST)
Lynne <dev@lynne.ee> wrote:

> From c79aa3ed01033f515cbb21251e83cb5bafdf83d7 Mon Sep 17 00:00:00 2001
> From: Lynne <dev@lynne.ee>
> Date: Wed, 7 Jun 2023 00:24:43 +0200
> Subject: [PATCH 1/2] vulkan: synchronize access to execution pool
> fences
> 
> vkResetFences is specified as being user-synchronized
> (yet vkWaitFences, is not).
> ---
>  libavcodec/vulkan_decode.c |  2 +-
>  libavutil/vulkan.c         | 12 ++++++++++--
>  libavutil/vulkan.h         |  2 ++
>  3 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/vulkan_decode.c b/libavcodec/vulkan_decode.c
> index 889c67a15f..9e3ebf6770 100644
> --- a/libavcodec/vulkan_decode.c
> +++ b/libavcodec/vulkan_decode.c
> @@ -1106,7 +1106,7 @@ int ff_vk_decode_init(AVCodecContext *avctx)
>  
>      /* Create decode exec context.
>       * 4 async contexts per thread seems like a good number. */
> -    err = ff_vk_exec_pool_init(s, &qf_dec, &ctx->exec_pool,
> 4*avctx->thread_count,
> +    err = ff_vk_exec_pool_init(s, &qf_dec, &ctx->exec_pool, 1,
>                                 nb_q,

Comment is now out of date?

> VK_QUERY_TYPE_RESULT_STATUS_ONLY_KHR, 0,
> session_create.pVideoProfile); if (err < 0)
> diff --git a/libavutil/vulkan.c b/libavutil/vulkan.c
> index bc4466e6c9..4b96c0c200 100644
> --- a/libavutil/vulkan.c
> +++ b/libavutil/vulkan.c
> @@ -241,6 +241,7 @@ void ff_vk_exec_pool_free(FFVulkanContext *s,
> FFVkExecPool *pool) vk->WaitForFences(s->hwctx->act_dev, 1,
> &e->fence, VK_TRUE, UINT64_MAX); vk->DestroyFence(s->hwctx->act_dev,
> e->fence, s->hwctx->alloc); }
> +        pthread_mutex_destroy(&e->lock);
>  
>          ff_vk_exec_discard_deps(s, e);
>  
> @@ -379,12 +380,17 @@ int ff_vk_exec_pool_init(FFVulkanContext *s,
> FFVkQueueFamilyCtx *qf, /* Init contexts */
>      for (int i = 0; i < pool->pool_size; i++) {
>          FFVkExecContext *e = &pool->contexts[i];
> -
> -        /* Fence */
>          VkFenceCreateInfo fence_create = {
>              .sType = VK_STRUCTURE_TYPE_FENCE_CREATE_INFO,
>              .flags = VK_FENCE_CREATE_SIGNALED_BIT,
>          };
> +
> +        /* Mutex */
> +        err = pthread_mutex_init(&e->lock, NULL);
> +        if (err != 0)
> +            return AVERROR(err);
> +
> +        /* Fence */
>          ret = vk->CreateFence(s->hwctx->act_dev, &fence_create,
> s->hwctx->alloc, &e->fence);
>          if (ret != VK_SUCCESS) {
> @@ -489,8 +495,10 @@ int ff_vk_exec_start(FFVulkanContext *s,
> FFVkExecContext *e) };
>  
>      /* Create the fence and don't wait for it initially */
> +    pthread_mutex_lock(&e->lock);
>      vk->WaitForFences(s->hwctx->act_dev, 1, &e->fence, VK_TRUE,
> UINT64_MAX); vk->ResetFences(s->hwctx->act_dev, 1, &e->fence);
> +    pthread_mutex_unlock(&e->lock);

If WaitForFences doesn't require synchronisation, would it be desirable
to call it outside the mutex locking? 

Otherwise, LGTM.

--phil
_______________________________________________
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] vulkan: synchronize access to execution pool fences

[Philip Langdale]

On Wed, 7 Jun 2023 01:59:55 +0200 (CEST)
Lynne <dev@lynne.ee> wrote:

> Jun 7, 2023, 01:45 by dev@lynne.ee:
> 
> > Jun 7, 2023, 01:22 by dev@lynne.ee:
> >
> >> vkResetFences is specified as being user-synchronized
> >> (yet vkWaitFences, is not).
> >>
> >> Patch attached.
> >>
> >
> > Stray change in vulkan_decode.c removed locally.
> >
> 
> Also removed the vkWaitForFences call during
> the _start function from the mutex lock, as
> it's safe to do so, and added a comment.
> 

Ah, you updated the diff.

This one is LGTM.

--phil
_______________________________________________
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 2/2] vulkan: discard dependencies when explicitly waiting for execution

[Philip Langdale]

On Wed, 7 Jun 2023 01:23:24 +0200 (CEST)
Lynne <dev@lynne.ee> wrote:

> From eb74297de8662c9fa66cd719c6315567966afe56 Mon Sep 17 00:00:00 2001
> From: Lynne <dev@lynne.ee>
> Date: Wed, 7 Jun 2023 01:16:29 +0200
> Subject: [PATCH 2/2] vulkan: discard dependencies when explicitly
> waiting for execution
> 
> This reduces memory needed dramatically, as unneeded resources
> can be immediately returned to the pool.
> Although waitforfences is threadsafe, we add a mutex wait around
> it, as the mutex fence in combination with waitforfences assures
> us that no other thread will reset the fence in the meanwhile
> whilst the mutex is locked. This allows is to call
> ff_vk_exec_discard_deps.
> ---
>  libavutil/vulkan.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/libavutil/vulkan.c b/libavutil/vulkan.c
> index 4b96c0c200..0cfe334db0 100644
> --- a/libavutil/vulkan.c
> +++ b/libavutil/vulkan.c
> @@ -480,7 +480,10 @@ FFVkExecContext *ff_vk_exec_get(FFVkExecPool
> *pool) void ff_vk_exec_wait(FFVulkanContext *s, FFVkExecContext *e)
>  {
>      FFVulkanFunctions *vk = &s->vkfn;
> +    pthread_mutex_lock(&e->lock);
>      vk->WaitForFences(s->hwctx->act_dev, 1, &e->fence, VK_TRUE,
> UINT64_MAX);
> +    ff_vk_exec_discard_deps(s, e);
> +    pthread_mutex_unlock(&e->lock);
>  }
>  
>  int ff_vk_exec_start(FFVulkanContext *s, FFVkExecContext *e)

LGTM.


--phil
_______________________________________________
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 2/2] vulkan: discard dependencies when explicitly waiting for execution

[Lynne]

Jun 7, 2023, 23:33 by philipl@overt.org:

> On Wed, 7 Jun 2023 01:23:24 +0200 (CEST)
> Lynne <dev@lynne.ee> wrote:
>
>> From eb74297de8662c9fa66cd719c6315567966afe56 Mon Sep 17 00:00:00 2001
>> From: Lynne <dev@lynne.ee>
>> Date: Wed, 7 Jun 2023 01:16:29 +0200
>> Subject: [PATCH 2/2] vulkan: discard dependencies when explicitly
>> waiting for execution
>>
>> This reduces memory needed dramatically, as unneeded resources
>> can be immediately returned to the pool.
>> Although waitforfences is threadsafe, we add a mutex wait around
>> it, as the mutex fence in combination with waitforfences assures
>> us that no other thread will reset the fence in the meanwhile
>> whilst the mutex is locked. This allows is to call
>> ff_vk_exec_discard_deps.
>> ---
>>  libavutil/vulkan.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/libavutil/vulkan.c b/libavutil/vulkan.c
>> index 4b96c0c200..0cfe334db0 100644
>> --- a/libavutil/vulkan.c
>> +++ b/libavutil/vulkan.c
>> @@ -480,7 +480,10 @@ FFVkExecContext *ff_vk_exec_get(FFVkExecPool
>> *pool) void ff_vk_exec_wait(FFVulkanContext *s, FFVkExecContext *e)
>>  {
>>  FFVulkanFunctions *vk = &s->vkfn;
>> +    pthread_mutex_lock(&e->lock);
>>  vk->WaitForFences(s->hwctx->act_dev, 1, &e->fence, VK_TRUE,
>> UINT64_MAX);
>> +    ff_vk_exec_discard_deps(s, e);
>> +    pthread_mutex_unlock(&e->lock);
>>  }
>>  
>>  int ff_vk_exec_start(FFVulkanContext *s, FFVkExecContext *e)
>>
>
> LGTM.
>

Thanks, pushed.
_______________________________________________
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] vulkan: synchronize access to execution pool fences

[Lynne]

Jun 7, 2023, 23:31 by philipl@overt.org:

> On Wed, 7 Jun 2023 01:59:55 +0200 (CEST)
> Lynne <dev@lynne.ee> wrote:
>
>> Jun 7, 2023, 01:45 by dev@lynne.ee:
>>
>> > Jun 7, 2023, 01:22 by dev@lynne.ee:
>> >
>> >> vkResetFences is specified as being user-synchronized
>> >> (yet vkWaitFences, is not).
>> >>
>> >> Patch attached.
>> >>
>> >
>> > Stray change in vulkan_decode.c removed locally.
>> >
>>
>> Also removed the vkWaitForFences call during
>> the _start function from the mutex lock, as
>> it's safe to do so, and added a comment.
>>
>
> Ah, you updated the diff.
>
> This one is LGTM.
>

Thanks, pushed.
_______________________________________________
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".