diff mbox

[FFmpeg-devel,v2] lavc/vaapi_encode: add support for AVC Trellis

Message ID 20190612152814.29665-1-linjie.fu@intel.com
State Superseded
Headers show

Commit Message

Fu, Linjie June 12, 2019, 3:28 p.m. UTC
Add support for VAAPI AVC Trellis Quantization with limitation:
    - VA-API version >= (1, 0, 0)

Use option "-trellis off/I/P/B" to disable or enable Trellis
quantization for I/P/B frames.

Signed-off-by: Linjie Fu <linjie.fu@intel.com>
---
[v2]: Since nonstandard struct for VAEncMiscParameterQuantization is
fixed: https://github.com/intel/libva/issues/265
update patch based on:
http://git.ffmpeg.org/gitweb/ffmpeg.git/commit/2880a32c668023bfee4745095c885450d547ae45
 libavcodec/vaapi_encode.c      | 48 ++++++++++++++++++++++++++++++++++
 libavcodec/vaapi_encode.h      |  9 +++++--
 libavcodec/vaapi_encode_h264.c |  9 +++++++
 3 files changed, 64 insertions(+), 2 deletions(-)

Comments

mypopy@gmail.com June 12, 2019, 8:14 a.m. UTC | #1
On Wed, Jun 12, 2019 at 3:28 PM Linjie Fu <linjie.fu@intel.com> wrote:
>
> Add support for VAAPI AVC Trellis Quantization with limitation:
>     - VA-API version >= (1, 0, 0)
>
> Use option "-trellis off/I/P/B" to disable or enable Trellis
> quantization for I/P/B frames.
>
> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> ---
> [v2]: Since nonstandard struct for VAEncMiscParameterQuantization is
> fixed: https://github.com/intel/libva/issues/265
> update patch based on:
> http://git.ffmpeg.org/gitweb/ffmpeg.git/commit/2880a32c668023bfee4745095c885450d547ae45
>  libavcodec/vaapi_encode.c      | 48 ++++++++++++++++++++++++++++++++++
>  libavcodec/vaapi_encode.h      |  9 +++++--
>  libavcodec/vaapi_encode_h264.c |  9 +++++++
>  3 files changed, 64 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> index dd2a24de04..fbfbe78c6b 100644
> --- a/libavcodec/vaapi_encode.c
> +++ b/libavcodec/vaapi_encode.c
> @@ -1671,6 +1671,48 @@ rc_mode_found:
>      return 0;
>  }
>
> +static av_cold int vaapi_encode_init_quantization(AVCodecContext *avctx)
> +{
> +#if VA_CHECK_VERSION(1, 0, 0)
> +    VAAPIEncodeContext *ctx = avctx->priv_data;
> +    VAStatus vas;
> +    VAConfigAttrib attr = { VAConfigAttribEncQuantization };
> +    int trellis = ctx->trellis;
> +
> +    vas = vaGetConfigAttributes(ctx->hwctx->display,
> +                                ctx->va_profile,
> +                                ctx->va_entrypoint,
> +                                &attr, 1);
> +    if (vas != VA_STATUS_SUCCESS) {
> +        av_log(avctx, AV_LOG_ERROR, "Failed to query quantization "
> +               "config attribute: %d (%s).\n", vas, vaErrorStr(vas));
> +        return AVERROR_EXTERNAL;
> +    }
> +
> +    if (attr.value == VA_ATTRIB_NOT_SUPPORTED ||
> +        attr.value == VA_ENC_QUANTIZATION_NONE) {
> +        av_log(avctx, AV_LOG_WARNING, "Special Quantization attribute is not "
> +                "supported: will use default quantization.\n");
> +    } else if (attr.value == VA_ENC_QUANTIZATION_TRELLIS_SUPPORTED){
> +        av_log(avctx, AV_LOG_VERBOSE, "Quantization Trellis supported.\n");
> +
> +        ctx->quantization_params = (VAEncMiscParameterQuantization) {
> +            .quantization_flags.value = trellis,
> +        };
> +
> +        vaapi_encode_add_global_param(avctx,
> +                                      VAEncMiscParameterTypeQuantization,
> +                                      &ctx->quantization_params,
> +                                      sizeof(ctx->quantization_params));
> +    }
> +#else
> +    av_log(avctx, AV_LOG_WARNING, "The encode quantization option (Trellis) is "
> +           "not supported with this VAAPI version.\n");
> +#endif
> +
> +    return 0;
> +}
> +
>  static av_cold int vaapi_encode_init_gop_structure(AVCodecContext *avctx)
>  {
>      VAAPIEncodeContext *ctx = avctx->priv_data;
> @@ -2132,6 +2174,12 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
>      if (err < 0)
>          goto fail;
>
> +    if (ctx->trellis) {
> +        err = vaapi_encode_init_quantization(avctx);
> +        if (err < 0)
> +            goto fail;
> +    }
> +
>      if (avctx->compression_level >= 0) {
>          err = vaapi_encode_init_quality(avctx);
>          if (err < 0)
> diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
> index eeec06036b..b24735da59 100644
> --- a/libavcodec/vaapi_encode.h
> +++ b/libavcodec/vaapi_encode.h
> @@ -37,7 +37,7 @@ struct VAAPIEncodePicture;
>
>  enum {
>      MAX_CONFIG_ATTRIBUTES  = 4,
> -    MAX_GLOBAL_PARAMS      = 4,
> +    MAX_GLOBAL_PARAMS      = 5,
>      MAX_DPB_SIZE           = 16,
>      MAX_PICTURE_REFERENCES = 2,
>      MAX_REORDER_DELAY      = 16,
> @@ -220,6 +220,9 @@ typedef struct VAAPIEncodeContext {
>      // Packed headers which will actually be sent.
>      unsigned int    va_packed_headers;
>
> +    // Quantization mode
> +    int trellis;
> +
>      // Configuration attributes to use when creating va_config.
>      VAConfigAttrib  config_attributes[MAX_CONFIG_ATTRIBUTES];
>      int          nb_config_attributes;
> @@ -256,7 +259,9 @@ typedef struct VAAPIEncodeContext {
>  #if VA_CHECK_VERSION(0, 36, 0)
>      VAEncMiscParameterBufferQualityLevel quality_params;
>  #endif
> -
> +#if VA_CHECK_VERSION(1, 0, 0)
> +    VAEncMiscParameterQuantization quantization_params;
> +#endif
>      // Per-sequence parameter structure (VAEncSequenceParameterBuffer*).
>      void           *codec_sequence_params;
>
> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
> index d1427112ea..427fb6320e 100644
> --- a/libavcodec/vaapi_encode_h264.c
> +++ b/libavcodec/vaapi_encode_h264.c
> @@ -72,6 +72,7 @@ typedef struct VAAPIEncodeH264Context {
>      int sei;
>      int profile;
>      int level;
> +    int trellis;
>
>      // Derived settings.
>      int mb_width;
> @@ -1233,6 +1234,8 @@ static av_cold int vaapi_encode_h264_init(AVCodecContext *avctx)
>      if (priv->qp > 0)
>          ctx->explicit_qp = priv->qp;
>
> +    ctx->trellis = priv->trellis;
> +
>      return ff_vaapi_encode_init(avctx);
>  }
>
> @@ -1263,6 +1266,12 @@ static const AVOption vaapi_encode_h264_options[] = {
>          { "cabac", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 1 }, INT_MIN, INT_MAX, FLAGS, "coder" },
>          { "vlc",   NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 0 }, INT_MIN, INT_MAX, FLAGS, "coder" },
>          { "ac",    NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 1 }, INT_MIN, INT_MAX, FLAGS, "coder" },
> +    { "trellis", "Trellis Quantization",
> +      OFFSET(trellis), AV_OPT_TYPE_FLAGS, { .i64 = 0}, 0, INT_MAX, FLAGS, "trellis"},
> +        { "off",   NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 1 }, INT_MIN, INT_MAX, FLAGS, "trellis"},
> +        { "I",     NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 2 }, INT_MIN, INT_MAX, FLAGS, "trellis"},
> +        { "P",     NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 4 }, INT_MIN, INT_MAX, FLAGS, "trellis"},
> +        { "B",     NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 8 }, INT_MIN, INT_MAX, FLAGS, "trellis"},
>
>      { "aud", "Include AUD",
>        OFFSET(aud), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, FLAGS },
> --
> 2.17.1
>

