On Thu, Jul 26, 2018 at 7:55 AM Li, Zhong <[email protected]> wrote:
> > -----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. > > 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 [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
