Introduce dedicated types for primary/secondary transform representation - #5369
Open
arnab-dey-12 wants to merge 4 commits into
Open
Introduce dedicated types for primary/secondary transform representation#5369arnab-dey-12 wants to merge 4 commits into
arnab-dey-12 wants to merge 4 commits into
Conversation
- Define PRIM_TX_TYPE, SEC_TX_TYPE, and SEC_TX_SET enums. - Replace all occurrences of TX_TYPES with PRIM_TX_TYPES. - Define temporary compatibility typedef for TX_TYPE. No stats changed
- Where a value only holds one part of the packed TX_TYPE,
give it the type for that part (i.e, PRIM_TX_TYPE, SEC_TX_TYPE,
SEC_TX_SET).
- Change TxfmParam's 4 transform fields as below
TX_TYPE tx_type -> PRIM_TX_TYPE prim_tx_type
TX_TYPE sec_tx_type -> SEC_TX_TYPE sec_tx_type
TX_TYPE sec_tx_set -> SEC_TX_SET sec_tx_set
TX_TYPE sec_tx_set_idx -> uint8_t sec_tx_set_idx
sec_tx_set_idx is left untyped as it holds a remapped kernel
index, not a set id.
- Callers now pass the primary type explicitly via
get_primary_tx_type() to functions that don't need the packed type.
- TX_TYPE is still a typedef of PRIM_TX_TYPE, so every remaining
use of it holds a packed value.
No stats changed.
- Define TX_TYPE as a bitfield union encapsulating prim_tx, sec_tx, and sec_set. - Update packed transform operation with access to union members. - Update the related comments. - Add av2_tx_type_in_range() to bound-check all three components together, replacing the size-only sec_set asserts that were spread across the encoder and decoder. No stats changed Change-Id: I5143108be2d2e3142887fb553071791da8f48002
- Delete get_primary_tx_type, get_secondary_tx_type, get_secondary_tx_set, set_secondary_tx_type, set_secondary_tx_set, disable_primary_tx_type, and disable_secondary_tx_type helpers. No stats changed Change-Id: I84ece9b20137de42216a008ae976a20e35c3d7ee
arnab-dey-12
marked this pull request as ready for review
September 4, 2026 13:43
This file contains hidden or 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
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.
@yunqingwang1, @urvangjoshi Please Review
/cc @vikasprasad10 @harishdm @ranjit-tulabandu @cherma-a @deepa-kg @RemyaPrakasan @ram-mohan @mudassir-sg
AV2 introduces a secondary transform type, but reuses AV1's
TX_TYPE(an enum) representing primary transform types, to represent every transform-related value: the primary type, the secondary set, the secondary type, and the packed representation. Overloading one enum across four distinct meanings creates readability issues and is prone to mistakes:TX_TYPEtx_type != DCT_DCTreads as "the primary type is notDCT_DCT", but sincetx_typeis actually the packed representation, the check really asks whether any of the three fields is set.TX_TYPEtype, so passing one where the other is expected isn't a compile error, it silently produces the wrong value at runtime. For example, in tx_search.c,primary_tx_typeandpacked_tx_typeare both plainTX_TYPEin the same function, swapping them would compile cleanly while corrupting the transform type used downstream.This PR introduces dedicated data structures so each concept is explicit and distinct:
PRIM_TX_TYPE— the primary transform typeSEC_TX_SET— the secondary transform setSEC_TX_TYPE— the secondary transform typeIt also refactors the existing
TX_TYPEenum into a bitfield union, and updates call sites to use the new types where appropriate. Together, these changes make the distinction between primary type, secondary set, secondary type, and packed representation explicit and make the code easier to read and maintain.We have verified that this PR is bit-exact for speeds 0 to 3 across RA, LD, and AI configurations on a few test clips.
Note : Each commit is independent, self-contained change and has been verified to be bit-exact. Please do not squash.