Do you test this function for this feature with the other
options/features, as I know, this feature have some limitation, I
believe is a hardware or driver  limitation, and we enable this
features about 2 years ago, but didn't submit the patch because the
limitation

Some question:
1. How is the impact on encoder performance and image quality?
2. What's the limitation with the other VA-API features?
3. Missed the docs
Mark Thompson June 12, 2019, 10:37 p.m. UTC | #2
On 12/06/2019 16:28, Linjie Fu wrote:
> Add support for VAAPI AVC Trellis Quantization with limitation:
>     - VA-API version >= (1, 0, 0)
> 
> Use option "-trellis off/I/P/B" to disable or enable Trellis
> quantization for I/P/B frames.
> 
> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> ---
> [v2]: Since nonstandard struct for VAEncMiscParameterQuantization is
> fixed: https://github.com/intel/libva/issues/265

Should the version check be for the first release this fix is in rather than for libva 2.0, then?

> update patch based on:
> http://git.ffmpeg.org/gitweb/ffmpeg.git/commit/2880a32c668023bfee4745095c885450d547ae45
>  libavcodec/vaapi_encode.c      | 48 ++++++++++++++++++++++++++++++++++
>  libavcodec/vaapi_encode.h      |  9 +++++--
>  libavcodec/vaapi_encode_h264.c |  9 +++++++
>  3 files changed, 64 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> index dd2a24de04..fbfbe78c6b 100644
> --- a/libavcodec/vaapi_encode.c
> +++ b/libavcodec/vaapi_encode.c
> @@ -1671,6 +1671,48 @@ rc_mode_found:
>      return 0;
>  }
>  
> +static av_cold int vaapi_encode_init_quantization(AVCodecContext *avctx)
> +{
> +#if VA_CHECK_VERSION(1, 0, 0)
> +    VAAPIEncodeContext *ctx = avctx->priv_data;
> +    VAStatus vas;
> +    VAConfigAttrib attr = { VAConfigAttribEncQuantization };
> +    int trellis = ctx->trellis;

This variable isn't really doing anything.

> +
> +    vas = vaGetConfigAttributes(ctx->hwctx->display,
> +                                ctx->va_profile,
> +                                ctx->va_entrypoint,
> +                                &attr, 1);
> +    if (vas != VA_STATUS_SUCCESS) {
> +        av_log(avctx, AV_LOG_ERROR, "Failed to query quantization "
> +               "config attribute: %d (%s).\n", vas, vaErrorStr(vas));
> +        return AVERROR_EXTERNAL;
> +    }
> +
> +    if (attr.value == VA_ATTRIB_NOT_SUPPORTED ||
> +        attr.value == VA_ENC_QUANTIZATION_NONE) {
> +        av_log(avctx, AV_LOG_WARNING, "Special Quantization attribute is not "
> +                "supported: will use default quantization.\n");
> +    } else if (attr.value == VA_ENC_QUANTIZATION_TRELLIS_SUPPORTED){
> +        av_log(avctx, AV_LOG_VERBOSE, "Quantization Trellis supported.\n");
> +
> +        ctx->quantization_params = (VAEncMiscParameterQuantization) {
> +            .quantization_flags.value = trellis,
> +        };
> +
> +        vaapi_encode_add_global_param(avctx,
> +                                      VAEncMiscParameterTypeQuantization,
> +                                      &ctx->quantization_params,
> +                                      sizeof(ctx->quantization_params));
> +    }
> +#else
> +    av_log(avctx, AV_LOG_WARNING, "The encode quantization option (Trellis) is "
> +           "not supported with this VAAPI version.\n");
> +#endif
> +
> +    return 0;
> +}
> +
>  static av_cold int vaapi_encode_init_gop_structure(AVCodecContext *avctx)
>  {
>      VAAPIEncodeContext *ctx = avctx->priv_data;
> @@ -2132,6 +2174,12 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
>      if (err < 0)
>          goto fail;
>  
> +    if (ctx->trellis) {
> +        err = vaapi_encode_init_quantization(avctx);
> +        if (err < 0)
> +            goto fail;
> +    }
> +
>      if (avctx->compression_level >= 0) {
>          err = vaapi_encode_init_quality(avctx);
>          if (err < 0)
> diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
> index eeec06036b..b24735da59 100644
> --- a/libavcodec/vaapi_encode.h
> +++ b/libavcodec/vaapi_encode.h
> @@ -37,7 +37,7 @@ struct VAAPIEncodePicture;
>  
>  enum {
>      MAX_CONFIG_ATTRIBUTES  = 4,
> -    MAX_GLOBAL_PARAMS      = 4,
> +    MAX_GLOBAL_PARAMS      = 5,
>      MAX_DPB_SIZE           = 16,
>      MAX_PICTURE_REFERENCES = 2,
>      MAX_REORDER_DELAY      = 16,
> @@ -220,6 +220,9 @@ typedef struct VAAPIEncodeContext {
>      // Packed headers which will actually be sent.
>      unsigned int    va_packed_headers;
>  
> +    // Quantization mode
> +    int trellis;

Put this field further up, above the "set before calling ff_vaapi_encode_init()" line.

> +
>      // Configuration attributes to use when creating va_config.
>      VAConfigAttrib  config_attributes[MAX_CONFIG_ATTRIBUTES];
>      int          nb_config_attributes;
> @@ -256,7 +259,9 @@ typedef struct VAAPIEncodeContext {
>  #if VA_CHECK_VERSION(0, 36, 0)
>      VAEncMiscParameterBufferQualityLevel quality_params;
>  #endif
> -
> +#if VA_CHECK_VERSION(1, 0, 0)
> +    VAEncMiscParameterQuantization quantization_params;
> +#endif
>      // Per-sequence parameter structure (VAEncSequenceParameterBuffer*).
>      void           *codec_sequence_params;
>  
> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
> index d1427112ea..427fb6320e 100644
> --- a/libavcodec/vaapi_encode_h264.c
> +++ b/libavcodec/vaapi_encode_h264.c
> @@ -72,6 +72,7 @@ typedef struct VAAPIEncodeH264Context {
>      int sei;
>      int profile;
>      int level;
> +    int trellis;
>  
>      // Derived settings.
>      int mb_width;
> @@ -1233,6 +1234,8 @@ static av_cold int vaapi_encode_h264_init(AVCodecContext *avctx)
>      if (priv->qp > 0)
>          ctx->explicit_qp = priv->qp;
>  
> +    ctx->trellis = priv->trellis;
> +
>      return ff_vaapi_encode_init(avctx);
>  }
>  
> @@ -1263,6 +1266,12 @@ static const AVOption vaapi_encode_h264_options[] = {
>          { "cabac", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 1 }, INT_MIN, INT_MAX, FLAGS, "coder" },
>          { "vlc",   NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 0 }, INT_MIN, INT_MAX, FLAGS, "coder" },
>          { "ac",    NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 1 }, INT_MIN, INT_MAX, FLAGS, "coder" },
> +    { "trellis", "Trellis Quantization",
> +      OFFSET(trellis), AV_OPT_TYPE_FLAGS, { .i64 = 0}, 0, INT_MAX, FLAGS, "trellis"},
> +        { "off",   NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 1 }, INT_MIN, INT_MAX, FLAGS, "trellis"},
> +        { "I",     NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 2 }, INT_MIN, INT_MAX, FLAGS, "trellis"},
> +        { "P",     NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 4 }, INT_MIN, INT_MAX, FLAGS, "trellis"},
> +        { "B",     NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 8 }, INT_MIN, INT_MAX, FLAGS, "trellis"},
>  
>      { "aud", "Include AUD",
>        OFFSET(aud), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, FLAGS },
> 

