[FFmpeg-devel,1/7] lavc/libopenh264enc: Add default qmin/qmax support

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)

Signed-off-by: Linjie Fu <linjie.fu@intel.com>
---
[1] <https://github.com/cisco/openh264/blob/master/codec/encoder/core/src/encoder_ext.cpp#L375>
[2] <https://github.com/cisco/openh264/issues/3259>

 libavcodec/libopenh264enc.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

On 3/31/2020 12:31 PM, 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)
> 
> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> ---
> [1] <https://github.com/cisco/openh264/blob/master/codec/encoder/core/src/encoder_ext.cpp#L375>
> [2] <https://github.com/cisco/openh264/issues/3259>
> 
>  libavcodec/libopenh264enc.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> index ae6d17c..7cd1efe 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, avctx->qmax) : 1;

Should be param.iMaxQp. avctx->qmax may be <= 0.

>      param.iTemporalLayerNum          = 1;
>      param.iSpatialLayerNum           = 1;
>      param.bEnableDenoise             = 0;
> @@ -332,6 +335,12 @@ static int svc_encode_frame(AVCodecContext *avctx, AVPacket *avpkt,
>      return 0;
>  }
>  
> +static const AVCodecDefault svc_enc_defaults[] = {
> +    { "qmin",      "-1"    },
> +    { "qmax",      "-1"    },
> +    { NULL },
> +};
> +
>  AVCodec ff_libopenh264_encoder = {
>      .name           = "libopenh264",
>      .long_name      = NULL_IF_CONFIG_SMALL("OpenH264 H.264 / AVC / MPEG-4 AVC / MPEG-4 part 10"),
> @@ -345,6 +354,7 @@ AVCodec ff_libopenh264_encoder = {
>      .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP,
>      .pix_fmts       = (const enum AVPixelFormat[]){ AV_PIX_FMT_YUV420P,
>                                                      AV_PIX_FMT_NONE },
> +    .defaults       = svc_enc_defaults,
>      .priv_class     = &class,
>      .wrapper_name   = "libopenh264",
>  };
>

> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> James Almer
> Sent: Tuesday, March 31, 2020 23:44
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 1/7] lavc/libopenh264enc: Add default
> qmin/qmax support
> 
> On 3/31/2020 12:31 PM, 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)
> >
> > Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> > ---
> > [1]
> <https://github.com/cisco/openh264/blob/master/codec/encoder/core/src/
> encoder_ext.cpp#L375>
> > [2] <https://github.com/cisco/openh264/issues/3259>
> >
> >  libavcodec/libopenh264enc.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> > index ae6d17c..7cd1efe 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,
> avctx->qmax) : 1;
> 
> Should be param.iMaxQp. avctx->qmax may be <= 0.
> 
Fixed locally, thanks for pointing this out and the elaborations in IRC.

- Linjie