[FFmpeg-devel] [PATCH v4 1/9] lavc/libopenh264enc: Add default qmin/qmax support

[Martin Storsjö]

On Wed, 15 Apr 2020, Linjie Fu wrote:

> 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 dd5d4ee..c7ae5b1 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;

If qmax/qmin < 0, i.e. wasn't specified by the user, wouldn't it be better 
to not touch param.iMax/MinQp at all (and use the default value of the 
library, which may change between versions), instead of overriding it with 
a value hardcoded here?

// Martin