On 17/07/18 17:07, 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 <maxim....@gmail.com>
> ---
>  libavutil/hwcontext_qsv.c | 38 +++++++++++++++++++++++++++++---------
>  1 file changed, 29 insertions(+), 9 deletions(-)
> 
> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> index b3eb4a3ea..390c3aac4 100644
> --- a/libavutil/hwcontext_qsv.c
> +++ b/libavutil/hwcontext_qsv.c
> @@ -18,6 +18,7 @@
>  
>  #include <stdint.h>
>  #include <string.h>
> +#include <stdatomic.h>
>  
>  #include <mfx/mfxvideo.h>
>  
> @@ -56,7 +57,9 @@ typedef struct QSVDeviceContext {
>  
>  typedef struct QSVFramesContext {
>      mfxSession session_download;
> +    atomic_int session_download_init;
>      mfxSession session_upload;
> +    atomic_int session_upload_init;
>  
>      AVBufferRef *child_frames_ref;
>      mfxFrameSurface1 *surfaces_internal;
> @@ -146,13 +149,15 @@ 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;
>  
>      av_freep(&s->mem_ids);
>      av_freep(&s->surface_ptrs);
> @@ -535,13 +540,10 @@ 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;
> -
> -    ret = qsv_init_internal_session(ctx, &s->session_upload, 1);
> -    if (ret < 0)
> -        return ret;
> +    s->session_download      = NULL;
> +    s->session_upload        = NULL;
> +    s->session_download_init = 0;
> +    s->session_upload_init   = 0;
>  
>      return 0;
>  }
> @@ -741,6 +743,15 @@ static int qsv_transfer_data_from(AVHWFramesContext 
> *ctx, AVFrame *dst,
>      mfxSyncPoint sync = NULL;
>      mfxStatus err;
>  
> +    while (!s->session_download_init && !s->session_download) {
> +     if (atomic_fetch_add(&s->session_download_init, 1) == 0) {
> +            qsv_init_internal_session(ctx, &s->session_download, 0);
> +        }
> +     else {
> +            av_usleep(1);

This races - consider what happens if the other thread is preempted for more 
than 1µs, or if the initialisation itself takes more than that long.

You need to actually do some synchronisation here (e.g. with a 'once' variable) 
- with only atomic flags there is no way to guarantee that the other thread has 
finished unless you spin, which isn't acceptable.

> +        }
> +    }
> +
>      if (!s->session_download) {
>          if (s->child_frames_ref)
>              return qsv_transfer_data_child(ctx, dst, src);
> @@ -788,6 +799,15 @@ static int qsv_transfer_data_to(AVHWFramesContext *ctx, 
> AVFrame *dst,
>      mfxSyncPoint sync = NULL;
>      mfxStatus err;
>  
> +    while (!s->session_upload_init && !s->session_upload) {
> +     if (atomic_fetch_add(&s->session_upload_init, 1) == 0) {
> +            qsv_init_internal_session(ctx, &s->session_upload, 1);
> +        }
> +     else {
> +            av_usleep(1);
> +        }
> +    }
> +
>      if (!s->session_upload) {
>          if (s->child_frames_ref)
>              return qsv_transfer_data_child(ctx, dst, src);
> 

Thanks,

- Mark
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to