> -----Original Message-----
> From: Rogozhkin, Dmitry V
> Sent: Wednesday, July 25, 2018 1:36 AM
> To: [email protected]
> Cc: Rogozhkin, Dmitry V <[email protected]>; Maxym
> Dmytrychenko <[email protected]>; Li, Zhong <[email protected]>
> 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 <[email protected]>
> Cc: Maxym Dmytrychenko <[email protected]>
> Cc: Zhong Li <[email protected]>
> ---
> 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.
> 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.
BTW, currently the option "async_depth" range is 0 ~ INT_MAX. Need to change it
to 1 ~ INT_MAX now, no?
> 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
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
