date:20180726

Re: [libav-devel] [PATCH] avcodec/qsv: fix async support

2018-07-26 Thread Rogozhkin, Dmitry V

On Thu, 2018-07-26 at 09:44 +0200, Maxym Dmytrychenko wrote:


On Thu, Jul 26, 2018 at 7:55 AM Li, Zhong 
mailto:zhong...@intel.com>> wrote:
> -Original Message-
> From: Rogozhkin, Dmitry V
> Sent: Wednesday, July 25, 2018 1:36 AM
> To: libav-devel@libav.org
> Cc: Rogozhkin, Dmitry V 
> mailto:dmitry.v.rogozh...@intel.com>>; Maxym
> Dmytrychenko mailto:maxim@gmail.com>>; Li, Zhong 
> mailto:zhong...@intel.com>>
> Subject: [PATCH] avcodec/qsv: fix async support
>
> Current implementations of qsv components incorrectly work with async
> level, they actually try to work in async+1 level stepping into
> MFX_WRN_DEVICE_BUSY and polling loop. This change address this
> misbehaviour.
>
> Signed-off-by: Dmitry Rogozhkin 
> mailto:dmitry.v.rogozh...@intel.com>>
> Cc: Maxym Dmytrychenko mailto:maxim@gmail.com>>
> Cc: Zhong Li mailto:zhong...@intel.com>>
> ---
>  libavcodec/qsvdec.c | 15 ---  libavcodec/qsvenc.c | 17
> +
>  2 files changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c index
> 32f1fe7..b9707f7 100644
> --- a/libavcodec/qsvdec.c
> +++ b/libavcodec/qsvdec.c
> @@ -110,6 +110,16 @@ static int qsv_init_session(AVCodecContext *avctx,
> QSVContext *q, mfxSession ses
>  return 0;
>  }
>
> +static inline unsigned int qsv_fifo_item_size(void) {
> +return sizeof(mfxSyncPoint*) + sizeof(QSVFrame*); }
> +
> +static inline unsigned int qsv_fifo_size(const AVFifoBuffer* fifo) {
> +return av_fifo_size(fifo)/qsv_fifo_item_size();
> +}
> +
>  static int qsv_decode_init(AVCodecContext *avctx, QSVContext *q)  {
>  const AVPixFmtDescriptor *desc;
> @@ -125,8 +135,7 @@ static int qsv_decode_init(AVCodecContext *avctx,
> QSVContext *q)
>  return AVERROR_BUG;
>
>  if (!q->async_fifo) {
> -q->async_fifo = av_fifo_alloc((1 + q->async_depth) *
> -  (sizeof(mfxSyncPoint*) +
> sizeof(QSVFrame*)));
> +q->async_fifo = av_fifo_alloc(q->async_depth *
> + qsv_fifo_item_size());
>  if (!q->async_fifo)
>  return AVERROR(ENOMEM);
>  }
> @@ -384,7 +393,7 @@ static int qsv_decode(AVCodecContext *avctx,
> QSVContext *q,
>  av_freep(&sync);
>  }
>
> -if (!av_fifo_space(q->async_fifo) ||
> +if ((qsv_fifo_size(q->async_fifo) >= q->async_depth) ||
>  (!avpkt->size && av_fifo_size(q->async_fifo))) {
>  AVFrame *src_frame;
>
> diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c index
> 3ce5ffe..40ddb34 100644
> --- a/libavcodec/qsvenc.c
> +++ b/libavcodec/qsvenc.c
> @@ -777,7 +777,7 @@ static int qsv_init_opaque_alloc(AVCodecContext
> *avctx, QSVEncContext *q)
>  mfxFrameSurface1 *surfaces;
>  int nb_surfaces, i;
>
> -nb_surfaces = qsv->nb_opaque_surfaces +
> q->req.NumFrameSuggested + q->async_depth;
> +nb_surfaces = qsv->nb_opaque_surfaces +
> q->req.NumFrameSuggested;
>
>  q->opaque_alloc_buf = av_buffer_allocz(sizeof(*surfaces) *
> nb_surfaces);
>  if (!q->opaque_alloc_buf)
> @@ -848,6 +848,16 @@ static int qsvenc_init_session(AVCodecContext
> *avctx, QSVEncContext *q)
>  return 0;
>  }
>
> +static inline unsigned int qsv_fifo_item_size(void) {
> +return sizeof(AVPacket) + sizeof(mfxSyncPoint*) +
> +sizeof(mfxBitstream*); }
> +
> +static inline unsigned int qsv_fifo_size(const AVFifoBuffer* fifo) {
> +return av_fifo_size(fifo)/qsv_fifo_item_size();
> +}
> +
Add a blank before and after "/" is a unified coding style.
Maybe better to move it to qsv.c since it is common for qsvdec/enc.


good point - agree.


I uploaded v2 of the patch. I added spaces, but I did not move the 
qsv_fifo_size to the common place. Indeed it is the same between dec/enc, but 
qsv_fifo_item_size is not the same (sizeof queue objects are different). Thus, 
I can move only qsv_fifo_size and rely on the symbol resolution that 
qsv_fifo_item_size will be found. I don't think this is a good idea. So, I left 
as is, but fixed white space.

>  int ff_qsv_enc_init(AVCodecContext *avctx, QSVEncContext *q)  {
>  int iopattern = 0;
> @@ -856,8 +866,7 @@ int ff_qsv_enc_init(AVCodecContext *avctx,
> QSVEncContext *q)
>
>  q->param.AsyncDepth = q->async_depth;
>
> -q->async_fifo = av_fifo_alloc((1 + q->async_depth) *
> -  (sizeof(AVPacket) +
> sizeof(mfxSyncPoint*) + sizeof(mfxBitstream*)));
> +q->async_fifo = av_fifo_alloc(q->async_depth *
> + qsv_fifo_item_size());

I agree with remove the "+1". And noted someone was also confused too: 
http://ffmpeg.org/pipermail/ffmpeg-devel/2017-November/219643.html
Any historic reason to add "+1" in the initial implementation? If no, I would 
like to see this patch applied.


just a minor reason.

BTW, currently the option "async_depth" range is 0 ~ INT_MAX. Need to change it 
to 1 ~ INT_MAX now, no?


yes, will adjust this as well

I adjusted the range. Actually this can be handled other way round. If

[libav-devel] [PATCH v2] avcodec/qsv: fix async support

2018-07-26 Thread Dmitry Rogozhkin

Current implementations of qsv components incorrectly work with async level, 
they
actually try to work in async+1 level stepping into MFX_WRN_DEVICE_BUSY and 
polling
loop. This change address this misbehaviour.

Signed-off-by: Dmitry Rogozhkin 
Cc: Maxym Dmytrychenko 
Cc: Zhong Li 
---
 libavcodec/qsvdec.c   | 15 ---
 libavcodec/qsvdec_h2645.c |  4 ++--
 libavcodec/qsvdec_other.c |  2 +-
 libavcodec/qsvenc.c   | 17 +
 libavcodec/qsvenc.h   |  2 +-
 5 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c
index 32f1fe7..22e7a46 100644
--- a/libavcodec/qsvdec.c
+++ b/libavcodec/qsvdec.c
@@ -110,6 +110,16 @@ static int qsv_init_session(AVCodecContext *avctx, 
QSVContext *q, mfxSession ses
 return 0;
 }
 
+static inline unsigned int qsv_fifo_item_size(void)
+{
+return sizeof(mfxSyncPoint*) + sizeof(QSVFrame*);
+}
+
+static inline unsigned int qsv_fifo_size(const AVFifoBuffer* fifo)
+{
+return av_fifo_size(fifo) / qsv_fifo_item_size();
+}
+
 static int qsv_decode_init(AVCodecContext *avctx, QSVContext *q)
 {
 const AVPixFmtDescriptor *desc;
@@ -125,8 +135,7 @@ static int qsv_decode_init(AVCodecContext *avctx, 
QSVContext *q)
 return AVERROR_BUG;
 
 if (!q->async_fifo) {
-q->async_fifo = av_fifo_alloc((1 + q->async_depth) *
-  (sizeof(mfxSyncPoint*) + 
sizeof(QSVFrame*)));
+q->async_fifo = av_fifo_alloc(q->async_depth * qsv_fifo_item_size());
 if (!q->async_fifo)
 return AVERROR(ENOMEM);
 }
@@ -384,7 +393,7 @@ static int qsv_decode(AVCodecContext *avctx, QSVContext *q,
 av_freep(&sync);
 }
 
-if (!av_fifo_space(q->async_fifo) ||
+if ((qsv_fifo_size(q->async_fifo) >= q->async_depth) ||
 (!avpkt->size && av_fifo_size(q->async_fifo))) {
 AVFrame *src_frame;
 
diff --git a/libavcodec/qsvdec_h2645.c b/libavcodec/qsvdec_h2645.c
index 831252f..d9d2318 100644
--- a/libavcodec/qsvdec_h2645.c
+++ b/libavcodec/qsvdec_h2645.c
@@ -186,7 +186,7 @@ static void qsv_decode_flush(AVCodecContext *avctx)
 
 #if CONFIG_HEVC_QSV_DECODER
 static const AVOption hevc_options[] = {
-{ "async_depth", "Internal parallelization depth, the higher the value the 
higher the latency.", OFFSET(qsv.async_depth), AV_OPT_TYPE_INT, { .i64 = 
ASYNC_DEPTH_DEFAULT }, 0, INT_MAX, VD },
+{ "async_depth", "Internal parallelization depth, the higher the value the 
higher the latency.", OFFSET(qsv.async_depth), AV_OPT_TYPE_INT, { .i64 = 
ASYNC_DEPTH_DEFAULT }, 1, INT_MAX, VD },
 
 { "load_plugin", "A user plugin to load in an internal session", 
OFFSET(load_plugin), AV_OPT_TYPE_INT, { .i64 = LOAD_PLUGIN_DEFAULT }, 
LOAD_PLUGIN_NONE, LOAD_PLUGIN_HEVC_HW, VD, "load_plugin" },
 { "none", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = LOAD_PLUGIN_NONE },
0, 0, VD, "load_plugin" },
@@ -229,7 +229,7 @@ AVCodec ff_hevc_qsv_decoder = {
 
 #if CONFIG_H264_QSV_DECODER
 static const AVOption options[] = {
-{ "async_depth", "Internal parallelization depth, the higher the value the 
higher the latency.", OFFSET(qsv.async_depth), AV_OPT_TYPE_INT, { .i64 = 
ASYNC_DEPTH_DEFAULT }, 0, INT_MAX, VD },
+{ "async_depth", "Internal parallelization depth, the higher the value the 
higher the latency.", OFFSET(qsv.async_depth), AV_OPT_TYPE_INT, { .i64 = 
ASYNC_DEPTH_DEFAULT }, 1, INT_MAX, VD },
 { NULL },
 };
 
diff --git a/libavcodec/qsvdec_other.c b/libavcodec/qsvdec_other.c
index 3c872dc..f6e08a2 100644
--- a/libavcodec/qsvdec_other.c
+++ b/libavcodec/qsvdec_other.c
@@ -159,7 +159,7 @@ static void qsv_decode_flush(AVCodecContext *avctx)
 #define OFFSET(x) offsetof(QSVOtherContext, x)
 #define VD AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_DECODING_PARAM
 static const AVOption options[] = {
-{ "async_depth", "Internal parallelization depth, the higher the value the 
higher the latency.", OFFSET(qsv.async_depth), AV_OPT_TYPE_INT, { .i64 = 
ASYNC_DEPTH_DEFAULT }, 0, INT_MAX, VD },
+{ "async_depth", "Internal parallelization depth, the higher the value the 
higher the latency.", OFFSET(qsv.async_depth), AV_OPT_TYPE_INT, { .i64 = 
ASYNC_DEPTH_DEFAULT }, 1, INT_MAX, VD },
 { NULL },
 };
 
diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
index 3ce5ffe..b08fa9d 100644
--- a/libavcodec/qsvenc.c
+++ b/libavcodec/qsvenc.c
@@ -777,7 +777,7 @@ static int qsv_init_opaque_alloc(AVCodecContext *avctx, 
QSVEncContext *q)
 mfxFrameSurface1 *surfaces;
 int nb_surfaces, i;
 
-nb_surfaces = qsv->nb_opaque_surfaces + q->req.NumFrameSuggested + 
q->async_depth;
+nb_surfaces = qsv->nb_opaque_surfaces + q->req.NumFrameSuggested;
 
 q->opaque_alloc_buf = av_buffer_allocz(sizeof(*surfaces) * nb_surfaces);
 if (!q->opaque_alloc_buf)
@@ -848,6 +848,16 @@ static int qsvenc_init_session(AVCodecContext *avctx, 
QSVEncContext *q)
 return 0;
 }
 