Can you add a bit more explanation about what the option does and when someone might want to set it?  (Ideally in a documentation patch...)

I don't think it should be specific to H.264, either - the technique can apply in any codec feeding sets of block coefficients into an entropy coder.  The check above does the right thing for any driver which doesn't support it for a given codec.

Thanks,

- Mark
Fu, Linjie June 13, 2019, 2:02 a.m. UTC | #3
> -----Original Message-----

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf

> Of Mark Thompson

> Sent: Thursday, June 13, 2019 06:37

> To: ffmpeg-devel@ffmpeg.org

> Subject: Re: [FFmpeg-devel] [PATCH, v2] lavc/vaapi_encode: add support for

> AVC Trellis

> 

> On 12/06/2019 16:28, Linjie Fu wrote:

> > Add support for VAAPI AVC Trellis Quantization with limitation:

> >     - VA-API version >= (1, 0, 0)

> >

> > Use option "-trellis off/I/P/B" to disable or enable Trellis

> > quantization for I/P/B frames.

> >

> > Signed-off-by: Linjie Fu <linjie.fu@intel.com>

> > ---

> > [v2]: Since nonstandard struct for VAEncMiscParameterQuantization is

> > fixed: https://github.com/intel/libva/issues/265

> 

> Should the version check be for the first release this fix is in rather than for

