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

[Fu, Linjie]

> 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/encoder_ext.cpp#L375 

The main reason is that "0" is not supported well, which is the default value of iMinQp.

- Linjie