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

Mark Thompson sw at jkqxz.net
Tue Jun 26 00:30:24 EEST 2018


On 21/06/18 18:03, Michael Niedermayer wrote:
> On Thu, Jun 21, 2018 at 12:10:04AM +0100, Mark Thompson wrote:
>> 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.) 
> 
> if i understand correctly, then yes, see vbv_ignore_qmax. If max rate
> cannot be achievied the mpegvideo encoders should attempt to encode the frame
> again without qmax and at lower quality

Ok, fair enough.

I've added a warning as below so that it's clear this case isn't supported.

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 53c3a2132a..25d89c65c9 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -1311,6 +1311,10 @@ static av_cold int vaapi_encode_init_rate_control(AVCodecContext *avctx)
         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;
+            if (avctx->bit_rate > 0 || avctx->rc_max_rate > 0) {
+                av_log(avctx, AV_LOG_WARNING, "Bitrate target parameters "
+                       "ignored in constant-quality mode.\n");
+            }
             return 0;
         } else {
             av_log(avctx, AV_LOG_ERROR, "Driver does not support "

Thanks,

- Mark


More information about the ffmpeg-devel mailing list