> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf > Of Mark Thompson > Sent: Wednesday, October 31, 2018 7:40 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH 1/5] lavc/qsvenc: add forced_idr opiton > > On 30/10/18 09:49, Li, Zhong wrote: > >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On > Behalf > >> Of Mark Thompson > >> Sent: Tuesday, October 30, 2018 5:06 AM > >> To: ffmpeg-devel@ffmpeg.org > >> Subject: Re: [FFmpeg-devel] [PATCH 1/5] lavc/qsvenc: add forced_idr > >> opiton > >> > >> On 25/10/18 13:36, Zhong Li wrote: > >>> This option can be used to repect original input I/IDR frame type. > >>> > >>> Signed-off-by: Zhong Li <zhong...@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..e534dcf 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; > >>> + > >>> + if (q->forced_idr >= 0 && 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; > >>> + } else > >>> + enc_ctrl->FrameType = MFX_FRAMETYPE_UNKNOWN; > >>> } > >>> > >>> ret = av_new_packet(&new_pkt, q->packet_size); diff --git > >>> a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h index 055b4a6..1f97f77 > >>> 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_INT, { .i64 = -1 }, -1, > >> 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); > >>> > >> > >> This seems confusing, because it doesn't match what forced_idr does > >> in any other encoder. > >> > >> Checking of pict_type for AV_PICTURE_TYPE_I in order to get a key > >> frame (of whatever kind) is always enabled if supported (many > >> encoders). The "forced_idr" option to H.26[45] encoders (libx264, > >> libx265) then forces that to be an IDR frame, not just an I frame. > >> > >> - Mark > > Yup, I know it doesn’t match other encoders such as > libx264/libx265/nvenc. > > However, it is my intentional behavior. I believe current implement in > libx264/libx265 is not complete. > > One case is ignored: user just want to keep the gop structure as input, not > to force all I frames as IDR frames. > > So I have an idea: > > Default value = -1, ignore the input gop structure. > > 0: respect input gop structure but don't force I frame as IDR frame. > > 1: force all I frame as IDR frame. > > This sounds like two independent options. One is the "forced-idr" option > implemented by several other encoders (notably libx264, which is the most > commonly-used external encoder), which looks like a sensible addition to me. > The second is an "ignore user-set pict_type" option, which I don't understand > the need for at all - it's never set unless the user deliberately wants to use > that feature (e.g. by using the force_key_frames option in the ffmpeg > utility), > so why would you want to have a special way to override that? > > Thanks, > > - Mark I may miss something. The default case (forced_idr = -1) is do nothing, ignoring the input I frames. Which is same as default case of x264/x265/nvenc forced_idr. Vaapi encoder is different from others, there is no forced_idr option but an internal variable named force_idr, always set IDR if the input frame is I frame. (Please correct me if I am wrong)
Compare with x264/x265 forced_idr. I just add this case: "0: respect input gop structure but don't force I frame as IDR frame." _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
