[FFmpeg-devel] [PATCH v2 11/36] vaapi_encode: Choose profiles dynamically
Mark Thompson
sw at jkqxz.net
Thu Jun 14 01:27:51 EEST 2018
On 12/06/18 08:22, Xiang, Haihao wrote:
> On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
>> Previously there was one fixed choice for each codec (e.g. H.265 -> Main
>> profile), and using anything else then required an explicit option from
>> the user. This changes to selecting the profile based on the input format
>> and the set of profiles actually supported by the driver (e.g. P010 input
>> will choose Main 10 profile for H.265 if the driver supports it).
>>
>> The entrypoint and render target format are also chosen dynamically in the
>> same way, removing those explicit selections from the per-codec code.
>> ---
>> doc/encoders.texi | 3 +
>> libavcodec/vaapi_encode.c | 271 ++++++++++++++++++++++++++++++++-------
>> -
>> libavcodec/vaapi_encode.h | 43 +++++--
>> libavcodec/vaapi_encode_h264.c | 45 ++-----
>> libavcodec/vaapi_encode_h265.c | 35 ++----
>> libavcodec/vaapi_encode_mjpeg.c | 13 +-
>> libavcodec/vaapi_encode_mpeg2.c | 36 ++----
>> libavcodec/vaapi_encode_vp8.c | 11 +-
>> libavcodec/vaapi_encode_vp9.c | 34 ++---
>> 9 files changed, 310 insertions(+), 181 deletions(-)
>>
>> ...
>> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
>> index 27ce792fbe..6104470b31 100644
>> --- a/libavcodec/vaapi_encode.c
>> +++ b/libavcodec/vaapi_encode.c
>> @@ -983,70 +983,247 @@ static av_cold void
>> vaapi_encode_add_global_param(AVCodecContext *avctx,
>> ++ctx->nb_global_params;
>> }
>>
>> -static av_cold int vaapi_encode_config_attributes(AVCodecContext *avctx)
>> +typedef struct VAAPIEncodeRTFormat {
>> + const char *name;
>> + unsigned int value;
>> + int depth;
>> + int components;
>
> How about adding a prefix of 'nb_' to this field? I think nb_components is more
> readable.
Sure. It should match the one in VAAPIEncodeProfile, so I'll change it there too.
>> + int log2_chroma_w;
>> + int log2_chroma_h;
>> +} VAAPIEncodeRTFormat;
>> +
>> +static const VAAPIEncodeRTFormat vaapi_encode_rt_formats[] = {
>> + { "YUV400", VA_RT_FORMAT_YUV400, 8, 1, },
>> + { "YUV420", VA_RT_FORMAT_YUV420, 8, 3, 1, 1 },
>> + { "YUV422", VA_RT_FORMAT_YUV422, 8, 3, 1, 0 },
>> + { "YUV444", VA_RT_FORMAT_YUV444, 8, 3, 0, 0 },
>> + { "YUV411", VA_RT_FORMAT_YUV411, 8, 3, 2, 0 },
>> +#if VA_CHECK_VERSION(0, 38, 1)
>> + { "YUV420_10", VA_RT_FORMAT_YUV420_10BPP, 10, 3, 1, 1 },
>> +#endif
>> +};
>> +
>> +static const VAEntrypoint vaapi_encode_entrypoints_normal[] = {
>> + VAEntrypointEncSlice,
>> + VAEntrypointEncPicture,
>> +#if VA_CHECK_VERSION(0, 39, 2)
>> + VAEntrypointEncSliceLP,
>> +#endif
>> + 0
>> +};
>> +#if VA_CHECK_VERSION(0, 39, 2)
>> +static const VAEntrypoint vaapi_encode_entrypoints_low_power[] = {
>> + VAEntrypointEncSliceLP,
>> + 0
>> +};
>> +#endif
>> +
>> +static av_cold int vaapi_encode_profile_entrypoint(AVCodecContext *avctx)
>> {
>> - VAAPIEncodeContext *ctx = avctx->priv_data;
>> + VAAPIEncodeContext *ctx = avctx->priv_data;
>> + VAProfile *va_profiles = NULL;
>> + VAEntrypoint *va_entrypoints = NULL;
>> VAStatus vas;
>> - int i, n, err;
>> - VAProfile *profiles = NULL;
>> - VAEntrypoint *entrypoints = NULL;
>> - VAConfigAttrib attr[] = {
>> - { VAConfigAttribRTFormat },
>> - { VAConfigAttribRateControl },
>> - { VAConfigAttribEncMaxRefFrames },
>> - { VAConfigAttribEncPackedHeaders },
>> - };
>> + const VAEntrypoint *usable_entrypoints;
>> + const VAAPIEncodeProfile *profile;
>> + const AVPixFmtDescriptor *desc;
>> + VAConfigAttrib rt_format_attr;
>> + const VAAPIEncodeRTFormat *rt_format;
>> + int i, j, n, depth, err;
>> +
>> +
>> + if (ctx->low_power) {
>> +#if VA_CHECK_VERSION(0, 39, 2)
>> + usable_entrypoints = vaapi_encode_entrypoints_low_power;
>> +#else
>> + av_log(avctx, AV_LOG_ERROR, "Low-power encoding is not "
>> + "supported with this VAAPI version.\n");
>
> Is it possible to report the minimal VAAPI version in the log in case user
> doesn't know the requirement on vaapi version 0.39.2?
The VAAPI version is pretty much useless to users (without reading any source code, how would you find what VAAPI version 0.39.2 means?).
Maybe libva version? That's still not quite right, though, because it's more "not supported in this build" (and will continue to not be supported if you upgrade libva but don't rebuild with the new headers).
>> + return AVERROR(EINVAL);
>> +#endif
>> + } else {
>> + usable_entrypoints = vaapi_encode_entrypoints_normal;
>> + }
>> +
>> + desc = av_pix_fmt_desc_get(ctx->input_frames->sw_format);
>> + if (!desc) {
>> + av_log(avctx, AV_LOG_ERROR, "Invalid input pixfmt (%d).\n",
>> + ctx->input_frames->sw_format);
>> + return AVERROR(EINVAL);
>> + }
>> + depth = desc->comp[0].depth;
>> + for (i = 1; i < desc->nb_components; i++) {
>> + if (desc->comp[i].depth != depth) {
>> + av_log(avctx, AV_LOG_ERROR, "Invalid input pixfmt (%s).\n",
>> + desc->name);
>> + return AVERROR(EINVAL);
>> + }
>> + }
>> + av_log(avctx, AV_LOG_VERBOSE, "Input surface format is %s.\n",
>> + desc->name);
>>
>> n = vaMaxNumProfiles(ctx->hwctx->display);
>> - profiles = av_malloc_array(n, sizeof(VAProfile));
>> - if (!profiles) {
>> + va_profiles = av_malloc_array(n, sizeof(VAProfile));
>> + if (!va_profiles) {
>> err = AVERROR(ENOMEM);
>> goto fail;
>> }
>> - vas = vaQueryConfigProfiles(ctx->hwctx->display, profiles, &n);
>> + vas = vaQueryConfigProfiles(ctx->hwctx->display, va_profiles, &n);
>> if (vas != VA_STATUS_SUCCESS) {
>> - av_log(ctx, AV_LOG_ERROR, "Failed to query profiles: %d (%s).\n",
>> + av_log(avctx, AV_LOG_ERROR, "Failed to query profiles: %d (%s).\n",
>> vas, vaErrorStr(vas));
>> - err = AVERROR(ENOSYS);
>> + err = AVERROR_EXTERNAL;
>> goto fail;
>> }
>> - for (i = 0; i < n; i++) {
>> - if (profiles[i] == ctx->va_profile)
>> - break;
>> +
>> + av_assert0(ctx->codec->profiles);
>> + for (i = 0; (ctx->codec->profiles[i].av_profile !=
>> + FF_PROFILE_UNKNOWN); i++) {
>> + profile = &ctx->codec->profiles[i];
>> + if (depth != profile->depth ||
>> + desc->nb_components != profile->components)
>> + continue;
>> + if (desc->nb_components > 1 &&
>> + (desc->log2_chroma_w != profile->log2_chroma_w ||
>> + desc->log2_chroma_h != profile->log2_chroma_h))
>> + continue;
>> + if (avctx->profile != profile->av_profile &&
>> + avctx->profile != FF_PROFILE_UNKNOWN)
>> + continue;
>> +
>> + for (j = 0; j < n; j++) {
>> + if (va_profiles[j] == profile->va_profile)
>> + break;
>> + }
>> + if (j >= n) {
>> + av_log(avctx, AV_LOG_VERBOSE, "Matching profile %d is "
>> + "not supported by driver.\n", profile->va_profile);
>
> Is it possible to report the profile string in the log as what you did below?
The #ifdefs are very nasty, but they seemed merited for the "I chose this one" log line. See below for a different method, not sure it's better.
>> + continue;
>> + }
>> +
>> + ctx->profile = profile;
>> + break;
>> }
>> - if (i >= n) {
>> - av_log(ctx, AV_LOG_ERROR, "Encoding profile not found (%d).\n",
>> - ctx->va_profile);
>> - err = AVERROR(ENOSYS);
>> - goto fail;
>> + if (!ctx->profile) {
>> + av_log(avctx, AV_LOG_ERROR, "No usable encoding profile found.\n");
>> + return AVERROR(ENOSYS);
>
> Set err to AVERROR(ENOSYS) then goto fail, otherwise it will result in memory
> leak.
Fixed.
>> }
>>
>> + avctx->profile = profile->av_profile;
>> + ctx->va_profile = profile->va_profile;
>> +#if VA_CHECK_VERSION(1, 0, 0)
>> + av_log(avctx, AV_LOG_VERBOSE, "Using VAAPI profile %s (%d).\n",
>> + vaProfileStr(ctx->va_profile), ctx->va_profile);
>> +#else
>> + av_log(avctx, AV_LOG_VERBOSE, "Using VAAPI profile %d.\n",
>> + ctx->va_profile);
>> +#endif
>> +
>> n = vaMaxNumEntrypoints(ctx->hwctx->display);
>> - entrypoints = av_malloc_array(n, sizeof(VAEntrypoint));
>> - if (!entrypoints) {
>> + va_entrypoints = av_malloc_array(n, sizeof(VAEntrypoint));
>> + if (!va_entrypoints) {
>> err = AVERROR(ENOMEM);
>> goto fail;
>> }
>> vas = vaQueryConfigEntrypoints(ctx->hwctx->display, ctx->va_profile,
>> - entrypoints, &n);
>> + va_entrypoints, &n);
>> if (vas != VA_STATUS_SUCCESS) {
>> - av_log(ctx, AV_LOG_ERROR, "Failed to query entrypoints for "
>> + av_log(avctx, AV_LOG_ERROR, "Failed to query entrypoints for "
>> "profile %u: %d (%s).\n", ctx->va_profile,
>
> Log profile string?
>
>> vas, vaErrorStr(vas));
>> - err = AVERROR(ENOSYS);
>> + err = AVERROR_EXTERNAL;
>> goto fail;
>> }
>> +
>> for (i = 0; i < n; i++) {
>> - if (entrypoints[i] == ctx->va_entrypoint)
>> + for (j = 0; usable_entrypoints[j]; j++) {
>> + if (va_entrypoints[i] == usable_entrypoints[j])
>> + break;
>> + }
>> + if (usable_entrypoints[j])
>> break;
>> }
>> if (i >= n) {
>> - av_log(ctx, AV_LOG_ERROR, "Encoding entrypoint not found "
>> - "(%d / %d).\n", ctx->va_profile, ctx->va_entrypoint);
>> + av_log(avctx, AV_LOG_ERROR, "No usable encoding entrypoint found "
>> + "for profile %d.\n", ctx->va_profile);
>
> Log profile string?
>
>> + err = AVERROR(ENOSYS);
>> + goto fail;
>> + }
>> +
>> + ctx->va_entrypoint = va_entrypoints[i];
>> +#if VA_CHECK_VERSION(1, 0, 0)
>> + av_log(avctx, AV_LOG_VERBOSE, "Using VAAPI entrypoint %s (%d).\n",
>> + vaEntrypointStr(ctx->va_entrypoint), ctx->va_entrypoint);
>> +#else
>> + av_log(avctx, AV_LOG_VERBOSE, "Using VAAPI entrypoint %d.\n",
>> + ctx->va_entrypoint);
>> +#endif
>> +
>> + for (i = 0; i < FF_ARRAY_ELEMS(vaapi_encode_rt_formats); i++) {
>> + rt_format = &vaapi_encode_rt_formats[i];
>> + if (rt_format->depth == depth &&
>> + rt_format->components == profile->components &&
>> + rt_format->log2_chroma_w == profile->log2_chroma_w &&
>> + rt_format->log2_chroma_h == profile->log2_chroma_h)
>> + break;
>> + }
>> + if (i >= FF_ARRAY_ELEMS(vaapi_encode_rt_formats)) {
>> + av_log(avctx, AV_LOG_ERROR, "No usable render target format "
>> + "found for profile %d entrypoint %d.\n",
>> + ctx->va_profile, ctx->va_entrypoint);
>
> Log profile and entrypoint strings?
>
>> err = AVERROR(ENOSYS);
>> goto fail;
>> }
>>
>> + rt_format_attr = (VAConfigAttrib) { VAConfigAttribRTFormat };
>> + vas = vaGetConfigAttributes(ctx->hwctx->display,
>> + ctx->va_profile, ctx->va_entrypoint,
>> + &rt_format_attr, 1);
>> + if (vas != VA_STATUS_SUCCESS) {
>> + av_log(avctx, AV_LOG_ERROR, "Failed to query RT format "
>> + "config attribute: %d (%s).\n", vas, vaErrorStr(vas));
>> + err = AVERROR_EXTERNAL;
>> + goto fail;
>> + }
>> +
>> + if (rt_format_attr.value == VA_ATTRIB_NOT_SUPPORTED) {
>> + av_log(avctx, AV_LOG_VERBOSE, "RT format config attribute not "
>> + "supported by driver: assuming surface RT format %s "
>> + "is valid.\n", rt_format->name);
>
> I think it would be better to log it as a warning.
I'm not sure what a user would be meant to do with such a warning. Even if the driver doesn't make it queryable, we know what the answer should be and it will either work or not after this point with the known value.
>> + } else if (!(rt_format_attr.value & rt_format->value)) {
>> + av_log(avctx, AV_LOG_ERROR, "Surface RT format %s not supported "
>> + "by driver for encoding profile %d entrypoint %d.\n",
>> + rt_format->name, ctx->va_profile, ctx->va_entrypoint);
>> + err = AVERROR(ENOSYS);
>> + goto fail;
>> + } else {
>> + av_log(avctx, AV_LOG_VERBOSE, "Using VAAPI render target "
>> + "format %s (%#x).\n", rt_format->name, rt_format->value);
>> + ctx->config_attributes[ctx->nb_config_attributes++] =
>> + (VAConfigAttrib) {
>> + .type = VAConfigAttribRTFormat,
>> + .value = rt_format->value,
>> + };
>> + }
>> +
>> + err = 0;
>> +fail:
>> + av_freep(&va_profiles);
>> + av_freep(&va_entrypoints);
>> + return err;
>> +}
>> +
>> ...
Here's a different way to do the strings. The result is kindof stupid on libva < 2, so I'm not sure I would really argue for it. Would you prefer this?
diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 2ed12beb0c..f838ee5bd5 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -1020,6 +1020,7 @@ static av_cold int vaapi_encode_profile_entrypoint(AVCodecContext *avctx)
const AVPixFmtDescriptor *desc;
VAConfigAttrib rt_format_attr;
const VAAPIEncodeRTFormat *rt_format;
+ const char *profile_string, *entrypoint_string;
int i, j, n, depth, err;
@@ -1081,6 +1082,12 @@ static av_cold int vaapi_encode_profile_entrypoint(AVCodecContext *avctx)
avctx->profile != FF_PROFILE_UNKNOWN)
continue;
+#if VA_CHECK_VERSION(1, 0, 0)
+ profile_string = vaProfileStr(profile->va_profile);
+#else
+ profile_string = "[no profile names]";
+#endif
+
for (j = 0; j < n; j++) {
if (va_profiles[j] == profile->va_profile)
break;
@@ -1102,13 +1109,8 @@ static av_cold int vaapi_encode_profile_entrypoint(AVCodecContext *avctx)
avctx->profile = profile->av_profile;
ctx->va_profile = profile->va_profile;
-#if VA_CHECK_VERSION(1, 0, 0)
av_log(avctx, AV_LOG_VERBOSE, "Using VAAPI profile %s (%d).\n",
- vaProfileStr(ctx->va_profile), ctx->va_profile);
-#else
- av_log(avctx, AV_LOG_VERBOSE, "Using VAAPI profile %d.\n",
- ctx->va_profile);
-#endif
+ profile_string, ctx->va_profile);
n = vaMaxNumEntrypoints(ctx->hwctx->display);
va_entrypoints = av_malloc_array(n, sizeof(VAEntrypoint));
@@ -1120,8 +1122,8 @@ static av_cold int vaapi_encode_profile_entrypoint(AVCodecContext *avctx)
va_entrypoints, &n);
if (vas != VA_STATUS_SUCCESS) {
av_log(avctx, AV_LOG_ERROR, "Failed to query entrypoints for "
- "profile %u: %d (%s).\n", ctx->va_profile,
- vas, vaErrorStr(vas));
+ "profile %s (%d): %d (%s).\n", profile_string,
+ ctx->va_profile, vas, vaErrorStr(vas));
err = AVERROR_EXTERNAL;
goto fail;
}
@@ -1136,19 +1138,19 @@ static av_cold int vaapi_encode_profile_entrypoint(AVCodecContext *avctx)
}
if (i >= n) {
av_log(avctx, AV_LOG_ERROR, "No usable encoding entrypoint found "
- "for profile %d.\n", ctx->va_profile);
+ "for profile %s (%d).\n", profile_string, ctx->va_profile);
err = AVERROR(ENOSYS);
goto fail;
}
ctx->va_entrypoint = va_entrypoints[i];
#if VA_CHECK_VERSION(1, 0, 0)
- av_log(avctx, AV_LOG_VERBOSE, "Using VAAPI entrypoint %s (%d).\n",
- vaEntrypointStr(ctx->va_entrypoint), ctx->va_entrypoint);
+ entrypoint_string = vaEntrypointStr(ctx->va_entrypoint);
#else
- av_log(avctx, AV_LOG_VERBOSE, "Using VAAPI entrypoint %d.\n",
- ctx->va_entrypoint);
+ entrypoint_string = "[no entrypoint names]";
#endif
+ av_log(avctx, AV_LOG_VERBOSE, "Using VAAPI entrypoint %s (%d).\n",
+ entrypoint_string, ctx->va_entrypoint);
for (i = 0; i < FF_ARRAY_ELEMS(vaapi_encode_rt_formats); i++) {
rt_format = &vaapi_encode_rt_formats[i];
@@ -1160,8 +1162,9 @@ static av_cold int vaapi_encode_profile_entrypoint(AVCodecContext *avctx)
}
if (i >= FF_ARRAY_ELEMS(vaapi_encode_rt_formats)) {
av_log(avctx, AV_LOG_ERROR, "No usable render target format "
- "found for profile %d entrypoint %d.\n",
- ctx->va_profile, ctx->va_entrypoint);
+ "found for profile %s (%d) entrypoint %s (%d).\n",
+ profile_string, ctx->va_profile,
+ entrypoint_string, ctx->va_entrypoint);
err = AVERROR(ENOSYS);
goto fail;
}
@@ -1183,8 +1186,9 @@ static av_cold int vaapi_encode_profile_entrypoint(AVCodecContext *avctx)
"is valid.\n", rt_format->name);
} else if (!(rt_format_attr.value & rt_format->value)) {
av_log(avctx, AV_LOG_ERROR, "Surface RT format %s not supported "
- "by driver for encoding profile %d entrypoint %d.\n",
- rt_format->name, ctx->va_profile, ctx->va_entrypoint);
+ "by driver for encoding profile %s (%d) entrypoint %s (%d).\n",
+ rt_format->name, profile_string, ctx->va_profile,
+ entrypoint_string, ctx->va_entrypoint);
err = AVERROR(ENOSYS);
goto fail;
} else {
More information about the ffmpeg-devel
mailing list