[FFmpeg-devel] [PATCH 1/5] lavc/qsvenc: add forced_idr opiton

[Rogozhkin, Dmitry V]

On Tue, 2018-10-30 at 18:05 +0800, Li, Zhong wrote:
> > > +        } else
> > > +            enc_ctrl->FrameType = MFX_FRAMETYPE_UNKNOWN;
> > 
> > "else" block don't make much sense to me. You eventually already
> > had
> > enc_ctrl structure passed to the encoder. Thus, it should be
> > initialized to
> > default (already). And you don't make anything specific/new in the
> > "else".
> > From my perspective "else" just obscures the code and should be
> > dropped.
> 
> This was a case I had concern. I doubt the default initialization is
> always zero 
> (you know MFX_FRAMETYPE_UNKNOWN is zero). Isn't it possible? 
> Please check the regression case I fixed: https://patchwork.ffmpeg.or
> g/patch/10517/ 

Patch 10517 deals with unitialized variable on a compilation level. As
for the enc_ctrl I very much hope that ffmpeg-qsv code takes care to
memset the mfcEncodeCtrl (as well as all other mediasdk structures)
_before_ usage. I.e. there should be a code somewhere similar to:
  memset(enc_ctrl, 0, sizeof(mfxEncodeCtrl));
If this is missing, there is a VERY big problem in the QSV code since
indeed compiler may initialize structures to everything it wants and
there will be very bad consequences.

As for the usage of mfxEncodeCtrl, the idea is the following. User
allocates this control and memsets it to 0. If this will be passed in
that way mediasdk will change nothing to encode the frame. If user
wants to change some parameter, it can do this changing only the
parameter it wants to take effect. So, the "else" should really not be
needed.

Dmitry.