> libva 2.0, then?


I've been thinking about the concerns before, too.
Since the nonstandard structure is supported more robustly, uint32_t data[]
in misc now can be point to the uint64_t quantization and make this feature
valid.
The remaining concern is that using  a uint32_t pointer to point a uint64_t one (should be uint32_t) which
is not functional. 
So I choose to check the version libva 2.0  which supported Trellis for the first time to support
more libva version.

Will update the version check to libva 2.5 to eliminate all concerns if it's more acceptable.

> 

> > update patch based on:

> >

> http://git.ffmpeg.org/gitweb/ffmpeg.git/commit/2880a32c668023bfee47450

> 95c885450d547ae45

> >  libavcodec/vaapi_encode.c      | 48

> ++++++++++++++++++++++++++++++++++

> >  libavcodec/vaapi_encode.h      |  9 +++++--

> >  libavcodec/vaapi_encode_h264.c |  9 +++++++

> >  3 files changed, 64 insertions(+), 2 deletions(-)

> >

> > diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c

> > index dd2a24de04..fbfbe78c6b 100644

> > --- a/libavcodec/vaapi_encode.c

> > +++ b/libavcodec/vaapi_encode.c

> > @@ -1671,6 +1671,48 @@ rc_mode_found:

> >      return 0;

> >  }

> >

> > +static av_cold int vaapi_encode_init_quantization(AVCodecContext

> *avctx)

> > +{

> > +#if VA_CHECK_VERSION(1, 0, 0)

> > +    VAAPIEncodeContext *ctx = avctx->priv_data;

> > +    VAStatus vas;

> > +    VAConfigAttrib attr = { VAConfigAttribEncQuantization };

> > +    int trellis = ctx->trellis;

> 

> This variable isn't really doing anything.

Yep, trying to match the behavior with quality but since this variable is not frequently
used, ctx->trellis seems to be enough.

> 

> > +

> > +    vas = vaGetConfigAttributes(ctx->hwctx->display,

> > +                                ctx->va_profile,

> > +                                ctx->va_entrypoint,

> > +                                &attr, 1);

> > +    if (vas != VA_STATUS_SUCCESS) {

> > +        av_log(avctx, AV_LOG_ERROR, "Failed to query quantization "

> > +               "config attribute: %d (%s).\n", vas, vaErrorStr(vas));

> > +        return AVERROR_EXTERNAL;

> > +    }

> > +

> > +    if (attr.value == VA_ATTRIB_NOT_SUPPORTED ||

> > +        attr.value == VA_ENC_QUANTIZATION_NONE) {

> > +        av_log(avctx, AV_LOG_WARNING, "Special Quantization attribute is

> not "

> > +                "supported: will use default quantization.\n");

> > +    } else if (attr.value == VA_ENC_QUANTIZATION_TRELLIS_SUPPORTED){

> > +        av_log(avctx, AV_LOG_VERBOSE, "Quantization Trellis supported.\n");

> > +

> > +        ctx->quantization_params = (VAEncMiscParameterQuantization) {

> > +            .quantization_flags.value = trellis,

> > +        };

> > +

> > +        vaapi_encode_add_global_param(avctx,

> > +                                      VAEncMiscParameterTypeQuantization,

> > +                                      &ctx->quantization_params,

> > +                                      sizeof(ctx->quantization_params));

> > +    }

> > +#else

> > +    av_log(avctx, AV_LOG_WARNING, "The encode quantization option

> (Trellis) is "

> > +           "not supported with this VAAPI version.\n");

> > +#endif

> > +

> > +    return 0;

> > +}

