On 16/07/18 14:26, 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 | 35 ++++++++++++++++++++++++++---------
> 1 file changed, 26 insertions(+), 9 deletions(-)
This makes sense, but it looks like it might need some sort of 'once'
construction for thread-safety?
I believe the current API intent is that performing simultaneous transfer
operations on different frames in the same frames context should be safe.
> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> index b3eb4a3ea..3e6c38037 100644
> --- a/libavutil/hwcontext_qsv.c
> +++ b/libavutil/hwcontext_qsv.c
> @@ -56,7 +56,9 @@ typedef struct QSVDeviceContext {
>
> typedef struct QSVFramesContext {
> mfxSession session_download;
> + int session_download_init;
> mfxSession session_upload;
> + int session_upload_init;
>
> AVBufferRef *child_frames_ref;
> mfxFrameSurface1 *surfaces_internal;
> @@ -146,13 +148,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 +539,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;
> }
> @@ -740,6 +741,14 @@ static int qsv_transfer_data_from(AVHWFramesContext
> *ctx, AVFrame *dst,
>
> mfxSyncPoint sync = NULL;
> mfxStatus err;
> + int ret = -1;
The initialisation is redundant? The -1 confused me, but I don't think it's
ever read anywhere.
> +
> + if (!s->session_download_init) {
> + s->session_download_init = 1;
> + ret = qsv_init_internal_session(ctx, &s->session_download, 0);
> + if (ret < 0)
> + return ret;
> + }
>
> if (!s->session_download) {
> if (s->child_frames_ref)
> @@ -787,6 +796,14 @@ static int qsv_transfer_data_to(AVHWFramesContext *ctx,
> AVFrame *dst,
>
> mfxSyncPoint sync = NULL;
> mfxStatus err;
> + int ret = -1;
Likewise this one.
> +
> + if (!s->session_upload_init) {
> + s->session_upload_init = 1;
> + ret = qsv_init_internal_session(ctx, &s->session_upload, 1);
> + if (ret < 0)
> + return ret;
> + }
>
> if (!s->session_upload) {
> if (s->child_frames_ref)
>
Thanks,
- Mark
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
