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

Li, Zhong zhong.li at intel.com
Tue Jul 3 08:14:09 EEST 2018


> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of Mark Thompson
> Sent: Tuesday, June 26, 2018 5:30 AM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v2 16/36] vaapi_encode: Clean up rate
> control configuration
> 
> 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

I have another idea: if rc_max_rate is set, then set rc_mode to be VBR (align with amfenc). Target bitrate can be set to a half (or anther value) of rc_max_rate if necessary.
CQP is not as good as CRF and should be low priority mode especially when max_rate is set.



More information about the ffmpeg-devel mailing list