On Tue, 28 Apr 2020, Fu, Linjie wrote:

From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
Martin Storsjö
Sent: Tuesday, April 28, 2020 14:31
To: Fu, Linjie <linjie...@intel.com>
Cc: FFmpeg development discussions and patches <ffmpeg-
de...@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ö <mar...@martin.st>
Sent: Tuesday, April 28, 2020 14:19
To: Fu, Linjie <linjie...@intel.com>
Cc: FFmpeg development discussions and patches <ffmpeg-
de...@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ö <mar...@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

Right, so looking at that code, it looks like it has a good intent - if the user hasn't specified that he wants a custom setting of min/max qp, then it uses the defaults.

So the problem is that this writes a warning in the log - and you want to avoid the warning.

Wouldn't it just be better to change this message to WELS_LOG_INFO within openh264, to avoid it being treated as a warning - as it's not a fault, it's the intended behaviour of picking sensible defaults when the user hasn't requested anything in particular?

// Martin
_______________________________________________
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".

Reply via email to