* [FFmpeg-devel] [PATCH 1/2] lavfi/qsvpp: fix after 85c938fa28
@ 2022-02-08 18:26 Anton Khirnov
2022-02-08 18:26 ` [FFmpeg-devel] [PATCH 2/2] lavu/hwcontext_qsv: fix a potential infinite loop Anton Khirnov
2022-02-08 22:53 ` [FFmpeg-devel] [PATCH 1/2] lavfi/qsvpp: fix after 85c938fa28 Eoff, Ullysses A
0 siblings, 2 replies; 4+ messages in thread
From: Anton Khirnov @ 2022-02-08 18:26 UTC (permalink / raw)
To: ffmpeg-devel
---
libavfilter/qsvvpp.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/libavfilter/qsvvpp.c b/libavfilter/qsvvpp.c
index 35769dfd60..954f882637 100644
--- a/libavfilter/qsvvpp.c
+++ b/libavfilter/qsvvpp.c
@@ -796,7 +796,7 @@ int ff_qsvvpp_filter_frame(QSVVPPContext *s, AVFilterLink *inlink, AVFrame *picr
AVFilterLink *outlink = ctx->outputs[0];
QSVAsyncFrame aframe;
mfxSyncPoint sync;
- QSVFrame *in_frame, *out_frame, *tmp;
+ QSVFrame *in_frame, *out_frame;
int ret, filter_ret;
while (s->eof && av_fifo_read(s->async_fifo, &aframe, 1) >= 0) {
@@ -857,15 +857,15 @@ int ff_qsvvpp_filter_frame(QSVVPPContext *s, AVFilterLink *inlink, AVFrame *picr
ret = MFXVideoCORE_SyncOperation(s->session, aframe.sync, 1000);
} while (ret == MFX_WRN_IN_EXECUTION);
- filter_ret = s->filter_frame(outlink, tmp->frame);
+ filter_ret = s->filter_frame(outlink, aframe.frame->frame);
if (filter_ret < 0) {
- av_frame_free(&tmp->frame);
+ av_frame_free(&aframe.frame->frame);
return filter_ret;
}
- tmp->queued--;
+ aframe.frame->queued--;
s->got_frame = 1;
- tmp->frame = NULL;
+ aframe.frame->frame = NULL;
}
} while(ret == MFX_ERR_MORE_SURFACE);
--
2.34.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
* [FFmpeg-devel] [PATCH 2/2] lavu/hwcontext_qsv: fix a potential infinite loop
2022-02-08 18:26 [FFmpeg-devel] [PATCH 1/2] lavfi/qsvpp: fix after 85c938fa28 Anton Khirnov
@ 2022-02-08 18:26 ` Anton Khirnov
2022-02-08 19:31 ` James Almer
2022-02-08 22:53 ` [FFmpeg-devel] [PATCH 1/2] lavfi/qsvpp: fix after 85c938fa28 Eoff, Ullysses A
1 sibling, 1 reply; 4+ messages in thread
From: Anton Khirnov @ 2022-02-08 18:26 UTC (permalink / raw)
To: ffmpeg-devel
Current code will loop forever if MFXVideoVPP_Init() fails.
Also, simplify the code.
---
libavutil/hwcontext_qsv.c | 80 +++++++++++++++------------------------
1 file changed, 31 insertions(+), 49 deletions(-)
diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
index d3d8f42c99..95f8071abe 100644
--- a/libavutil/hwcontext_qsv.c
+++ b/libavutil/hwcontext_qsv.c
@@ -16,6 +16,7 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/
+#include <stdatomic.h>
#include <stdint.h>
#include <string.h>
@@ -71,12 +72,11 @@ typedef struct QSVDeviceContext {
typedef struct QSVFramesContext {
mfxSession session_download;
- int session_download_init;
+ atomic_int session_download_init;
mfxSession session_upload;
- int session_upload_init;
+ atomic_int session_upload_init;
#if HAVE_PTHREADS
pthread_mutex_t session_lock;
- pthread_cond_t session_cond;
#endif
AVBufferRef *child_frames_ref;
@@ -297,7 +297,6 @@ static void qsv_frames_uninit(AVHWFramesContext *ctx)
#if HAVE_PTHREADS
pthread_mutex_destroy(&s->session_lock);
- pthread_cond_destroy(&s->session_cond);
#endif
av_freep(&s->mem_ids);
@@ -744,7 +743,6 @@ static int qsv_frames_init(AVHWFramesContext *ctx)
#if HAVE_PTHREADS
pthread_mutex_init(&s->session_lock, NULL);
- pthread_cond_init(&s->session_cond, NULL);
#endif
return 0;
@@ -1024,6 +1022,32 @@ static int map_frame_to_surface(const AVFrame *frame, mfxFrameSurface1 *surface)
return 0;
}
+static int qsv_internal_session_check_init(AVHWFramesContext *ctx, int upload)
+{
+ QSVFramesContext *s = ctx->internal->priv;
+ atomic_int *inited = upload ? &s->session_upload_init : &s->session_download_init;
+ mfxSession *session = upload ? &s->session_upload : &s->session_download;
+ int ret = 0;
+
+ if (atomic_load(inited))
+ return 0;
+
+#if HAVE_PTHREADS
+ pthread_mutex_lock(&s->session_lock);
+#endif
+
+ 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
+
+ return ret;
+}
+
static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
const AVFrame *src)
{
@@ -1035,28 +1059,7 @@ static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
mfxStatus err;
int ret = 0;
- while (!s->session_download_init && !s->session_download && !ret) {
-#if HAVE_PTHREADS
- if (pthread_mutex_trylock(&s->session_lock) == 0) {
-#endif
- if (!s->session_download_init) {
- ret = qsv_init_internal_session(ctx, &s->session_download, 0);
- if (s->session_download)
- s->session_download_init = 1;
- }
-#if HAVE_PTHREADS
- pthread_mutex_unlock(&s->session_lock);
- pthread_cond_signal(&s->session_cond);
- } else {
- pthread_mutex_lock(&s->session_lock);
- while (!s->session_download_init && !s->session_download) {
- pthread_cond_wait(&s->session_cond, &s->session_lock);
- }
- pthread_mutex_unlock(&s->session_lock);
- }
-#endif
- }
-
+ ret = qsv_internal_session_check_init(ctx, 0);
if (ret < 0)
return ret;
@@ -1109,28 +1112,7 @@ static int qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
const AVFrame *src_frame;
int realigned = 0;
-
- while (!s->session_upload_init && !s->session_upload && !ret) {
-#if HAVE_PTHREADS
- if (pthread_mutex_trylock(&s->session_lock) == 0) {
-#endif
- if (!s->session_upload_init) {
- ret = qsv_init_internal_session(ctx, &s->session_upload, 1);
- if (s->session_upload)
- s->session_upload_init = 1;
- }
-#if HAVE_PTHREADS
- pthread_mutex_unlock(&s->session_lock);
- pthread_cond_signal(&s->session_cond);
- } else {
- pthread_mutex_lock(&s->session_lock);
- while (!s->session_upload_init && !s->session_upload) {
- pthread_cond_wait(&s->session_cond, &s->session_lock);
- }
- pthread_mutex_unlock(&s->session_lock);
- }
-#endif
- }
+ ret = qsv_internal_session_check_init(ctx, 1);
if (ret < 0)
return ret;
--
2.34.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 2/2] lavu/hwcontext_qsv: fix a potential infinite loop
2022-02-08 18:26 ` [FFmpeg-devel] [PATCH 2/2] lavu/hwcontext_qsv: fix a potential infinite loop Anton Khirnov
@ 2022-02-08 19:31 ` James Almer
0 siblings, 0 replies; 4+ messages in thread
From: James Almer @ 2022-02-08 19:31 UTC (permalink / raw)
To: ffmpeg-devel
On 2/8/2022 3:26 PM, Anton Khirnov wrote:
> Current code will loop forever if MFXVideoVPP_Init() fails.
> Also, simplify the code.
> ---
> libavutil/hwcontext_qsv.c | 80 +++++++++++++++------------------------
> 1 file changed, 31 insertions(+), 49 deletions(-)
>
> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> index d3d8f42c99..95f8071abe 100644
> --- a/libavutil/hwcontext_qsv.c
> +++ b/libavutil/hwcontext_qsv.c
> @@ -16,6 +16,7 @@
> * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> */
>
> +#include <stdatomic.h>
> #include <stdint.h>
> #include <string.h>
>
> @@ -71,12 +72,11 @@ typedef struct QSVDeviceContext {
>
> typedef struct QSVFramesContext {
> mfxSession session_download;
> - int session_download_init;
> + atomic_int session_download_init;
> mfxSession session_upload;
> - int session_upload_init;
> + atomic_int session_upload_init;
> #if HAVE_PTHREADS
> pthread_mutex_t session_lock;
> - pthread_cond_t session_cond;
> #endif
>
> AVBufferRef *child_frames_ref;
> @@ -297,7 +297,6 @@ static void qsv_frames_uninit(AVHWFramesContext *ctx)
>
> #if HAVE_PTHREADS
> pthread_mutex_destroy(&s->session_lock);
> - pthread_cond_destroy(&s->session_cond);
> #endif
>
> av_freep(&s->mem_ids);
> @@ -744,7 +743,6 @@ static int qsv_frames_init(AVHWFramesContext *ctx)
>
> #if HAVE_PTHREADS
> pthread_mutex_init(&s->session_lock, NULL);
> - pthread_cond_init(&s->session_cond, NULL);
> #endif
>
> return 0;
> @@ -1024,6 +1022,32 @@ static int map_frame_to_surface(const AVFrame *frame, mfxFrameSurface1 *surface)
> return 0;
> }
>
> +static int qsv_internal_session_check_init(AVHWFramesContext *ctx, int upload)
> +{
> + QSVFramesContext *s = ctx->internal->priv;
> + atomic_int *inited = upload ? &s->session_upload_init : &s->session_download_init;
> + mfxSession *session = upload ? &s->session_upload : &s->session_download;
> + int ret = 0;
> +
> + if (atomic_load(inited))
> + return 0;
> +
> +#if HAVE_PTHREADS
> + pthread_mutex_lock(&s->session_lock);
> +#endif
> +
> + if (!atomic_load(inited)) {
> + ret = qsv_init_internal_session(ctx, session, upload);
> + atomic_store(inited, 1);
Nit: How about
if (atomic_compare_exchange_strong(inited, &ret, 1))
ret = qsv_init_internal_session(ctx, session, upload);
Or using some other variable for the second argument (It must be 0
before the call, and will become 1 if *inited was already 1).
Since atomic_compare_exchange_strong() is not used anywhere in the tree,
it would let us know if any of the atomics emulation implementations are
faulty (win32, pthreads, old gcc).
> + }
> +
> +#if HAVE_PTHREADS
> + pthread_mutex_unlock(&s->session_lock);
> +#endif
> +
> + return ret;
> +}
> +
> static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
> const AVFrame *src)
> {
> @@ -1035,28 +1059,7 @@ static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
> mfxStatus err;
> int ret = 0;
>
> - while (!s->session_download_init && !s->session_download && !ret) {
> -#if HAVE_PTHREADS
> - if (pthread_mutex_trylock(&s->session_lock) == 0) {
> -#endif
> - if (!s->session_download_init) {
> - ret = qsv_init_internal_session(ctx, &s->session_download, 0);
> - if (s->session_download)
> - s->session_download_init = 1;
> - }
> -#if HAVE_PTHREADS
> - pthread_mutex_unlock(&s->session_lock);
> - pthread_cond_signal(&s->session_cond);
> - } else {
> - pthread_mutex_lock(&s->session_lock);
> - while (!s->session_download_init && !s->session_download) {
> - pthread_cond_wait(&s->session_cond, &s->session_lock);
> - }
> - pthread_mutex_unlock(&s->session_lock);
> - }
> -#endif
> - }
> -
> + ret = qsv_internal_session_check_init(ctx, 0);
> if (ret < 0)
> return ret;
>
> @@ -1109,28 +1112,7 @@ static int qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
> const AVFrame *src_frame;
> int realigned = 0;
>
> -
> - while (!s->session_upload_init && !s->session_upload && !ret) {
> -#if HAVE_PTHREADS
> - if (pthread_mutex_trylock(&s->session_lock) == 0) {
> -#endif
> - if (!s->session_upload_init) {
> - ret = qsv_init_internal_session(ctx, &s->session_upload, 1);
> - if (s->session_upload)
> - s->session_upload_init = 1;
> - }
> -#if HAVE_PTHREADS
> - pthread_mutex_unlock(&s->session_lock);
> - pthread_cond_signal(&s->session_cond);
> - } else {
> - pthread_mutex_lock(&s->session_lock);
> - while (!s->session_upload_init && !s->session_upload) {
> - pthread_cond_wait(&s->session_cond, &s->session_lock);
> - }
> - pthread_mutex_unlock(&s->session_lock);
> - }
> -#endif
> - }
> + ret = qsv_internal_session_check_init(ctx, 1);
> if (ret < 0)
> return ret;
>
_______________________________________________
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 1/2] lavfi/qsvpp: fix after 85c938fa28
2022-02-08 18:26 [FFmpeg-devel] [PATCH 1/2] lavfi/qsvpp: fix after 85c938fa28 Anton Khirnov
2022-02-08 18:26 ` [FFmpeg-devel] [PATCH 2/2] lavu/hwcontext_qsv: fix a potential infinite loop Anton Khirnov
@ 2022-02-08 22:53 ` Eoff, Ullysses A
1 sibling, 0 replies; 4+ messages in thread
From: Eoff, Ullysses A @ 2022-02-08 22:53 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton Khirnov
> Sent: Tuesday, February 8, 2022 10:27 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH 1/2] lavfi/qsvpp: fix after 85c938fa28
>
> ---
> libavfilter/qsvvpp.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/libavfilter/qsvvpp.c b/libavfilter/qsvvpp.c
> index 35769dfd60..954f882637 100644
> --- a/libavfilter/qsvvpp.c
> +++ b/libavfilter/qsvvpp.c
> @@ -796,7 +796,7 @@ int ff_qsvvpp_filter_frame(QSVVPPContext *s, AVFilterLink *inlink, AVFrame *picr
> AVFilterLink *outlink = ctx->outputs[0];
> QSVAsyncFrame aframe;
> mfxSyncPoint sync;
> - QSVFrame *in_frame, *out_frame, *tmp;
> + QSVFrame *in_frame, *out_frame;
> int ret, filter_ret;
>
> while (s->eof && av_fifo_read(s->async_fifo, &aframe, 1) >= 0) {
> @@ -857,15 +857,15 @@ int ff_qsvvpp_filter_frame(QSVVPPContext *s, AVFilterLink *inlink, AVFrame *picr
> ret = MFXVideoCORE_SyncOperation(s->session, aframe.sync, 1000);
> } while (ret == MFX_WRN_IN_EXECUTION);
>
> - filter_ret = s->filter_frame(outlink, tmp->frame);
> + filter_ret = s->filter_frame(outlink, aframe.frame->frame);
> if (filter_ret < 0) {
> - av_frame_free(&tmp->frame);
> + av_frame_free(&aframe.frame->frame);
> return filter_ret;
> }
>
> - tmp->queued--;
> + aframe.frame->queued--;
> s->got_frame = 1;
> - tmp->frame = NULL;
> + aframe.frame->frame = NULL;
> }
> } while(ret == MFX_ERR_MORE_SURFACE);
>
> --
> 2.34.1
>
Fixes https://trac.ffmpeg.org/ticket/9629 for me.
LGTM
> _______________________________________________
> 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 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:[~2022-02-08 22:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-08 18:26 [FFmpeg-devel] [PATCH 1/2] lavfi/qsvpp: fix after 85c938fa28 Anton Khirnov
2022-02-08 18:26 ` [FFmpeg-devel] [PATCH 2/2] lavu/hwcontext_qsv: fix a potential infinite loop Anton Khirnov
2022-02-08 19:31 ` James Almer
2022-02-08 22:53 ` [FFmpeg-devel] [PATCH 1/2] lavfi/qsvpp: fix after 85c938fa28 Eoff, Ullysses A
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