On Wed, 2018-07-25 at 16:14 +0000, Rogozhkin, Dmitry V wrote: > On Tue, 2018-07-24 at 09:53 -0700, Dmitry Rogozhkin wrote: > > On Wed, 2018-07-18 at 14:07 +0200, Maxym Dmytrychenko wrote: > > > 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 <[email protected]> > > > --- > > > 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. > > > + 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. > > > + 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; > > 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; > } > > #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 > > > > > > > > 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; > > 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; > } > > #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 > > > > > if (!s->session_upload) { > > > if (s->child_frames_ref) > > > return qsv_transfer_data_child(ctx, dst, src); > > _______________________________________________ > libav-devel mailing list > [email protected] > https://lists.libav.org/mailman/listinfo/libav-devel
Maxim, did you miss these comments? or I missed a reply? _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
