| Message ID | 20231003225313.407161-1-vigneshv@google.com |
|---|---|
| State | Accepted |
| Commit | e8465aa5dda03b4f30d24f192ee165f88a912762 |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/svt-av1: Set force_key_frames only when gop_size == 1 | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
Hi, On Tue, Oct 3, 2023 at 6:53 PM Vignesh Venkatasubramanian via ffmpeg-devel < ffmpeg-devel@ffmpeg.org> wrote: > SVT-AV1 does not support requesting keyframes at arbitrary points > by setting pic_type to EB_AV1_KEY_PICTURE. So set force_key_frames > to 1 only when gop_size == 1. > > Please see the comments in > https://gitlab.com/AOMediaCodec/SVT-AV1/-/issues/2076 for a bit more > details. > > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com> > --- > libavcodec/libsvtav1.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/libsvtav1.c b/libavcodec/libsvtav1.c > index 5015169244..8d2c7f3be4 100644 > --- a/libavcodec/libsvtav1.c > +++ b/libavcodec/libsvtav1.c > @@ -253,8 +253,13 @@ static int config_enc_params(EbSvtAv1EncConfiguration > *param, > // In order for SVT-AV1 to force keyframes by setting pic_type to > // EB_AV1_KEY_PICTURE on any frame, force_key_frames has to be set. > Note > // that this does not force all frames to be keyframes (it only > forces a > - // keyframe with pic_type is set to EB_AV1_KEY_PICTURE). > - param->force_key_frames = 1; > + // keyframe with pic_type is set to EB_AV1_KEY_PICTURE). As of now, > SVT-AV1 > + // does not support arbitrary keyframe requests by setting pic_type to > + // EB_AV1_KEY_PICTURE, so it is done only when gop_size == 1. > + // FIXME: When SVT-AV1 supports arbitrary keyframe requests, this > code needs > + // to be updated to set force_key_frames accordingly. > + if (avctx->gop_size == 1) > + param->force_key_frames = 1; > > if (avctx->framerate.num > 0 && avctx->framerate.den > 0) { > param->frame_rate_numerator = avctx->framerate.num; > -- > 2.42.0.582.g8ccd20d70d-goog > Seems reasonable to me. Can you merge, or should I merge for you? Ronald
On Wed, Oct 4, 2023 at 10:23 AM Ronald S. Bultje <rsbultje@gmail.com> wrote: > > Hi, > > On Tue, Oct 3, 2023 at 6:53 PM Vignesh Venkatasubramanian via ffmpeg-devel <ffmpeg-devel@ffmpeg.org> wrote: >> >> SVT-AV1 does not support requesting keyframes at arbitrary points >> by setting pic_type to EB_AV1_KEY_PICTURE. So set force_key_frames >> to 1 only when gop_size == 1. >> >> Please see the comments in >> https://gitlab.com/AOMediaCodec/SVT-AV1/-/issues/2076 for a bit more >> details. >> >> Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com> >> --- >> libavcodec/libsvtav1.c | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/libavcodec/libsvtav1.c b/libavcodec/libsvtav1.c >> index 5015169244..8d2c7f3be4 100644 >> --- a/libavcodec/libsvtav1.c >> +++ b/libavcodec/libsvtav1.c >> @@ -253,8 +253,13 @@ static int config_enc_params(EbSvtAv1EncConfiguration *param, >> // In order for SVT-AV1 to force keyframes by setting pic_type to >> // EB_AV1_KEY_PICTURE on any frame, force_key_frames has to be set. Note >> // that this does not force all frames to be keyframes (it only forces a >> - // keyframe with pic_type is set to EB_AV1_KEY_PICTURE). >> - param->force_key_frames = 1; >> + // keyframe with pic_type is set to EB_AV1_KEY_PICTURE). As of now, SVT-AV1 >> + // does not support arbitrary keyframe requests by setting pic_type to >> + // EB_AV1_KEY_PICTURE, so it is done only when gop_size == 1. >> + // FIXME: When SVT-AV1 supports arbitrary keyframe requests, this code needs >> + // to be updated to set force_key_frames accordingly. >> + if (avctx->gop_size == 1) >> + param->force_key_frames = 1; >> >> if (avctx->framerate.num > 0 && avctx->framerate.den > 0) { >> param->frame_rate_numerator = avctx->framerate.num; >> -- >> 2.42.0.582.g8ccd20d70d-goog > > > Seems reasonable to me. > > Can you merge, or should I merge for you? > Thanks Ronald. I don't have commit access. Can you please merge it? > Ronald
Hi, On Wed, Oct 4, 2023 at 5:05 PM Vignesh Venkat <vigneshv@google.com> wrote: > On Wed, Oct 4, 2023 at 10:23 AM Ronald S. Bultje <rsbultje@gmail.com> > wrote: > > > > Hi, > > > > On Tue, Oct 3, 2023 at 6:53 PM Vignesh Venkatasubramanian via > ffmpeg-devel <ffmpeg-devel@ffmpeg.org> wrote: > >> > >> SVT-AV1 does not support requesting keyframes at arbitrary points > >> by setting pic_type to EB_AV1_KEY_PICTURE. So set force_key_frames > >> to 1 only when gop_size == 1. > >> > >> Please see the comments in > >> https://gitlab.com/AOMediaCodec/SVT-AV1/-/issues/2076 for a bit more > >> details. > >> > >> Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com> > >> --- > >> libavcodec/libsvtav1.c | 9 +++++++-- > >> 1 file changed, 7 insertions(+), 2 deletions(-) > >> > >> diff --git a/libavcodec/libsvtav1.c b/libavcodec/libsvtav1.c > >> index 5015169244..8d2c7f3be4 100644 > >> --- a/libavcodec/libsvtav1.c > >> +++ b/libavcodec/libsvtav1.c > >> @@ -253,8 +253,13 @@ static int > config_enc_params(EbSvtAv1EncConfiguration *param, > >> // In order for SVT-AV1 to force keyframes by setting pic_type to > >> // EB_AV1_KEY_PICTURE on any frame, force_key_frames has to be > set. Note > >> // that this does not force all frames to be keyframes (it only > forces a > >> - // keyframe with pic_type is set to EB_AV1_KEY_PICTURE). > >> - param->force_key_frames = 1; > >> + // keyframe with pic_type is set to EB_AV1_KEY_PICTURE). As of > now, SVT-AV1 > >> + // does not support arbitrary keyframe requests by setting > pic_type to > >> + // EB_AV1_KEY_PICTURE, so it is done only when gop_size == 1. > >> + // FIXME: When SVT-AV1 supports arbitrary keyframe requests, this > code needs > >> + // to be updated to set force_key_frames accordingly. > >> + if (avctx->gop_size == 1) > >> + param->force_key_frames = 1; > >> > >> if (avctx->framerate.num > 0 && avctx->framerate.den > 0) { > >> param->frame_rate_numerator = avctx->framerate.num; > >> -- > >> 2.42.0.582.g8ccd20d70d-goog > > > > > > Seems reasonable to me. > > > > Can you merge, or should I merge for you? > > > > Thanks Ronald. I don't have commit access. Can you please merge it? > Done. Ronald
diff --git a/libavcodec/libsvtav1.c b/libavcodec/libsvtav1.c index 5015169244..8d2c7f3be4 100644 --- a/libavcodec/libsvtav1.c +++ b/libavcodec/libsvtav1.c @@ -253,8 +253,13 @@ static int config_enc_params(EbSvtAv1EncConfiguration *param, // In order for SVT-AV1 to force keyframes by setting pic_type to // EB_AV1_KEY_PICTURE on any frame, force_key_frames has to be set. Note // that this does not force all frames to be keyframes (it only forces a - // keyframe with pic_type is set to EB_AV1_KEY_PICTURE). - param->force_key_frames = 1; + // keyframe with pic_type is set to EB_AV1_KEY_PICTURE). As of now, SVT-AV1 + // does not support arbitrary keyframe requests by setting pic_type to + // EB_AV1_KEY_PICTURE, so it is done only when gop_size == 1. + // FIXME: When SVT-AV1 supports arbitrary keyframe requests, this code needs + // to be updated to set force_key_frames accordingly. + if (avctx->gop_size == 1) + param->force_key_frames = 1; if (avctx->framerate.num > 0 && avctx->framerate.den > 0) { param->frame_rate_numerator = avctx->framerate.num;
SVT-AV1 does not support requesting keyframes at arbitrary points by setting pic_type to EB_AV1_KEY_PICTURE. So set force_key_frames to 1 only when gop_size == 1. Please see the comments in https://gitlab.com/AOMediaCodec/SVT-AV1/-/issues/2076 for a bit more details. Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com> --- libavcodec/libsvtav1.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)