[FFmpeg-devel] [PATCH v2 2/7] lavc/qsvenc: add forced_idr opiton

[Li, Zhong]

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of Mark Thompson
> Sent: Tuesday, November 6, 2018 2:13 AM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v2 2/7] lavc/qsvenc: add forced_idr
> opiton
> 
> On 05/11/18 12:15, Zhong Li wrote:
> > This option can be used to repect original input I/IDR frame type.
> >
> > Signed-off-by: Zhong Li <zhong.li at intel.com>
> > ---
> >  libavcodec/qsvenc.c | 7 +++++++
> >  libavcodec/qsvenc.h | 2 ++
> >  2 files changed, 9 insertions(+)
> >
> > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c index
> > 948751d..7098ec7 100644
> > --- a/libavcodec/qsvenc.c
> > +++ b/libavcodec/qsvenc.c
> > @@ -1192,6 +1192,13 @@ static int encode_frame(AVCodecContext
> *avctx, QSVEncContext *q,
> >      if (qsv_frame) {
> >          surf = &qsv_frame->surface;
> >          enc_ctrl = &qsv_frame->enc_ctrl;
> > +        memset(enc_ctrl, 0, sizeof(mfxEncodeCtrl));
> > +
> > +        if (q->forced_idr && frame->pict_type == AV_PICTURE_TYPE_I)
> {
> > +            enc_ctrl->FrameType = MFX_FRAMETYPE_I |
> MFX_FRAMETYPE_REF;
> > +            if (q->forced_idr || frame->key_frame)
> > +                enc_ctrl->FrameType |= MFX_FRAMETYPE_IDR;
> > +        }
> 
> This still seems confused.  I think you don't want the q->forced_idr in the
> first condition?

Yup, good catch. Sorry for the stupid mistake, will update.

> I wouldn't include the check against frame->key_frame either, since no other
> encoder uses that.

Noted that for x264/x265 too. But I think they are mixing key_frame and general I frame. 
Not all I frames are key frames and they haven't respected original key_frame well. 
If you agree with that too, I will also send patches to libx264/libx265 to keep alignment.

> >      }
> >
> >      ret = av_new_packet(&new_pkt, q->packet_size); diff --git
> > a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h index 055b4a6..fbdc199
> > 100644
> > --- a/libavcodec/qsvenc.h
> > +++ b/libavcodec/qsvenc.h
> > @@ -87,6 +87,7 @@
> >  { "adaptive_i",     "Adaptive I-frame placement",
> OFFSET(qsv.adaptive_i),     AV_OPT_TYPE_INT, { .i64 = -1 }, -1,
> 1, VE },                         \
> >  { "adaptive_b",     "Adaptive B-frame placement",
> OFFSET(qsv.adaptive_b),     AV_OPT_TYPE_INT, { .i64 = -1 }, -1,
> 1, VE },                         \
> >  { "b_strategy",     "Strategy to choose between I/P/B-frames",
> OFFSET(qsv.b_strategy),    AV_OPT_TYPE_INT, { .i64 = -1 }, -1,
> 1, VE },                         \
> > +{ "forced_idr",     "Forcing I frames as IDR frames",
> OFFSET(qsv.forced_idr),     AV_OPT_TYPE_BOOL,{ .i64 = 0  },  0,
> 1, VE },                         \
> >
> >  typedef int SetEncodeCtrlCB (AVCodecContext *avctx,
> >                               const AVFrame *frame,
> mfxEncodeCtrl*
> > enc_ctrl); @@ -168,6 +169,7 @@ typedef struct QSVEncContext {  #endif
> >      char *load_plugins;
> >      SetEncodeCtrlCB *set_encode_ctrl_cb;
> > +    int forced_idr;
> >  } QSVEncContext;
> >
> >  int ff_qsv_enc_init(AVCodecContext *avctx, QSVEncContext *q);
> >
> 
> - Mark