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-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? > > 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 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB it is not once nor twice but times without number that the same ideas make their appearance in the world. -- Aristotle
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
