[FFmpeg-devel] [PATCH 01/10] lavc/libopenh264enc: Add default qmin/qmax support

[Fu, Linjie]

> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Anton Khirnov
> Sent: Friday, April 10, 2020 18:12
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 01/10] lavc/libopenh264enc: Add
> default qmin/qmax support
> 
> Quoting Linjie Fu (2020-04-06 13:14:44)
> > Set default QP range to (1, 51) instead of (2, 32).
> >
> > QP = 0 is not well supported currently in libopenh264. If iMaxQp/iMinQp
> > equals 0, the QP range would be changed unexpectedly inside libopenh264
> > with a warning:
> >
> > Warning:Change QP Range from(0,51) to (12,42)
> >
> > [1]
> <https://github.com/cisco/openh264/blob/master/codec/encoder/core/src/
> encoder_ext.cpp#L375>
> > [2] <https://github.com/cisco/openh264/issues/3259>
> >
> > Signed-off-by: Linjie Fu <linjie.fu at intel.com>
> > ---
> >  libavcodec/libopenh264enc.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> > index ae6d17c..364926f 100644
> > --- a/libavcodec/libopenh264enc.c
> > +++ b/libavcodec/libopenh264enc.c
> > @@ -135,6 +135,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >      param.iTargetBitrate             = avctx->bit_rate;
> >      param.iMaxBitrate                = FFMAX(avctx->rc_max_rate, avctx-
> >bit_rate);
> >      param.iRCMode                    = RC_QUALITY_MODE;
> > +    // QP = 0 is not well supported, so default to (1, 51)
> > +    param.iMaxQp                     = avctx->qmax >= 0 ? av_clip(avctx->qmax, 1,
> 51) : 51;
> > +    param.iMinQp                     = avctx->qmin >= 0 ? av_clip(avctx->qmin, 1,
> param.iMaxQp) : 1;
> 
> Should we set them at all if they are not specified by the user?

The answer is no and that's why I set qmin/qmax to -1 by default, otherwise
they would be set to (2, 31) by default [1] in VBR mode in FFmpeg.

> Wouldn't it be better to leave the unset, as is done now?

Most encoders would prefer to get a default QP range specifically if it's not
explicitly set by user, either choosing a default value inside ffmpeg or leave it
unset to be determined in core library/driver, like libx264, vaapi, qsv, nvenc.

For libopenh264enc specifically, the supported range is (1, 51), qp = 0 is not well
Supported yet in libopenh264 library [2]. If either of qmin or qmax equals 0, the
qp range would be reduced to (12, 42) inside libopenh264 library.

The default QP range is supposed to be as wide as enough IMHO, so set it to (1, 51)
and avoid the warning inside the library. (and MaxQp =51 is the recommended value
in the demo config file in ..../openh264/testbin/welsenc.cfg)

Thanks for the review.

- Linjie

[1] https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/options_table.h#L97
[2] <https://github.com/cisco/openh264/issues/3259>