diff mbox series

[FFmpeg-devel] libavcodec/qsvenc: add low latency P-pyramid support for qsv

Message ID 20201210060510.6594-1-wenbin.chen@intel.com
State Superseded
Headers show
Series [FFmpeg-devel] libavcodec/qsvenc: add low latency P-pyramid support for qsv | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Wenbin Chen Dec. 10, 2020, 6:05 a.m. UTC
From: Wenbinc-Bin <wenbin.chen@intel.com>

Add low latency P-pyramid support for qsv, and it relates to a new
command line parameter "-p_strategy". To enable this flag, user also
need to set "-bf" to -1 or 0.

Signed-off-by Wenbin Chen <wenbin.chen@intel.com>
---
 libavcodec/qsvenc.c | 22 ++++++++++++++++++++++
 libavcodec/qsvenc.h |  2 ++
 2 files changed, 24 insertions(+)

Comments

Linjie Fu Dec. 10, 2020, 7:48 a.m. UTC | #1
On Thu, Dec 10, 2020 at 2:07 PM <wenbin.chen@intel.com> wrote:
>
> From: Wenbinc-Bin <wenbin.chen@intel.com>
>
> Add low latency P-pyramid support for qsv, and it relates to a new
> command line parameter "-p_strategy". To enable this flag, user also
> need to set "-bf" to -1 or 0.

IMO correcting the related options and prompting a warning would be
more user-friendly.

> Signed-off-by Wenbin Chen <wenbin.chen@intel.com>
> ---
>  libavcodec/qsvenc.c | 22 ++++++++++++++++++++++
>  libavcodec/qsvenc.h |  2 ++
>  2 files changed, 24 insertions(+)
>
> diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
> index 2bd2a56227..c3b41374e4 100644
> --- a/libavcodec/qsvenc.c
> +++ b/libavcodec/qsvenc.c
> @@ -268,6 +268,13 @@ static void dump_video_param(AVCodecContext *avctx, QSVEncContext *q,
>      case MFX_B_REF_PYRAMID: av_log(avctx, AV_LOG_VERBOSE, "pyramid");   break;
>      default:                av_log(avctx, AV_LOG_VERBOSE, "auto");      break;
>      }
> +    av_log(avctx, AV_LOG_VERBOSE, "; PRefType: ");
> +    switch(co3->PRefType){
> +        case MFX_P_REF_DEFAULT: av_log(avctx, AV_LOG_VERBOSE, "default");   break;
> +        case MFX_P_REF_SIMPLE:  av_log(avctx, AV_LOG_VERBOSE, "simple");   break;
> +        case MFX_P_REF_PYRAMID: av_log(avctx, AV_LOG_VERBOSE, "pyramid");   break;
> +        default:    break;
> +    }
>      av_log(avctx, AV_LOG_VERBOSE, "\n");
>  #endif
>
> @@ -777,6 +784,21 @@ FF_ENABLE_DEPRECATION_WARNINGS
>  #if QSV_HAVE_CO3
>          q->extco3.Header.BufferId      = MFX_EXTBUFF_CODING_OPTION3;
>          q->extco3.Header.BufferSz      = sizeof(q->extco3);
> +        switch(q->p_strategy){
> +            case 0:
> +                q->extco3.PRefType = MFX_P_REF_DEFAULT;
> +                break;
> +            case 1:
> +                q->extco3.PRefType = MFX_P_REF_SIMPLE;
> +                break;
> +            case 2:
> +                q->extco3.PRefType = MFX_P_REF_PYRAMID;
> +                break;
> +            default:
> +                q->extco3.PRefType = MFX_P_REF_DEFAULT;
> +                av_log(avctx, AV_LOG_VERBOSE, "invalid p_strategy, set to default\n");
> +                break;
> +        }

It'll be great if one could help to provide some links/background
information like [1] in commit messages or docs,
to elaborate more about "default" and "simple", and make a new option
easier to be understood:

