[FFmpeg-devel] [PATCH v4 8/9] lavc/qsvenc: enable QVBR mode
Li, Zhong
zhong.li at intel.com
Mon Dec 10 08:50:48 EET 2018
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of Mark Thompson
> Sent: Monday, December 10, 2018 3:10 AM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v4 8/9] lavc/qsvenc: enable QVBR mode
>
> On 29/11/2018 08:29, Zhong Li wrote:
> > QVBR mode is to use the variable bitrate control algorithm with
> > constant quality.
> > mfxExtCodingOption3 should be supported to enable QVBR mode.
> >
> > Example usage: ffmpeg -hwaccel qsv -c:v h264_qsv -i input.mp4 -c:v
> > h264_qsv -global_quality 25 -maxrate 2M test_qvbr.mp4 -v verbose
> >
> > Signed-off-by: Zhong Li <zhong.li at intel.com>
> > ---
> > libavcodec/qsvenc.c | 39 +++++++++++++++++++++++++++++++++++++--
> > libavcodec/qsvenc.h | 7 +++++--
> > 2 files changed, 42 insertions(+), 4 deletions(-)
> >
> > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c index
> > ba74821..2dd41d7 100644
> > --- a/libavcodec/qsvenc.c
> > +++ b/libavcodec/qsvenc.c
> > @@ -136,6 +136,9 @@ static void dump_video_param(AVCodecContext
> > *avctx, QSVEncContext *q, #if QSV_HAVE_CO2
> > mfxExtCodingOption2 *co2 =
> (mfxExtCodingOption2*)coding_opts[1];
> > #endif
> > +#if QSV_HAVE_CO3
> > + mfxExtCodingOption3 *co3 =
> (mfxExtCodingOption3*)coding_opts[2];
> > +#endif
>
> With a slightly older header, I got:
>
> src/libavcodec/qsvenc.c: In function ‘dump_video_param’:
> src/libavcodec/qsvenc.c:140:26: warning: unused variable ‘co3’
> [-Wunused-variable]
> mfxExtCodingOption3 *co3 = (mfxExtCodingOption3*)coding_opts[2];
> ^~~
>
> av_unused or condition on QVBR rather than CO3 to avoid that?
The problem I see is that no CO3 is supported except QVBR.
So my plan to support more CO3 parameters (win_brc is one of them and is part of this patch series),
and I do find many of them are useful, such as EnableMBQP/ WeightedPred/ GPB.
> >
> > av_log(avctx, AV_LOG_VERBOSE, "profile: %s; level: %"PRIu16"\n",
> > print_profile(info->CodecProfile), info->CodecLevel); @@
> > -190,7 +193,12 @@ static void dump_video_param(AVCodecContext
> *avctx, QSVEncContext *q,
> > info->ICQQuality, co2->LookAheadDepth);
> > }
> > #endif
> > -
> > +#if QSV_HAVE_QVBR
> > + else if (info->RateControlMethod == MFX_RATECONTROL_QVBR) {
> > + av_log(avctx, AV_LOG_VERBOSE, "QVBRQuality: %"PRIu16"\n",
> > + co3->QVBRQuality);
> > + }
> > +#endif
> > av_log(avctx, AV_LOG_VERBOSE, "NumSlice: %"PRIu16";
> NumRefFrame: %"PRIu16"\n",
> > info->NumSlice, info->NumRefFrame);
> > av_log(avctx, AV_LOG_VERBOSE, "RateDistortionOpt: %s\n", @@
> > -326,7 +334,7 @@ static int select_rc_mode(AVCodecContext *avctx,
> QSVEncContext *q)
> > }
> > #endif
> > #if QSV_HAVE_ICQ
> > - else if (avctx->global_quality > 0) {
> > + else if (avctx->global_quality > 0 && !avctx->rc_max_rate) {
> > rc_mode = MFX_RATECONTROL_ICQ;
> > rc_desc = "intelligent constant quality (ICQ)";
> > }
> > @@ -341,6 +349,12 @@ static int select_rc_mode(AVCodecContext
> *avctx, QSVEncContext *q)
> > rc_desc = "average variable bitrate (AVBR)";
> > }
> > #endif
> > +#if QSV_HAVE_QVBR
> > + else if (avctx->global_quality > 0) {
> > + rc_mode = MFX_RATECONTROL_QVBR;
> > + rc_desc = "constant quality with VBR algorithm (QVBR)";
> > + }
> > +#endif
> > else {
> > rc_mode = MFX_RATECONTROL_VBR;
> > rc_desc = "variable bitrate (VBR)"; @@ -551,10 +565,17 @@
> > static int init_video_param(AVCodecContext *avctx, QSVEncContext *q)
> > #if QSV_HAVE_VCM
> > case MFX_RATECONTROL_VCM:
> > #endif
> > +#if QSV_HAVE_QVBR
> > + case MFX_RATECONTROL_QVBR:
> > +#endif
> > q->param.mfx.BufferSizeInKB = avctx->rc_buffer_size /
> 8000;
> > q->param.mfx.InitialDelayInKB =
> avctx->rc_initial_buffer_occupancy / 1000;
> > q->param.mfx.TargetKbps = avctx->bit_rate / 1000;
> > q->param.mfx.MaxKbps = avctx->rc_max_rate /
> 1000;
> > +#if QSV_HAVE_QVBR
> > + if (q->param.mfx.RateControlMethod ==
> MFX_RATECONTROL_QVBR)
> > + q->extco3.QVBRQuality = avctx->global_quality;
>
> I think you don't want bit_rate / TargetKbps to be set in this case? (Though
> if it's definitely just ignored then I guess it's fine to pass whatever value.)
I guess so too. However, as the MSDK documentation: "It uses the same set of parameters as VBR
and quality factor specified by mfxExtCodingOption3::QVBRQuality."
So just pass all and let it be decided by MSDK.
> > +#endif
> > break;
> > case MFX_RATECONTROL_CQP:
> > quant = avctx->global_quality / FF_QP2LAMBDA; @@ -699,6
> > +720,11 @@ FF_ENABLE_DEPRECATION_WARNINGS
> > }
> > #endif
> > }
> > +#if QSV_HAVE_CO3
> > + q->extco3.Header.BufferId =
> MFX_EXTBUFF_CODING_OPTION3;
> > + q->extco3.Header.BufferSz = sizeof(q->extco3);
> > + q->extparam_internal[q->nb_extparam_internal++] =
> > +(mfxExtBuffer *)&q->extco3; #endif
> > }
> >
> > if (!check_enc_param(avctx,q)) {
> > @@ -753,6 +779,12 @@ static int
> qsv_retrieve_enc_params(AVCodecContext *avctx, QSVEncContext *q)
> > .Header.BufferSz = sizeof(co2),
> > };
> > #endif
> > +#if QSV_HAVE_CO3
> > + mfxExtCodingOption3 co3 = {
> > + .Header.BufferId = MFX_EXTBUFF_CODING_OPTION3,
> > + .Header.BufferSz = sizeof(co3),
> > + };
> > +#endif
> >
> > mfxExtBuffer *ext_buffers[] = {
> > (mfxExtBuffer*)&extradata,
> > @@ -760,6 +792,9 @@ static int
> qsv_retrieve_enc_params(AVCodecContext
> > *avctx, QSVEncContext *q) #if QSV_HAVE_CO2
> > (mfxExtBuffer*)&co2,
> > #endif
> > +#if QSV_HAVE_CO3
> > + (mfxExtBuffer*)&co3,
> > +#endif
> > };
> >
> > int need_pps = avctx->codec_id != AV_CODEC_ID_MPEG2VIDEO;
> diff
> > --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h index
> > c2aa88e..075c86b 100644
> > --- a/libavcodec/qsvenc.h
> > +++ b/libavcodec/qsvenc.h
> > @@ -55,7 +55,7 @@
> > #define QSV_HAVE_AVBR 0
> > #define QSV_HAVE_ICQ QSV_VERSION_ATLEAST(1, 28)
> > #define QSV_HAVE_VCM 0
> > -#define QSV_HAVE_QVBR 0
> > +#define QSV_HAVE_QVBR QSV_VERSION_ATLEAST(1, 28)
> > #define QSV_HAVE_MF QSV_VERSION_ATLEAST(1, 25)
> > #endif
> >
> > @@ -110,6 +110,9 @@ typedef struct QSVEncContext { #if
> QSV_HAVE_CO2
> > mfxExtCodingOption2 extco2;
> > #endif
> > +#if QSV_HAVE_CO3
> > + mfxExtCodingOption3 extco3;
> > +#endif
> > #if QSV_HAVE_MF
> > mfxExtMultiFrameParam extmfp;
> > mfxExtMultiFrameControl extmfc;
> > @@ -118,7 +121,7 @@ typedef struct QSVEncContext {
> > mfxFrameSurface1 **opaque_surfaces;
> > AVBufferRef *opaque_alloc_buf;
> >
> > - mfxExtBuffer *extparam_internal[2 + QSV_HAVE_CO2 +
> (QSV_HAVE_MF * 2)];
> > + mfxExtBuffer *extparam_internal[2 + QSV_HAVE_CO2 +
> QSV_HAVE_CO3
> > + + (QSV_HAVE_MF * 2)];
> > int nb_extparam_internal;
> >
> > mfxExtBuffer **extparam;
> >
>
> There should probably be a check somewhere that the quality value is
> actually in the 1-51 supported range.
It has been checked by MSDK: https://github.com/Intel-Media-SDK/MediaSDK/blob/master/_studio/mfx_lib/shared/src/mfx_h264_enc_common_hw.cpp#L3883
However, checking in ffmpeg level is still benefit to keep robust and easier to let user know.
The only concern is that: For vp8/vp9 encoding, quality range should be 1~127 as my memory. (Though they are not supported right now, they are in my To-Do list).
More information about the ffmpeg-devel
mailing list