[FFmpeg-devel] [PATCH v2 19/36] vaapi_encode: Clean up the packed header configuration
Xiang, Haihao
haihao.xiang at intel.com
Thu Jun 14 10:15:12 EEST 2018
On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
> Add a larger warning more clearly explaining the consequences of missing
> packed header support in the driver. Also only write the extradata if the
> user actually requests it via the GLOBAL_HEADER flag.
> ---
> libavcodec/vaapi_encode.c | 118 +++++++++++++++++++++----------------
> ---
> libavcodec/vaapi_encode.h | 7 ++-
> libavcodec/vaapi_encode_h264.c | 2 +-
> libavcodec/vaapi_encode_h265.c | 2 +-
> libavcodec/vaapi_encode_mjpeg.c | 2 +-
> libavcodec/vaapi_encode_mpeg2.c | 4 +-
> libavcodec/vaapi_encode_vp8.c | 6 +-
> libavcodec/vaapi_encode_vp9.c | 6 +-
> 8 files changed, 79 insertions(+), 68 deletions(-)
>
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> index af9224c98f..14d1846ea3 100644
> --- a/libavcodec/vaapi_encode.c
> +++ b/libavcodec/vaapi_encode.c
> @@ -1210,60 +1210,6 @@ fail:
> return err;
> }
>
> -static av_cold int vaapi_encode_config_attributes(AVCodecContext *avctx)
> -{
> - VAAPIEncodeContext *ctx = avctx->priv_data;
> - VAStatus vas;
> - int i;
> -
> - VAConfigAttrib attr[] = {
> - { VAConfigAttribEncPackedHeaders },
> - };
> -
> - vas = vaGetConfigAttributes(ctx->hwctx->display,
> - ctx->va_profile, ctx->va_entrypoint,
> - attr, FF_ARRAY_ELEMS(attr));
> - if (vas != VA_STATUS_SUCCESS) {
> - av_log(avctx, AV_LOG_ERROR, "Failed to fetch config "
> - "attributes: %d (%s).\n", vas, vaErrorStr(vas));
> - return AVERROR(EINVAL);
> - }
> -
> - for (i = 0; i < FF_ARRAY_ELEMS(attr); i++) {
> - if (attr[i].value == VA_ATTRIB_NOT_SUPPORTED) {
> - // Unfortunately we have to treat this as "don't know" and hope
> - // for the best, because the Intel MJPEG encoder returns this
> - // for all the interesting attributes.
> - av_log(avctx, AV_LOG_DEBUG, "Attribute (%d) is not supported.\n",
> - attr[i].type);
> - continue;
> - }
> - switch (attr[i].type) {
> - case VAConfigAttribEncPackedHeaders:
> - if (ctx->va_packed_headers & ~attr[i].value) {
> - // This isn't fatal, but packed headers are always
> - // preferable because they are under our control.
> - // When absent, the driver is generating them and some
> - // features may not work (e.g. VUI or SEI in H.264).
> - av_log(avctx, AV_LOG_WARNING, "Warning: some packed "
> - "headers are not supported (want %#x, got %#x).\n",
> - ctx->va_packed_headers, attr[i].value);
> - ctx->va_packed_headers &= attr[i].value;
> - }
> - ctx->config_attributes[ctx->nb_config_attributes++] =
> - (VAConfigAttrib) {
> - .type = VAConfigAttribEncPackedHeaders,
> - .value = ctx->va_packed_headers,
> - };
> - break;
> - default:
> - av_assert0(0 && "Unexpected config attribute.");
> - }
> - }
> -
> - return 0;
> -}
> -
> static av_cold int vaapi_encode_init_rate_control(AVCodecContext *avctx)
> {
> VAAPIEncodeContext *ctx = avctx->priv_data;
> @@ -1494,6 +1440,65 @@ static av_cold int
> vaapi_encode_init_gop_structure(AVCodecContext *avctx)
> return 0;
> }
>
> +static av_cold int vaapi_encode_init_packed_headers(AVCodecContext *avctx)
> +{
> + VAAPIEncodeContext *ctx = avctx->priv_data;
> + VAStatus vas;
> + VAConfigAttrib attr = { VAConfigAttribEncPackedHeaders };
> +
> + 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 packed headers "
> + "attribute: %d (%s).\n", vas, vaErrorStr(vas));
> + return AVERROR_EXTERNAL;
> + }
> +
> + if (attr.value == VA_ATTRIB_NOT_SUPPORTED) {
> + if (ctx->packed_headers) {
> + av_log(avctx, AV_LOG_WARNING, "Driver does not support any "
> + "packed headers (wanted %#x).\n", ctx->packed_headers);
> + } else {
> + av_log(avctx, AV_LOG_VERBOSE, "Driver does not support any "
> + "packed headers (none wanted).\n");
> + }
> + ctx->va_packed_headers = 0;
> + } else {
> + if (ctx->packed_headers & ~attr.value) {
> + av_log(avctx, AV_LOG_WARNING, "Driver does not support some "
> + "wanted packed headers (wanted %#x, found %#x).\n",
> + ctx->packed_headers, attr.value);
> + } else {
> + av_log(avctx, AV_LOG_VERBOSE, "All wanted packed headers "
> + "available (wanted %#x, found %#x).\n",
> + ctx->packed_headers, attr.value);
> + }
> + ctx->va_packed_headers = ctx->packed_headers & attr.value;
> + }
> +
> + if (ctx->va_packed_headers) {
> + ctx->config_attributes[ctx->nb_config_attributes++] =
> + (VAConfigAttrib) {
> + .type = VAConfigAttribEncPackedHeaders,
> + .value = ctx->va_packed_headers,
> + };
> + }
> +
> + if ( (ctx->packed_headers & VA_ENC_PACKED_HEADER_SEQUENCE) &&
> + !(ctx->va_packed_headers & VA_ENC_PACKED_HEADER_SEQUENCE) &&
> + (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER)) {
> + av_log(avctx, AV_LOG_WARNING, "Driver does not support packed "
> + "sequence headers, but a global header is requested.\n");
> + av_log(avctx, AV_LOG_WARNING, "No global header will be written: "
> + "this may result in a stream which is not usable for some "
> + "purposes (e.g. not muxable to some containers).\n");
> + }
> +
> + return 0;
> +}
> +
> static av_cold int vaapi_encode_init_quality(AVCodecContext *avctx)
> {
> #if VA_CHECK_VERSION(0, 36, 0)
> @@ -1724,7 +1729,7 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
> if (err < 0)
> goto fail;
>
> - err = vaapi_encode_config_attributes(avctx);
> + err = vaapi_encode_init_packed_headers(avctx);
> if (err < 0)
> goto fail;
>
> @@ -1813,7 +1818,8 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
> ctx->issue_mode = ISSUE_MODE_MAXIMISE_THROUGHPUT;
>
> if (ctx->va_packed_headers & VA_ENC_PACKED_HEADER_SEQUENCE &&
> - ctx->codec->write_sequence_header) {
> + ctx->codec->write_sequence_header &&
> + avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
> char data[MAX_PARAM_BUFFER_SIZE];
> size_t bit_len = 8 * sizeof(data);
>
> diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
> index bbec721bca..e6acd933d6 100644
> --- a/libavcodec/vaapi_encode.h
> +++ b/libavcodec/vaapi_encode.h
> @@ -116,9 +116,8 @@ typedef struct VAAPIEncodeContext {
> // Use low power encoding mode.
> int low_power;
>
> - // Supported packed headers (initially the desired set, modified
> - // later to what is actually supported).
> - unsigned int va_packed_headers;
> + // Desired packed headers.
> + unsigned int packed_headers;
>
How about adding a prefix of desired_ to this field? I got a little bit confused
when reviewing this patch :-)
> // The required size of surfaces. This is probably the input
> // size (AVCodecContext.width|height) aligned up to whatever
> @@ -140,6 +139,8 @@ typedef struct VAAPIEncodeContext {
> unsigned int va_rc_mode;
> // Bitrate for codec-specific encoder parameters.
> unsigned int va_bit_rate;
> + // Packed headers which will actually be sent.
> + unsigned int va_packed_headers;
>
> // Configuration attributes to use when creating va_config.
> VAConfigAttrib config_attributes[MAX_CONFIG_ATTRIBUTES];
> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
> index 717be024ca..e2f4f4f2f5 100644
> --- a/libavcodec/vaapi_encode_h264.c
> +++ b/libavcodec/vaapi_encode_h264.c
> @@ -930,7 +930,7 @@ static av_cold int vaapi_encode_h264_init(AVCodecContext
> *avctx)
> return AVERROR_PATCHWELCOME;
> }
>
> - ctx->va_packed_headers =
> + ctx->packed_headers =
> VA_ENC_PACKED_HEADER_SEQUENCE | // SPS and PPS.
> VA_ENC_PACKED_HEADER_SLICE | // Slice headers.
> VA_ENC_PACKED_HEADER_MISC; // SEI.
> diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
> index b2d6b8d24d..04dfef27c8 100644
> --- a/libavcodec/vaapi_encode_h265.c
> +++ b/libavcodec/vaapi_encode_h265.c
> @@ -1064,7 +1064,7 @@ static av_cold int vaapi_encode_h265_init(AVCodecContext
> *avctx)
> if (avctx->level == FF_LEVEL_UNKNOWN)
> avctx->level = priv->level;
>
> - ctx->va_packed_headers =
> + ctx->packed_headers =
> VA_ENC_PACKED_HEADER_SEQUENCE | // VPS, SPS and PPS.
> VA_ENC_PACKED_HEADER_SLICE | // Slice headers.
> VA_ENC_PACKED_HEADER_MISC; // SEI
> diff --git a/libavcodec/vaapi_encode_mjpeg.c b/libavcodec/vaapi_encode_mjpeg.c
> index 4982cd166f..f76645425a 100644
> --- a/libavcodec/vaapi_encode_mjpeg.c
> +++ b/libavcodec/vaapi_encode_mjpeg.c
> @@ -389,7 +389,7 @@ static av_cold int vaapi_encode_mjpeg_init(AVCodecContext
> *avctx)
> ctx->codec = &vaapi_encode_type_mjpeg;
>
> // The JPEG image header - see note above.
> - ctx->va_packed_headers =
> + ctx->packed_headers =
> VA_ENC_PACKED_HEADER_RAW_DATA;
>
> ctx->surface_width = FFALIGN(avctx->width, 8);
> diff --git a/libavcodec/vaapi_encode_mpeg2.c b/libavcodec/vaapi_encode_mpeg2.c
> index b6edca9158..1afd753fc1 100644
> --- a/libavcodec/vaapi_encode_mpeg2.c
> +++ b/libavcodec/vaapi_encode_mpeg2.c
> @@ -615,8 +615,8 @@ static av_cold int vaapi_encode_mpeg2_init(AVCodecContext
> *avctx)
> return AVERROR(EINVAL);
> }
>
> - ctx->va_packed_headers = VA_ENC_PACKED_HEADER_SEQUENCE |
> - VA_ENC_PACKED_HEADER_PICTURE;
> + ctx->packed_headers = VA_ENC_PACKED_HEADER_SEQUENCE |
> + VA_ENC_PACKED_HEADER_PICTURE;
>
> ctx->surface_width = FFALIGN(avctx->width, 16);
> ctx->surface_height = FFALIGN(avctx->height, 16);
> diff --git a/libavcodec/vaapi_encode_vp8.c b/libavcodec/vaapi_encode_vp8.c
> index 4512609a19..51ba601c5e 100644
> --- a/libavcodec/vaapi_encode_vp8.c
> +++ b/libavcodec/vaapi_encode_vp8.c
> @@ -200,8 +200,10 @@ static av_cold int vaapi_encode_vp8_init(AVCodecContext
> *avctx)
>
> ctx->codec = &vaapi_encode_type_vp8;
>
> - // Packed headers are not currently supported.
> - ctx->va_packed_headers = 0;
> + // No packed headers are currently desired. VP8 has no metadata
> + // which would be useful to write, and no existing driver supports
> + // adding them anyway.
> + ctx->packed_headers = 0;
>
> ctx->surface_width = FFALIGN(avctx->width, 16);
> ctx->surface_height = FFALIGN(avctx->height, 16);
> diff --git a/libavcodec/vaapi_encode_vp9.c b/libavcodec/vaapi_encode_vp9.c
> index d4069ec850..537a92c48c 100644
> --- a/libavcodec/vaapi_encode_vp9.c
> +++ b/libavcodec/vaapi_encode_vp9.c
> @@ -228,8 +228,10 @@ static av_cold int vaapi_encode_vp9_init(AVCodecContext
> *avctx)
>
> ctx->codec = &vaapi_encode_type_vp9;
>
> - // Packed headers are not currently supported.
> - ctx->va_packed_headers = 0;
> + // No packed headers are currently desired. They could be written,
> + // but there isn't any reason to do so - the one usable driver (i965)
> + // can write its own headers and there is no metadata to include.
> + ctx->packed_headers = 0;
>
> // Surfaces must be aligned to superblock boundaries.
> ctx->surface_width = FFALIGN(avctx->width, 64);
More information about the ffmpeg-devel
mailing list