> > +

> >  static av_cold int vaapi_encode_init_gop_structure(AVCodecContext

> *avctx)

> >  {

> >      VAAPIEncodeContext *ctx = avctx->priv_data;

> > @@ -2132,6 +2174,12 @@ av_cold int

> ff_vaapi_encode_init(AVCodecContext *avctx)

> >      if (err < 0)

> >          goto fail;

> >

> > +    if (ctx->trellis) {

> > +        err = vaapi_encode_init_quantization(avctx);

> > +        if (err < 0)

> > +            goto fail;

> > +    }

> > +

> >      if (avctx->compression_level >= 0) {

> >          err = vaapi_encode_init_quality(avctx);

> >          if (err < 0)

> > diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h

> > index eeec06036b..b24735da59 100644

> > --- a/libavcodec/vaapi_encode.h

> > +++ b/libavcodec/vaapi_encode.h

> > @@ -37,7 +37,7 @@ struct VAAPIEncodePicture;

> >

> >  enum {

> >      MAX_CONFIG_ATTRIBUTES  = 4,

> > -    MAX_GLOBAL_PARAMS      = 4,

> > +    MAX_GLOBAL_PARAMS      = 5,

> >      MAX_DPB_SIZE           = 16,

> >      MAX_PICTURE_REFERENCES = 2,

> >      MAX_REORDER_DELAY      = 16,

> > @@ -220,6 +220,9 @@ typedef struct VAAPIEncodeContext {

> >      // Packed headers which will actually be sent.

> >      unsigned int    va_packed_headers;

> >

> > +    // Quantization mode

> > +    int trellis;

> 

> Put this field further up, above the "set before calling

> ff_vaapi_encode_init()" line.


Will follow, thanks.

> 

> > +

> >      // Configuration attributes to use when creating va_config.

> >      VAConfigAttrib  config_attributes[MAX_CONFIG_ATTRIBUTES];

> >      int          nb_config_attributes;

> > @@ -256,7 +259,9 @@ typedef struct VAAPIEncodeContext {

> >  #if VA_CHECK_VERSION(0, 36, 0)

> >      VAEncMiscParameterBufferQualityLevel quality_params;

> >  #endif

> > -

> > +#if VA_CHECK_VERSION(1, 0, 0)

> > +    VAEncMiscParameterQuantization quantization_params;

> > +#endif

> >      // Per-sequence parameter structure

> (VAEncSequenceParameterBuffer*).

> >      void           *codec_sequence_params;

> >

> > diff --git a/libavcodec/vaapi_encode_h264.c

> b/libavcodec/vaapi_encode_h264.c

> > index d1427112ea..427fb6320e 100644

> > --- a/libavcodec/vaapi_encode_h264.c

> > +++ b/libavcodec/vaapi_encode_h264.c

> > @@ -72,6 +72,7 @@ typedef struct VAAPIEncodeH264Context {

> >      int sei;

> >      int profile;

> >      int level;

> > +    int trellis;

> >

> >      // Derived settings.

> >      int mb_width;

> > @@ -1233,6 +1234,8 @@ static av_cold int

> vaapi_encode_h264_init(AVCodecContext *avctx)

> >      if (priv->qp > 0)

> >          ctx->explicit_qp = priv->qp;

> >

> > +    ctx->trellis = priv->trellis;

> > +

> >      return ff_vaapi_encode_init(avctx);

> >  }

> >

> > @@ -1263,6 +1266,12 @@ static const AVOption

> vaapi_encode_h264_options[] = {

> >          { "cabac", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 1 }, INT_MIN,

> INT_MAX, FLAGS, "coder" },

> >          { "vlc",   NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 0 }, INT_MIN,

> INT_MAX, FLAGS, "coder" },

> >          { "ac",    NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 1 }, INT_MIN,

> INT_MAX, FLAGS, "coder" },

> > +    { "trellis", "Trellis Quantization",

> > +      OFFSET(trellis), AV_OPT_TYPE_FLAGS, { .i64 = 0}, 0, INT_MAX, FLAGS,

> "trellis"},

> > +        { "off",   NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 1 }, INT_MIN,

> INT_MAX, FLAGS, "trellis"},

> > +        { "I",     NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 2 }, INT_MIN,

> INT_MAX, FLAGS, "trellis"},

> > +        { "P",     NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 4 }, INT_MIN,

> INT_MAX, FLAGS, "trellis"},

> > +        { "B",     NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 8 }, INT_MIN,

> INT_MAX, FLAGS, "trellis"},

> >

> >      { "aud", "Include AUD",

> >        OFFSET(aud), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, FLAGS },

> >

> 

> Can you add a bit more explanation about what the option does and when

> someone might want to set it?  (Ideally in a documentation patch...)


Yes, will add docs as Jun has advised, too.

> I don't think it should be specific to H.264, either - the technique can apply in

> any codec feeding sets of block coefficients into an entropy coder.  The check

> above does the right thing for any driver which doesn't support it for a given

> codec.


