* [FFmpeg-devel] [PATCH] libavutil/hwcontext_qsv: Make qsv hardware transfers thread safe
@ 2024-04-13 12:57 Mark Samuelson
2024-04-17 8:15 ` Xiang, Haihao
0 siblings, 1 reply; 4+ messages in thread
From: Mark Samuelson @ 2024-04-13 12:57 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Mark Samuelson
The QSV hardware context currently uses pthreads to lock initilization,
which is not available on windows builds. Instead, use the AVMutex
object. Also lock uses of the realigned_upload_frame and
realigned_download_frame objects, so multiple threads do not attempt
to write to them at the same time.
---
libavutil/hwcontext_qsv.c | 75 ++++++++++++++++++++++++---------------
1 file changed, 46 insertions(+), 29 deletions(-)
diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
index c7c7878644..92bab134e4 100644
--- a/libavutil/hwcontext_qsv.c
+++ b/libavutil/hwcontext_qsv.c
@@ -23,10 +23,7 @@
#include <mfxvideo.h>
#include "config.h"
-
-#if HAVE_PTHREADS
-#include <pthread.h>
-#endif
+#include "thread.h"
#define COBJMACROS
#if CONFIG_VAAPI
@@ -98,9 +95,7 @@ typedef struct QSVFramesContext {
atomic_int session_download_init;
mfxSession session_upload;
atomic_int session_upload_init;
-#if HAVE_PTHREADS
- pthread_mutex_t session_lock;
-#endif
+ AVMutex session_lock;
AVBufferRef *child_frames_ref;
mfxFrameSurface1 *surfaces_internal;
@@ -354,9 +349,7 @@ static void qsv_frames_uninit(AVHWFramesContext *ctx)
s->session_upload = NULL;
s->session_upload_init = 0;
-#if HAVE_PTHREADS
- pthread_mutex_destroy(&s->session_lock);
-#endif
+ ff_mutex_destroy(&s->session_lock);
av_freep(&s->mem_ids);
#if QSV_HAVE_OPAQUE
@@ -1302,9 +1295,7 @@ static int qsv_frames_init(AVHWFramesContext *ctx)
s->session_download_init = 0;
s->session_upload_init = 0;
-#if HAVE_PTHREADS
- pthread_mutex_init(&s->session_lock, NULL);
-#endif
+ ff_mutex_init(&s->session_lock, NULL);
return 0;
}
@@ -1629,24 +1620,20 @@ static int qsv_internal_session_check_init(AVHWFramesContext *ctx, int upload)
if (atomic_load(inited))
return 0;
-#if HAVE_PTHREADS
- pthread_mutex_lock(&s->session_lock);
-#endif
+ ff_mutex_lock(&s->session_lock);
if (!atomic_load(inited)) {
ret = qsv_init_internal_session(ctx, session, upload);
atomic_store(inited, 1);
}
-#if HAVE_PTHREADS
- pthread_mutex_unlock(&s->session_lock);
-#endif
+ ff_mutex_unlock(&s->session_lock);
return ret;
}
-static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
- const AVFrame *src)
+static int qsv_transfer_data_from_internal(AVHWFramesContext *ctx, AVFrame *dst,
+ const AVFrame *src, int realigned)
{
QSVFramesContext *s = ctx->hwctx;
mfxFrameSurface1 out = {{ 0 }};
@@ -1658,7 +1645,6 @@ static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
/* download to temp frame if the output is not padded as libmfx requires */
AVFrame *tmp_frame = &s->realigned_download_frame;
AVFrame *dst_frame;
- int realigned = 0;
ret = qsv_internal_session_check_init(ctx, 0);
if (ret < 0)
@@ -1667,8 +1653,7 @@ static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
/* According to MSDK spec for mfxframeinfo, "Width must be a multiple of 16.
* Height must be a multiple of 16 for progressive frame sequence and a
* multiple of 32 otherwise.", so allign all frames to 16 before downloading. */
- if (dst->height & 15 || dst->linesize[0] & 15) {
- realigned = 1;
+ if (realigned) {
if (tmp_frame->format != dst->format ||
tmp_frame->width != FFALIGN(dst->linesize[0], 16) ||
tmp_frame->height != FFALIGN(dst->height, 16)) {
@@ -1728,8 +1713,25 @@ static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
return 0;
}
-static int qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
- const AVFrame *src)
+static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
+ const AVFrame *src)
+{
+ QSVFramesContext *s = ctx->hwctx;
+ int realigned = 0;
+ if (dst->height & 15 || dst->linesize[0] & 15)
+ {
+ realigned = 1;
+ ff_mutex_lock(&s->session_lock);
+ }
+ int ret = qsv_transfer_data_from_internal(ctx, dst, src, realigned);
+ if (realigned)
+ ff_mutex_unlock(&s->session_lock);
+
+ return ret;
+}
+
+static int qsv_transfer_data_to_internal(AVHWFramesContext *ctx, AVFrame *dst,
+ const AVFrame *src, int realigned)
{
QSVFramesContext *s = ctx->hwctx;
mfxFrameSurface1 in = {{ 0 }};
@@ -1742,7 +1744,6 @@ static int qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
/* make a copy if the input is not padded as libmfx requires */
AVFrame *tmp_frame = &s->realigned_upload_frame;
const AVFrame *src_frame;
- int realigned = 0;
ret = qsv_internal_session_check_init(ctx, 1);
if (ret < 0)
@@ -1751,8 +1752,7 @@ static int qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
/* According to MSDK spec for mfxframeinfo, "Width must be a multiple of 16.
* Height must be a multiple of 16 for progressive frame sequence and a
* multiple of 32 otherwise.", so allign all frames to 16 before uploading. */
- if (src->height & 15 || src->linesize[0] & 15) {
- realigned = 1;
+ if (realigned) {
if (tmp_frame->format != src->format ||
tmp_frame->width != FFALIGN(src->width, 16) ||
tmp_frame->height != FFALIGN(src->height, 16)) {
@@ -1821,6 +1821,23 @@ static int qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
return 0;
}
+static int qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
+ const AVFrame *src)
+{
+ QSVFramesContext *s = ctx->hwctx;
+ int realigned = 0;
+ if (src->height & 15 || src->linesize[0] & 15)
+ {
+ realigned = 1;
+ ff_mutex_lock(&s->session_lock);
+ }
+ int ret = qsv_transfer_data_to_internal(ctx, dst, src, realigned);
+ if (realigned)
+ ff_mutex_unlock(&s->session_lock);
+
+ return ret;
+}
+
static int qsv_frames_derive_to(AVHWFramesContext *dst_ctx,
AVHWFramesContext *src_ctx, int flags)
{
--
2.44.0.windows.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [FFmpeg-devel] [PATCH] libavutil/hwcontext_qsv: Make qsv hardware transfers thread safe
2024-04-13 12:57 [FFmpeg-devel] [PATCH] libavutil/hwcontext_qsv: Make qsv hardware transfers thread safe Mark Samuelson
@ 2024-04-17 8:15 ` Xiang, Haihao
2024-04-17 14:46 ` Mark Samuelson
0 siblings, 1 reply; 4+ messages in thread
From: Xiang, Haihao @ 2024-04-17 8:15 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: samuelsonm
On Sa, 2024-04-13 at 07:57 -0500, Mark Samuelson wrote:
> The QSV hardware context currently uses pthreads to lock initilization,
> which is not available on windows builds. Instead, use the AVMutex
> object. Also lock uses of the realigned_upload_frame and
> realigned_download_frame objects, so multiple threads do not attempt
> to write to them at the same time.
> ---
> libavutil/hwcontext_qsv.c | 75 ++++++++++++++++++++++++---------------
> 1 file changed, 46 insertions(+), 29 deletions(-)
>
> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> index c7c7878644..92bab134e4 100644
> --- a/libavutil/hwcontext_qsv.c
> +++ b/libavutil/hwcontext_qsv.c
> @@ -23,10 +23,7 @@
> #include <mfxvideo.h>
>
> #include "config.h"
> -
> -#if HAVE_PTHREADS
> -#include <pthread.h>
> -#endif
> +#include "thread.h"
>
> #define COBJMACROS
> #if CONFIG_VAAPI
> @@ -98,9 +95,7 @@ typedef struct QSVFramesContext {
> atomic_int session_download_init;
> mfxSession session_upload;
> atomic_int session_upload_init;
> -#if HAVE_PTHREADS
> - pthread_mutex_t session_lock;
> -#endif
> + AVMutex session_lock;
>
> AVBufferRef *child_frames_ref;
> mfxFrameSurface1 *surfaces_internal;
> @@ -354,9 +349,7 @@ static void qsv_frames_uninit(AVHWFramesContext *ctx)
> s->session_upload = NULL;
> s->session_upload_init = 0;
>
> -#if HAVE_PTHREADS
> - pthread_mutex_destroy(&s->session_lock);
> -#endif
> + ff_mutex_destroy(&s->session_lock);
>
> av_freep(&s->mem_ids);
> #if QSV_HAVE_OPAQUE
> @@ -1302,9 +1295,7 @@ static int qsv_frames_init(AVHWFramesContext *ctx)
> s->session_download_init = 0;
> s->session_upload_init = 0;
>
> -#if HAVE_PTHREADS
> - pthread_mutex_init(&s->session_lock, NULL);
> -#endif
> + ff_mutex_init(&s->session_lock, NULL);
>
> return 0;
> }
> @@ -1629,24 +1620,20 @@ static int
> qsv_internal_session_check_init(AVHWFramesContext *ctx, int upload)
> if (atomic_load(inited))
> return 0;
>
> -#if HAVE_PTHREADS
> - pthread_mutex_lock(&s->session_lock);
> -#endif
> + ff_mutex_lock(&s->session_lock);
ff_mutex_lock is called twice in the same thread when realigned is 1, the
current thread is blocked. You may run the command below to reproduce this
issue:
./ffmpeg -init_hw_device qsv -f lavfi -i testsrc=size=352x280,format=nv12 -vf
"hwupload=extra_hw_frames=16" -f null -
>
> if (!atomic_load(inited)) {
> ret = qsv_init_internal_session(ctx, session, upload);
> atomic_store(inited, 1);
> }
>
> -#if HAVE_PTHREADS
> - pthread_mutex_unlock(&s->session_lock);
> -#endif
> + ff_mutex_unlock(&s->session_lock);
>
> return ret;
> }
>
> -static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
> - const AVFrame *src)
> +static int qsv_transfer_data_from_internal(AVHWFramesContext *ctx, AVFrame
> *dst,
> + const AVFrame *src, int realigned)
> {
> QSVFramesContext *s = ctx->hwctx;
> mfxFrameSurface1 out = {{ 0 }};
> @@ -1658,7 +1645,6 @@ static int qsv_transfer_data_from(AVHWFramesContext
> *ctx, AVFrame *dst,
> /* download to temp frame if the output is not padded as libmfx requires
> */
> AVFrame *tmp_frame = &s->realigned_download_frame;
> AVFrame *dst_frame;
> - int realigned = 0;
>
> ret = qsv_internal_session_check_init(ctx, 0);
> if (ret < 0)
> @@ -1667,8 +1653,7 @@ static int qsv_transfer_data_from(AVHWFramesContext
> *ctx, AVFrame *dst,
> /* According to MSDK spec for mfxframeinfo, "Width must be a multiple of
> 16.
> * Height must be a multiple of 16 for progressive frame sequence and a
> * multiple of 32 otherwise.", so allign all frames to 16 before
> downloading. */
> - if (dst->height & 15 || dst->linesize[0] & 15) {
> - realigned = 1;
> + if (realigned) {
> if (tmp_frame->format != dst->format ||
> tmp_frame->width != FFALIGN(dst->linesize[0], 16) ||
> tmp_frame->height != FFALIGN(dst->height, 16)) {
> @@ -1728,8 +1713,25 @@ static int qsv_transfer_data_from(AVHWFramesContext
> *ctx, AVFrame *dst,
> return 0;
> }
>
> -static int qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
> - const AVFrame *src)
> +static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
> + const AVFrame *src)
> +{
> + QSVFramesContext *s = ctx->hwctx;
Please do not use tab for indentation.
> + int realigned = 0;
> + if (dst->height & 15 || dst->linesize[0] & 15)
> + {
{ and if statement should be on the same line to keep the consistent code style.
> + realigned = 1;
> + ff_mutex_lock(&s->session_lock);
> + }
> + int ret = qsv_transfer_data_from_internal(ctx, dst, src, realigned);
Please do not mix declaration and code.
> + if (realigned)
> + ff_mutex_unlock(&s->session_lock);
> +
> + return ret;
> +}
> +
> +static int qsv_transfer_data_to_internal(AVHWFramesContext *ctx, AVFrame
> *dst,
> + const AVFrame *src, int realigned)
> {
> QSVFramesContext *s = ctx->hwctx;
> mfxFrameSurface1 in = {{ 0 }};
> @@ -1742,7 +1744,6 @@ static int qsv_transfer_data_to(AVHWFramesContext *ctx,
> AVFrame *dst,
> /* make a copy if the input is not padded as libmfx requires */
> AVFrame *tmp_frame = &s->realigned_upload_frame;
> const AVFrame *src_frame;
> - int realigned = 0;
>
> ret = qsv_internal_session_check_init(ctx, 1);
> if (ret < 0)
> @@ -1751,8 +1752,7 @@ static int qsv_transfer_data_to(AVHWFramesContext *ctx,
> AVFrame *dst,
> /* According to MSDK spec for mfxframeinfo, "Width must be a multiple of
> 16.
> * Height must be a multiple of 16 for progressive frame sequence and a
> * multiple of 32 otherwise.", so allign all frames to 16 before
> uploading. */
> - if (src->height & 15 || src->linesize[0] & 15) {
> - realigned = 1;
> + if (realigned) {
> if (tmp_frame->format != src->format ||
> tmp_frame->width != FFALIGN(src->width, 16) ||
> tmp_frame->height != FFALIGN(src->height, 16)) {
> @@ -1821,6 +1821,23 @@ static int qsv_transfer_data_to(AVHWFramesContext *ctx,
> AVFrame *dst,
> return 0;
> }
>
> +static int qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
> + const AVFrame *src)
> +{
> + QSVFramesContext *s = ctx->hwctx;
Please do not use tab for indentation too.
> + int realigned = 0;
> + if (src->height & 15 || src->linesize[0] & 15)
> + {
{ and if statement should be on the same line
> + realigned = 1;
> + ff_mutex_lock(&s->session_lock);
> + }
> + int ret = qsv_transfer_data_to_internal(ctx, dst, src, realigned);
Please do not mix declaration and code.
Thanks
Haihao
> + if (realigned)
> + ff_mutex_unlock(&s->session_lock);
> +
> + return ret;
> +}
> +
> static int qsv_frames_derive_to(AVHWFramesContext *dst_ctx,
> AVHWFramesContext *src_ctx, int flags)
> {
_______________________________________________
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".
^ permalink raw reply [flat|nested] 4+ messages in thread* [FFmpeg-devel] [PATCH] libavutil/hwcontext_qsv: Make qsv hardware transfers thread safe
2024-04-17 8:15 ` Xiang, Haihao
@ 2024-04-17 14:46 ` Mark Samuelson
2024-04-24 5:06 ` Xiang, Haihao
0 siblings, 1 reply; 4+ messages in thread
From: Mark Samuelson @ 2024-04-17 14:46 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Mark Samuelson
The QSV hardware context currently uses pthreads to lock initilization,
which is not available on windows builds. Instead, use the AVMutex
object. Also lock uses of the realigned_upload_frame and
realigned_download_frame objects, so multiple threads do not attempt
to write to them at the same time.
---
Here is a new patch addressing your comments
Fixed the nested calls to ff_mutex_lock
Fixed the two accidental tabs
Fixed the two violations of K&R style
Fixed the two incidents of mixing declaration and code
libavutil/hwcontext_qsv.c | 93 +++++++++++++++++++++++----------------
1 file changed, 56 insertions(+), 37 deletions(-)
diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
index c7c7878644..ed462d440a 100644
--- a/libavutil/hwcontext_qsv.c
+++ b/libavutil/hwcontext_qsv.c
@@ -23,10 +23,7 @@
#include <mfxvideo.h>
#include "config.h"
-
-#if HAVE_PTHREADS
-#include <pthread.h>
-#endif
+#include "thread.h"
#define COBJMACROS
#if CONFIG_VAAPI
@@ -98,9 +95,7 @@ typedef struct QSVFramesContext {
atomic_int session_download_init;
mfxSession session_upload;
atomic_int session_upload_init;
-#if HAVE_PTHREADS
- pthread_mutex_t session_lock;
-#endif
+ AVMutex session_lock;
AVBufferRef *child_frames_ref;
mfxFrameSurface1 *surfaces_internal;
@@ -354,9 +349,7 @@ static void qsv_frames_uninit(AVHWFramesContext *ctx)
s->session_upload = NULL;
s->session_upload_init = 0;
-#if HAVE_PTHREADS
- pthread_mutex_destroy(&s->session_lock);
-#endif
+ ff_mutex_destroy(&s->session_lock);
av_freep(&s->mem_ids);
#if QSV_HAVE_OPAQUE
@@ -1302,9 +1295,7 @@ static int qsv_frames_init(AVHWFramesContext *ctx)
s->session_download_init = 0;
s->session_upload_init = 0;
-#if HAVE_PTHREADS
- pthread_mutex_init(&s->session_lock, NULL);
-#endif
+ ff_mutex_init(&s->session_lock, NULL);
return 0;
}
@@ -1629,24 +1620,20 @@ static int qsv_internal_session_check_init(AVHWFramesContext *ctx, int upload)
if (atomic_load(inited))
return 0;
-#if HAVE_PTHREADS
- pthread_mutex_lock(&s->session_lock);
-#endif
+ ff_mutex_lock(&s->session_lock);
if (!atomic_load(inited)) {
ret = qsv_init_internal_session(ctx, session, upload);
atomic_store(inited, 1);
}
-#if HAVE_PTHREADS
- pthread_mutex_unlock(&s->session_lock);
-#endif
+ ff_mutex_unlock(&s->session_lock);
return ret;
}
-static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
- const AVFrame *src)
+static int qsv_transfer_data_from_internal(AVHWFramesContext *ctx, AVFrame *dst,
+ const AVFrame *src, int realigned)
{
QSVFramesContext *s = ctx->hwctx;
mfxFrameSurface1 out = {{ 0 }};
@@ -1658,17 +1645,11 @@ static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
/* download to temp frame if the output is not padded as libmfx requires */
AVFrame *tmp_frame = &s->realigned_download_frame;
AVFrame *dst_frame;
- int realigned = 0;
-
- ret = qsv_internal_session_check_init(ctx, 0);
- if (ret < 0)
- return ret;
/* According to MSDK spec for mfxframeinfo, "Width must be a multiple of 16.
* Height must be a multiple of 16 for progressive frame sequence and a
* multiple of 32 otherwise.", so allign all frames to 16 before downloading. */
- if (dst->height & 15 || dst->linesize[0] & 15) {
- realigned = 1;
+ if (realigned) {
if (tmp_frame->format != dst->format ||
tmp_frame->width != FFALIGN(dst->linesize[0], 16) ||
tmp_frame->height != FFALIGN(dst->height, 16)) {
@@ -1728,8 +1709,30 @@ static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
return 0;
}
-static int qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
- const AVFrame *src)
+static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
+ const AVFrame *src)
+{
+ QSVFramesContext *s = ctx->hwctx;
+ int realigned = 0;
+ int ret = 0;
+
+ ret = qsv_internal_session_check_init(ctx, 0);
+ if (ret < 0)
+ return ret;
+
+ if (dst->height & 15 || dst->linesize[0] & 15) {
+ realigned = 1;
+ ff_mutex_lock(&s->session_lock);
+ }
+ ret = qsv_transfer_data_from_internal(ctx, dst, src, realigned);
+ if (realigned)
+ ff_mutex_unlock(&s->session_lock);
+
+ return ret;
+}
+
+static int qsv_transfer_data_to_internal(AVHWFramesContext *ctx, AVFrame *dst,
+ const AVFrame *src, int realigned)
{
QSVFramesContext *s = ctx->hwctx;
mfxFrameSurface1 in = {{ 0 }};
@@ -1742,17 +1745,11 @@ static int qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
/* make a copy if the input is not padded as libmfx requires */
AVFrame *tmp_frame = &s->realigned_upload_frame;
const AVFrame *src_frame;
- int realigned = 0;
-
- ret = qsv_internal_session_check_init(ctx, 1);
- if (ret < 0)
- return ret;
/* According to MSDK spec for mfxframeinfo, "Width must be a multiple of 16.
* Height must be a multiple of 16 for progressive frame sequence and a
* multiple of 32 otherwise.", so allign all frames to 16 before uploading. */
- if (src->height & 15 || src->linesize[0] & 15) {
- realigned = 1;
+ if (realigned) {
if (tmp_frame->format != src->format ||
tmp_frame->width != FFALIGN(src->width, 16) ||
tmp_frame->height != FFALIGN(src->height, 16)) {
@@ -1821,6 +1818,28 @@ static int qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
return 0;
}
+static int qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
+ const AVFrame *src)
+{
+ QSVFramesContext *s = ctx->hwctx;
+ int realigned = 0;
+ int ret = 0;
+
+ ret = qsv_internal_session_check_init(ctx, 1);
+ if (ret < 0)
+ return ret;
+
+ if (src->height & 15 || src->linesize[0] & 15) {
+ realigned = 1;
+ ff_mutex_lock(&s->session_lock);
+ }
+ ret = qsv_transfer_data_to_internal(ctx, dst, src, realigned);
+ if (realigned)
+ ff_mutex_unlock(&s->session_lock);
+
+ return ret;
+}
+
static int qsv_frames_derive_to(AVHWFramesContext *dst_ctx,
AVHWFramesContext *src_ctx, int flags)
{
--
2.44.0.windows.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [FFmpeg-devel] [PATCH] libavutil/hwcontext_qsv: Make qsv hardware transfers thread safe
2024-04-17 14:46 ` Mark Samuelson
@ 2024-04-24 5:06 ` Xiang, Haihao
0 siblings, 0 replies; 4+ messages in thread
From: Xiang, Haihao @ 2024-04-24 5:06 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: samuelsonm
On Wo, 2024-04-17 at 09:46 -0500, Mark Samuelson wrote:
> The QSV hardware context currently uses pthreads to lock initilization,
> which is not available on windows builds. Instead, use the AVMutex
> object. Also lock uses of the realigned_upload_frame and
> realigned_download_frame objects, so multiple threads do not attempt
> to write to them at the same time.
> ---
>
> Here is a new patch addressing your comments
> Fixed the nested calls to ff_mutex_lock
> Fixed the two accidental tabs
> Fixed the two violations of K&R style
> Fixed the two incidents of mixing declaration and code
>
>
> libavutil/hwcontext_qsv.c | 93 +++++++++++++++++++++++----------------
> 1 file changed, 56 insertions(+), 37 deletions(-)
>
> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> index c7c7878644..ed462d440a 100644
> --- a/libavutil/hwcontext_qsv.c
> +++ b/libavutil/hwcontext_qsv.c
> @@ -23,10 +23,7 @@
> #include <mfxvideo.h>
>
> #include "config.h"
> -
> -#if HAVE_PTHREADS
> -#include <pthread.h>
> -#endif
> +#include "thread.h"
>
> #define COBJMACROS
> #if CONFIG_VAAPI
> @@ -98,9 +95,7 @@ typedef struct QSVFramesContext {
> atomic_int session_download_init;
> mfxSession session_upload;
> atomic_int session_upload_init;
> -#if HAVE_PTHREADS
> - pthread_mutex_t session_lock;
> -#endif
> + AVMutex session_lock;
>
> AVBufferRef *child_frames_ref;
> mfxFrameSurface1 *surfaces_internal;
> @@ -354,9 +349,7 @@ static void qsv_frames_uninit(AVHWFramesContext *ctx)
> s->session_upload = NULL;
> s->session_upload_init = 0;
>
> -#if HAVE_PTHREADS
> - pthread_mutex_destroy(&s->session_lock);
> -#endif
> + ff_mutex_destroy(&s->session_lock);
>
> av_freep(&s->mem_ids);
> #if QSV_HAVE_OPAQUE
> @@ -1302,9 +1295,7 @@ static int qsv_frames_init(AVHWFramesContext *ctx)
> s->session_download_init = 0;
> s->session_upload_init = 0;
>
> -#if HAVE_PTHREADS
> - pthread_mutex_init(&s->session_lock, NULL);
> -#endif
> + ff_mutex_init(&s->session_lock, NULL);
>
> return 0;
> }
> @@ -1629,24 +1620,20 @@ static int
> qsv_internal_session_check_init(AVHWFramesContext *ctx, int upload)
> if (atomic_load(inited))
> return 0;
>
> -#if HAVE_PTHREADS
> - pthread_mutex_lock(&s->session_lock);
> -#endif
> + ff_mutex_lock(&s->session_lock);
>
> if (!atomic_load(inited)) {
> ret = qsv_init_internal_session(ctx, session, upload);
> atomic_store(inited, 1);
> }
>
> -#if HAVE_PTHREADS
> - pthread_mutex_unlock(&s->session_lock);
> -#endif
> + ff_mutex_unlock(&s->session_lock);
>
> return ret;
> }
>
> -static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
> - const AVFrame *src)
> +static int qsv_transfer_data_from_internal(AVHWFramesContext *ctx, AVFrame
> *dst,
> + const AVFrame *src, int realigned)
> {
> QSVFramesContext *s = ctx->hwctx;
> mfxFrameSurface1 out = {{ 0 }};
> @@ -1658,17 +1645,11 @@ static int qsv_transfer_data_from(AVHWFramesContext
> *ctx, AVFrame *dst,
> /* download to temp frame if the output is not padded as libmfx requires
> */
> AVFrame *tmp_frame = &s->realigned_download_frame;
> AVFrame *dst_frame;
> - int realigned = 0;
> -
> - ret = qsv_internal_session_check_init(ctx, 0);
> - if (ret < 0)
> - return ret;
>
> /* According to MSDK spec for mfxframeinfo, "Width must be a multiple of
> 16.
> * Height must be a multiple of 16 for progressive frame sequence and a
> * multiple of 32 otherwise.", so allign all frames to 16 before
> downloading. */
> - if (dst->height & 15 || dst->linesize[0] & 15) {
> - realigned = 1;
> + if (realigned) {
> if (tmp_frame->format != dst->format ||
> tmp_frame->width != FFALIGN(dst->linesize[0], 16) ||
> tmp_frame->height != FFALIGN(dst->height, 16)) {
> @@ -1728,8 +1709,30 @@ static int qsv_transfer_data_from(AVHWFramesContext
> *ctx, AVFrame *dst,
> return 0;
> }
>
> -static int qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
> - const AVFrame *src)
> +static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
> + const AVFrame *src)
> +{
> + QSVFramesContext *s = ctx->hwctx;
> + int realigned = 0;
> + int ret = 0;
> +
> + ret = qsv_internal_session_check_init(ctx, 0);
> + if (ret < 0)
> + return ret;
> +
> + if (dst->height & 15 || dst->linesize[0] & 15) {
> + realigned = 1;
> + ff_mutex_lock(&s->session_lock);
Needn't lock the use if changing realigned_upload_frame and
realigned_download_frame to local variables.
Thanks
Haihao
> + }
> + ret = qsv_transfer_data_from_internal(ctx, dst, src, realigned);
> + if (realigned)
> + ff_mutex_unlock(&s->session_lock);
> +
> + return ret;
> +}
> +
> +static int qsv_transfer_data_to_internal(AVHWFramesContext *ctx, AVFrame
> *dst,
> + const AVFrame *src, int realigned)
> {
> QSVFramesContext *s = ctx->hwctx;
> mfxFrameSurface1 in = {{ 0 }};
> @@ -1742,17 +1745,11 @@ static int qsv_transfer_data_to(AVHWFramesContext
> *ctx, AVFrame *dst,
> /* make a copy if the input is not padded as libmfx requires */
> AVFrame *tmp_frame = &s->realigned_upload_frame;
> const AVFrame *src_frame;
> - int realigned = 0;
> -
> - ret = qsv_internal_session_check_init(ctx, 1);
> - if (ret < 0)
> - return ret;
>
> /* According to MSDK spec for mfxframeinfo, "Width must be a multiple of
> 16.
> * Height must be a multiple of 16 for progressive frame sequence and a
> * multiple of 32 otherwise.", so allign all frames to 16 before
> uploading. */
> - if (src->height & 15 || src->linesize[0] & 15) {
> - realigned = 1;
> + if (realigned) {
> if (tmp_frame->format != src->format ||
> tmp_frame->width != FFALIGN(src->width, 16) ||
> tmp_frame->height != FFALIGN(src->height, 16)) {
> @@ -1821,6 +1818,28 @@ static int qsv_transfer_data_to(AVHWFramesContext *ctx,
> AVFrame *dst,
> return 0;
> }
>
> +static int qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
> + const AVFrame *src)
> +{
> + QSVFramesContext *s = ctx->hwctx;
> + int realigned = 0;
> + int ret = 0;
> +
> + ret = qsv_internal_session_check_init(ctx, 1);
> + if (ret < 0)
> + return ret;
> +
> + if (src->height & 15 || src->linesize[0] & 15) {
> + realigned = 1;
> + ff_mutex_lock(&s->session_lock);
> + }
> + ret = qsv_transfer_data_to_internal(ctx, dst, src, realigned);
> + if (realigned)
> + ff_mutex_unlock(&s->session_lock);
> +
> + return ret;
> +}
> +
> static int qsv_frames_derive_to(AVHWFramesContext *dst_ctx,
> AVHWFramesContext *src_ctx, int flags)
> {
_______________________________________________
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".
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-24 5:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-13 12:57 [FFmpeg-devel] [PATCH] libavutil/hwcontext_qsv: Make qsv hardware transfers thread safe Mark Samuelson
2024-04-17 8:15 ` Xiang, Haihao
2024-04-17 14:46 ` Mark Samuelson
2024-04-24 5:06 ` Xiang, Haihao
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git