[FFmpeg-devel] [PATCH, v2] lavc/qsvenc: replace assert with error return

[Fu, Linjie]

> -----Original Message-----
> From: Li, Zhong
> Sent: Thursday, December 6, 2018 20:30
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Cc: Fu, Linjie <linjie.fu at intel.com>
> Subject: RE: [FFmpeg-devel] [PATCH, v2] lavc/qsvenc: replace assert with
> error return
> 
> > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On
> Behalf
> > Of Linjie Fu
> > Sent: Thursday, December 6, 2018 7:19 PM
> > To: ffmpeg-devel at ffmpeg.org
> > Cc: Fu, Linjie <linjie.fu at intel.com>
> > Subject: [FFmpeg-devel] [PATCH, v2] lavc/qsvenc: replace assert with error
> > return
> >
> > bs->FrameType is not set in MSDK in some cases (mjpeg encode for
> > bs->example),
> > and assert on a value coming from an external library is not proper.
> >
> > Add default type check for bs->FrameType, and return invalid data error in
> > function ff_qsv_encode to avoid using uninitialized value.
> >
> > Fix #7593.
> >
> > Signed-off-by: Linjie Fu <linjie.fu at intel.com>
> > ---
> > [v2]: Add default bs->FrameType check.
> >
> >  libavcodec/qsvenc.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c index
> > 7f4592f878..917344b60c 100644
> > --- a/libavcodec/qsvenc.c
> > +++ b/libavcodec/qsvenc.c
> > @@ -1337,8 +1337,10 @@ int ff_qsv_encode(AVCodecContext *avctx,
> > QSVEncContext *q,
> >              pict_type = AV_PICTURE_TYPE_P;
> >          else if (bs->FrameType & MFX_FRAMETYPE_B || bs->FrameType
> > & MFX_FRAMETYPE_xB)
> >              pict_type = AV_PICTURE_TYPE_B;
> > +        else if (bs->FrameType == MFX_FRAMETYPE_UNKNOWN)
> 
> Unknown frame type has potential risk and would better give a waring log
> message here.

Thanks, will add. 

> 
> > +            pict_type = AV_PICTURE_TYPE_NONE;
> >          else
> > -            av_assert0(!"Uninitialized pict_type!");
> > +            return AVERROR_INVALIDDATA;
> 
> If frame type is MFX_FRAMETYPE_IDR or MFX_FRAMETYPE_xIDR, will go
> here but this is not invalid data.
> So pict_tpye should be set to AV_PICTURE_TYPE_I in this case I believe.

It seems that  Frame types such as MFX_FRAMETYPE_IDR and 
MFX_FRAMETYPE_xIDR will be set along with MFX_FRAMETYPE_I in MSDK. 
But frame type which only contains IDR or xIDR should also be set to type I.

Will also add an log message to report an error.

Thanks
Linjie