Trellis is not supported in HEVC currently:
https://github.com/intel/media-driver/issues/574
That's one of the reasons to keep it private for AVC.
However since the query/check is available , it would be better to take it as a
common option for all codec.

Thanks a lot,
- Linjie
Fu, Linjie June 17, 2019, 5:47 a.m. UTC | #4
> -----Original Message-----

> From: mypopy@gmail.com [mailto:mypopy@gmail.com]

> Sent: Wednesday, June 12, 2019 16:14

> To: FFmpeg development discussions and patches <ffmpeg-

> devel@ffmpeg.org>

> Cc: Fu, Linjie <linjie.fu@intel.com>

> Subject: Re: [FFmpeg-devel] [PATCH, v2] lavc/vaapi_encode: add support for

> AVC Trellis

> 

> On Wed, Jun 12, 2019 at 3:28 PM Linjie Fu <linjie.fu@intel.com> wrote:

> >

> > Add support for VAAPI AVC Trellis Quantization with limitation:

> >     - VA-API version >= (1, 0, 0)

> >

> > Use option "-trellis off/I/P/B" to disable or enable Trellis

> > quantization for I/P/B frames.

> >

> > Signed-off-by: Linjie Fu <linjie.fu@intel.com>

> > ---

> > [v2]: Since nonstandard struct for VAEncMiscParameterQuantization is

> > fixed: https://github.com/intel/libva/issues/265

> > update patch based on:

> >

> http://git.ffmpeg.org/gitweb/ffmpeg.git/commit/2880a32c668023bfee47450

> 95c885450d547ae45

> >  libavcodec/vaapi_encode.c      | 48

> ++++++++++++++++++++++++++++++++++

> >  libavcodec/vaapi_encode.h      |  9 +++++--

> >  libavcodec/vaapi_encode_h264.c |  9 +++++++

> >  3 files changed, 64 insertions(+), 2 deletions(-)

> >

> > diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c

> > index dd2a24de04..fbfbe78c6b 100644

> > --- a/libavcodec/vaapi_encode.c

> > +++ b/libavcodec/vaapi_encode.c

> > @@ -1671,6 +1671,48 @@ rc_mode_found:

> >      return 0;

> >  }

> >

> > +static av_cold int vaapi_encode_init_quantization(AVCodecContext

> *avctx)

> > +{

> > +#if VA_CHECK_VERSION(1, 0, 0)

> > +    VAAPIEncodeContext *ctx = avctx->priv_data;

> > +    VAStatus vas;

> > +    VAConfigAttrib attr = { VAConfigAttribEncQuantization };

> > +    int trellis = ctx->trellis;

> > +

> > +    vas = vaGetConfigAttributes(ctx->hwctx->display,

> > +                                ctx->va_profile,

> > +                                ctx->va_entrypoint,

> > +                                &attr, 1);

> > +    if (vas != VA_STATUS_SUCCESS) {

> > +        av_log(avctx, AV_LOG_ERROR, "Failed to query quantization "

> > +               "config attribute: %d (%s).\n", vas, vaErrorStr(vas));

> > +        return AVERROR_EXTERNAL;

> > +    }

> > +

> > +    if (attr.value == VA_ATTRIB_NOT_SUPPORTED ||

> > +        attr.value == VA_ENC_QUANTIZATION_NONE) {

> > +        av_log(avctx, AV_LOG_WARNING, "Special Quantization attribute is

> not "

> > +                "supported: will use default quantization.\n");

> > +    } else if (attr.value == VA_ENC_QUANTIZATION_TRELLIS_SUPPORTED){

> > +        av_log(avctx, AV_LOG_VERBOSE, "Quantization Trellis supported.\n");

> > +

> > +        ctx->quantization_params = (VAEncMiscParameterQuantization) {

> > +            .quantization_flags.value = trellis,

> > +        };

> > +

> > +        vaapi_encode_add_global_param(avctx,

> > +                                      VAEncMiscParameterTypeQuantization,

> > +                                      &ctx->quantization_params,

> > +                                      sizeof(ctx->quantization_params));

> > +    }

> > +#else

> > +    av_log(avctx, AV_LOG_WARNING, "The encode quantization option

> (Trellis) is "

> > +           "not supported with this VAAPI version.\n");

> > +#endif

> > +

> > +    return 0;

> > +}

> > +

> >  static av_cold int vaapi_encode_init_gop_structure(AVCodecContext

> *avctx)

