Minor cleanup: pNext chaining of structures#192
Conversation
krajunv
commented
Jan 21, 2026
krajunv
commented
- Cleanup pNext chain of structures that extends VkVideoReferenceSlotInfoKHR
- Group together pNext chaining of structure that extends VkVideoEncodeInfoKHR in the codec specific EnodeFrameInfo reset methods (eg. VkVideoEncodeFrameInfoH265::Reset())
|
@zlatinski @srinathkr-nv Please take a look at the small cleanup. |
srinathkr-nv
left a comment
srinathkr-nv
left a comment
There was a problem hiding this comment.
Instead of manually adding code for manipulating the pNext chain, please look at using vk::ChainNextVkStruct() wherever needed.
|
I initially considered using this approach. However, I wanted to group the population of one structure parameters in a single place and follow the order I’ll try updating the implementation based on your suggestion and see if I can still preserve the desired grouping and ordering I wanted which is mentioned above. |
|
I updated the files. Just to note, if multiple features are enabled conditionally, maintaining a specific chaining order (or any predefined order we may want to follow in the future) is not possible without introducing conditional chaining. Since the ordering is not a requirement in Vulkan, so I changed the coded accordingly. |
zlatinski
left a comment
zlatinski
left a comment
There was a problem hiding this comment.
Nice clean-up! Thank you, Raju!
srinathkr-nv
left a comment
srinathkr-nv
left a comment
There was a problem hiding this comment.
Please squash together the commits in this PR, so that there is a single commit with the required description of changes.
| m_dpbAV1->GetDirtyIntraRefreshRegions((int8_t)dpbIdx); | ||
|
|
||
| pFrameInfo->dpbSlotInfo[numReferenceSlots].pNext = &pFrameInfo->referenceIntraRefreshInfo[numReferenceSlots]; | ||
| vk::ChainNextVkStruct(pFrameInfo->dpbSlotInfo[numReferenceSlots], pFrameInfo->referenceIntraRefreshInfo[numReferenceSlots]); |
There was a problem hiding this comment.
While this is technically correct and we don't have strict usage rules for vk::ChainNextVkStruct, I think that we should use this helper in such a way that it naturally follows the specification language.
Whenever the specification refers to a given pNext chain, it is with respect to the base structure in an API call. Using a modified version of the example from a previous comment i.e. VkVideoReferenceSlotInfoKHR -> VkVideoDecodeAV1DpbSlotInfoKHR -> VkVideoReferenceIntraRefreshInfoKHR, both VkVideoDecodeAV1DpbSlotInfoKHR and VkVideoReferenceIntraRefreshInfoKHR are said to be present in the pNext chain of VkVideoReferenceSlotInfoKHR, and the ordering between VkVideoDecodeAV1DpbSlotInfoKHR and VkVideoReferenceIntraRefreshInfoKHR is not relevant.
For this reason, I suggest rewriting this statement as:
vk::ChainNextVkStruct(pFrameInfo->referenceSlotsInfo[numReferenceSlots], pFrameInfo->referenceIntraRefreshInfo[numReferenceSlots]);, in the intra-refresh-specific blocks for all codecs.
All the other changes look fine to me.
There was a problem hiding this comment.
Updated the chaining as per your suggestion.
krajunv
force-pushed
the
pNext-chain-cleanup
branch
from
9a5e52e to
bdd69a7
Compare
* Cleanup pNext chain of structures that extends VkVideoReferenceSlotInfoKHR. Use `vk::ChainNextVkStruct()` helper function to chain the structures. * Group together pNext chaining of structure that extends VkVideoEncodeInfoKHR in the codec specific EnodeFrameInfo reset methods (eg. VkVideoEncodeFrameInfoH265::Reset()) Signed-off-by: Raju Konda <kraju@nvidia.com>
krajunv
force-pushed
the
pNext-chain-cleanup
branch
from
bdd69a7 to
6b2cb8f
Compare
|
Squashed the commits. @zlatinski Please check and merge. |
3 participants
