Re: [FFmpeg-devel] [PATCH v4 8/9] lavc/qsvenc: enable QVBR mode

2018-12-09 Thread Li, Zhong
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Mark Thompson
> Sent: Monday, December 10, 2018 3:10 AM
> To: ffmpeg-devel@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 
> > ---
> >  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_

Re: [FFmpeg-devel] [PATCH v4 8/9] lavc/qsvenc: enable QVBR mode

2018-12-09 Thread Mark Thompson
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 
> ---
>  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?

>  
>  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.)

> +#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 
> *)>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*),
> @@ -760,6 +792,9 @@ static int qsv_retrieve_enc_params(AVCodecContext *avctx, 
> QSVEncContext *q)
>  #if QSV_HAVE_CO2
>  (mfxExtBuffer*),
>  #endif
> +#if QSV_HAVE_CO3
> +(mfxExtBuffer*),
> +#endif
>  };
>  
>  int need_pps = avctx->codec_id != AV_CODEC_ID_MPEG2VIDEO;
> diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
> index 

Re: [FFmpeg-devel] [PATCH v4 8/9] lavc/qsvenc: enable QVBR mode

2018-12-05 Thread Li, Zhong
> From: Li, Zhong
> Sent: Thursday, November 29, 2018 4:29 PM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Li, Zhong 
> Subject: [PATCH v4 8/9] lavc/qsvenc: enable QVBR mode
> 
> 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 
> ---
>  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
> 
>  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; #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
> +*)>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*),
> @@ -760,6 +792,9 @@ static int qsv_retrieve_enc_params(AVCodecContext
> *avctx, QSVEncContext *q)  #if QSV_HAVE_CO2
>  (mfxExtBuffer*),
>  #endif
> +#if QSV_HAVE_CO3
> +(mfxExtBuffer*),
> +#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_ICQQSV_VERSION_ATLEAST(1, 28)
>  #define QSV_HAVE_VCM0
> -#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