> >  {

> >      VAAPIEncodeContext *ctx = avctx->priv_data;

> > @@ -2132,6 +2174,12 @@ av_cold int

> ff_vaapi_encode_init(AVCodecContext *avctx)

> >      if (err < 0)

> >          goto fail;

> >

> > +    if (ctx->trellis) {

> > +        err = vaapi_encode_init_quantization(avctx);

> > +        if (err < 0)

> > +            goto fail;

> > +    }

> > +

> >      if (avctx->compression_level >= 0) {

> >          err = vaapi_encode_init_quality(avctx);

> >          if (err < 0)

> > diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h

> > index eeec06036b..b24735da59 100644

> > --- a/libavcodec/vaapi_encode.h

> > +++ b/libavcodec/vaapi_encode.h

> > @@ -37,7 +37,7 @@ struct VAAPIEncodePicture;

> >

> >  enum {

> >      MAX_CONFIG_ATTRIBUTES  = 4,

> > -    MAX_GLOBAL_PARAMS      = 4,

> > +    MAX_GLOBAL_PARAMS      = 5,

> >      MAX_DPB_SIZE           = 16,

> >      MAX_PICTURE_REFERENCES = 2,

> >      MAX_REORDER_DELAY      = 16,

> > @@ -220,6 +220,9 @@ typedef struct VAAPIEncodeContext {

> >      // Packed headers which will actually be sent.

> >      unsigned int    va_packed_headers;

> >

> > +    // Quantization mode

> > +    int trellis;

> > +

> >      // Configuration attributes to use when creating va_config.

> >      VAConfigAttrib  config_attributes[MAX_CONFIG_ATTRIBUTES];

> >      int          nb_config_attributes;

> > @@ -256,7 +259,9 @@ typedef struct VAAPIEncodeContext {

> >  #if VA_CHECK_VERSION(0, 36, 0)

> >      VAEncMiscParameterBufferQualityLevel quality_params;

> >  #endif

> > -

> > +#if VA_CHECK_VERSION(1, 0, 0)

> > +    VAEncMiscParameterQuantization quantization_params;

> > +#endif

> >      // Per-sequence parameter structure

> (VAEncSequenceParameterBuffer*).

> >      void           *codec_sequence_params;

> >

> > diff --git a/libavcodec/vaapi_encode_h264.c

> b/libavcodec/vaapi_encode_h264.c

> > index d1427112ea..427fb6320e 100644

> > --- a/libavcodec/vaapi_encode_h264.c

> > +++ b/libavcodec/vaapi_encode_h264.c

> > @@ -72,6 +72,7 @@ typedef struct VAAPIEncodeH264Context {

> >      int sei;

> >      int profile;

> >      int level;

> > +    int trellis;

> >

> >      // Derived settings.

> >      int mb_width;

> > @@ -1233,6 +1234,8 @@ static av_cold int

> vaapi_encode_h264_init(AVCodecContext *avctx)

> >      if (priv->qp > 0)

> >          ctx->explicit_qp = priv->qp;

> >

> > +    ctx->trellis = priv->trellis;

> > +

> >      return ff_vaapi_encode_init(avctx);

> >  }

> >

> > @@ -1263,6 +1266,12 @@ static const AVOption

> vaapi_encode_h264_options[] = {

> >          { "cabac", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 1 }, INT_MIN,

> INT_MAX, FLAGS, "coder" },

> >          { "vlc",   NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 0 }, INT_MIN,

> INT_MAX, FLAGS, "coder" },

> >          { "ac",    NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 1 }, INT_MIN,

> INT_MAX, FLAGS, "coder" },

> > +    { "trellis", "Trellis Quantization",

> > +      OFFSET(trellis), AV_OPT_TYPE_FLAGS, { .i64 = 0}, 0, INT_MAX, FLAGS,

> "trellis"},

> > +        { "off",   NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 1 }, INT_MIN,

> INT_MAX, FLAGS, "trellis"},

> > +        { "I",     NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 2 }, INT_MIN,

> INT_MAX, FLAGS, "trellis"},

> > +        { "P",     NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 4 }, INT_MIN,

> INT_MAX, FLAGS, "trellis"},

> > +        { "B",     NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 8 }, INT_MIN,

> INT_MAX, FLAGS, "trellis"},

> >

> >      { "aud", "Include AUD",

> >        OFFSET(aud), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, FLAGS },

> > --

> > 2.17.1

> >

> 

> Do you test this function for this feature with the other

> options/features, as I know, this feature have some limitation, I

> believe is a hardware or driver  limitation, and we enable this

> features about 2 years ago, but didn't submit the patch because the

> limitation


Thanks for the remind, I’ve no idea about previous patches on trellis, which is
not mentioned in the forked ffmpeg repo when I take this task.

> Some question:

> 1. How is the impact on encoder performance and image quality?


Enabling trellis will gain better quality in the low QP range.
During my test, enable trellis can increase ~1% quality (PSNR).
Please feel free let me know if you have any concerns to offer this option within spec and increasing the quality.

> 2. What's the limitation with the other VA-API features?

As far as I know, Quality gain is QP dependent, with lower QP value, we could see higher quality gain.
In BRC mode, it might be disabled, but I haven't noticed this during my test. 
We have the daily CI to ensure ffmpeg-vaaapi/qsv mainline quality, and pre-patch check-in service in github to ensure our patches quality. And I already pass the pre-checkin

> 3. Missed the docs

Will add docs.


Thanks,
Linjie
diff mbox

Patch

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index dd2a24de04..fbfbe78c6b 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -1671,6 +1671,48 @@  rc_mode_found:
     return 0;
 }
 
