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

[Mark Thompson]

On 13/11/18 11:58, 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..63d0939 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 (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;
> +        }
>      }
>  
>      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);
> 

Testing this, I think the reason not to look at key_frame in an encoder is that it isn't usually written by the user.  Rather, it gets copied (including through copy_props) from whatever the original source of the frame was.

You can see that it results in surprising behaviour in ffmpeg (which doesn't touch that field) with your patch as-is:

$ ffprobe h264.mp4
...
    Stream #0:0(und): Video: h264 (High) (avc1 / 0x31637661), yuv420p, 1920x1080 [SAR 1:1 DAR 16:9], 11244 kb/s, 60 fps, 60 tbr, 15360 tbn, 120 tbc (default)
...
$ ffprobe jpeg.mp4
...
    Stream #0:0(und): Video: mjpeg (Baseline) (mp4v / 0x7634706D), yuvj420p(pc, bt470bg/unknown/unknown, progressive), 1920x1080, 92213 kb/s, SAR 1:1 DAR 16:9, 60 fps, 60 tbr, 15360 tbn, 15360 tbc (default)
...
$ LIBVA_DRIVER_NAME=iHD ./ffmpeg_g -y -i h264.mp4 -an -c:v h264_qsv -force_key_frames expr:1 -b:v 1M -frames:v 10 -bsf:v trace_headers -f null - 2>&1 | grep 'nal_unit_type.*\(1\|5\)$' 
[AVBSFContext @ 0x55dd71352700] 3           nal_unit_type                                           00101 = 5
[AVBSFContext @ 0x55dd71352700] 3           nal_unit_type                                           00001 = 1
[AVBSFContext @ 0x55dd71352700] 3           nal_unit_type                                           00001 = 1
[AVBSFContext @ 0x55dd71352700] 3           nal_unit_type                                           00001 = 1
[AVBSFContext @ 0x55dd71352700] 3           nal_unit_type                                           00001 = 1
[AVBSFContext @ 0x55dd71352700] 3           nal_unit_type                                           00001 = 1
[AVBSFContext @ 0x55dd71352700] 3           nal_unit_type                                           00001 = 1
[AVBSFContext @ 0x55dd71352700] 3           nal_unit_type                                           00001 = 1
[AVBSFContext @ 0x55dd71352700] 3           nal_unit_type                                           00001 = 1
[AVBSFContext @ 0x55dd71352700] 3           nal_unit_type                                           00001 = 1
$ LIBVA_DRIVER_NAME=iHD ./ffmpeg_g -y -i jpeg.mp4 -an -c:v h264_qsv -force_key_frames expr:1 -b:v 1M -frames:v 10 -bsf:v trace_headers -f null - 2>&1 | grep 'nal_unit_type.*\(1\|5\)$' 
[AVBSFContext @ 0x55989b59b540] 3           nal_unit_type                                           00101 = 5
[AVBSFContext @ 0x55989b59b540] 3           nal_unit_type                                           00101 = 5
[AVBSFContext @ 0x55989b59b540] 3           nal_unit_type                                           00101 = 5
[AVBSFContext @ 0x55989b59b540] 3           nal_unit_type                                           00101 = 5
[AVBSFContext @ 0x55989b59b540] 3           nal_unit_type                                           00101 = 5
[AVBSFContext @ 0x55989b59b540] 3           nal_unit_type                                           00101 = 5
[AVBSFContext @ 0x55989b59b540] 3           nal_unit_type                                           00101 = 5
[AVBSFContext @ 0x55989b59b540] 3           nal_unit_type                                           00101 = 5
[AVBSFContext @ 0x55989b59b540] 3           nal_unit_type                                           00101 = 5
[AVBSFContext @ 0x55989b59b540] 3           nal_unit_type                                           00101 = 5
$ LIBVA_DRIVER_NAME=iHD ./ffmpeg_g -y -i h264.mp4 -an -c:v h264_qsv -force_key_frames expr:1 -b:v 1M -forced_idr 1 -frames:v 10 -bsf:v trace_headers -f null - 2>&1 | grep 'nal_unit_type.*\(1\|5\)$' 
[AVBSFContext @ 0x560d28291180] 3           nal_unit_type                                           00101 = 5
[AVBSFContext @ 0x560d28291180] 3           nal_unit_type                                           00101 = 5
[AVBSFContext @ 0x560d28291180] 3           nal_unit_type                                           00101 = 5
[AVBSFContext @ 0x560d28291180] 3           nal_unit_type                                           00101 = 5
[AVBSFContext @ 0x560d28291180] 3           nal_unit_type                                           00101 = 5
[AVBSFContext @ 0x560d28291180] 3           nal_unit_type                                           00101 = 5
[AVBSFContext @ 0x560d28291180] 3           nal_unit_type                                           00101 = 5
[AVBSFContext @ 0x560d28291180] 3           nal_unit_type                                           00101 = 5
[AVBSFContext @ 0x560d28291180] 3           nal_unit_type                                           00101 = 5
[AVBSFContext @ 0x560d28291180] 3           nal_unit_type                                           00101 = 5

So, LGTM with the reference to key_frame dropped and the title typo fixed.

Thanks,

- Mark