[1] https://github.com/Intel-Media-SDK/MediaSDK/blob/master/doc/mediasdk-man.md#preftype

- linjie
Wenbin Chen Dec. 28, 2020, 4:04 a.m. UTC | #2
> -----Original Message-----
> From: Linjie Fu <linjie.justin.fu@gmail.com>
> Sent: Thursday, December 10, 2020 3:49 PM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Cc: Chen, Wenbin <wenbin.chen@intel.com>
> Subject: Re: [FFmpeg-devel] [PATCH] libavcodec/qsvenc: add low latency P-
> pyramid support for qsv
> 
> On Thu, Dec 10, 2020 at 2:07 PM <wenbin.chen@intel.com> wrote:
> >
> > From: Wenbinc-Bin <wenbin.chen@intel.com>
> >
> > Add low latency P-pyramid support for qsv, and it relates to a new
> > command line parameter "-p_strategy". To enable this flag, user also
> > need to set "-bf" to -1 or 0.
> 
> IMO correcting the related options and prompting a warning would be more
> user-friendly.
> 
> > Signed-off-by Wenbin Chen <wenbin.chen@intel.com>
> > ---
> >  libavcodec/qsvenc.c | 22 ++++++++++++++++++++++  libavcodec/qsvenc.h
> > |  2 ++
> >  2 files changed, 24 insertions(+)
> >
> > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c index
> > 2bd2a56227..c3b41374e4 100644
> > --- a/libavcodec/qsvenc.c
> > +++ b/libavcodec/qsvenc.c
> > @@ -268,6 +268,13 @@ static void dump_video_param(AVCodecContext
> *avctx, QSVEncContext *q,
> >      case MFX_B_REF_PYRAMID: av_log(avctx, AV_LOG_VERBOSE,
> "pyramid");   break;
> >      default:                av_log(avctx, AV_LOG_VERBOSE, "auto");      break;
> >      }
> > +    av_log(avctx, AV_LOG_VERBOSE, "; PRefType: ");
> > +    switch(co3->PRefType){
> > +        case MFX_P_REF_DEFAULT: av_log(avctx, AV_LOG_VERBOSE,
> "default");   break;
> > +        case MFX_P_REF_SIMPLE:  av_log(avctx, AV_LOG_VERBOSE, "simple");
> break;
> > +        case MFX_P_REF_PYRAMID: av_log(avctx, AV_LOG_VERBOSE,
> "pyramid");   break;
> > +        default:    break;
> > +    }
> >      av_log(avctx, AV_LOG_VERBOSE, "\n");  #endif
> >
> > @@ -777,6 +784,21 @@ FF_ENABLE_DEPRECATION_WARNINGS  #if
> QSV_HAVE_CO3
> >          q->extco3.Header.BufferId      = MFX_EXTBUFF_CODING_OPTION3;
> >          q->extco3.Header.BufferSz      = sizeof(q->extco3);
> > +        switch(q->p_strategy){
> > +            case 0:
> > +                q->extco3.PRefType = MFX_P_REF_DEFAULT;
> > +                break;
> > +            case 1:
> > +                q->extco3.PRefType = MFX_P_REF_SIMPLE;
> > +                break;
> > +            case 2:
> > +                q->extco3.PRefType = MFX_P_REF_PYRAMID;
> > +                break;
> > +            default:
> > +                q->extco3.PRefType = MFX_P_REF_DEFAULT;
> > +                av_log(avctx, AV_LOG_VERBOSE, "invalid p_strategy, set to
> default\n");
> > +                break;
> > +        }
> 
> It'll be great if one could help to provide some links/background information
> like [1] in commit messages or docs, to elaborate more about "default" and
> "simple", and make a new option easier to be understood:
> 
> [1] https://github.com/Intel-Media-
> SDK/MediaSDK/blob/master/doc/mediasdk-man.md#preftype
> 
> - linjie

Sorry, I missed your replay. 
Thanks for your advice and I will resubmit the patch.

Wenbin
diff mbox series

Patch

diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
index 2bd2a56227..c3b41374e4 100644
--- a/libavcodec/qsvenc.c
+++ b/libavcodec/qsvenc.c
@@ -268,6 +268,13 @@  static void dump_video_param(AVCodecContext *avctx, QSVEncContext *q,
     case MFX_B_REF_PYRAMID: av_log(avctx, AV_LOG_VERBOSE, "pyramid");   break;
     default:                av_log(avctx, AV_LOG_VERBOSE, "auto");      break;
     }
+    av_log(avctx, AV_LOG_VERBOSE, "; PRefType: ");
+    switch(co3->PRefType){
+        case MFX_P_REF_DEFAULT: av_log(avctx, AV_LOG_VERBOSE, "default");   break;
+        case MFX_P_REF_SIMPLE:  av_log(avctx, AV_LOG_VERBOSE, "simple");   break;
+        case MFX_P_REF_PYRAMID: av_log(avctx, AV_LOG_VERBOSE, "pyramid");   break;
+        default:    break;
+    }
     av_log(avctx, AV_LOG_VERBOSE, "\n");
 #endif
 
@@ -777,6 +784,21 @@  FF_ENABLE_DEPRECATION_WARNINGS
 #if QSV_HAVE_CO3
         q->extco3.Header.BufferId      = MFX_EXTBUFF_CODING_OPTION3;
         q->extco3.Header.BufferSz      = sizeof(q->extco3);
+        switch(q->p_strategy){
+            case 0:
+                q->extco3.PRefType = MFX_P_REF_DEFAULT;
+                break;
+            case 1:
+                q->extco3.PRefType = MFX_P_REF_SIMPLE;
+                break;
+            case 2:
+                q->extco3.PRefType = MFX_P_REF_PYRAMID;
+                break;
+            default:
+                q->extco3.PRefType = MFX_P_REF_DEFAULT;
+                av_log(avctx, AV_LOG_VERBOSE, "invalid p_strategy, set to default\n");
+                break;
+        }
 #if QSV_HAVE_GPB
         if (avctx->codec_id == AV_CODEC_ID_HEVC)
             q->extco3.GPB              = q->gpb ? MFX_CODINGOPTION_ON : MFX_CODINGOPTION_OFF;
diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
index 6d305f87dd..30aaf72ebd 100644
--- a/libavcodec/qsvenc.h
+++ b/libavcodec/qsvenc.h
@@ -95,6 +95,7 @@ 
 { "adaptive_i",     "Adaptive I-frame placement",             OFFSET(qsv.adaptive_i),     AV_OPT_TYPE_INT, { .i64 = -1 }, -1,          1, VE },                         \
 { "adaptive_b",     "Adaptive B-frame placement",             OFFSET(qsv.adaptive_b),     AV_OPT_TYPE_INT, { .i64 = -1 }, -1,          1, VE },                         \
 { "b_strategy",     "Strategy to choose between I/P/B-frames", OFFSET(qsv.b_strategy),    AV_OPT_TYPE_INT, { .i64 = -1 }, -1,          1, VE },                         \
+{ "p_strategy",     "enable P-pyramid 0-default 1-simple 2-pyramis",    OFFSET(qsv.p_strategy), AV_OPT_TYPE_INT,    { .i64 = 0}, 0,    2, VE },                       \
 { "forced_idr",     "Forcing I frames as IDR frames",         OFFSET(qsv.forced_idr),     AV_OPT_TYPE_BOOL,{ .i64 = 0  },  0,          1, VE },                         \
 { "low_power", "enable low power mode(experimental: many limitations by mfx version, BRC modes, etc.)", OFFSET(qsv.low_power), AV_OPT_TYPE_BOOL, { .i64 = 0}, 0, 1, VE},\
 
@@ -182,6 +183,7 @@  typedef struct QSVEncContext {
     int adaptive_i;
     int adaptive_b;
     int b_strategy;
+    int p_strategy;
     int cavlc;
 
     int int_ref_type;