Since the resolution of the bug 769266 this enhancement has become trickier.

The patches for bug 769266 assume that the encoder has context already configured (a pair profile/entrypoint) which will provide all the supported color formats to upload.

Nonetheless, in the case of HEVC, there are two VAEntrypointEncSlice for HEVC:
Main and Main10, where the difference is the accepted color space to upload. The first admits NV12, I420, YV12 and IMC3; the former one NV12 and P010.

So we have an exception to the rule: two profiles for one encoder. That means that gst_vaapi_encoder_get_surface_formats() needs to be aware of this exception. It will need to try all the available profiles.

Created attachment 349403 [details] [review]
vaapiencode: enhance logs of negotiated caps

Created attachment 349404 [details] [review]
libs: encoder: initialize chroma_type

Instead of initialize the chroma_type with a undefined value, which
will be converted to GST_VAAPI_CHROMA_TYPE_YUV420 by GstVaapiContext,
this patch queries the VA config, given the received
GstVaapiContextInfo's parameters, and gets the first response.

In order to get the GstVaapiChromaType value, also it was needed to
add a new utility function: to_GstVaapiChromaType(), which, given a
VA_RT_FORMAT_* will return the associated GstVaapiChromaType.

Created attachment 349405 [details] [review]
libs: encoder: refactor init_context_info()

In order to generate vaapi contexts iterative, the function
init_context_info() is refactored to pass, as parameters the
GstVaapiContextInfo and the GstVaapiProfile.

Created attachment 349406 [details] [review]
libs: encoder: dummy context for get_surface_formats()

