[FFmpeg-devel,v2,18/36] vaapi_encode: Clean up the GOP structure configuration

Submitted by Mark Thompson on June 7, 2018, 11:43 p.m.

Details

Message ID 20180607234331.32139-19-sw@jkqxz.net
State New
Headers show

Commit Message

Mark Thompson June 7, 2018, 11:43 p.m.
Choose what types of reference frames will be used based on what types
are available, and make the intra-only mode explicit (GOP size one,
which must be used for MJPEG).
---
 libavcodec/vaapi_encode.c       | 83 +++++++++++++++++++++++++++--------------
 libavcodec/vaapi_encode.h       |  1 +
 libavcodec/vaapi_encode_h264.c  |  4 +-
 libavcodec/vaapi_encode_h265.c  |  4 +-
 libavcodec/vaapi_encode_mjpeg.c |  1 +
 libavcodec/vaapi_encode_mpeg2.c |  2 +-
 libavcodec/vaapi_encode_vp8.c   |  7 +---
 libavcodec/vaapi_encode_vp9.c   |  7 ++--
 8 files changed, 68 insertions(+), 41 deletions(-)

Comments

Haihao Xiang June 14, 2018, 6:48 a.m.
On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
> Choose what types of reference frames will be used based on what types

> are available, and make the intra-only mode explicit (GOP size one,

> which must be used for MJPEG).

> ---

>  libavcodec/vaapi_encode.c       | 83 +++++++++++++++++++++++++++-------------

> -

>  libavcodec/vaapi_encode.h       |  1 +

>  libavcodec/vaapi_encode_h264.c  |  4 +-

>  libavcodec/vaapi_encode_h265.c  |  4 +-

>  libavcodec/vaapi_encode_mjpeg.c |  1 +

>  libavcodec/vaapi_encode_mpeg2.c |  2 +-

>  libavcodec/vaapi_encode_vp8.c   |  7 +---

>  libavcodec/vaapi_encode_vp9.c   |  7 ++--

>  8 files changed, 68 insertions(+), 41 deletions(-)

> 

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

> index 0e806a29e3..af9224c98f 100644

> --- a/libavcodec/vaapi_encode.c

> +++ b/libavcodec/vaapi_encode.c