+static inline unsigned int qsv_fifo_item_size(void)
+{
+return sizeo

Re: [libav-devel] [PATCH v6] qsvvpp: Fix to perform full init only when needed

2018-07-26 Thread Rogozhkin, Dmitry V

On Thu, 2018-07-26 at 15:48 +0200, Joe Olivas wrote:
> Removing unused VPP sessions by initializing only when used in order
> to help reduce CPU utilization. Thanks to Maxym for the guidance.
> 
> Signed-off-by: Joe Olivas 
Please, add:
Cc: Maxym Dmytrychenko 
Cc: Dmitry Rogozhkin 

To keep us in the review loop.
> ---
>  libavutil/hwcontext_qsv.c | 78 -
> --
>  1 file changed, 72 insertions(+), 6 deletions(-)
> 
> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> index 250091c4e8..fc4a4127e3 100644
> --- a/libavutil/hwcontext_qsv.c
> +++ b/libavutil/hwcontext_qsv.c
> @@ -23,6 +23,10 @@
>  
>  #include "config.h"
>  
> +#if HAVE_PTHREADS
> +#include 
> +#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;
> @@ -147,12 +157,19 @@ static void qsv_frames_uninit(AVHWFramesContext
> *ctx)
>  MFXClose(s->session_download);
>  }
>  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_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,29 @@ static int
> qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
>  mfxSyncPoint sync = NULL;
>  mfxStatus err;
>  
> +while (!s->session_download_init && !s->session_download) {

I don't think you need this while at all.

> +#if HAVE_PTHREADS
> + if (pthread_mutex_trylock(&s->session_lock) == 0) {
> +#endif
> +if (!s->session_download_init) {
> +qsv_init_internal_session(ctx, &s->session_download, 
> 0);

I can imagine the only one situation why you need the while above: this
function fails and you need to try again, again and again. Is that
possible or if function failed it failed permanently?

> +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) {
This condition starts to be _very_ strange especially if you will
motivate why you need a while above:). If you don't need the while
above (and I think you don't), then this condition is somewhat like an
error handling in the case when for some reason you failed to
initialize session_download. Unfortunately I don't know whether you can
or can not meet such a situation.

>  if (s->child_frames_ref)
>  return qsv_transfer_data_child(ctx, dst, src);
> @@ -788,6 +831,29 @@ static int
> qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
>  mfxSyncPoint sync = NULL;
>  mfxStatus err;
>  
> +while (!s->session_upload_init && !s->session_upload) {

Same comment as above.

> +#if HAVE_PTHREADS
> + if (pthread_mutex_trylock(&s->session_lock) == 0) {
> +#endif
> +if (!s->session_upload_init) {
> +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);
> +}
> +

[libav-devel] [PATCH v6] qsvvpp: Fix to perform full init only when needed

2018-07-26 Thread Joe Olivas

Removing unused VPP sessions by initializing only when used in order to help 
reduce CPU utilization. Thanks to Maxym for the guidance.

Signed-off-by: Joe Olivas 
---
 libavutil/hwcontext_qsv.c | 78 ---
 1 file changed, 72 insertions(+), 6 deletions(-)

diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
index 250091c4e8..fc4a4127e3 100644
--- a/libavutil/hwcontext_qsv.c
+++ b/libavutil/hwcontext_qsv.c
@@ -23,6 +23,10 @@
 
 #include "config.h"
 
+#if HAVE_PTHREADS
+#include 
+#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;
@@ -147,12 +157,19 @@ static void qsv_frames_uninit(AVHWFramesContext *ctx)
 MFXClose(s->session_download);
 }
 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_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,29 @@ 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
+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 +831,29 @@ 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
+if (!s->session_upload_init) {
+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
+}
+
 if (!s->session_upload) {
 if (s->child_frames_ref)
 return qsv_transfer_data_child(ctx, dst, src);
-- 
2.18.0

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

Re: [libav-devel] [PATCH] avcodec/qsv: fix async support

2018-07-26 Thread Maxym Dmytrychenko

On Thu, Jul 26, 2018 at 7:55 AM Li, Zhong  wrote:

> > -Original Message-
> > From: Rogozhkin, Dmitry V
> > Sent: Wednesday, July 25, 2018 1:36 AM
> > To: libav-devel@libav.org
> > Cc: Rogozhkin, Dmitry V ; Maxym
> > Dmytrychenko ; Li, Zhong 
> > Subject: [PATCH] avcodec/qsv: fix async support
> >
> > Current implementations of qsv components incorrectly work with async
> > level, they actually try to work in async+1 level stepping into
> > MFX_WRN_DEVICE_BUSY and polling loop. This change address this
> > misbehaviour.
> >
> > Signed-off-by: Dmitry Rogozhkin 
> > Cc: Maxym Dmytrychenko 
> > Cc: Zhong Li 
> > ---
> >  libavcodec/qsvdec.c | 15 ---  libavcodec/qsvenc.c | 17
> > +
> >  2 files changed, 25 insertions(+), 7 deletions(-)
> >
> > diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c index
> > 32f1fe7..b9707f7 100644
> > --- a/libavcodec/qsvdec.c
> > +++ b/libavcodec/qsvdec.c
> > @@ -110,6 +110,16 @@ static int qsv_init_session(AVCodecContext *avctx,
> > QSVContext *q, mfxSession ses
> >  return 0;
> >  }
> >
> > +static inline unsigned int qsv_fifo_item_size(void) {
> > +return sizeof(mfxSyncPoint*) + sizeof(QSVFrame*); }
> > +
> > +static inline unsigned int qsv_fifo_size(const AVFifoBuffer* fifo) {
> > +return av_fifo_size(fifo)/qsv_fifo_item_size();
> > +}
> > +
> >  static int qsv_decode_init(AVCodecContext *avctx, QSVContext *q)  {
> >  const AVPixFmtDescriptor *desc;
> > @@ -125,8 +135,7 @@ static int qsv_decode_init(AVCodecContext *avctx,
> > QSVContext *q)
> >  return AVERROR_BUG;
> >
> >  if (!q->async_fifo) {
> > -q->async_fifo = av_fifo_alloc((1 + q->async_depth) *
> > -  (sizeof(mfxSyncPoint*) +
> > sizeof(QSVFrame*)));
> > +q->async_fifo = av_fifo_alloc(q->async_depth *
> > + qsv_fifo_item_size());
> >  if (!q->async_fifo)
> >  return AVERROR(ENOMEM);
> >  }
> > @@ -384,7 +393,7 @@ static int qsv_decode(AVCodecContext *avctx,
> > QSVContext *q,
> >  av_freep(&sync);
> >  }
> >
> > -if (!av_fifo_space(q->async_fifo) ||
> > +if ((qsv_fifo_size(q->async_fifo) >= q->async_depth) ||
> >  (!avpkt->size && av_fifo_size(q->async_fifo))) {
> >  AVFrame *src_frame;
> >
> > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c index
> > 3ce5ffe..40ddb34 100644
> > --- a/libavcodec/qsvenc.c
> > +++ b/libavcodec/qsvenc.c
> > @@ -777,7 +777,7 @@ static int qsv_init_opaque_alloc(AVCodecContext
> > *avctx, QSVEncContext *q)
> >  mfxFrameSurface1 *surfaces;
> >  int nb_surfaces, i;
> >
> > -nb_surfaces = qsv->nb_opaque_surfaces +
> > q->req.NumFrameSuggested + q->async_depth;
> > +nb_surfaces = qsv->nb_opaque_surfaces +
> > q->req.NumFrameSuggested;
> >
> >  q->opaque_alloc_buf = av_buffer_allocz(sizeof(*surfaces) *
> > nb_surfaces);
> >  if (!q->opaque_alloc_buf)
> > @@ -848,6 +848,16 @@ static int qsvenc_init_session(AVCodecContext
> > *avctx, QSVEncContext *q)
> >  return 0;
> >  }
> >
> > +static inline unsigned int qsv_fifo_item_size(void) {
> > +return sizeof(AVPacket) + sizeof(mfxSyncPoint*) +
> > +sizeof(mfxBitstream*); }
> > +
> > +static inline unsigned int qsv_fifo_size(const AVFifoBuffer* fifo) {
> > +return av_fifo_size(fifo)/qsv_fifo_item_size();
> > +}
> > +
> Add a blank before and after "/" is a unified coding style.
> Maybe better to move it to qsv.c since it is common for qsvdec/enc.
>
>
good point - agree.


> >  int ff_qsv_enc_init(AVCodecContext *avctx, QSVEncContext *q)  {
> >  int iopattern = 0;
> > @@ -856,8 +866,7 @@ int ff_qsv_enc_init(AVCodecContext *avctx,
> > QSVEncContext *q)
> >
> >  q->param.AsyncDepth = q->async_depth;
> >
> > -q->async_fifo = av_fifo_alloc((1 + q->async_depth) *
> > -  (sizeof(AVPacket) +
> > sizeof(mfxSyncPoint*) + sizeof(mfxBitstream*)));
> > +q->async_fifo = av_fifo_alloc(q->async_depth *
> > + qsv_fifo_item_size());
>
> I agree with remove the "+1". And noted someone was also confused too:
> http://ffmpeg.org/pipermail/ffmpeg-devel/2017-November/219643.html
> Any historic reason to add "+1" in the initial implementation? If no, I
> would like to see this patch applied.
>
>
just a minor reason.


> BTW, currently the option "async_depth" range is 0 ~ INT_MAX. Need to
> change it to 1 ~ INT_MAX now, no?
>
>
yes, will adjust this as well


>  if (!q->async_fifo)
> >  return AVERROR(ENOMEM);
> >
> > @@ -1214,7 +1223,7 @@ int ff_qsv_encode(AVCodecContext *avctx,
> > QSVEncContext *q,
> >  if (ret < 0)
> >  return ret;
> >
> > -if (!av_fifo_space(q->async_fifo) ||
> > +if ((qsv_fifo_size(q->async_fifo) >= q->async_depth) ||
> >  (!frame && av_fifo_size(q->async_fifo))) {
> >  AVPacket new_pkt;
> >  mfxBitstream *bs;
> > --
> > 1.8.3.1
>
>
___
libav-devel mailing list
libav-devel@libav.org
https:/

Re: [libav-devel] [PATCH] avcodec/qsv: fix async support

[libav-devel] [PATCH v2] avcodec/qsv: fix async support

Re: [libav-devel] [PATCH v6] qsvvpp: Fix to perform full init only when needed

[libav-devel] [PATCH v6] qsvvpp: Fix to perform full init only when needed

Re: [libav-devel] [PATCH] avcodec/qsv: fix async support

5 matches

Site Navigation

Mail list logo

Footer information