Discussion:
[PATCH v5] qsvvpp: Fix to perform full init only when needed
(too old to reply)
Maxym Dmytrychenko
2018-07-18 12:07:17 UTC
Permalink
Not used VPP sessions, like for hwupload/hwdownload handling,
can increase CPU utilization and this patch fixes it.
thank you,Joe, for the contribution.

Signed-off-by: Maxym Dmytrychenko <***@gmail.com>
---
libavutil/hwcontext_qsv.c | 72 +++++++++++++++++++++++++++++++++++++++++------
1 file changed, 64 insertions(+), 8 deletions(-)

diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
index b3eb4a3ea..6bc2a38ff 100644
--- a/libavutil/hwcontext_qsv.c
+++ b/libavutil/hwcontext_qsv.c
@@ -23,6 +23,10 @@

#include "config.h"

+#if HAVE_PTHREADS
+#include <pthread.h>
+#endif
+
#if CONFIG_VAAPI
#include "hwcontext_vaapi.h"
#endif
@@ -56,7 +60,13 @@ typedef struct QSVDeviceContext {

typedef struct QSVFramesContext {
mfxSession session_download;
+ int session_download_init;
mfxSession session_upload;
+ int session_upload_init;
+#if HAVE_PTHREADS
+ pthread_mutex_t session_lock;
+ pthread_cond_t session_cond;
+#endif

AVBufferRef *child_frames_ref;
mfxFrameSurface1 *surfaces_internal;
@@ -146,13 +156,20 @@ static void qsv_frames_uninit(AVHWFramesContext *ctx)
MFXVideoVPP_Close(s->session_download);
MFXClose(s->session_download);
}
- s->session_download = NULL;
+ s->session_download = NULL;
+ s->session_download_init = 0;

if (s->session_upload) {
MFXVideoVPP_Close(s->session_upload);
MFXClose(s->session_upload);
}
- s->session_upload = NULL;
+ s->session_upload = NULL;
+ s->session_upload_init = 0;
+
+#if HAVE_PTHREADS
+ pthread_mutex_destroy(&s->session_lock);
+ pthread_cond_destroy(&s->session_cond);
+#endif

av_freep(&s->mem_ids);
av_freep(&s->surface_ptrs);
@@ -535,13 +552,16 @@ static int qsv_frames_init(AVHWFramesContext *ctx)
s->mem_ids[i] = frames_hwctx->surfaces[i].Data.MemId;
}

- ret = qsv_init_internal_session(ctx, &s->session_download, 0);
- if (ret < 0)
- return ret;
+ s->session_download = NULL;
+ s->session_upload = NULL;

- ret = qsv_init_internal_session(ctx, &s->session_upload, 1);
- if (ret < 0)
- return ret;
+ s->session_download_init = 0;
+ s->session_upload_init = 0;
+
+#if HAVE_PTHREADS
+ pthread_mutex_init(&s->session_lock, NULL);
+ pthread_cond_init(&s->session_cond, NULL);
+#endif

return 0;
}
@@ -741,6 +761,24 @@ static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
mfxSyncPoint sync = NULL;
mfxStatus err;

