Re: [FFmpeg-devel] [PATCH v2 11/36] vaapi_encode: Choose profiles dynamically
On 14/06/18 05:51, Xiang, Haihao wrote:
> On Wed, 2018-06-13 at 23:27 +0100, Mark Thompson wrote:
>> 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(-)
...
+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?).
>>
>
> Ok, I agree it is useless to user.
>
>> 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).
>>
>
> May we ask user to rebuild ffmpeg once user upgrades libva in the log?
On further thought, I don't think a recommendation is useful here. You end up
having to suggest upgrading everything (libva, driver, ffmpeg), and even then
it still may not be there (if you are using a driver or codec which doesn't
expose this option).
So, I think it's most sensible to keep the simple text as originally written.
If you have a string opinion then feel free to suggest your own patch for how
this should work. (And it should probably be consistent with other similar
cases, such as the quality-speed tradeoff option.)
+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, );
+vas =
Re: [FFmpeg-devel] [PATCH v2 11/36] vaapi_encode: Choose profiles dynamically
On Wed, 2018-06-13 at 23:27 +0100, Mark Thompson wrote:
> 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?).
>
Ok, I agree it is useless to user.
> 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).
>
May
Re: [FFmpeg-devel] [PATCH v2 11/36] vaapi_encode: Choose profiles dynamically
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 =
Re: [FFmpeg-devel] [PATCH v2 11/36] vaapi_encode: Choose profiles dynamically
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/doc/encoders.texi b/doc/encoders.texi
> index 7b095754d1..16be6359b3 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -2565,6 +2565,9 @@ The following standard libavcodec options are used:
> @option{bf} / @option{max_b_frames}
> @item
> @option{profile}
> +
> +If not set, this will be determined automatically from the format of the
> input
> +frames and the profiles supported by the driver.
> @item
> @option{level}
> @item
> 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.
> +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?
> +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,
