[FFmpeg-devel] [PATCH v4] avcodec/libvpxenc: add a way to set VP9E_SET_SVC_REF_FRAME_CONFIG

[James Zern]

Hi,

On Wed, Jan 6, 2021 at 8:59 AM Wonkap Jang <wonkapjang at gmail.com> wrote:
>
> HI JAmes,
>
> On Tue, Dec 29, 2020 at 11:54 AM Wonkap Jang <
> wonkap-at-google.com at ffmpeg.org> wrote:
>
> > Hi James,
> >
> > On Tue, Dec 29, 2020 at 11:51 AM Wonkap Jang <wonkap at google.com> wrote:
> >
> > > In order to fine-control referencing schemes in VP9 encoding, there
> > > is a need to use VP9E_SET_SVC_REF_FRAME_CONFIG method. This commit
> > > provides a way to use the API through frame metadata.
> > > ---
> > >  doc/encoders.texi      | 32 +++++++++++++++++
> > >  libavcodec/libvpxenc.c | 81 ++++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 113 insertions(+)
> > >

A change like this should also bump the micro version in avcodec.h

> > > diff --git a/doc/encoders.texi b/doc/encoders.texi
> > > index 0b1c69e982..11cf4d89d0 100644
> > > --- a/doc/encoders.texi
> > > +++ b/doc/encoders.texi
> > > @@ -2129,6 +2129,38 @@ midpoint is passed in rather than calculated for a
> > > specific clip or chunk.
> > >  The valid range is [0, 10000]. 0 (default) uses standard VBR.
> > >  @item enable-tpl @var{boolean}
> > >  Enable temporal dependency model.
> > > + at item ref-frame-config
> > > +Using per-frame metadata, set members of the structure
> > > @code{vpx_svc_ref_frame_config_t} in @code{vpx/vp8cx.h} to fine-control
> > > referencing schemes and frame buffer management.
> > > +@*Use a :-separated list of key=value pairs.
> > > +For example,
> > > + at example
> > > +av_dict_set(&av_frame->metadata, "ref-frame-config", \
> > >
> > >
> > +"rfc_update_buffer_slot=7:rfc_lst_fb_idx=0:rfc_gld_fb_idx=1:rfc_alt_fb_idx=2:rfc_reference_last=0:rfc_reference_golden=0:rfc_reference_alt_ref=0");
> > > + at end example
> > > + at table @option
> > > + at item rfc_update_buffer_slot
> > > +Indicates the buffer slot number to update
> > > + at item rfc_update_last
> > > +Indicates whether to update the LAST frame
> > > + at item rfc_update_golden
> > > +Indicates whether to update GOLDEN frame
> > > + at item rfc_update_alt_ref
> > > +Indicates whether to update ALT_REF frame
> > > + at item rfc_lst_fb_idx
> > > +LAST frame buffer index
> > > + at item rfc_gld_fb_idx
> > > +GOLDEN frame buffer index
> > > + at item rfc_alt_fb_idx
> > > +ALT_REF frame buffer index
> > > + at item rfc_reference_last
> > > +Indicates whetehr to reference LAST frame

whether

> > > + at item rfc_reference_golden
> > > +Indicates whether to reference GOLDEN frame
> > > + at item rfc_reference_alt_ref
> > > +Indicates whether to reference ALT_REF frame
> > > + at item rfc_reference_duration
> > > +Indicates frame duration
> > > + at end table
> > >  @end table
> > >
> > >  @end table
> > > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> > > index a7c76eb835..4943a9e32c 100644
> > > --- a/libavcodec/libvpxenc.c
> > > +++ b/libavcodec/libvpxenc.c
> > > @@ -125,6 +125,10 @@ typedef struct VPxEncoderContext {
> > >       * encounter a frame with ROI side data.
> > >       */
> > >      int roi_warned;
> > > +#if CONFIG_LIBVPX_VP9_ENCODER && defined
> > > (VPX_CTRL_VP9E_SET_MAX_INTER_BITRATE_PCT)

It's uncommon to add a space before ( with defined.

> > > [...]
> > >
> > > +#if CONFIG_LIBVPX_VP9_ENCODER && defined
> > > (VPX_CTRL_VP9E_SET_MAX_INTER_BITRATE_PCT)
> > > +static int vpx_ref_frame_config_parse(VPxContext *ctx, const struct
> > > vpx_codec_enc_cfg *enccfg,
> > > +                              char *key, char *value, enum AVCodecID
> > > codec_id)

indent is off

> > > +{
> > > +    size_t value_len = strlen(value);
> > > +    int ss_number_layers = enccfg->ss_number_layers;
> > > +    vpx_svc_ref_frame_config_t *ref_frame_config =
> > &ctx->ref_frame_config;
> > > +
> > > +    if (!value_len)
> > > +        return -1;
> > > +
> > > +    if (codec_id != AV_CODEC_ID_VP9)
> > > +        return -1;
> > > +

You might want to just skip the call altogether and add 1 warning
output if it's VP8.

> > > [...]
> > > +    }
> > > +
> > > +    return 0;
> > > +}
> > > +#endif
> > > +
> > > +

this line can be removed.