| Message ID | 20240905051710.2012-1-aicommander@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/amfenc: Add support for on-demand key frames | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On 05/09/2024 07:16, Cameron Gutman wrote: > Signed-off-by: Cameron Gutman <aicommander@gmail.com> > --- > libavcodec/amfenc.c | 34 +++++++++++++++++++++++++++++++++- > 1 file changed, 33 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c > index 41eaef9758..6f2b211d14 100644 > --- a/libavcodec/amfenc.c > +++ b/libavcodec/amfenc.c > @@ -766,11 +766,43 @@ int ff_amf_receive_packet(AVCodecContext *avctx, AVPacket *avpkt) > switch (avctx->codec->id) { > case AV_CODEC_ID_H264: > AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_INSERT_AUD, !!ctx->aud); > + if (frame->flags & AV_FRAME_FLAG_KEY) { > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_INSERT_SPS, 1); > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_INSERT_PPS, 1); > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_IDR); > + } else { > + switch (frame->pict_type) { > + case AV_PICTURE_TYPE_I: > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_I); break; > + case AV_PICTURE_TYPE_P: > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_P); break; > + case AV_PICTURE_TYPE_B: > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_B); break; > + } > + } > break; > case AV_CODEC_ID_HEVC: > AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_INSERT_AUD, !!ctx->aud); > + if (frame->flags & AV_FRAME_FLAG_KEY) { > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_INSERT_HEADER, 1); > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_HEVC_PICTURE_TYPE_IDR); > + } else { > + switch (frame->pict_type) { > + case AV_PICTURE_TYPE_I: > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_HEVC_PICTURE_TYPE_I); break; > + case AV_PICTURE_TYPE_P: > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_HEVC_PICTURE_TYPE_P); break; > + } > + } > + break; > + case AV_CODEC_ID_AV1: > + if (frame->flags & AV_FRAME_FLAG_KEY) { > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_AV1_FORCE_INSERT_SEQUENCE_HEADER, 1); > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_AV1_FORCE_FRAME_TYPE, AMF_VIDEO_ENCODER_AV1_FORCE_FRAME_TYPE_KEY); > + } else if (frame->pict_type == AV_PICTURE_TYPE_I) { > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_AV1_FORCE_FRAME_TYPE, AMF_VIDEO_ENCODER_AV1_FORCE_FRAME_TYPE_INTRA_ONLY); > + } > break; > - //case AV_CODEC_ID_AV1 not supported > default: > break; > } You should hook up FFCodec.flush instead. That's what nvenc uses to reset the GOP.
Quoting Lynne via ffmpeg-devel (2024-09-05 23:27:40) > On 05/09/2024 07:16, Cameron Gutman wrote: > > Signed-off-by: Cameron Gutman <aicommander@gmail.com> > > --- > > libavcodec/amfenc.c | 34 +++++++++++++++++++++++++++++++++- > > 1 file changed, 33 insertions(+), 1 deletion(-) > > > > diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c > > index 41eaef9758..6f2b211d14 100644 > > --- a/libavcodec/amfenc.c > > +++ b/libavcodec/amfenc.c > > @@ -766,11 +766,43 @@ int ff_amf_receive_packet(AVCodecContext *avctx, AVPacket *avpkt) > > switch (avctx->codec->id) { > > case AV_CODEC_ID_H264: > > AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_INSERT_AUD, !!ctx->aud); > > + if (frame->flags & AV_FRAME_FLAG_KEY) { > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_INSERT_SPS, 1); > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_INSERT_PPS, 1); > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_IDR); > > + } else { > > + switch (frame->pict_type) { > > + case AV_PICTURE_TYPE_I: > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_I); break; > > + case AV_PICTURE_TYPE_P: > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_P); break; > > + case AV_PICTURE_TYPE_B: > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_B); break; > > + } > > + } > > break; > > case AV_CODEC_ID_HEVC: > > AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_INSERT_AUD, !!ctx->aud); > > + if (frame->flags & AV_FRAME_FLAG_KEY) { > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_INSERT_HEADER, 1); > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_HEVC_PICTURE_TYPE_IDR); > > + } else { > > + switch (frame->pict_type) { > > + case AV_PICTURE_TYPE_I: > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_HEVC_PICTURE_TYPE_I); break; > > + case AV_PICTURE_TYPE_P: > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_HEVC_PICTURE_TYPE_P); break; > > + } > > + } > > + break; > > + case AV_CODEC_ID_AV1: > > + if (frame->flags & AV_FRAME_FLAG_KEY) { > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_AV1_FORCE_INSERT_SEQUENCE_HEADER, 1); > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_AV1_FORCE_FRAME_TYPE, AMF_VIDEO_ENCODER_AV1_FORCE_FRAME_TYPE_KEY); > > + } else if (frame->pict_type == AV_PICTURE_TYPE_I) { > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_AV1_FORCE_FRAME_TYPE, AMF_VIDEO_ENCODER_AV1_FORCE_FRAME_TYPE_INTRA_ONLY); > > + } > > break; > > - //case AV_CODEC_ID_AV1 not supported > > default: > > break; > > } > > You should hook up FFCodec.flush instead. That's what nvenc uses to > reset the GOP. No, the patch does almost the right thing, though most other encoders look at pict_type instead.
On Fri, Sep 6, 2024 at 1:57 AM Anton Khirnov <anton@khirnov.net> wrote: > > Quoting Lynne via ffmpeg-devel (2024-09-05 23:27:40) > > On 05/09/2024 07:16, Cameron Gutman wrote: > > > Signed-off-by: Cameron Gutman <aicommander@gmail.com> > > > --- > > > libavcodec/amfenc.c | 34 +++++++++++++++++++++++++++++++++- > > > 1 file changed, 33 insertions(+), 1 deletion(-) > > > > > > diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c > > > index 41eaef9758..6f2b211d14 100644 > > > --- a/libavcodec/amfenc.c > > > +++ b/libavcodec/amfenc.c > > > @@ -766,11 +766,43 @@ int ff_amf_receive_packet(AVCodecContext *avctx, AVPacket *avpkt) > > > switch (avctx->codec->id) { > > > case AV_CODEC_ID_H264: > > > AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_INSERT_AUD, !!ctx->aud); > > > + if (frame->flags & AV_FRAME_FLAG_KEY) { > > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_INSERT_SPS, 1); > > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_INSERT_PPS, 1); > > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_IDR); > > > + } else { > > > + switch (frame->pict_type) { > > > + case AV_PICTURE_TYPE_I: > > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_I); break; > > > + case AV_PICTURE_TYPE_P: > > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_P); break; > > > + case AV_PICTURE_TYPE_B: > > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_B); break; > > > + } > > > + } > > > break; > > > case AV_CODEC_ID_HEVC: > > > AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_INSERT_AUD, !!ctx->aud); > > > + if (frame->flags & AV_FRAME_FLAG_KEY) { > > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_INSERT_HEADER, 1); > > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_HEVC_PICTURE_TYPE_IDR); > > > + } else { > > > + switch (frame->pict_type) { > > > + case AV_PICTURE_TYPE_I: > > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_HEVC_PICTURE_TYPE_I); break; > > > + case AV_PICTURE_TYPE_P: > > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_HEVC_PICTURE_TYPE_P); break; > > > + } > > > + } > > > + break; > > > + case AV_CODEC_ID_AV1: > > > + if (frame->flags & AV_FRAME_FLAG_KEY) { > > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_AV1_FORCE_INSERT_SEQUENCE_HEADER, 1); > > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_AV1_FORCE_FRAME_TYPE, AMF_VIDEO_ENCODER_AV1_FORCE_FRAME_TYPE_KEY); > > > + } else if (frame->pict_type == AV_PICTURE_TYPE_I) { > > > + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_AV1_FORCE_FRAME_TYPE, AMF_VIDEO_ENCODER_AV1_FORCE_FRAME_TYPE_INTRA_ONLY); > > > + } > > > break; > > > - //case AV_CODEC_ID_AV1 not supported > > > default: > > > break; > > > } > > > > You should hook up FFCodec.flush instead. That's what nvenc uses to > > reset the GOP. > > No, the patch does almost the right thing, though most other encoders > look at pict_type instead. > I took a look at what other common encoders do. Most of the ones I looked at use pict_type as you mentioned, and in some cases there's an additional codec option called 'forced-idr' or 'forced_idr' that causes AV_PICTURE_TYPE_I to generate an IDR frame instead of a normal I frame. Some of these encoders that don't have a 'forced-idr' option may require a closed GOP to get IDR frames via AV_PICTURE_TYPE_I but I couldn't easily tell that from code review so I'm ignoring it here. Here is a brief summary of what I found is required to generate an IDR frame: - qsvenc: AV_PICTURE_TYPE_I and either forced_idr=1 or AV_FRAME_FLAG_KEY - nvenc: AV_PICTURE_TYPE_I and forced-idr=1 - libx265: AV_PICTURE_TYPE_I and forced-idr=1 - libx264: AV_PICTURE_TYPE_I and forced-idr=1 - d3d12va: AV_PICTURE_TYPE_I alone - vaapi: AV_PICTURE_TYPE_I alone - videotoolbox: AV_PICTURE_TYPE_I alone - mfenc: AV_PICTURE_TYPE_I alone Given that it's basically a tie between encoders that give an IDR frame for any I frame request and those that require a secondary option to do so, I think I will follow the lead of NVENC and the software encoders and add a 'forced-idr' option to the AMF encoders that will determine the behavior of AV_PICTURE_TYPE_I for IDR frame generation. Cameron
diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c index 41eaef9758..6f2b211d14 100644 --- a/libavcodec/amfenc.c +++ b/libavcodec/amfenc.c @@ -766,11 +766,43 @@ int ff_amf_receive_packet(AVCodecContext *avctx, AVPacket *avpkt) switch (avctx->codec->id) { case AV_CODEC_ID_H264: AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_INSERT_AUD, !!ctx->aud); + if (frame->flags & AV_FRAME_FLAG_KEY) { + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_INSERT_SPS, 1); + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_INSERT_PPS, 1); + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_IDR); + } else { + switch (frame->pict_type) { + case AV_PICTURE_TYPE_I: + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_I); break; + case AV_PICTURE_TYPE_P: + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_P); break; + case AV_PICTURE_TYPE_B: + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_B); break; + } + } break; case AV_CODEC_ID_HEVC: AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_INSERT_AUD, !!ctx->aud); + if (frame->flags & AV_FRAME_FLAG_KEY) { + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_INSERT_HEADER, 1); + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_HEVC_PICTURE_TYPE_IDR); + } else { + switch (frame->pict_type) { + case AV_PICTURE_TYPE_I: + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_HEVC_PICTURE_TYPE_I); break; + case AV_PICTURE_TYPE_P: + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_HEVC_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_HEVC_PICTURE_TYPE_P); break; + } + } + break; + case AV_CODEC_ID_AV1: + if (frame->flags & AV_FRAME_FLAG_KEY) { + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_AV1_FORCE_INSERT_SEQUENCE_HEADER, 1); + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_AV1_FORCE_FRAME_TYPE, AMF_VIDEO_ENCODER_AV1_FORCE_FRAME_TYPE_KEY); + } else if (frame->pict_type == AV_PICTURE_TYPE_I) { + AMF_ASSIGN_PROPERTY_INT64(res, surface, AMF_VIDEO_ENCODER_AV1_FORCE_FRAME_TYPE, AMF_VIDEO_ENCODER_AV1_FORCE_FRAME_TYPE_INTRA_ONLY); + } break; - //case AV_CODEC_ID_AV1 not supported default: break; }
Signed-off-by: Cameron Gutman <aicommander@gmail.com> --- libavcodec/amfenc.c | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-)