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

[Fu, Linjie]

> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Martin Storsjö
> Sent: Tuesday, April 28, 2020 14:31
> To: Fu, Linjie <linjie.fu at intel.com>
> Cc: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v4 1/9] lavc/libopenh264enc: Add
> default qmin/qmax support
> 
> On Tue, 28 Apr 2020, Fu, Linjie wrote:
> 
> >> From: Martin Storsjö <martin at martin.st>
> >> Sent: Tuesday, April 28, 2020 14:19
> >> To: Fu, Linjie <linjie.fu at intel.com>
> >> Cc: FFmpeg development discussions and patches <ffmpeg-
> >> devel at ffmpeg.org>
> >> Subject: RE: [FFmpeg-devel] [PATCH v4 1/9] lavc/libopenh264enc: Add
> >> default qmin/qmax support
> >>
> >> On Tue, 28 Apr 2020, Fu, Linjie wrote:
> >>
> >>>> From: Martin Storsjö <martin at martin.st>
> >>>> Sent: Tuesday, April 28, 2020 03:27
> >>>>
> >>>> 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?
> >>>>
> >>> Okay, this seems more natural if the recommended QP range varies
> >> between
> >>> versions, though one of my original purposes is to avoid the warning in
> >> default
> >>> situation for changing the QP inside libopenh264 library.
> >>
> >> Well in general I'd want to avoid hardcoding opinionated defaults within
> >> our own wrapper - I'd like it to behave as close to what upstream
> >> intended, so that whatever issues we see with defaults, are the same
> >> issues that everyone else sees as well, so any fixes to those defaults
> >> upstream also end up for us - so we don't get stuck on whatever we
> thought
> >> was a good default at some point.
> >
> > Agree, this makes more sense.
> >
> >> What warnings about changing QP are you referring to?
> > Warning:Change QP Range from(0,51) to (12,42)
> >
> https://github.com/cisco/openh264/blob/master/codec/encoder/core/src/e
> ncoder_ext.cpp#L375
> >
> > The main reason is that "0" is not supported well, which is the default
> > value of iMinQp.
> 
> Ah, right.
> 
> In that case, setting some other default might make sense indeed.
> How/where does OpenH264 set suitable values for this, in order not to get
> the warnings everywhere? Are the values e.g. 12-42 hardcoded everywhere
> in
> every caller?
> 
IIRC, it depends on the iUsageType, and hardcoded in the libopenh264 library function
ParamValidation() in [1]:
For SCREEN_CONTENT_REAL_TIME,
	it's (MIN_SCREEN_QP, MAX_SCREEN_QP), e.g. (26, 35);
For other usage like CAMERA_VIDEO_REAL_TIME by default,
	It's (GOM_MIN_QP_MODE, MAX_LOW_BR_QP), e.g. (12, 42)

- Linjie

[1] https://github.com/cisco/openh264/blob/master/codec/encoder/core/src/encoder_ext.cpp#L379