Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

va: add av1 profile2 #828

Merged
merged 1 commit into from
Jul 25, 2024
Merged

va: add av1 profile2 #828

merged 1 commit into from
Jul 25, 2024

Conversation

davidwuAMD
Copy link
Contributor

Adding VAProfileAV1Profile2 to support 12bit AV1 decoding

Adding VAProfileAV1Profile2 to support 12bit AV1 decoding

Signed-off-by: David (Ming Qiang) Wu <[email protected]>
Copy link
Contributor

@XinfengZhang XinfengZhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it enough to add a profile only? will there be a platform support this profile?

@davidwuAMD
Copy link
Contributor Author

davidwuAMD commented Jul 18, 2024

Hi Xinfeng,
I am working on enabling 12bit AV1 decoding in ffmpeg and Mesa - so far it is working with this patch on amdgpu vaapi use case - I hope once this patch is in I will enable/push those supports to upstream.
David

@davidwuAMD
Copy link
Contributor Author

Hi Xinfeng,
any concern about letting this pull request merged? Let me know.
Thanks,
David

@XinfengZhang
Copy link
Contributor

LGTM, once the patch in mesa and ffmpeg merged, please paste the link here, thanks.

@davidwuAMD
Copy link
Contributor Author

https://gitlab.freedesktop.org/davidwu2/mesa/-/commit/8fa17b068a1fef987c53dff881b643bcf632e258
this is the link for 12bit AV1 support in mesa - I will need final touch once I know the new libva version - right now I am using 1.23 for testing but this needs to be 1.23+.
Wondering when there will a new version for libva?

@XinfengZhang
Copy link
Contributor

release is 2.xx.x , such as https://github.com/intel/libva/releases/tag/2.22.0, it is latest release.
master is 2.xx+1.0.pre with newest patches, current , it is 2.23.0.pre, suppose this patch will be merged into master, and will be in release 2.23.0

@XinfengZhang XinfengZhang merged commit 0115352 into intel:master Jul 25, 2024
28 checks passed
@davidwuAMD
Copy link
Contributor Author

thank you Xinfeng!

in meson.build (or configure.ac) we have
va_api_major_version = 1
va_api_minor_version = 23
va_api_micro_version = 0

We have a check in Mesa (and ffmpeg) for these info above. For 12bit AV1 support we will need a new version.
I expect it should be something like
va_api_major_version = 1
va_api_minor_version = 24
va_api_micro_version = 0
or
va_api_major_version = 1
va_api_minor_version = 23
va_api_micro_version = 1
in this case I can enable 12bit AV1 in Mesa and ffmpeg. Should I make a pull request to bump up these info?

Let me know if you have any question.
David

@XinfengZhang
Copy link
Contributor

you just need 1.23.0 withouth "pre1"
you could use macro #if VA_CHECK_VERSION(1, 23, 0), the release version including your patch is 2.23.0, current master is 2.23.0.pre1, it is not a release, so it may cache some feature un-released.
image

@davidwuAMD
Copy link
Contributor Author

I see - I did use the "#if VA_CHECK_VERSION(1, 23, 0)" in my mesa and ffmpeg - I thought 1.23.0 is has released - my bad
OK - sounds good -
thanks,

@fhvwy
Copy link
Contributor

fhvwy commented Aug 11, 2024

Based on the Mesa commits above this isn't actually what the decoder support needs? The specific case being implemented is for the subset of VAProfileAV1Profile2 which is 12-bit 4:2:0 only. A decoder which supports that only can't advertise profile 2 support because it doesn't cover all cases as required by the standard in section A.2.

I think this needs something else to advertise a subset of a profile. Maybe VAProfileAV1Profile2_420Only as a profile, or some other query could be added to check for additional support beyond the profiles which are fully supported (so advertise VAProfileAV1Profile0 since that profile is fully supported, but then have some extra attribute to say it works with 12-bit 4:2:0 as well).

@XinfengZhang
Copy link
Contributor

how about the VASurfaceAttrib, if it is Profile2, and the RT format is P010, it may represent 12bit 420 only. if it also include Y210 or P208, it represent 422?

@davidwuAMD
Copy link
Contributor Author

I realized the limitation for AV1 Profile2 - I will submit another patch to call something like VAProfileAV1Profile2_420Only or VAProfileAV1Profile2_420. thanks for pointing that out.

@davidwuAMD
Copy link
Contributor Author

another thought - can we keep VAProfileAV1Profile2 - as is? then in Mesa we check for the bitstream and allow for 12bit 420 only. I think ffmpeg might need more changes to support it

@davidwuAMD
Copy link
Contributor Author

I made another pull request - #832
I will investigate in ffmpeg and Mesa to make sure only 12bit 420 is allowed there - once done I will post the MR.

@davidwuAMD
Copy link
Contributor Author

Hi @XinfengZhang,
Would you be able to help get this merged?
thanks,
David

@XinfengZhang
Copy link
Contributor

hi DavidwuAMD, looks driver could report the capability by vaSurfaceAttrib, like my previous comments #828 (comment)

@davidwuAMD
Copy link
Contributor Author

oh - the driver only reports P010 and did not report Y210 or P208 etc. As the Hardware only supports 12bit 420. So I think #832 is needed to make it more accurate - so please help with #832

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants