Quoting Fu, Linjie (2020-04-10 15:27:02) > > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of > > Anton Khirnov > > Sent: Friday, April 10, 2020 18:12 > > To: FFmpeg development discussions and patches <ffmpeg- > > de...@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...@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.
ok then -- Anton Khirnov _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
