> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Mark Thompson
> Sent: Sunday, June 17, 2018 9:51 PM
> To: ffmpeg-devel@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-boun...@ffmpeg.org] On
> Behalf
> >> Of Xiang, Haihao
> >> Sent: Thursday, June 14, 2018 2:08 PM
> >> To: ffmpeg-devel@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?
>
> >>>>> +
> >>>>> + // We only have a target bitrate, but VAAPI requires
> >> that a
> >>>>> + // maximum rate be supplied as well. Since the user
> >> has
> >>>>> + // offered no particular constraint, arbitrarily pick a
> >>>>> + // maximum rate of double the target rate.
> >>>>> + rc_bits_per_second = 2 * avctx->bit_rate;
> >>>>> + rc_target_percentage = 50;
> >>>>> + } else {
> >>>>> + ctx->va_rc_mode = VA_RC_CBR;
> >>>>> +
> >>>>> rc_bits_per_second = avctx->bit_rate;
> >>>>> rc_target_percentage = 100;
> >>>>> - } else {
> >>>>> - rc_bits_per_second = avctx->rc_max_rate;
> >>>>> - rc_target_percentage = (avctx->bit_rate * 100) /
> >>>>> rc_bits_per_second;
> >>>>> }
> >>>>> - rc_window_size = (hrd_buffer_size * 1000) /
> avctx->bit_rate;
> >>>>> }
> >>>>> ...
>
> Thanks,
>
> - Mark
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
