[FFmpeg-devel] [PATCH] avcodec/svt-av1: Set pic_type only when gop_size == 1

[Ronald S. Bultje]

Hi Vignesh,

On Thu, Sep 28, 2023 at 12:14 AM Vignesh Venkatasubramanian via
ffmpeg-devel <ffmpeg-devel at ffmpeg.org> wrote:

> SVT-AV1 does not support requesting keyframes at arbitrary points
> by setting pic_type to EB_AV1_KEY_PICTURE.
>
> This patch changes the following:
>  * Set pic_type to EB_AV1_KEY_PICTURE only when gop_size == 1. This
>    only has an effect in this case (combined with force_key_frames).
>    In all other cases, setting this has no effect.
>  * Set force_key_frames to 1 only when gop_size == 1, this is
>    needed for pic_type request above to work.
>
> Please see the comments in
> https://gitlab.com/AOMediaCodec/SVT-AV1/-/issues/2076 for a bit more
> details.
>

Right. So, if I put my archeologist hat on, is it fair to say that what
probably happened is that force_key_frames used to not exist, and pic_type
worked as per above code. Then force_key_frames was required (because of
quality implications), breaking the above code. And now we're removing the
broken code because otherwise there's a quality penalty. Is that fair?

I agree it's probably unfair to ask you to fix the broken use case that was
previously possible (I suppose this comes down to exposing an option to set
force_key_frames by having the user indicate s/he'll be using pic_type).
However, maybe we should not remove the dead code, because it's
functionally OK. Maybe we even need a FIXME above the line where we set
force_key_frames only if gop_size==1, explaining this is a bug. WDYT?

(It feels a bit weird to remove it because it's broken, is what I'm trying
to say, especially because we know how to fix it.)

Ronald