Fix validation warnings #57
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
Only tested on Linux. Signed-off-by: Charlie Turner <[email protected]>
VK_LAYER_KHRONOS_validation is the more modern layer name. VK_EXT_DEBUG_REPORT_EXTENSION_NAME was mistakenly being requested as a layer, request the instance extension instead. Signed-off-by: Charlie Turner <[email protected]>
This happens in noPresent mode, since we aren't supposed to be retrieving queues from these families. Signed-off-by: Charlie Turner <[email protected]>
And use it to ignore the buffer profile information errors. These will become optional soon, and wiring them into the sample app is problematic, since the bitstream buffers are created before the video profile is known! Signed-off-by: Charlie Turner <[email protected]>
Various initialization paths will need this information to become validation clean Signed-off-by: Charlie Turner <[email protected]>
This is not complete, but silences some validation warnings. Before all features were being turned off! Signed-off-by: Charlie Turner <[email protected]>
Needs better testing Signed-off-by: Charlie Turner <[email protected]>
Signed-off-by: Charlie Turner <[email protected]>
Need to figure out better how to handle these Signed-off-by: Charlie Turner <[email protected]>
Signed-off-by: Charlie Turner <[email protected]>
charlie-ht
force-pushed
the
cturner/validation-fixes
branch
from
e50bf19
to
aa21fe5
Compare
Signed-off-by: Charlie Turner <[email protected]>
Signed-off-by: Charlie Turner <[email protected]>
Signed-off-by: Charlie Turner <[email protected]>
Signed-off-by: Charlie Turner <[email protected]>
Signed-off-by: Charlie Turner <[email protected]>
Signed-off-by: Charlie Turner <[email protected]>
Signed-off-by: Charlie Turner <[email protected]>
For linear mode on coincident implementations, this gives an validation error (although it feels OK to do...) Just make the layer happy, I suspect we should be using the image copying approach in general, and rather copy to a buffer. Signed-off-by: Charlie Turner <[email protected]>
Signed-off-by: Charlie Turner <[email protected]>
This is now unconditionally used, perhaps we should flag it off if not supported, so as to run on more devices. Signed-off-by: Charlie Turner <[email protected]>
…orted Demonstration for Tony, per our discussion in the weekly meeting. + assert(!outputImageSupportsLinearTiling); Please check if this is a driver bug. In general it might not be supported though, and we will need the ImageToBuffer fallback anyway. Signed-off-by: Charlie Turner <[email protected]>
Lets rely on CopyImageToBuffer, and tidy up the image handling code in the process. Signed-off-by: Charlie Turner <[email protected]>
This is not proposed to merge, rather to help a discussion. Of course the code should be abstracted a bit, looking for early feedback. This also doesn't work for 10-bit out of the box. Need to do some bit manipulations of the packed 10-bit values to convert it properly to RAW YUV format (bottom 6 bits clear, rather than top 6) Before doing that, want to check if this is an acceptable approach in general, or whether we should enforce linear tiling for output images. Signed-off-by: Charlie Turner <[email protected]>
charlie-ht
force-pushed
the
cturner/validation-fixes
branch
from
aa21fe5
to
b18ab00
Compare
zlatinski
requested changes
Nov 3, 2023
| uint32_t lumaBufferSize = NextPowerOf2U32(imageWidth*imageHeight*bytesPerPixel); | ||
| uint32_t chromaBufferSize = NextPowerOf2U32(imageWidth*secondaryPlaneHeight*bytesPerPixel); | ||
| result = | ||
| VkBufferResource::Create(m_vkDevCtx, |
There was a problem hiding this comment.
Are we creating and destroying resources with each frame here?
Also, why is using a buffer copy better than an image copy?
|
Hi @charlie-ht, Can you please rebase your fixes against the ToT? Sorry, it took so long! |
|
I so look forward to using this so I can have 100% clean validation logs in my engine. It's also worrisome that some of the data wasn't even passed like the HEVC profile of the video in question during certain buffer creation, I had started to fix that but realized it needed some more serious refactoring to access that data from higher up the callstack. |
Hi @zlatinski - unfortunately main has moved along in a way that makes a lot of this conflict. Perhaps best to close this one, and I'll try and find some time to pick over any validation errors that remain in the latest main branch. #69 partly overlaps some of this work (the transfer queue exposure). Can you merge those fixes for AMD into the encode branch and the main branch? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There remain some less serious warnings after this series, but to fix some of the more difficult ones, changes in how the image handling and download happens were required.
Those commits start at "Use concurrent sharing when the transfer queue != decode queue". I'd like some review that this is an OK direction to take the sample app in before going much further.
/cc @zlatinski