+static av_cold int vaapi_encode_init_quantization(AVCodecContext *avctx)
+{
+#if VA_CHECK_VERSION(1, 0, 0)
+    VAAPIEncodeContext *ctx = avctx->priv_data;
+    VAStatus vas;
+    VAConfigAttrib attr = { VAConfigAttribEncQuantization };
+    int trellis = ctx->trellis;
+
+    vas = vaGetConfigAttributes(ctx->hwctx->display,
+                                ctx->va_profile,
+                                ctx->va_entrypoint,
+                                &attr, 1);
+    if (vas != VA_STATUS_SUCCESS) {
+        av_log(avctx, AV_LOG_ERROR, "Failed to query quantization "
+               "config attribute: %d (%s).\n", vas, vaErrorStr(vas));
+        return AVERROR_EXTERNAL;
+    }
+
+    if (attr.value == VA_ATTRIB_NOT_SUPPORTED ||
+        attr.value == VA_ENC_QUANTIZATION_NONE) {
+        av_log(avctx, AV_LOG_WARNING, "Special Quantization attribute is not "
+                "supported: will use default quantization.\n");
+    } else if (attr.value == VA_ENC_QUANTIZATION_TRELLIS_SUPPORTED){
+        av_log(avctx, AV_LOG_VERBOSE, "Quantization Trellis supported.\n");
+
+        ctx->quantization_params = (VAEncMiscParameterQuantization) {
+            .quantization_flags.value = trellis,
+        };
+
+        vaapi_encode_add_global_param(avctx,
+                                      VAEncMiscParameterTypeQuantization,
+                                      &ctx->quantization_params,
+                                      sizeof(ctx->quantization_params));
+    }
+#else
+    av_log(avctx, AV_LOG_WARNING, "The encode quantization option (Trellis) is "
+           "not supported with this VAAPI version.\n");
+#endif
+
+    return 0;
+}
+
 static av_cold int vaapi_encode_init_gop_structure(AVCodecContext *avctx)
 {
     VAAPIEncodeContext *ctx = avctx->priv_data;
@@ -2132,6 +2174,12 @@  av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
     if (err < 0)
         goto fail;
 
+    if (ctx->trellis) {
+        err = vaapi_encode_init_quantization(avctx);
+        if (err < 0)
+            goto fail;
+    }
+
     if (avctx->compression_level >= 0) {
         err = vaapi_encode_init_quality(avctx);
         if (err < 0)
diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
index eeec06036b..b24735da59 100644
--- a/libavcodec/vaapi_encode.h
+++ b/libavcodec/vaapi_encode.h
@@ -37,7 +37,7 @@  struct VAAPIEncodePicture;
 
 enum {
     MAX_CONFIG_ATTRIBUTES  = 4,
-    MAX_GLOBAL_PARAMS      = 4,
+    MAX_GLOBAL_PARAMS      = 5,
     MAX_DPB_SIZE           = 16,
     MAX_PICTURE_REFERENCES = 2,
     MAX_REORDER_DELAY      = 16,
@@ -220,6 +220,9 @@  typedef struct VAAPIEncodeContext {
     // Packed headers which will actually be sent.
     unsigned int    va_packed_headers;
 
+    // Quantization mode
+    int trellis;
+
     // Configuration attributes to use when creating va_config.
     VAConfigAttrib  config_attributes[MAX_CONFIG_ATTRIBUTES];
     int          nb_config_attributes;
@@ -256,7 +259,9 @@  typedef struct VAAPIEncodeContext {
 #if VA_CHECK_VERSION(0, 36, 0)
     VAEncMiscParameterBufferQualityLevel quality_params;
 #endif
-
+#if VA_CHECK_VERSION(1, 0, 0)
+    VAEncMiscParameterQuantization quantization_params;
+#endif
     // Per-sequence parameter structure (VAEncSequenceParameterBuffer*).
     void           *codec_sequence_params;
 
diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
index d1427112ea..427fb6320e 100644
--- a/libavcodec/vaapi_encode_h264.c
+++ b/libavcodec/vaapi_encode_h264.c
@@ -72,6 +72,7 @@  typedef struct VAAPIEncodeH264Context {
     int sei;
     int profile;
     int level;
+    int trellis;
 
     // Derived settings.
     int mb_width;
@@ -1233,6 +1234,8 @@  static av_cold int vaapi_encode_h264_init(AVCodecContext *avctx)
     if (priv->qp > 0)
         ctx->explicit_qp = priv->qp;
 
+    ctx->trellis = priv->trellis;
+
     return ff_vaapi_encode_init(avctx);
 }
 
@@ -1263,6 +1266,12 @@  static const AVOption vaapi_encode_h264_options[] = {
         { "cabac", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 1 }, INT_MIN, INT_MAX, FLAGS, "coder" },
         { "vlc",   NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 0 }, INT_MIN, INT_MAX, FLAGS, "coder" },
         { "ac",    NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 1 }, INT_MIN, INT_MAX, FLAGS, "coder" },
+    { "trellis", "Trellis Quantization",
+      OFFSET(trellis), AV_OPT_TYPE_FLAGS, { .i64 = 0}, 0, INT_MAX, FLAGS, "trellis"},
+        { "off",   NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 1 }, INT_MIN, INT_MAX, FLAGS, "trellis"},
+        { "I",     NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 2 }, INT_MIN, INT_MAX, FLAGS, "trellis"},
+        { "P",     NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 4 }, INT_MIN, INT_MAX, FLAGS, "trellis"},
+        { "B",     NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 8 }, INT_MIN, INT_MAX, FLAGS, "trellis"},
 
     { "aud", "Include AUD",
       OFFSET(aud), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, FLAGS },