Accept empty packed headers attribute in vaCreateConfig() #358
Conversation
|
If the driver reports the attribute as unsupported, then shouldn't unsupported be the only acceptable value? |
|
See fe0cac2 |
|
Also, libavcodec already does the right thing... see intel/libva#178 (comment) |
Yes. However, the problem here is for VP9 which does report some packed headers as supported.
I agree. Unfortunately, the current Intel driver rejects this behaviour and therefore VP9 encoding is broken when using it on all versions of libavcodec. (Hence considering it a ABI compatibility break, because programs which previously worked on the version 1.8 or 2.0 drivers no longer do.) |
|
Ah, I see, so VP9 RAW packed header is optional. And therefore, all packed headers must be treated as optional since there is no way to explicitly relay it otherwise :-/ |
|
Would you mind adding a unit test, at least for the VP9 case, so we can detect if this gets broken again in the future? |
Driver should allow 0x0 (i.e. VA_ENC_PACKED_HEADER_NONE) value for the VAConfigAttribEncPackedHeaders attribute during vaCreateConfig when the attribute is supported. This test supercedes intel/intel-vaapi-driver#361 Also see intel/intel-vaapi-driver#358 and intel/libva#178 Signed-off-by: U. Artie Eoff <[email protected]>
|
@xhaihao bump |
|
It seems we may want to continue to support VA_ENC_PACKED_HEADER_NONE until it's deprecated and removed. |
|
@xhaihao Updated with a warning for the empty set case. |
src/i965_drv_video.c
Outdated
| i965_log_info(ctx, "vaCreateConfig: invalid EncPackedHeaders attribute %#x: " | ||
| "packed headers are not supported.\n", attrib_found->value); | ||
| vaStatus = VA_STATUS_ERROR_INVALID_VALUE; | ||
| } |
There was a problem hiding this comment.
If the driver doesn't support packed header at all, application should not pass VAConfigAttribEncPackedHeaders to the driver. so I think the driver should return error no matter what is attrib_found->value
|
@xhaihao Sure, updated. |
|
Could you update the commit log as well? Per the commit log, vaCreateConfig() can accept VAConfigAttribEncPackedHeaders when packed header is not supported. |
If packed headers are supported, then the provided value must be some (possibly empty) subset of the supported headers. This fixes config creation with an empty packed header set, which the user may pass if they don't want to provide any packed headers. This pattern is used in at least libavcodec, where VP9 encoding is broken prior to this change. Since there is intent to deprecate this behaviour in future, also add warnings to this case and the other possible failure modes here. Fixes intel#362. Signed-off-by: Mark Thompson <[email protected]>
|
@xhaihao Sure, updated. |
|
Thanks for the patch, applied. @fhvwy |
Driver should allow 0x0 (i.e. VA_ENC_PACKED_HEADER_NONE) value for the VAConfigAttribEncPackedHeaders attribute during vaCreateConfig when the attribute is supported. This test supercedes intel/intel-vaapi-driver#361 Also see intel/intel-vaapi-driver#358 and intel/libva#178 Signed-off-by: U. Artie Eoff <[email protected]>
This indicates to the driver that the user will not supply any packed
headers, even though the possibility is available. This was an API/ABI
break in cccf2a3, and existing programs
such as libavcodec do use this pattern.