| Message ID | 5540c08e-fd41-0138-fea0-c7a33da66621@jkqxz.net |
|---|---|
| State | Superseded |
| Headers | show |
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Mark Thompson > Sent: Tuesday, June 26, 2018 5:30 AM > To: ffmpeg-devel@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@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-bounces@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? > >> > >> 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.
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 "