[FFmpeg-devel] [PATCH v2 16/36] vaapi_encode: Clean up rate control configuration

Mark Thompson sw at jkqxz.net
Thu Jun 21 02:10:04 EEST 2018


On 20/06/18 10:44, Li, Zhong wrote:
>> -----Original Message-----
>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
>> Of Mark Thompson
>> Sent: Sunday, June 17, 2018 9:51 PM
>> To: ffmpeg-devel at ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH v2 16/36] vaapi_encode: Clean up rate
>> control configuration
>>
>> On 14/06/18 08:22, Li, Zhong wrote:
>>>> -----Original Message-----
>>>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On
>> Behalf
>>>> Of Xiang, Haihao
>>>> Sent: Thursday, June 14, 2018 2:08 PM
>>>> To: ffmpeg-devel at ffmpeg.org
>>>> Subject: Re: [FFmpeg-devel] [PATCH v2 16/36] vaapi_encode: Clean up
>>>> rate control configuration
>>>>
>>>> On Wed, 2018-06-13 at 23:42 +0100, Mark Thompson wrote:
>>>>> On 13/06/18 08:03, Xiang, Haihao wrote:
>>>>>> On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
>>>>>>> Query which modes are supported and select between VBR and CBR
>>>>>>> based on that - this removes all of the codec-specific rate
>>>>>>> control mode selection code.
>>>>>>> ---
>>>>>>>  doc/encoders.texi               |   2 -
>>>>>>>  libavcodec/vaapi_encode.c       | 173
>>>> ++++++++++++++++++++++++++++-------
>>>>>>> ----
>>>>>>> -
>>>>>>>  libavcodec/vaapi_encode.h       |   6 +-
>>>>>>>  libavcodec/vaapi_encode_h264.c  |  18 +----
>>>>>>> libavcodec/vaapi_encode_h265.c  |  14 +---
>>>>>>>  libavcodec/vaapi_encode_mjpeg.c |   3 +-
>>>>>>>  libavcodec/vaapi_encode_mpeg2.c |   9 +--
>>>>>>>  libavcodec/vaapi_encode_vp8.c   |  13 +--
>>>>>>>  libavcodec/vaapi_encode_vp9.c   |  13 +--
>>>>>>>  9 files changed, 137 insertions(+), 114 deletions(-)
>>>>>>>
>>>>>>> ...
>>>>>>> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
>>>>>>> index f4c063734c..5de5483454 100644
>>>>>>> --- a/libavcodec/vaapi_encode.c
>>>>>>> +++ b/libavcodec/vaapi_encode.c
>>>>>>> ...
>>>>>>> +    if (avctx->flags & AV_CODEC_FLAG_QSCALE ||
>>>>>>> +        avctx->bit_rate <= 0) {
>>
>> This condition ^
>>
>>>>>>> +        if (rc_attr.value & VA_RC_CQP) {
>>>>>>> +            av_log(avctx, AV_LOG_VERBOSE, "Using
>>>> constant-quality
>>>>>>> mode.\n");
>>>>>>> +            ctx->va_rc_mode = VA_RC_CQP;
>>>>>>> +            return 0;
>>>>>>> +        } else {
>>>>>>> +            av_log(avctx, AV_LOG_ERROR, "Driver does not
>>>> support "
>>>>>>> +                   "constant-quality mode (%#x).\n",
>>>> rc_attr.value);
>>>>>>> +            return AVERROR(EINVAL);
>>>>>>> +        }
>>>>>>> +    }
>>>>>>> ...
>>>>>>> +    } else if (avctx->rc_max_rate == avctx->bit_rate) {
>>>>>>> +        if (!(rc_attr.value & VA_RC_CBR)) {
>>>>>>> +            av_log(avctx, AV_LOG_WARNING, "Driver does not
>>>> support "
>>>>>>> +                   "CBR mode (%#x), using VBR mode
>>>> instead.\n",
>>>>>>> +                   rc_attr.value);
>>>>>>> +            ctx->va_rc_mode = VA_RC_VBR;
>>>>>>> +        } else {
>>>>>>> +            ctx->va_rc_mode = VA_RC_CBR;
>>>>>>> +        }
>>>>>>>
>>>>>>> -    if (ctx->va_rc_mode == VA_RC_CBR) {
>>>>>>>          rc_bits_per_second   = avctx->bit_rate;
>>>>>>>          rc_target_percentage = 100;
>>>>>>> -        rc_window_size       = 1000;
>>>>>>> +
>>>>>>>      } else {
>>>>>>> -        if (avctx->rc_max_rate < avctx->bit_rate) {
>>>>>>> -            // Max rate is unset or invalid, just use the normal
>>>> bitrate.
>>>>>>> +        if (rc_attr.value & VA_RC_VBR) {
>>>>>>> +            ctx->va_rc_mode = VA_RC_VBR;
>>>>>>
>>>>>> Is it better to take it as CBR when avctx->rc_max_rate is 0 and CBR
>>>>>> is supported by driver?
>>>>>
>>>>> I don't think so?  VBR with the specified target is probably what
>>>>> you want in most cases, and I think anyone with specific constraints
>>>>> that want constant bitrate should expect to set maxrate to achieve that.
>>>>>
>>>>
>>>> I agree VBR is probably what an user wants in most case, however
>>>> target percent set to 50% is not suitable for most case. To get a
>>>> specific target percent, user should set both target bitrate and max
>>>> bitrate, so it is reasonable to ask user must set both target bitrate
>>>> and max bitrate for VBR cases, and for CBR user may set target bitrate
>> only.
>>>
>>> How about set the max_rate to be a very larger number such as INT_MAX
>> if user hasn't set it?
>>> User may don't set max_rate on purpose, expecting better quality with
>> unlimited bitrate fluctuation (common requirement for local video files).
>>> Double of target_bit_rate is too strict IMHO. And I haven't such a
>> limitation in x264 ABR mode.
>>
>> This unconstrained setup you describe was my intent (as you say, it's usually
>> what you want for local files), but unfortunately the API doesn't really let us
>> do it - the target bitrate is specified as an integer percentage of the max
>> bitrate.  50% was an arbitrary value picked to not have a strong constraint
>> but also not be small enough that we need to think about rounding/overflow
>> problems.
>>
>> We could try to pick a large value with the right properties (for example: if
>> target < 2^32 / 100 then choose maxrate = target * 100, percentage = 1;
>> otherwise choose percentage = 2^32 * 100 / bitrate, maxrate = bitrate * 100
>> / percentage), but that would be complex to test and I don't think the drivers
>> handle this sort of setup very well.
>>
>>>> I just saw it is also VBR in QSV when max bitrate is not set so I'm
>>>> fine to keep consistency with QSV for this case.
>>>
>>> What will happen if user set a max_rate but without a setting for
>> target_bitrate?
>>> The mode will be VBR (if driver support) with target_bitrate=0.
>>> Tried this on qsv, MSDK returns an error of invalid video parameters.
>>> Is it ok for vaapi? And also with iHD driver?
>>
>> If AVCodecContext.bit_rate isn't set then we use constant-quality mode
>> instead - see the block I've pointed out above.
>>
>> There isn't currently any constant-quality with max-rate constraint mode in
>> VAAPI.
> 
> Then the problem I see is that -max_rate hasn't been respected well if user set it (he may don't care about the target bitrate except the peak value). 
> Maybe we can add a warning at least? 

Given that it's really CQP, I don't think anyone would ever expect this to work?  Encoders generally don't warn about ignoring extra irrelevant options in AVCodecContext.

(Is there any encoder at all which supports that combination?  E.g. libx264 supports maxrate in CRF but not CQP mode.) 

- Mark


More information about the ffmpeg-devel mailing list