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       | 119 +++++++++++++++++---------------
 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, 80 insertions(+), 68 deletions(-)

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index e48e703ab4..2c34cdce2c 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -1206,60 +1206,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;
@@ -1498,6 +1444,66 @@ 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->desired_packed_headers) {
+            av_log(avctx, AV_LOG_WARNING, "Driver does not support any "
+                   "packed headers (wanted %#x).\n",
+                   ctx->desired_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->desired_packed_headers & ~attr.value) {
+            av_log(avctx, AV_LOG_WARNING, "Driver does not support some "
+                   "wanted packed headers (wanted %#x, found %#x).\n",
+                   ctx->desired_packed_headers, attr.value);
+        } else {
+            av_log(avctx, AV_LOG_VERBOSE, "All wanted packed headers "
+                   "available (wanted %#x, found %#x).\n",
+                   ctx->desired_packed_headers, attr.value);
+        }
+        ctx->va_packed_headers = ctx->desired_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->desired_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)
@@ -1728,7 +1734,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;
 
@@ -1817,7 +1823,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 5d6fdcf336..091889f9ae 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    desired_packed_headers;
 
     // 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 5c93fe35ce..b233a91d04 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->desired_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..c40782a78e 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->desired_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..332053ae34 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->desired_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..1377eeb9bb 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->desired_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..697b465787 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->desired_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..39bc868f3a 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->desired_packed_headers = 0;
 
     // Surfaces must be aligned to superblock boundaries.
     ctx->surface_width  = FFALIGN(avctx->width,  64);
-- 
2.18.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to