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

vabackend: support 10/12 bit formats #120

Merged
merged 1 commit into from
Oct 20, 2022

Conversation

philipl
Copy link
Contributor

@philipl philipl commented Oct 17, 2022

The latest nvidia 520 drivers include support for the required DRM
formats to allow exporting of 10/12 bit surfaces.

The driver currently has incomplete support and various bits and
pieces commented out.

This change is an initial attempt to implement the required support for
the relevant profiles.

Things are simple where the profile and bit depth have a 1:1
correspondence, but AV1Profile0 includes both 8 and 10 bit content,
while VP9Profile2 includes 10 and 12 bit.

In these cases, we have to have a way to learn from the caller what
format is desired. In the most straight-forward case, the call to
createConfig will specify which format the caller wants as an RtFormat,
but this is optional. They are allowed to specify nothing.

On the other hand, other callers can provide pre-allocated surfaces
when calling createContext, and we can look at the surface format to
work out what they want.

What if a caller didn't provide either hint? We'd end up having to
guess and just assume 8bit. In practice, we believe that all relevant
callers use one of the above patterns.

Fixes #34

@philipl
Copy link
Contributor Author

philipl commented Oct 17, 2022

Hmm. Probably what's required is that the NVConfig must store the fact that multiple formats and bit depths are associated with the profile, and then return the various pixel formats in the surface attributes, so that we don't care what RT_FORMAT is passed to createConfig.

@philipl
Copy link
Contributor Author

philipl commented Oct 17, 2022

And createContext probably has to look at the render targets to see what format they are to set the right format on the decoder.

@elFarto
Copy link
Owner

elFarto commented Oct 17, 2022

Patch looks good so far. But yeah, you ran straight into the issue I spotted, two codecs that support multiple bit-depths. There are a couple of ways I can see at the moment.

  1. Defer the creation of the decoder until we know all the details. I don't really like this idea, as we end up having to lie on some of the methods (e.g. create context) and it errors to show up were calling apps may not expect.
  2. Keep track of the created surface formats and use that to create the context. This isn't too bad, but Chrome (if we ever get around to supporting that) doesn't create the surfaces before the context, so we can't use that technique there.

I think we need to understand how the other drivers are handling it, but I have a suspicion that NVDEC is the only one that requires knowing the bit-depth before creating the context meaning VA-API isn't designed to handle that scenario.

@philipl
Copy link
Contributor Author

philipl commented Oct 18, 2022

Pushed a change that uses render target at context creation time to set decoder output format. Works with mpv. I don't think delaying until nvBeginPicture is too crazy. I suspect you are right that vaapi assumes format decisions can made after context initialisation and this is a fundamental mismatch.

@elFarto
Copy link
Owner

elFarto commented Oct 18, 2022

Well some good news at least, it seems that Chrome always provides the RTFormat to vaCreateConfig, so we will be able to get the correct format either by using that, or the passed in render targets when creating the context. Althought, it seems it only supports 10-bit for VP9 profiles 2 and 3.

The changes look good. Looks like I'll have to manually install the 520 driver as the normal package I rely on hasn't been updated yet.

Should we be doing a doesGPUSupportCodec call inside nvCreateConfig to make sure that particular configuration is supported? I'm not sure if it's possible to have a GPU that can only decode one of the bit-depths a profile supports. Infact does the nBitDepthMinus8 parameter we pass to NVDEC represent the bitstream's format, or the render target's bit-depth? The documentation is vague on this. They should be the same, but it's not impossible for them to be different.

We may end up trying to patch ffmpeg to pass in the RTFormat just so we're never left guessing, but I think that can wait for a while.

src/vabackend.c Outdated Show resolved Hide resolved
@philipl philipl force-pushed the 10-12-bit-surfaces branch from 0c0a53c to c742daa Compare October 19, 2022 04:15
@philipl
Copy link
Contributor Author

philipl commented Oct 19, 2022

Addressed comments and tidied it up a bit. I think we will have to keep 12bit disabled, as it's currently failing for non-obvious reasons.

@philipl
Copy link
Contributor Author

philipl commented Oct 19, 2022

Yeah, there's a cuda problem. Of course. CU_EGL_COLOR_FORMAT_Y12V12U12_420_SEMIPLANAR fails to export. If I tell it to use the Y10V10Y10 format, it exports (and I guess truncates). So I think we have to consider that a gap for now. @cubanismo

@elFarto
Copy link
Owner

elFarto commented Oct 19, 2022

It can't truncate as it's sharing memory and the 10 and 12 bit formats share the same underlying format so they should be identical. So using the 10 bit one should work for 12 bit formats, in theory.

The latest nvidia 520 drivers include support for the required DRM
formats to allow exporting of 10/12 bit surfaces.

The driver currently has incomplete support and various bits and
pieces commented out.

This change is an initial attempt to implement the required support for
the relevant profiles.

Things are simple where the profile and bit depth have a 1:1
correspondence, but AV1Profile0 includes both 8 and 10 bit content,
while VP9Profile2 includes 10 and 12 bit.

In these cases, we have to have a way to learn from the caller what
format is desired. In the most straight-forward case, the call to
createConfig will specify which format the caller wants as an RtFormat,
but this is optional. They are allowed to specify nothing.

On the other hand, other callers can provide pre-allocated surfaces
when calling createContext, and we can look at the surface format to
work out what they want.

What if a caller didn't provide either hint? We'd end up having to
guess and just assume 8bit. In practice, we believe that all relevant
callers use one of the above patterns.
@philipl philipl force-pushed the 10-12-bit-surfaces branch from c742daa to 90413a7 Compare October 20, 2022 03:09
@philipl philipl changed the title vabackend: WIP: Support 10/12 bit formats vabackend: support 10/12 bit formats Oct 20, 2022
@philipl
Copy link
Contributor Author

philipl commented Oct 20, 2022

Updated to use the Y10 format for 12 bit content.

@elFarto elFarto merged commit 8335038 into elFarto:master Oct 20, 2022
@elFarto
Copy link
Owner

elFarto commented Oct 20, 2022

Managed to get 520 installed. Looks good, but I was only able to test HEVC 10 and 12-bit as my card doesn't support the 10 and 12-bit VP9 profile 2.

Thanks for the patch!

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.

Cannot export surface formats needed for 10/12bit video
2 participants