> @@ -680,7 +680,7 @@ static int vaapi_encode_get_next(AVCodecContext *avctx,

>          return AVERROR(ENOMEM);

>  

>      if (ctx->input_order == 0 || ctx->force_idr ||

> -        ctx->gop_counter >= avctx->gop_size) {

> +        ctx->gop_counter >= ctx->gop_size) {

>          pic->type = PICTURE_TYPE_IDR;

>          ctx->force_idr = 0;

>          ctx->gop_counter = 1;

> @@ -703,7 +703,7 @@ static int vaapi_encode_get_next(AVCodecContext *avctx,

>          // encode-after it, but not exceeding the GOP size.

>  

>          for (i = 0; i < ctx->b_per_p &&

> -             ctx->gop_counter < avctx->gop_size; i++) {

> +             ctx->gop_counter < ctx->gop_size; i++) {

>              pic = vaapi_encode_alloc(avctx);

>              if (!pic)

>                  goto fail;

> @@ -1217,7 +1217,6 @@ static av_cold int

> vaapi_encode_config_attributes(AVCodecContext *avctx)

>      int i;

>  

>      VAConfigAttrib attr[] = {

> -        { VAConfigAttribEncMaxRefFrames  },

>          { VAConfigAttribEncPackedHeaders },

>      };

>  

> @@ -1240,24 +1239,6 @@ static av_cold int

> vaapi_encode_config_attributes(AVCodecContext *avctx)

>              continue;

>          }

>          switch (attr[i].type) {

> -        case VAConfigAttribEncMaxRefFrames:

> -        {

> -            unsigned int ref_l0 = attr[i].value & 0xffff;

> -            unsigned int ref_l1 = (attr[i].value >> 16) & 0xffff;

> -

> -            if (avctx->gop_size > 1 && ref_l0 < 1) {

> -                av_log(avctx, AV_LOG_ERROR, "P frames are not "

> -                       "supported (%#x).\n", attr[i].value);

> -                return AVERROR(EINVAL);

> -            }

> -            if (avctx->max_b_frames > 0 && ref_l1 < 1) {

> -                av_log(avctx, AV_LOG_WARNING, "B frames are not "

> -                       "supported (%#x) by the underlying driver.\n",

> -                       attr[i].value);

> -                avctx->max_b_frames = 0;

> -            }

> -        }

> -        break;

>          case VAConfigAttribEncPackedHeaders:

>              if (ctx->va_packed_headers & ~attr[i].value) {

>                  // This isn't fatal, but packed headers are always

> @@ -1465,6 +1446,54 @@ static av_cold int

> vaapi_encode_init_rate_control(AVCodecContext *avctx)

>      return 0;

>  }

>  

> +static av_cold int vaapi_encode_init_gop_structure(AVCodecContext *avctx)

> +{

> +    VAAPIEncodeContext *ctx = avctx->priv_data;

> +    VAStatus vas;

> +    VAConfigAttrib attr = { VAConfigAttribEncMaxRefFrames };

> +    uint32_t ref_l0, ref_l1;

> +

> +    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 reference frames "

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

> +        return AVERROR_EXTERNAL;

> +    }

> +

> +    if (attr.value == VA_ATTRIB_NOT_SUPPORTED) {

> +        ref_l0 = ref_l1 = 0;

> +    } else {

> +        ref_l0 = attr.value       & 0xffff;

> +        ref_l1 = attr.value >> 16 & 0xffff;

> +    }

> +

> +    if (avctx->gop_size <= 1) {

> +        av_log(avctx, AV_LOG_VERBOSE, "Using intra frames only.\n");

> +        ctx->gop_size = 1;

> +    } else if (ref_l0 < 1) {

> +        av_log(avctx, AV_LOG_ERROR, "Driver does not support any "

> +               "reference frames.\n");

> +        return AVERROR(EINVAL);

> +    } else if (ref_l1 < 1 || avctx->max_b_frames < 1) {

> +        av_log(avctx, AV_LOG_VERBOSE, "Using intra and P-frames "

> +               "(supported references: %d / %d).\n", ref_l0, ref_l1);

> +        ctx->gop_size = avctx->gop_size;

> +        ctx->p_per_i  = INT_MAX;


How about removing p_per_i? I see p_per_i is used only in the following 'else
if' statement and I don't think ctx->p_counter can be INT_MAX in reality. 

    } else if (ctx->p_counter >= ctx->p_per_i) {...}

> +        ctx->b_per_p  = 0;

> +    } else {

> +        av_log(avctx, AV_LOG_VERBOSE, "Using intra, P- and B-frames "

> +               "(supported references: %d / %d).\n", ref_l0, ref_l1);

> +        ctx->gop_size = avctx->gop_size;

> +        ctx->p_per_i  = INT_MAX;

> +        ctx->b_per_p  = avctx->max_b_frames;

> +    }

> +

> +    return 0;

> +}

> +

>  static av_cold int vaapi_encode_init_quality(AVCodecContext *avctx)

>  {

>  #if VA_CHECK_VERSION(0, 36, 0)

> @@ -1636,7 +1665,7 @@ static av_cold int

> vaapi_encode_create_recon_frames(AVCodecContext *avctx)

>      ctx->recon_frames->height    = ctx->surface_height;

>      // At most three IDR/I/P frames and two runs of B frames can be in

>      // flight at any one time.

> -    ctx->recon_frames->initial_pool_size = 3 + 2 * avctx->max_b_frames;

> +    ctx->recon_frames->initial_pool_size = 3 + 2 * ctx->b_per_p;

>  

>      err = av_hwframe_ctx_init(ctx->recon_frames_ref);

>      if (err < 0) {

> @@ -1691,6 +1720,10 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)

>      if (err < 0)

>          goto fail;

>  

> +    err = vaapi_encode_init_gop_structure(avctx);

> +    if (err < 0)

> +        goto fail;

> +

>      err = vaapi_encode_config_attributes(avctx);

>      if (err < 0)

>          goto fail;

> @@ -1745,14 +1778,10 @@ av_cold int ff_vaapi_encode_init(AVCodecContext

> *avctx)

>      }

>  

>      ctx->input_order  = 0;

> -    ctx->output_delay = avctx->max_b_frames;

> +    ctx->output_delay = ctx->b_per_p;

>      ctx->decode_delay = 1;

>      ctx->output_order = - ctx->output_delay - 1;

>  

> -    // Currently we never generate I frames, only IDR.

> -    ctx->p_per_i = INT_MAX;

> -    ctx->b_per_p = avctx->max_b_frames;

> -

>      if (ctx->codec->sequence_params_size > 0) {

>          ctx->codec_sequence_params =

>              av_mallocz(ctx->codec->sequence_params_size);

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

> index 6fac4781c3..bbec721bca 100644

> --- a/libavcodec/vaapi_encode.h

> +++ b/libavcodec/vaapi_encode.h

> @@ -224,6 +224,7 @@ typedef struct VAAPIEncodeContext {

>      int64_t         ts_ring[MAX_REORDER_DELAY * 3];

>  

>      // Frame type decision.

> +    int gop_size;

>      int p_per_i;

>      int b_per_p;

>      int force_idr;

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

> index 87c0d9acf3..717be024ca 100644

> --- a/libavcodec/vaapi_encode_h264.c

> +++ b/libavcodec/vaapi_encode_h264.c

> @@ -493,8 +493,8 @@ static int

> vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)

>      *vseq = (VAEncSequenceParameterBufferH264) {

>          .seq_parameter_set_id = sps->seq_parameter_set_id,

>          .level_idc        = sps->level_idc,

> -        .intra_period     = avctx->gop_size,

> -        .intra_idr_period = avctx->gop_size,

> +        .intra_period     = ctx->gop_size,

> +        .intra_idr_period = ctx->gop_size,

>          .ip_period        = ctx->b_per_p + 1,

>  

>          .bits_per_second       = ctx->va_bit_rate,

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

> index 13ddad79ae..b2d6b8d24d 100644

> --- a/libavcodec/vaapi_encode_h265.c

> +++ b/libavcodec/vaapi_encode_h265.c

> @@ -509,8 +509,8 @@ static int

> vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)

>          .general_level_idc   = vps->profile_tier_level.general_level_idc,

>          .general_tier_flag   = vps->profile_tier_level.general_tier_flag,

>  

> -        .intra_period     = avctx->gop_size,

> -        .intra_idr_period = avctx->gop_size,

> +        .intra_period     = ctx->gop_size,

> +        .intra_idr_period = ctx->gop_size,

>          .ip_period        = ctx->b_per_p + 1,

>          .bits_per_second  = ctx->va_bit_rate,

>  

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

> index 67ac2fba96..4982cd166f 100644

> --- a/libavcodec/vaapi_encode_mjpeg.c

> +++ b/libavcodec/vaapi_encode_mjpeg.c

> @@ -401,6 +401,7 @@ static av_cold int vaapi_encode_mjpeg_init(AVCodecContext

> *avctx)

>  static const AVCodecDefault vaapi_encode_mjpeg_defaults[] = {

>      { "global_quality", "80" },

>      { "b",              "0"  },

> +    { "g",              "1"  },

>      { NULL },

>  };

>  

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

> index db72516187..b6edca9158 100644

> --- a/libavcodec/vaapi_encode_mpeg2.c

> +++ b/libavcodec/vaapi_encode_mpeg2.c

> @@ -355,7 +355,7 @@ static int

> vaapi_encode_mpeg2_init_sequence_params(AVCodecContext *avctx)

>  

>  

>      *vseq = (VAEncSequenceParameterBufferMPEG2) {

> -        .intra_period = avctx->gop_size,

> +        .intra_period = ctx->gop_size,

>          .ip_period    = ctx->b_per_p + 1,

>  

>          .picture_width  = avctx->width,

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

> index db67136556..4512609a19 100644

> --- a/libavcodec/vaapi_encode_vp8.c

> +++ b/libavcodec/vaapi_encode_vp8.c

> @@ -66,7 +66,7 @@ static int

> vaapi_encode_vp8_init_sequence_params(AVCodecContext *avctx)

>  

>      if (!(ctx->va_rc_mode & VA_RC_CQP)) {

>          vseq->bits_per_second = ctx->va_bit_rate;

> -        vseq->intra_period    = avctx->gop_size;

> +        vseq->intra_period    = ctx->gop_size;

>      }

>  

>      return 0;

> @@ -198,11 +198,6 @@ static av_cold int vaapi_encode_vp8_init(AVCodecContext

> *avctx)

>  {

>      VAAPIEncodeContext *ctx = avctx->priv_data;

>  

> -    if (avctx->max_b_frames > 0) {

> -        av_log(avctx, AV_LOG_ERROR, "B-frames are not supported.\n");

> -        return AVERROR_PATCHWELCOME;

> -    }

> -

>      ctx->codec = &vaapi_encode_type_vp8;

>  

>      // Packed headers are not currently supported.

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

> index 2b0658ec1f..d4069ec850 100644

> --- a/libavcodec/vaapi_encode_vp9.c

> +++ b/libavcodec/vaapi_encode_vp9.c

> @@ -72,7 +72,7 @@ static int

> vaapi_encode_vp9_init_sequence_params(AVCodecContext *avctx)

>  

>      if (!(ctx->va_rc_mode & VA_RC_CQP)) {

>          vseq->bits_per_second = ctx->va_bit_rate;

> -        vseq->intra_period    = avctx->gop_size;

> +        vseq->intra_period    = ctx->gop_size;

>      }

>  

>      vpic->frame_width_src  = avctx->width;

> @@ -86,6 +86,7 @@ static int

> vaapi_encode_vp9_init_sequence_params(AVCodecContext *avctx)

>  static int vaapi_encode_vp9_init_picture_params(AVCodecContext *avctx,

>                                                  VAAPIEncodePicture *pic)

>  {

> +    VAAPIEncodeContext              *ctx = avctx->priv_data;

>      VAAPIEncodeVP9Context          *priv = avctx->priv_data;

>      VAEncPictureParameterBufferVP9 *vpic = pic->codec_picture_params;

>      int i;

> @@ -102,7 +103,7 @@ static int

> vaapi_encode_vp9_init_picture_params(AVCodecContext *avctx,

>          break;

>      case PICTURE_TYPE_P:

>          av_assert0(pic->nb_refs == 1);

> -        if (avctx->max_b_frames > 0) {

> +        if (ctx->b_per_p > 0) {

>              if (priv->last_ref_dir) {

>                  vpic->ref_flags.bits.ref_frame_ctrl_l0  = 2;

>                  vpic->ref_flags.bits.ref_gf_idx         = 1;

> @@ -174,7 +175,7 @@ static int

> vaapi_encode_vp9_init_picture_params(AVCodecContext *avctx,

>      vpic->filter_level    = priv->loop_filter_level;

>      vpic->sharpness_level = priv->loop_filter_sharpness;

>  

> -    if (avctx->max_b_frames > 0 && pic->type == PICTURE_TYPE_P)

> +    if (ctx->b_per_p > 0 && pic->type == PICTURE_TYPE_P)

>          priv->last_ref_dir = !priv->last_ref_dir;

>  

>      return 0;
Mark Thompson June 17, 2018, 1:21 p.m.
On 14/06/18 07:48, Xiang, Haihao wrote:
> On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
>> Choose what types of reference frames will be used based on what types
>> are available, and make the intra-only mode explicit (GOP size one,
>> which must be used for MJPEG).
>> ---
>>  libavcodec/vaapi_encode.c       | 83 +++++++++++++++++++++++++++-------------
>> -
>>  libavcodec/vaapi_encode.h       |  1 +
>>  libavcodec/vaapi_encode_h264.c  |  4 +-
>>  libavcodec/vaapi_encode_h265.c  |  4 +-
>>  libavcodec/vaapi_encode_mjpeg.c |  1 +
>>  libavcodec/vaapi_encode_mpeg2.c |  2 +-
>>  libavcodec/vaapi_encode_vp8.c   |  7 +---
>>  libavcodec/vaapi_encode_vp9.c   |  7 ++--
>>  8 files changed, 68 insertions(+), 41 deletions(-)
>>
>> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
>> index 0e806a29e3..af9224c98f 100644
>> --- a/libavcodec/vaapi_encode.c
>> +++ b/libavcodec/vaapi_encode.c
>> @@ -680,7 +680,7 @@ static int vaapi_encode_get_next(AVCodecContext *avctx,
>>          return AVERROR(ENOMEM);
>>  
>>      if (ctx->input_order == 0 || ctx->force_idr ||
>> -        ctx->gop_counter >= avctx->gop_size) {
>> +        ctx->gop_counter >= ctx->gop_size) {
>>          pic->type = PICTURE_TYPE_IDR;
>>          ctx->force_idr = 0;
>>          ctx->gop_counter = 1;
>> @@ -703,7 +703,7 @@ static int vaapi_encode_get_next(AVCodecContext *avctx,
>>          // encode-after it, but not exceeding the GOP size.
>>  
>>          for (i = 0; i < ctx->b_per_p &&
>> -             ctx->gop_counter < avctx->gop_size; i++) {
>> +             ctx->gop_counter < ctx->gop_size; i++) {
>>              pic = vaapi_encode_alloc(avctx);
>>              if (!pic)
>>                  goto fail;
>> @@ -1217,7 +1217,6 @@ static av_cold int
>> vaapi_encode_config_attributes(AVCodecContext *avctx)
>>      int i;
>>  
>>      VAConfigAttrib attr[] = {
>> -        { VAConfigAttribEncMaxRefFrames  },
>>          { VAConfigAttribEncPackedHeaders },
>>      };
>>  
>> @@ -1240,24 +1239,6 @@ static av_cold int
>> vaapi_encode_config_attributes(AVCodecContext *avctx)
>>              continue;
>>          }
>>          switch (attr[i].type) {
>> -        case VAConfigAttribEncMaxRefFrames:
>> -        {
>> -            unsigned int ref_l0 = attr[i].value & 0xffff;
>> -            unsigned int ref_l1 = (attr[i].value >> 16) & 0xffff;
>> -
>> -            if (avctx->gop_size > 1 && ref_l0 < 1) {
>> -                av_log(avctx, AV_LOG_ERROR, "P frames are not "
>> -                       "supported (%#x).\n", attr[i].value);
>> -                return AVERROR(EINVAL);
>> -            }
>> -            if (avctx->max_b_frames > 0 && ref_l1 < 1) {
>> -                av_log(avctx, AV_LOG_WARNING, "B frames are not "
>> -                       "supported (%#x) by the underlying driver.\n",
>> -                       attr[i].value);
>> -                avctx->max_b_frames = 0;
>> -            }
>> -        }
>> -        break;
>>          case VAConfigAttribEncPackedHeaders:
>>              if (ctx->va_packed_headers & ~attr[i].value) {
>>                  // This isn't fatal, but packed headers are always
>> @@ -1465,6 +1446,54 @@ static av_cold int
>> vaapi_encode_init_rate_control(AVCodecContext *avctx)
>>      return 0;
>>  }
>>  
>> +static av_cold int vaapi_encode_init_gop_structure(AVCodecContext *avctx)
>> +{
>> +    VAAPIEncodeContext *ctx = avctx->priv_data;
>> +    VAStatus vas;
>> +    VAConfigAttrib attr = { VAConfigAttribEncMaxRefFrames };
>> +    uint32_t ref_l0, ref_l1;
>> +
>> +    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 reference frames "
>> +               "attribute: %d (%s).\n", vas, vaErrorStr(vas));
>> +        return AVERROR_EXTERNAL;
>> +    }
>> +
>> +    if (attr.value == VA_ATTRIB_NOT_SUPPORTED) {
>> +        ref_l0 = ref_l1 = 0;
>> +    } else {
>> +        ref_l0 = attr.value       & 0xffff;
>> +        ref_l1 = attr.value >> 16 & 0xffff;
>> +    }
>> +
>> +    if (avctx->gop_size <= 1) {
>> +        av_log(avctx, AV_LOG_VERBOSE, "Using intra frames only.\n");
>> +        ctx->gop_size = 1;
>> +    } else if (ref_l0 < 1) {
>> +        av_log(avctx, AV_LOG_ERROR, "Driver does not support any "
>> +               "reference frames.\n");
>> +        return AVERROR(EINVAL);
>> +    } else if (ref_l1 < 1 || avctx->max_b_frames < 1) {
>> +        av_log(avctx, AV_LOG_VERBOSE, "Using intra and P-frames "
>> +               "(supported references: %d / %d).\n", ref_l0, ref_l1);
>> +        ctx->gop_size = avctx->gop_size;
>> +        ctx->p_per_i  = INT_MAX;
> 
> How about removing p_per_i? I see p_per_i is used only in the following 'else
> if' statement and I don't think ctx->p_counter can be INT_MAX in reality. 
> 
>     } else if (ctx->p_counter >= ctx->p_per_i) {...}

It's not currently user-visible, but changing these values does work to test the open-GOP handling in the codec-specific code.  (E.g. in H.264, I frames as recovery points rather than IDR frames.)

I do intend to expose this at some point.

>> +        ctx->b_per_p  = 0;
>> +    } else {
>> +        av_log(avctx, AV_LOG_VERBOSE, "Using intra, P- and B-frames "
>> +               "(supported references: %d / %d).\n", ref_l0, ref_l1);
>> +        ctx->gop_size = avctx->gop_size;
>> +        ctx->p_per_i  = INT_MAX;
>> +        ctx->b_per_p  = avctx->max_b_frames;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>  static av_cold int vaapi_encode_init_quality(AVCodecContext *avctx)
>>  {
>>  #if VA_CHECK_VERSION(0, 36, 0)
>> @@ -1636,7 +1665,7 @@ static av_cold int
>> vaapi_encode_create_recon_frames(AVCodecContext *avctx)
>>      ctx->recon_frames->height    = ctx->surface_height;
>>      // At most three IDR/I/P frames and two runs of B frames can be in
>>      // flight at any one time.
>> -    ctx->recon_frames->initial_pool_size = 3 + 2 * avctx->max_b_frames;
>> +    ctx->recon_frames->initial_pool_size = 3 + 2 * ctx->b_per_p;
>>  
>>      err = av_hwframe_ctx_init(ctx->recon_frames_ref);
>>      if (err < 0) {
>> @@ -1691,6 +1720,10 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
>>      if (err < 0)
>>          goto fail;
>>  
>> +    err = vaapi_encode_init_gop_structure(avctx);
>> +    if (err < 0)
>> +        goto fail;
>> +
>>      err = vaapi_encode_config_attributes(avctx);
>>      if (err < 0)
>>          goto fail;
>> @@ -1745,14 +1778,10 @@ av_cold int ff_vaapi_encode_init(AVCodecContext
>> *avctx)
>>      }
>>  
>>      ctx->input_order  = 0;
>> -    ctx->output_delay = avctx->max_b_frames;
>> +    ctx->output_delay = ctx->b_per_p;
>>      ctx->decode_delay = 1;
>>      ctx->output_order = - ctx->output_delay - 1;
>>  
>> -    // Currently we never generate I frames, only IDR.
>> -    ctx->p_per_i = INT_MAX;
>> -    ctx->b_per_p = avctx->max_b_frames;
>> -
>>      if (ctx->codec->sequence_params_size > 0) {
>>          ctx->codec_sequence_params =
>>              av_mallocz(ctx->codec->sequence_params_size);

Thanks,

- Mark

Patch hide | download patch | download mbox

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 0e806a29e3..af9224c98f 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -680,7 +680,7 @@  static int vaapi_encode_get_next(AVCodecContext *avctx,
         return AVERROR(ENOMEM);
 
     if (ctx->input_order == 0 || ctx->force_idr ||
-        ctx->gop_counter >= avctx->gop_size) {
+        ctx->gop_counter >= ctx->gop_size) {
         pic->type = PICTURE_TYPE_IDR;
         ctx->force_idr = 0;
         ctx->gop_counter = 1;
@@ -703,7 +703,7 @@  static int vaapi_encode_get_next(AVCodecContext *avctx,
         // encode-after it, but not exceeding the GOP size.
 
         for (i = 0; i < ctx->b_per_p &&
-             ctx->gop_counter < avctx->gop_size; i++) {
+             ctx->gop_counter < ctx->gop_size; i++) {
             pic = vaapi_encode_alloc(avctx);
             if (!pic)
                 goto fail;
@@ -1217,7 +1217,6 @@  static av_cold int vaapi_encode_config_attributes(AVCodecContext *avctx)
     int i;
 
     VAConfigAttrib attr[] = {
-        { VAConfigAttribEncMaxRefFrames  },
         { VAConfigAttribEncPackedHeaders },
     };
 
@@ -1240,24 +1239,6 @@  static av_cold int vaapi_encode_config_attributes(AVCodecContext *avctx)
             continue;
         }
         switch (attr[i].type) {
-        case VAConfigAttribEncMaxRefFrames:
-        {
-            unsigned int ref_l0 = attr[i].value & 0xffff;
-            unsigned int ref_l1 = (attr[i].value >> 16) & 0xffff;
-
-            if (avctx->gop_size > 1 && ref_l0 < 1) {
-                av_log(avctx, AV_LOG_ERROR, "P frames are not "
-                       "supported (%#x).\n", attr[i].value);
-                return AVERROR(EINVAL);
-            }
-            if (avctx->max_b_frames > 0 && ref_l1 < 1) {
-                av_log(avctx, AV_LOG_WARNING, "B frames are not "
-                       "supported (%#x) by the underlying driver.\n",
-                       attr[i].value);
-                avctx->max_b_frames = 0;
-            }
-        }
-        break;
         case VAConfigAttribEncPackedHeaders:
             if (ctx->va_packed_headers & ~attr[i].value) {
                 // This isn't fatal, but packed headers are always
@@ -1465,6 +1446,54 @@  static av_cold int vaapi_encode_init_rate_control(AVCodecContext *avctx)
     return 0;
 }
 
+static av_cold int vaapi_encode_init_gop_structure(AVCodecContext *avctx)
+{
+    VAAPIEncodeContext *ctx = avctx->priv_data;
+    VAStatus vas;
+    VAConfigAttrib attr = { VAConfigAttribEncMaxRefFrames };
+    uint32_t ref_l0, ref_l1;
+
+    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 reference frames "
+               "attribute: %d (%s).\n", vas, vaErrorStr(vas));
+        return AVERROR_EXTERNAL;
+    }
+
+    if (attr.value == VA_ATTRIB_NOT_SUPPORTED) {
+        ref_l0 = ref_l1 = 0;
+    } else {
+        ref_l0 = attr.value       & 0xffff;
+        ref_l1 = attr.value >> 16 & 0xffff;
+    }
+
+    if (avctx->gop_size <= 1) {
+        av_log(avctx, AV_LOG_VERBOSE, "Using intra frames only.\n");
+        ctx->gop_size = 1;
+    } else if (ref_l0 < 1) {
+        av_log(avctx, AV_LOG_ERROR, "Driver does not support any "
+               "reference frames.\n");
+        return AVERROR(EINVAL);
+    } else if (ref_l1 < 1 || avctx->max_b_frames < 1) {
+        av_log(avctx, AV_LOG_VERBOSE, "Using intra and P-frames "
+               "(supported references: %d / %d).\n", ref_l0, ref_l1);
+        ctx->gop_size = avctx->gop_size;
+        ctx->p_per_i  = INT_MAX;
+        ctx->b_per_p  = 0;
+    } else {
+        av_log(avctx, AV_LOG_VERBOSE, "Using intra, P- and B-frames "
+               "(supported references: %d / %d).\n", ref_l0, ref_l1);
+        ctx->gop_size = avctx->gop_size;
+        ctx->p_per_i  = INT_MAX;
+        ctx->b_per_p  = avctx->max_b_frames;
+    }
+
+    return 0;
+}
+
 static av_cold int vaapi_encode_init_quality(AVCodecContext *avctx)
 {
 #if VA_CHECK_VERSION(0, 36, 0)
@@ -1636,7 +1665,7 @@  static av_cold int vaapi_encode_create_recon_frames(AVCodecContext *avctx)
     ctx->recon_frames->height    = ctx->surface_height;
     // At most three IDR/I/P frames and two runs of B frames can be in
     // flight at any one time.
-    ctx->recon_frames->initial_pool_size = 3 + 2 * avctx->max_b_frames;
+    ctx->recon_frames->initial_pool_size = 3 + 2 * ctx->b_per_p;
 
     err = av_hwframe_ctx_init(ctx->recon_frames_ref);
     if (err < 0) {
@@ -1691,6 +1720,10 @@  av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
     if (err < 0)
         goto fail;
 
+    err = vaapi_encode_init_gop_structure(avctx);
+    if (err < 0)
+        goto fail;
+
     err = vaapi_encode_config_attributes(avctx);
     if (err < 0)
         goto fail;
@@ -1745,14 +1778,10 @@  av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
     }
 
     ctx->input_order  = 0;
-    ctx->output_delay = avctx->max_b_frames;
+    ctx->output_delay = ctx->b_per_p;
     ctx->decode_delay = 1;
     ctx->output_order = - ctx->output_delay - 1;
 
-    // Currently we never generate I frames, only IDR.
-    ctx->p_per_i = INT_MAX;
-    ctx->b_per_p = avctx->max_b_frames;
-
     if (ctx->codec->sequence_params_size > 0) {
         ctx->codec_sequence_params =
             av_mallocz(ctx->codec->sequence_params_size);
diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
index 6fac4781c3..bbec721bca 100644
--- a/libavcodec/vaapi_encode.h
+++ b/libavcodec/vaapi_encode.h
@@ -224,6 +224,7 @@  typedef struct VAAPIEncodeContext {
     int64_t         ts_ring[MAX_REORDER_DELAY * 3];
 
     // Frame type decision.
+    int gop_size;
     int p_per_i;
     int b_per_p;
     int force_idr;
diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
index 87c0d9acf3..717be024ca 100644
--- a/libavcodec/vaapi_encode_h264.c
+++ b/libavcodec/vaapi_encode_h264.c
@@ -493,8 +493,8 @@  static int vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
     *vseq = (VAEncSequenceParameterBufferH264) {
         .seq_parameter_set_id = sps->seq_parameter_set_id,
         .level_idc        = sps->level_idc,
-        .intra_period     = avctx->gop_size,
-        .intra_idr_period = avctx->gop_size,
+        .intra_period     = ctx->gop_size,
+        .intra_idr_period = ctx->gop_size,
         .ip_period        = ctx->b_per_p + 1,
 
         .bits_per_second       = ctx->va_bit_rate,
diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
index 13ddad79ae..b2d6b8d24d 100644
--- a/libavcodec/vaapi_encode_h265.c
+++ b/libavcodec/vaapi_encode_h265.c
@@ -509,8 +509,8 @@  static int vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
         .general_level_idc   = vps->profile_tier_level.general_level_idc,
         .general_tier_flag   = vps->profile_tier_level.general_tier_flag,
 
-        .intra_period     = avctx->gop_size,
-        .intra_idr_period = avctx->gop_size,
+        .intra_period     = ctx->gop_size,
+        .intra_idr_period = ctx->gop_size,
         .ip_period        = ctx->b_per_p + 1,
         .bits_per_second  = ctx->va_bit_rate,
 
diff --git a/libavcodec/vaapi_encode_mjpeg.c b/libavcodec/vaapi_encode_mjpeg.c
index 67ac2fba96..4982cd166f 100644
--- a/libavcodec/vaapi_encode_mjpeg.c
+++ b/libavcodec/vaapi_encode_mjpeg.c
@@ -401,6 +401,7 @@  static av_cold int vaapi_encode_mjpeg_init(AVCodecContext *avctx)
 static const AVCodecDefault vaapi_encode_mjpeg_defaults[] = {
     { "global_quality", "80" },
     { "b",              "0"  },
+    { "g",              "1"  },
     { NULL },
 };
 
diff --git a/libavcodec/vaapi_encode_mpeg2.c b/libavcodec/vaapi_encode_mpeg2.c
index db72516187..b6edca9158 100644
--- a/libavcodec/vaapi_encode_mpeg2.c
+++ b/libavcodec/vaapi_encode_mpeg2.c
@@ -355,7 +355,7 @@  static int vaapi_encode_mpeg2_init_sequence_params(AVCodecContext *avctx)
 
 
     *vseq = (VAEncSequenceParameterBufferMPEG2) {
-        .intra_period = avctx->gop_size,
+        .intra_period = ctx->gop_size,
         .ip_period    = ctx->b_per_p + 1,
 
         .picture_width  = avctx->width,
diff --git a/libavcodec/vaapi_encode_vp8.c b/libavcodec/vaapi_encode_vp8.c
index db67136556..4512609a19 100644
--- a/libavcodec/vaapi_encode_vp8.c
+++ b/libavcodec/vaapi_encode_vp8.c
@@ -66,7 +66,7 @@  static int vaapi_encode_vp8_init_sequence_params(AVCodecContext *avctx)
 
     if (!(ctx->va_rc_mode & VA_RC_CQP)) {
         vseq->bits_per_second = ctx->va_bit_rate;
-        vseq->intra_period    = avctx->gop_size;
+        vseq->intra_period    = ctx->gop_size;
     }
 
     return 0;
@@ -198,11 +198,6 @@  static av_cold int vaapi_encode_vp8_init(AVCodecContext *avctx)
 {
     VAAPIEncodeContext *ctx = avctx->priv_data;
 
-    if (avctx->max_b_frames > 0) {
-        av_log(avctx, AV_LOG_ERROR, "B-frames are not supported.\n");
-        return AVERROR_PATCHWELCOME;
-    }
-
     ctx->codec = &vaapi_encode_type_vp8;
 
     // Packed headers are not currently supported.
diff --git a/libavcodec/vaapi_encode_vp9.c b/libavcodec/vaapi_encode_vp9.c
index 2b0658ec1f..d4069ec850 100644
--- a/libavcodec/vaapi_encode_vp9.c
+++ b/libavcodec/vaapi_encode_vp9.c
@@ -72,7 +72,7 @@  static int vaapi_encode_vp9_init_sequence_params(AVCodecContext *avctx)
 
     if (!(ctx->va_rc_mode & VA_RC_CQP)) {
         vseq->bits_per_second = ctx->va_bit_rate;
-        vseq->intra_period    = avctx->gop_size;
+        vseq->intra_period    = ctx->gop_size;
     }
 
     vpic->frame_width_src  = avctx->width;
@@ -86,6 +86,7 @@  static int vaapi_encode_vp9_init_sequence_params(AVCodecContext *avctx)
 static int vaapi_encode_vp9_init_picture_params(AVCodecContext *avctx,
                                                 VAAPIEncodePicture *pic)
 {
+    VAAPIEncodeContext              *ctx = avctx->priv_data;
     VAAPIEncodeVP9Context          *priv = avctx->priv_data;
     VAEncPictureParameterBufferVP9 *vpic = pic->codec_picture_params;
     int i;
@@ -102,7 +103,7 @@  static int vaapi_encode_vp9_init_picture_params(AVCodecContext *avctx,
         break;
     case PICTURE_TYPE_P:
         av_assert0(pic->nb_refs == 1);
-        if (avctx->max_b_frames > 0) {
+        if (ctx->b_per_p > 0) {
             if (priv->last_ref_dir) {
                 vpic->ref_flags.bits.ref_frame_ctrl_l0  = 2;
                 vpic->ref_flags.bits.ref_gf_idx         = 1;
@@ -174,7 +175,7 @@  static int vaapi_encode_vp9_init_picture_params(AVCodecContext *avctx,
     vpic->filter_level    = priv->loop_filter_level;
     vpic->sharpness_level = priv->loop_filter_sharpness;
 
-    if (avctx->max_b_frames > 0 && pic->type == PICTURE_TYPE_P)
+    if (ctx->b_per_p > 0 && pic->type == PICTURE_TYPE_P)
         priv->last_ref_dir = !priv->last_ref_dir;
 
     return 0;