[FFmpeg-devel] [PATCH v3 3/6] lavc/qsvdec: Replace current parser with MFXVideoDECODE_DecodeHeader()

[Li, Zhong]

> From: Rogozhkin, Dmitry V
> Sent: Tuesday, March 12, 2019 8:04 AM
> To: Li, Zhong <zhong.li at intel.com>; ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v3 3/6] lavc/qsvdec: Replace current
> parser with MFXVideoDECODE_DecodeHeader()
> 
> pix_fmts[1] is misleading. And I think it can be quite easily avoided.
> 
> If I understand the code correctly, qsv_decode_preinit is the place (and the
> only place) where you need an array of pix_fmts. You actually request
> ffmpeg to select one of the formats and program it into avctx.
> Is that right?
> 
> If so, could we try to define the array exactly in this function and avoid
> passing the whole array around? See comments below for suggestions...
> 
> 
> On Mon, 2019-03-11 at 17:23 +0800, Li, Zhong wrote:
> > > -----Original Message-----
> > > From: Rogozhkin, Dmitry V
> > > Sent: Saturday, March 9, 2019 8:48 AM
> > > To: ffmpeg-devel at ffmpeg.org
> > > Cc: Li, Zhong <zhong.li at intel.com>
> > > Subject: Re: [FFmpeg-devel] [PATCH v3 3/6] lavc/qsvdec: Replace
> > > current parser with MFXVideoDECODE_DecodeHeader()
> > >
> > > On Fri, 2019-03-08 at 15:40 +0800, Zhong Li wrote:
> > > > Using MSDK parser can improve qsv decoder pass rate in some cases
> > > > (E.g:
> > > > sps declares a wrong level_idc, smaller than it should be).
> > > > And it is necessary for adding new qsv decoders such as MJPEG and
> > > > VP9
> > > > since current parser can't provide enough information.
> > > > Actually using MFXVideoDECODE_DecodeHeader() was disscussed at
> > > > https://ffmpeg.org/pipermail/ffmpeg-devel/2015-July/175734.html
> > > > and
> > > > merged as commit 1acb19d, but was overwritten when merged libav
> > > > patches (commit: 1f26a23) without any explain.
> > > >
> > > > v2: split decode header from decode_init, and call it for
> > > > everyframe to detect format/resoultion change. It can fix some
> > > > regression issues such as hevc 10bits decoding.
> > > >
> > > > Signed-off-by: Zhong Li <zhong.li at intel.com>
> > > > ---
> > > >  libavcodec/qsvdec.c       | 172
> ++++++++++++++++++++----------
> > > > ----
> > > > ----
> > > >  libavcodec/qsvdec.h       |   2 +
> > > >  libavcodec/qsvdec_h2645.c |   1 +
> > > >  libavcodec/qsvdec_other.c |   1 +
> > > >  4 files changed, 93 insertions(+), 83 deletions(-)
> > > >
> > > > diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c index
> > > > 4a0be811fb..b78026e14d 100644
> > > > --- a/libavcodec/qsvdec.c
> > > > +++ b/libavcodec/qsvdec.c
> > > > @@ -120,19 +120,17 @@ 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)
> > > > +static int qsv_decode_preinit(AVCodecContext *avctx, QSVContext
> > > > *q,
> > > > enum AVPixelFormat *pix_fmts, mfxVideoParam *param)
> 
> Just: enum AVPixelFormat pix_fmt. Single one we need.

No, it must be array, and the last element of this array must be AV_PIX_FMT_NONE (else function ff_get_format() doesn't know how many elements contained by this array). 
The first element of pix_fmt[] is HW pixel format, the second element of pix_fmt[] is SW pixel format. 
Once again, I would like you to take a look the implementation detail of ff_get_format ().