+ while (!s->session_download_init && !s->session_download) {
+#if HAVE_PTHREADS
+ if (pthread_mutex_trylock(&s->session_lock) == 0) {
+#endif
+ qsv_init_internal_session(ctx, &s->session_download, 0);
+ s->session_download_init = 1;
+#if HAVE_PTHREADS
+ pthread_cond_signal(&s->session_cond);
+ pthread_mutex_unlock(&s->session_lock);
+ }
+ else {
+ pthread_mutex_lock(&s->session_lock);
+ pthread_cond_wait(&s->session_cond, &s->session_lock);
+ pthread_mutex_unlock(&s->session_lock);
+ }
+#endif
+ }
+
if (!s->session_download) {
if (s->child_frames_ref)
return qsv_transfer_data_child(ctx, dst, src);
@@ -788,6 +826,24 @@ static int qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
mfxSyncPoint sync = NULL;
mfxStatus err;

+ while (!s->session_upload_init && !s->session_upload) {
+#if HAVE_PTHREADS
+ if (pthread_mutex_trylock(&s->session_lock) == 0) {
+#endif
+ qsv_init_internal_session(ctx, &s->session_upload, 1);
+ s->session_upload_init = 1;
+#if HAVE_PTHREADS
+ pthread_cond_signal(&s->session_cond);
+ pthread_mutex_unlock(&s->session_lock);
+ }
+ else {
+ pthread_mutex_lock(&s->session_lock);
+ pthread_cond_wait(&s->session_cond, &s->session_lock);
+ pthread_mutex_unlock(&s->session_lock);
+ }
+#endif
+ }
+
if (!s->session_upload) {
if (s->child_frames_ref)
return qsv_transfer_data_child(ctx, dst, src);
--
2.15.2 (Apple Git-101.1)
Rogozhkin, Dmitry V
2018-07-25 00:56:59 UTC
Permalink
Post by Maxym Dmytrychenko
Not used VPP sessions, like for hwupload/hwdownload handling,
can increase CPU utilization and this patch fixes it.
thank you,Joe, for the contribution.
---
 libavutil/hwcontext_qsv.c | 72
+++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 64 insertions(+), 8 deletions(-)
diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
index b3eb4a3ea..6bc2a38ff 100644
--- a/libavutil/hwcontext_qsv.c
+++ b/libavutil/hwcontext_qsv.c
@@ -23,6 +23,10 @@
 
 #include "config.h"
 
+#if HAVE_PTHREADS
+#include <pthread.h>
+#endif
+
 #if CONFIG_VAAPI
 #include "hwcontext_vaapi.h"
 #endif
@@ -56,7 +60,13 @@ typedef struct QSVDeviceContext {
 
 typedef struct QSVFramesContext {
     mfxSession session_download;
+    int session_download_init;
     mfxSession session_upload;
+    int session_upload_init;
+#if HAVE_PTHREADS
+    pthread_mutex_t session_lock;
+    pthread_cond_t session_cond;
+#endif
 
     AVBufferRef *child_frames_ref;
     mfxFrameSurface1 *surfaces_internal;
@@ -146,13 +156,20 @@ static void qsv_frames_uninit(AVHWFramesContext *ctx)
         MFXVideoVPP_Close(s->session_download);
         MFXClose(s->session_download);
     }
-    s->session_download = NULL;
+    s->session_download      = NULL;
+    s->session_download_init = 0;
 
     if (s->session_upload) {
         MFXVideoVPP_Close(s->session_upload);
         MFXClose(s->session_upload);
     }
-    s->session_upload = NULL;
+    s->session_upload      = NULL;
+    s->session_upload_init = 0;
+
+#if HAVE_PTHREADS
+    pthread_mutex_destroy(&s->session_lock);
+    pthread_cond_destroy(&s->session_cond);
+#endif
 
     av_freep(&s->mem_ids);
     av_freep(&s->surface_ptrs);
@@ -535,13 +552,16 @@ static int qsv_frames_init(AVHWFramesContext *ctx)
             s->mem_ids[i] = frames_hwctx->surfaces[i].Data.MemId;
     }
 
-    ret = qsv_init_internal_session(ctx, &s->session_download, 0);
-    if (ret < 0)
-        return ret;
+    s->session_download = NULL;
+    s->session_upload   = NULL;
 
-    ret = qsv_init_internal_session(ctx, &s->session_upload, 1);
-    if (ret < 0)
-        return ret;
+    s->session_download_init = 0;
+    s->session_upload_init   = 0;
+
+#if HAVE_PTHREADS
+    pthread_mutex_init(&s->session_lock, NULL);
+    pthread_cond_init(&s->session_cond, NULL);
+#endif
 
     return 0;
 }
@@ -741,6 +761,24 @@ static int
qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
     mfxSyncPoint sync = NULL;
     mfxStatus err;
 
+    while (!s->session_download_init && !s->session_download) {
+#if HAVE_PTHREADS
+        if (pthread_mutex_trylock(&s->session_lock) == 0) {
+#endif
+            qsv_init_internal_session(ctx, &s->session_download, 0);
+            s->session_download_init = 1;
+#if HAVE_PTHREADS
+            pthread_cond_signal(&s->session_cond);
You don't need to signal under the mutex. More efficiently is to do
that without.
Post by Maxym Dmytrychenko
+            pthread_mutex_unlock(&s->session_lock);
+        }
+        else {
+            pthread_mutex_lock(&s->session_lock);
+            pthread_cond_wait(&s->session_cond, &s->session_lock);
This is incorrect usage of condition variables. Look into example in
'man 3 pthread_cond_wait': you should check predicate under the mutex.
Post by Maxym Dmytrychenko
+            pthread_mutex_unlock(&s->session_lock);
+        }
+#endif
+    }
+
As I said the above code is an example of incorrect usage of condition
variables. I believe this should be written in this way:

#if HAVE_PTHREADS
    if (pthread_mutex_trylock(&s->session_lock) == 0) {
#endif
        qsv_init_internal_session(ctx, &s->session_download, 0);
        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
Post by Maxym Dmytrychenko
     if (!s->session_download) {
         if (s->child_frames_ref)
             return qsv_transfer_data_child(ctx, dst, src);
@@ -788,6 +826,24 @@ static int
qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
     mfxSyncPoint sync = NULL;
     mfxStatus err;
 
+    while (!s->session_upload_init && !s->session_upload) {
+#if HAVE_PTHREADS
+        if (pthread_mutex_trylock(&s->session_lock) == 0) {
+#endif
+            qsv_init_internal_session(ctx, &s->session_upload, 1);
+            s->session_upload_init = 1;
+#if HAVE_PTHREADS
+            pthread_cond_signal(&s->session_cond);
+            pthread_mutex_unlock(&s->session_lock);
+        }
+        else {
+            pthread_mutex_lock(&s->session_lock);
+            pthread_cond_wait(&s->session_cond, &s->session_lock);
+            pthread_mutex_unlock(&s->session_lock);
+        }
+#endif
+    }
+
Same comment as above. Correct code would be:

#if HAVE_PTHREADS
    if (pthread_mutex_trylock(&s->session_lock) == 0) {
#endif
        qsv_init_internal_session(ctx, &s->session_upload, 0);
        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
Post by Maxym Dmytrychenko
     if (!s->session_upload) {
         if (s->child_frames_ref)
             return qsv_transfer_data_child(ctx, dst, src);
Rogozhkin, Dmitry V
2018-07-25 16:14:19 UTC
Permalink
Post by Rogozhkin, Dmitry V
Post by Maxym Dmytrychenko
Not used VPP sessions, like for hwupload/hwdownload handling,
can increase CPU utilization and this patch fixes it.
thank you,Joe, for the contribution.
---
 libavutil/hwcontext_qsv.c | 72
+++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 64 insertions(+), 8 deletions(-)
diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
index b3eb4a3ea..6bc2a38ff 100644
--- a/libavutil/hwcontext_qsv.c
+++ b/libavutil/hwcontext_qsv.c
@@ -23,6 +23,10 @@
 
 #include "config.h"
 
+#if HAVE_PTHREADS
+#include <pthread.h>
+#endif
+
 #if CONFIG_VAAPI
 #include "hwcontext_vaapi.h"
 #endif
@@ -56,7 +60,13 @@ typedef struct QSVDeviceContext {
 
 typedef struct QSVFramesContext {
     mfxSession session_download;
+    int session_download_init;
     mfxSession session_upload;
+    int session_upload_init;
+#if HAVE_PTHREADS
+    pthread_mutex_t session_lock;
+    pthread_cond_t session_cond;
+#endif
 
     AVBufferRef *child_frames_ref;
     mfxFrameSurface1 *surfaces_internal;
@@ -146,13 +156,20 @@ static void
qsv_frames_uninit(AVHWFramesContext
*ctx)
         MFXVideoVPP_Close(s->session_download);
         MFXClose(s->session_download);
     }
-    s->session_download = NULL;
+    s->session_download      = NULL;
+    s->session_download_init = 0;
 
     if (s->session_upload) {
         MFXVideoVPP_Close(s->session_upload);
         MFXClose(s->session_upload);
     }
-    s->session_upload = NULL;
+    s->session_upload      = NULL;
+    s->session_upload_init = 0;
+
+#if HAVE_PTHREADS
+    pthread_mutex_destroy(&s->session_lock);
+    pthread_cond_destroy(&s->session_cond);
+#endif
 
     av_freep(&s->mem_ids);
     av_freep(&s->surface_ptrs);
@@ -535,13 +552,16 @@ static int qsv_frames_init(AVHWFramesContext *ctx)
             s->mem_ids[i] = frames_hwctx->surfaces[i].Data.MemId;
     }
 
-    ret = qsv_init_internal_session(ctx, &s->session_download, 0);
-    if (ret < 0)
-        return ret;
+    s->session_download = NULL;
+    s->session_upload   = NULL;
 
-    ret = qsv_init_internal_session(ctx, &s->session_upload, 1);
-    if (ret < 0)
-        return ret;
+    s->session_download_init = 0;
+    s->session_upload_init   = 0;
+
+#if HAVE_PTHREADS
+    pthread_mutex_init(&s->session_lock, NULL);
+    pthread_cond_init(&s->session_cond, NULL);
+#endif
 
     return 0;
 }
@@ -741,6 +761,24 @@ static int
qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
     mfxSyncPoint sync = NULL;
     mfxStatus err;
 
+    while (!s->session_download_init && !s->session_download) {
+#if HAVE_PTHREADS
+        if (pthread_mutex_trylock(&s->session_lock) == 0) {
+#endif
+            qsv_init_internal_session(ctx, &s->session_download, 0);
+            s->session_download_init = 1;
+#if HAVE_PTHREADS
Post by Rogozhkin, Dmitry V
Post by Maxym Dmytrychenko
+            pthread_cond_signal(&s->session_cond);
You don't need to signal under the mutex. More efficiently is to do
that without.
Post by Maxym Dmytrychenko
+            pthread_mutex_unlock(&s->session_lock);
+        }
+        else {
+            pthread_mutex_lock(&s->session_lock);
+            pthread_cond_wait(&s->session_cond, &s->session_lock);
This is incorrect usage of condition variables. Look into example in
'man 3 pthread_cond_wait': you should check predicate under the mutex.
Post by Maxym Dmytrychenko
+            pthread_mutex_unlock(&s->session_lock);
+        }
+#endif
+    }
+
As I said the above code is an example of incorrect usage of
condition
#if HAVE_PTHREADS
    if (pthread_mutex_trylock(&s->session_lock) == 0) {
#endif
        qsv_init_internal_session(ctx, &s->session_download, 0);
        s->session_download_init = 1;
Never do something 2 mins before the meeting. This should probably be:        if (!s->session_download_init) {
            qsv_init_internal_session(ctx, &s->session_download, 0);
            if (s->session_download)
                s->session_download_init = 1;
        }
Post by Rogozhkin, Dmitry V
#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
Post by Maxym Dmytrychenko
     if (!s->session_download) {
         if (s->child_frames_ref)
             return qsv_transfer_data_child(ctx, dst, src);
@@ -788,6 +826,24 @@ static int
qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
     mfxSyncPoint sync = NULL;
     mfxStatus err;
 
+    while (!s->session_upload_init && !s->session_upload) {
+#if HAVE_PTHREADS
+        if (pthread_mutex_trylock(&s->session_lock) == 0) {
+#endif
+            qsv_init_internal_session(ctx, &s->session_upload, 1);
+            s->session_upload_init = 1;
+#if HAVE_PTHREADS
+            pthread_cond_signal(&s->session_cond);
+            pthread_mutex_unlock(&s->session_lock);
+        }
+        else {
+            pthread_mutex_lock(&s->session_lock);
+            pthread_cond_wait(&s->session_cond, &s->session_lock);
+            pthread_mutex_unlock(&s->session_lock);
+        }
+#endif
+    }
+
#if HAVE_PTHREADS
    if (pthread_mutex_trylock(&s->session_lock) == 0) {
#endif
        qsv_init_internal_session(ctx, &s->session_upload, 0);
        s->session_upload_init = 1;
And here:
        if (!s->session_upload_init) {
            qsv_init_internal_session(ctx, &s->session_upload, 0);
            if (s->session_upload)
                s->session_upload_init = 1;
        }
Post by Rogozhkin, Dmitry V
#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
Post by Maxym Dmytrychenko
     if (!s->session_upload) {
         if (s->child_frames_ref)
             return qsv_transfer_data_child(ctx, dst, src);
Rogozhkin, Dmitry V
2018-08-23 20:51:06 UTC
Permalink
Post by Maxym Dmytrychenko
Post by Rogozhkin, Dmitry V
Post by Maxym Dmytrychenko
Not used VPP sessions, like for hwupload/hwdownload handling,
can increase CPU utilization and this patch fixes it.
thank you,Joe, for the contribution.
---
 libavutil/hwcontext_qsv.c | 72
+++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 64 insertions(+), 8 deletions(-)
diff --git a/libavutil/hwcontext_qsv.c
b/libavutil/hwcontext_qsv.c
index b3eb4a3ea..6bc2a38ff 100644
--- a/libavutil/hwcontext_qsv.c
+++ b/libavutil/hwcontext_qsv.c
@@ -23,6 +23,10 @@
 
 #include "config.h"
 
+#if HAVE_PTHREADS
+#include <pthread.h>
+#endif
+
 #if CONFIG_VAAPI
 #include "hwcontext_vaapi.h"
 #endif
@@ -56,7 +60,13 @@ typedef struct QSVDeviceContext {
 
 typedef struct QSVFramesContext {
     mfxSession session_download;
+    int session_download_init;
     mfxSession session_upload;
+    int session_upload_init;
+#if HAVE_PTHREADS
+    pthread_mutex_t session_lock;
+    pthread_cond_t session_cond;
+#endif
 
     AVBufferRef *child_frames_ref;
     mfxFrameSurface1 *surfaces_internal;
@@ -146,13 +156,20 @@ static void
qsv_frames_uninit(AVHWFramesContext
*ctx)
         MFXVideoVPP_Close(s->session_download);
         MFXClose(s->session_download);
     }
-    s->session_download = NULL;
+    s->session_download      = NULL;
+    s->session_download_init = 0;
 
     if (s->session_upload) {
         MFXVideoVPP_Close(s->session_upload);
         MFXClose(s->session_upload);
     }
-    s->session_upload = NULL;
+    s->session_upload      = NULL;
+    s->session_upload_init = 0;
+
+#if HAVE_PTHREADS
+    pthread_mutex_destroy(&s->session_lock);
+    pthread_cond_destroy(&s->session_cond);
+#endif
 
     av_freep(&s->mem_ids);
     av_freep(&s->surface_ptrs);
@@ -535,13 +552,16 @@ static int
qsv_frames_init(AVHWFramesContext
*ctx)
             s->mem_ids[i] = frames_hwctx-
Post by Maxym Dmytrychenko
surfaces[i].Data.MemId;
     }
 
-    ret = qsv_init_internal_session(ctx, &s->session_download, 0);
-    if (ret < 0)
-        return ret;
+    s->session_download = NULL;
+    s->session_upload   = NULL;
 
-    ret = qsv_init_internal_session(ctx, &s->session_upload, 1);
-    if (ret < 0)
-        return ret;
+    s->session_download_init = 0;
+    s->session_upload_init   = 0;
+
+#if HAVE_PTHREADS
+    pthread_mutex_init(&s->session_lock, NULL);
+    pthread_cond_init(&s->session_cond, NULL);
+#endif
 
     return 0;
 }
@@ -741,6 +761,24 @@ static int
qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
     mfxSyncPoint sync = NULL;
     mfxStatus err;
 
+    while (!s->session_download_init && !s->session_download) {
+#if HAVE_PTHREADS
+        if (pthread_mutex_trylock(&s->session_lock) == 0) {
+#endif
+            qsv_init_internal_session(ctx, &s->session_download, 0);
+            s->session_download_init = 1;
+#if HAVE_PTHREADS
Post by Rogozhkin, Dmitry V
Post by Maxym Dmytrychenko
+            pthread_cond_signal(&s->session_cond);
You don't need to signal under the mutex. More efficiently is to do
that without.
Post by Maxym Dmytrychenko
+            pthread_mutex_unlock(&s->session_lock);
+        }
+        else {
+            pthread_mutex_lock(&s->session_lock);
+            pthread_cond_wait(&s->session_cond, &s-
Post by Maxym Dmytrychenko
session_lock);
This is incorrect usage of condition variables. Look into example in
'man 3 pthread_cond_wait': you should check predicate under the mutex.
Post by Maxym Dmytrychenko
+            pthread_mutex_unlock(&s->session_lock);
+        }
+#endif
+    }
+
As I said the above code is an example of incorrect usage of
condition
#if HAVE_PTHREADS
    if (pthread_mutex_trylock(&s->session_lock) == 0) {
#endif
        qsv_init_internal_session(ctx, &s->session_download, 0);
        s->session_download_init = 1;
Never do something 2 mins before the meeting. This should probably
be:        if (!s->session_download_init) {
            qsv_init_internal_session(ctx, &s->session_download, 0);
            if (s->session_download)
                s->session_download_init = 1;
        }
Post by Rogozhkin, Dmitry V
#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
Post by Maxym Dmytrychenko
     if (!s->session_download) {
         if (s->child_frames_ref)
             return qsv_transfer_data_child(ctx, dst, src);
@@ -788,6 +826,24 @@ static int
qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
     mfxSyncPoint sync = NULL;
     mfxStatus err;
 
+    while (!s->session_upload_init && !s->session_upload) {
+#if HAVE_PTHREADS
+        if (pthread_mutex_trylock(&s->session_lock) == 0) {
+#endif
+            qsv_init_internal_session(ctx, &s->session_upload, 1);
+            s->session_upload_init = 1;
+#if HAVE_PTHREADS
+            pthread_cond_signal(&s->session_cond);
+            pthread_mutex_unlock(&s->session_lock);
+        }
+        else {
+            pthread_mutex_lock(&s->session_lock);
+            pthread_cond_wait(&s->session_cond, &s-
Post by Maxym Dmytrychenko
session_lock);
+            pthread_mutex_unlock(&s->session_lock);
+        }
+#endif
+    }
+
#if HAVE_PTHREADS
    if (pthread_mutex_trylock(&s->session_lock) == 0) {
#endif
        qsv_init_internal_session(ctx, &s->session_upload, 0);
        s->session_upload_init = 1;
        if (!s->session_upload_init) {
            qsv_init_internal_session(ctx, &s->session_upload, 0);
            if (s->session_upload)
                s->session_upload_init = 1;
        }
Post by Rogozhkin, Dmitry V
#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
Post by Maxym Dmytrychenko
     if (!s->session_upload) {
         if (s->child_frames_ref)
             return qsv_transfer_data_child(ctx, dst, src);
_______________________________________________
libav-devel mailing list
https://lists.libav.org/mailman/listinfo/libav-devel
Maxim, did you miss these comments? or I missed a reply?

Maxym Dmytrychenko
2018-08-23 20:35:59 UTC
Permalink
Post by Maxym Dmytrychenko
Not used VPP sessions, like for hwupload/hwdownload handling,
can increase CPU utilization and this patch fixes it.
thank you,Joe, for the contribution.
---
libavutil/hwcontext_qsv.c | 72
+++++++++++++++++++++++++++++++++++++++++------
1 file changed, 64 insertions(+), 8 deletions(-)
diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
index b3eb4a3ea..6bc2a38ff 100644
--- a/libavutil/hwcontext_qsv.c
+++ b/libavutil/hwcontext_qsv.c
@@ -23,6 +23,10 @@
#include "config.h"
+#if HAVE_PTHREADS
+#include <pthread.h>
+#endif
+
#if CONFIG_VAAPI
#include "hwcontext_vaapi.h"
#endif
@@ -56,7 +60,13 @@ typedef struct QSVDeviceContext {
typedef struct QSVFramesContext {
mfxSession session_download;
+ int session_download_init;
mfxSession session_upload;
+ int session_upload_init;
+#if HAVE_PTHREADS
+ pthread_mutex_t session_lock;
+ pthread_cond_t session_cond;
+#endif
AVBufferRef *child_frames_ref;
mfxFrameSurface1 *surfaces_internal;
@@ -146,13 +156,20 @@ static void qsv_frames_uninit(AVHWFramesContext *ctx)
MFXVideoVPP_Close(s->session_download);
MFXClose(s->session_download);
}
- s->session_download = NULL;
+ s->session_download = NULL;
+ s->session_download_init = 0;
if (s->session_upload) {
MFXVideoVPP_Close(s->session_upload);
MFXClose(s->session_upload);
}
- s->session_upload = NULL;
+ s->session_upload = NULL;
+ s->session_upload_init = 0;
+
+#if HAVE_PTHREADS
+ pthread_mutex_destroy(&s->session_lock);
+ pthread_cond_destroy(&s->session_cond);
+#endif
av_freep(&s->mem_ids);
av_freep(&s->surface_ptrs);
@@ -535,13 +552,16 @@ static int qsv_frames_init(AVHWFramesContext *ctx)
s->mem_ids[i] = frames_hwctx->surfaces[i].Data.MemId;
}
- ret = qsv_init_internal_session(ctx, &s->session_download, 0);
- if (ret < 0)
- return ret;
+ s->session_download = NULL;
+ s->session_upload = NULL;
- ret = qsv_init_internal_session(ctx, &s->session_upload, 1);
- if (ret < 0)
- return ret;
+ s->session_download_init = 0;
+ s->session_upload_init = 0;
+
+#if HAVE_PTHREADS
+ pthread_mutex_init(&s->session_lock, NULL);
+ pthread_cond_init(&s->session_cond, NULL);
+#endif
return 0;
}
@@ -741,6 +761,24 @@ static int qsv_transfer_data_from(AVHWFramesContext
*ctx, AVFrame *dst,
mfxSyncPoint sync = NULL;
mfxStatus err;
+ while (!s->session_download_init && !s->session_download) {
+#if HAVE_PTHREADS
+ if (pthread_mutex_trylock(&s->session_lock) == 0) {
+#endif
+ qsv_init_internal_session(ctx, &s->session_download, 0);
+ s->session_download_init = 1;
+#if HAVE_PTHREADS
+ pthread_cond_signal(&s->session_cond);
+ pthread_mutex_unlock(&s->session_lock);
+ }
+ else {
+ pthread_mutex_lock(&s->session_lock);
+ pthread_cond_wait(&s->session_cond, &s->session_lock);
+ pthread_mutex_unlock(&s->session_lock);
+ }
+#endif
+ }
+
if (!s->session_download) {
if (s->child_frames_ref)
return qsv_transfer_data_child(ctx, dst, src);
@@ -788,6 +826,24 @@ static int qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
mfxSyncPoint sync = NULL;
mfxStatus err;
+ while (!s->session_upload_init && !s->session_upload) {
+#if HAVE_PTHREADS
+ if (pthread_mutex_trylock(&s->session_lock) == 0) {
+#endif
+ qsv_init_internal_session(ctx, &s->session_upload, 1);
+ s->session_upload_init = 1;
+#if HAVE_PTHREADS
+ pthread_cond_signal(&s->session_cond);
+ pthread_mutex_unlock(&s->session_lock);
+ }
+ else {
+ pthread_mutex_lock(&s->session_lock);
+ pthread_cond_wait(&s->session_cond, &s->session_lock);
+ pthread_mutex_unlock(&s->session_lock);
+ }
+#endif
+ }
+
if (!s->session_upload) {
if (s->child_frames_ref)
return qsv_transfer_data_child(ctx, dst, src);
--
2.15.2 (Apple Git-101.1)
ping if any other comments

thanks
Max
Loading...