[FFmpeg-devel] [PATCH v5 3/5] lavc/libopenh264enc: add bit rate control select support

[Fu, Linjie]

> From: Martin Storsjö <martin at martin.st>
> Sent: Tuesday, April 28, 2020 18:31
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Cc: Fu, Linjie <linjie.fu at intel.com>
> Subject: Re: [FFmpeg-devel] [PATCH v5 3/5] lavc/libopenh264enc: add bit
> rate control select support
> 
> On Tue, 28 Apr 2020, Linjie Fu wrote:
> 
> > RC_BITRATE_MODE:
> >    set BITS_EXCEEDED to iCurrentBitsLevel and allows QP adjust
> >    in RcCalculatePictureQp().
> >
> > RC_BUFFERBASED_MODE:
> >    use buffer status to adjust the video quality, introduced in
> >    release 1.2.
> >
> > RC_TIMESTAMP_MODE:
> >    bit rate control based on timestamp, introduced in release 1.4.
> >
> > Default to use RC_QUALITY_MODE.
> >
> > Signed-off-by: Linjie Fu <linjie.fu at intel.com>
> > ---
> > libavcodec/libopenh264enc.c | 15 +++++++++++++++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> > index 0951412..93d3de2 100644
> > --- a/libavcodec/libopenh264enc.c
> > +++ b/libavcodec/libopenh264enc.c
> > @@ -49,6 +49,9 @@ typedef struct SVCContext {
> >     int skip_frames;
> >     int skipped;
> >     int cabac;
> > +
> > +    // rate control mode
> > +    int rc_mode;
> > } SVCContext;
> >
> > #define OFFSET(x) offsetof(SVCContext, x)
> > @@ -73,6 +76,18 @@ static const AVOption options[] = {
> >     { "max_nal_size", "set maximum NAL size in bytes",
> OFFSET(max_nal_size), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE },
> >     { "allow_skip_frames", "allow skipping frames to hit the target bitrate",
> OFFSET(skip_frames), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
> >     { "cabac", "Enable cabac", OFFSET(cabac), AV_OPT_TYPE_INT, { .i64 = 0 },
> 0, 1, VE },
> > +
> > +    { "rc_mode", "Select rate control mode", OFFSET(rc_mode),
> AV_OPT_TYPE_INT, { .i64 = RC_QUALITY_MODE }, RC_OFF_MODE,
> RC_TIMESTAMP_MODE, VE, "rc_mode" },
> > +        { "off",       "bit rate control off",                                                 0,
> AV_OPT_TYPE_CONST, { .i64 = RC_OFF_MODE },         0, 0, VE, "rc_mode" },
> > +        { "quality",   "quality mode",                                                         0,
> AV_OPT_TYPE_CONST, { .i64 = RC_QUALITY_MODE },     0, 0, VE, "rc_mode" },
> > +        { "bitrate",   "bitrate mode",                                                         0,
> AV_OPT_TYPE_CONST, { .i64 = RC_BITRATE_MODE },     0, 0, VE, "rc_mode" },
> > +#if OPENH264_VER_AT_LEAST(1, 2)
> > +        { "buffer",    "using buffer status to adjust the video quality (no bitrate
> control)", 0, AV_OPT_TYPE_CONST, { .i64 = RC_BUFFERBASED_MODE }, 0, 0,
> VE, "rc_mode" },
> > +#endif
> 
> We don't need checks for 1.2 - the initial version of openh264 that we
> support is 1.3, so we only need checks for 1.4 and newer.

Ahh, didn't noticed this until checked the commit message of
8a3d9ca603f4d15ecaa9ca379cbaab4ecaec8ce4.

IMHO it seems to be better if we add this version check in the configure as well.
(Did a quick verification with version modified in pkgconfig to 1.1.0, configure is
still good for libopenh264 )

Will send another fix for this if no against.
 
> > +#if OPENH264_VER_AT_LEAST(1, 4)
> > +        { "timestamp", "bit rate control based on timestamp",
> 0, AV_OPT_TYPE_CONST, { .i64 = RC_TIMESTAMP_MODE },   0, 0, VE,
> "rc_mode" },
> > +#endif
> > +
> >     { NULL }
> > };
> 
> This commit seems to lack something that actually sets the rc_mode value
> in the parameters struct though - wasn't that part of the previous version
> of the patch?
> 
Did you mean setting rc_mode through parameters like bit_rate/qp?
This patch enables explicitly setting the rc_mode as part of that logic. 

As to full enhancement, it's WIP and I'm still considering some parameters
like -qp to specify exact QP. (libopenh264 seems to only accept iMax/MinQp
in RC_QUALITY_MODE) For specific QP settings like -qp 26, we have to select
RC_OFF_MODE and specify QP by iDLayerQp in sSpatialLayers, which seems
a little bit confusing. (setting -qp 26 which leads to a non-quality mode)

- Linjie