Instead of creating (if it doesn't exist, yet) the encoder's context
the method gst_vaapi_encoder_get_surface_formats() now it creates
dummy contexts, unless the encoder has it already created.

The purpose of this is to avoid setting a encoder's context with a
wrong profile.

Created attachment 349407 [details] [review]
libs: encoder: pass profile to get_surface_formats()

In order to get the supported surface formats within a specific
profile this patch adds the GstVaapiProfile as property to
gst_vaapi_encoder_get_surface_formats().

Currently the extracted formats are only those related with the
default profile of the element's codec.

Created attachment 349408 [details] [review]
vaapiencode: add get_profile() vmethod

This new virtual method, get_profile(), if implemented by specific
encoders, will return the VA profile potentially determined by the
source caps.

Also it is implemented by h264 and h265 encoders, which are the main
users of this vmethod.

Created attachment 349409 [details] [review]
libs: encode: merge all possible surface formats

When the function gst_vaapi_encoder_get_surface_formats() was added
it was under the assumption that any VA profile of the specific codec
supported the same format colors. But it is not, for example the
profiles that support 10bit formats.

In other words, different VA profiles of a same codec may support
different color formats in their upload surfaces.

In order to expose all the possible color formats, if no profile is
specified via source caps, or if the encoder doesn't have yet a
context, all the possible VA profiles for the specific codec are
iterated and their color formats are merged.

Created attachment 349410 [details] [review]
libs: encoder: h265: ensures profile given format

Set the VA profile as GST_VAAPI_PROFILE_H265_MAIN10 if the
configured color format is P010_10LE.

Otherwise, keep GST_VAAPI_PROFILE_H265_MAIN

Created attachment 349411 [details] [review]
libs: encoder: admit YUV420_10BPP as valid chroma

Accepts as supported the GST_VAAPI_CHROMA_TYPE_YUV420_10BPP chroma
type.

Created attachment 349412 [details] [review]
libs: encoder: h265: chroma and luma with format

If the profile is main-10 the bit_depth_luma_minus8, in the sequence
parameter buffer, shall be the color format bit depth minus 8, 10-8
which is 2. Also for bit_depth_chroma_minus8.

This patch gets the negotiated sink caps format and queries its
luma's depth and uses that value to fill the mentioned parameters.

Created attachment 349413 [details] [review]
vaapiencode: h265: add main-10 in caps template

This patch adds h265's main-10 profile in encoder src caps template.

These are the patches required to enable main-10 in HEVC encoder.

I had to extend what it was done in bug 769266, by merging all the caps extracted from all the available codec profiles. Also added an optimization to only return the color formats of the specified profile in dowstream caps.

I know these are complicated patches, but if you can look through them would be great.

Since tomorrow afternoon (EEST) is the code freeze, I'll push them by the morning. Sorry.

Review of attachment 349404 [details] [review]:

::: gst-libs/gst/vaapi/gstvaapiencoder.c
@@ +598,3 @@
+          gst_vaapi_profile_get_va_profile (cip->profile),
+          gst_vaapi_entrypoint_get_va_entrypoint (cip->entrypoint),
+  guint value;

a query for VAConfigAttribRTFormat returns a bit field with supported RT_FORMATS or'd together, it won't give a single preferred default format.

Review of attachment 349412 [details] [review]:

::: gst-libs/gst/vaapi/gstvaapiencoder_h265.c
@@ +1439,3 @@
+  const GstVideoFormat format =
+      GST_VIDEO_INFO_FORMAT (GST_VAAPI_ENCODER_VIDEO_INFO (encoder));
+  guint bits_depth_luma =

small nit: may be a good idea to add the minus8 to the variable name here so it's obvious that -8 doesn't need to be applied below.

(In reply to Scott D Phillips from comment #14)
> a query for VAConfigAttribRTFormat returns a bit field with supported
> RT_FORMATS or'd together, it won't give a single preferred default format.

Maybe a different approach here would be to make a map from profile to rt_format? for h.264 and h.265 each profile supports a single subsampling type.

All other patches lgtm

(In reply to Scott D Phillips from comment #14)
> Review of attachment 349404 [details] [review] [review]:
> 
> ::: gst-libs/gst/vaapi/gstvaapiencoder.c
> @@ +598,3 @@
> +          gst_vaapi_profile_get_va_profile (cip->profile),
> +          gst_vaapi_entrypoint_get_va_entrypoint (cip->entrypoint),
> +  guint value;
> 
> a query for VAConfigAttribRTFormat returns a bit field with supported
> RT_FORMATS or'd together, it won't give a single preferred default format.

True. But still I prefer to rely on this "less hard" assumption rather than hard-coded map of preferred rt_format per VA profile, because it heavily relies on the specific driver.

(In reply to Scott D Phillips from comment #15)
> Review of attachment 349412 [details] [review] [review]:
> 
> ::: gst-libs/gst/vaapi/gstvaapiencoder_h265.c
> @@ +1439,3 @@
> +  const GstVideoFormat format =
> +      GST_VIDEO_INFO_FORMAT (GST_VAAPI_ENCODER_VIDEO_INFO (encoder));
> +  guint bits_depth_luma =
> 
> small nit: may be a good idea to add the minus8 to the variable name here so
> it's obvious that -8 doesn't need to be applied below.

Agree.

Attachment 349403 [details] pushed as 914a471 - vaapiencode: enhance logs of negotiated caps
Attachment 349404 [details] pushed as db72681 - libs: encoder: initialize chroma_type
Attachment 349405 [details] pushed as 31d326c - libs: encoder: refactor init_context_info()
Attachment 349406 [details] pushed as 9aa63e4 - libs: encoder: dummy context for get_surface_formats()
Attachment 349407 [details] pushed as 7153b45 - libs: encoder: pass profile to get_surface_formats()
Attachment 349408 [details] pushed as 5ccadd6 - vaapiencode: add get_profile() vmethod
Attachment 349409 [details] pushed as e534ff5 - libs: encode: merge all possible surface formats
Attachment 349410 [details] pushed as 48e21d6 - libs: encoder: h265: ensures profile given format
Attachment 349411 [details] pushed as d744aeb - libs: encoder: admit YUV420_10BPP as valid chroma
Attachment 349412 [details] pushed as 3532dca - libs: encoder: h265: chroma and luma with format
Attachment 349413 [details] pushed as b2815a3 - vaapiencode: h265: add main-10 in caps template