encode: codec profile and level fixes - #171
Conversation
49f442f to
4fba129
Compare
|
@srinathkr-nv ping :) |
|
Sorry for the delay; I have previously noticed your comments but I haven't been able to get around to addressing them. I'll try doing that next week. |
|
I have added new commits to this PR to address some of the main issues with the profile and level determination logic. This essentially reverses the direction I had taken in the earlier commits in this series, so I will be reworking this set of changes again to have a clean history. |
|
Thanks for this set of patches, I confirm that capabilities is retrieved before determining the right profile and level. Could you please squash the commit history in order to avoid the intermediate non working state ? |
Great, thank you for trying it out and confirming that it works for you. I was waiting for this confirmation before I reorganized the commits; I will work on this part next. |
This makes it easier to read and compare these limits against the data from table A-1 of the H.264 specification (from where these limits have been copied), and to add new entries if required in the future. Signed-off-by: Srinath Kumarapuram <[email protected]>
The values in this column have been taken from table A-4 of the H.264 specification and specify the value to be used for the SPS frame_mbs_only_flag syntax element for a given level. These values are not used anywhere in the code as interlaced encoding is not supported (and therefore frame_mbs_only_flag = 1 always). Signed-off-by: Srinath Kumarapuram <[email protected]>
These are defined in table A-1 of the H.264 specification but were missing from the level limits table defined in our code. These new entries also allow removing some workarounds where the level was previously artifically limited to level 5.2 as the maximum supported level. Signed-off-by: Srinath Kumarapuram <[email protected]>
The level determination code in EncoderConfigH264::DetermineLevel() uses a multipler of 1200 for the MaxBR and MaxCPB entries in the level limits table, taken from table A-1 of the H.264 specification. However, this multipler is to be used only for the Baseline, Constrained Baseline, Main and Extended profiles. Profiles greater than these use a different (larger) multipler, as specified in table A-2 of the H.264 specification. Due to the use of a smaller multipler, this code selects a greater level than is strictly necessary in some situations, based on the input settings. This commit updates this code to use a multipler dependent on the codec profile. Signed-off-by: Srinath Kumarapuram <[email protected]>
…ion step Currently, the codec level determination logic does not follow a consistent pattern between all the codec implementations. This change introduces DetermineLevelTier() as a pure virtual method on EncoderConfig, called from VkVideoEncoder::InitEncoder() after InitDeviceCapabilities() and before InitDpbCount(). This ensures that device capabilities and preferred settings for the selected quality level are available when determining the codec level. An improvement over the existing codec-specific level determination logic found in some of the codecs is that the new method returns a bool instead of void, to allow signaling failures in determining the appropriate level (as can happen for certain combinations of input parameters). Each codec then provides an overriding implementation for this method: - H.264: the the level computation has been extracted from InitDpbCount() into the new method. - H.265: GetLevelTier() has been renamed to DetermineLevelTier(). Also, instead of returning a struct containing the level and tier, the existing (unused) members for the same purpose in the config structure are now assigned. - AV1: DetermineLevelTier() now returns false instead of clamping to LEVEL_7_3 when the resolution/bitrate exceeds all defined levels. For H.265, in addition, some of the code involved in setting the codec profile has been simplified to remove various conditionals at the time of populating the SPS. Future changes will ensure correct codec profile initialization early on when initializing the encoder configuration. Signed-off-by: Srinath Kumarapuram <[email protected]>
This change is a refactoring of video profile initialization rather than
a plain renaming.
In the prior code, InitVideoProfile() would call
GetDefaultVideoProfileIdc() if videoProfileIdc was not initialized.
However, no code would initialize videoProfileIdc to a proper value, so
GetDefaultVideoProfileIdc() would get called always and return a fixed
value for each codec. This was problematic in some cases as the fixed
codec profile did not necessarily support all of the coding tools
requested by the user when encoding, or would be incompatible with the
codec profile actually needed for encoding.
InitVideoProfile() has been changed to MakeVideoProfile(), a pure
factory that takes the codec profile as an explicit parameter. This
removes the implicit dependency on GetDefaultVideoProfileIdc() and the
videoProfileIdc member, and allows deleting both of these.
Another motivation for this refactoring is that we will require more
flexibility in constructing and updating video profile instances in the
future, so this change is a step towards that goal.
One more improvement with this change is to move the
encodeBitDepth{Luma,Chroma} defaulting into InitializeParameters() so
that it happens before any profile-dependent logic, and there was no
specific reason previously to tie it to the video profile
initialization.
MakeVideoProfile() now gets called from the capability query code, so
the InitDeviceCapabilities() method has also been renamed to
InitVideoProfileCapabilities() to better reflect its new responsibility
of selecting the video profile first and then querying capabilities.
Currently, the codec profile used for the MakeVideoProfile() call is a
default value per codec (HIGH for H.264, MAIN for H.265/AV1). Proper
logic to determine an appropriate profile will be added in a subsequent
commit.
Signed-off-by: Srinath Kumarapuram <[email protected]>
…pabilities Each codec's InitVideoProfileCapabilities() implementation now determines the video profile from the encoding parameters when the user hasn't specified one explicitly. For H.265 and AV1, the logic is straightforward and uses the bit-depth and/or chroma subsampling to auto-select the profile. For H.264, the code also takes into account the number of B-frames, entropy coding mode and tuning mode (all of which can be configured by the user, but some have default values). The auto-selection logic also has been cleaned up a bit because we previously had hard-coded assumptions (like the adaptive transform would always be supported), which would lead to the code choosing a codec profile which may not be supported by the underlying device. The profile auto-selection logic has not been placed in a separate helper because although it would be easy to do this for H.265 and AV1, it would be non-trivial for H.264, where the correct profile determination depends on device capabilities (as will be introduced in a future change), so we need the profile determination logic adjacent to the capabilities query. Signed-off-by: Srinath Kumarapuram <[email protected]>
…bilities This helps the automatic profile selection logic select an appropriate profile based both on the requested features and the codec profiles supported by the device. The prior logic would prefer only the requested features, potentially configuring a codec profile which would not be supported by the device. Signed-off-by: Srinath Kumarapuram <[email protected]>
With this change, the adaptiveTransformMode parameter is either ENABLE or DISABLE at the end of InitVideoProfileCapabilities(), based on hardware support for transform_8x8_mode and the final determined profile. This removes the need for a three-way branch in InitSpsPpsParameters() and simplifies code for setting this flag. Signed-off-by: Srinath Kumarapuram <[email protected]>
Add the missing luma sample rate (MaxLumaSr) check from Annex A of the H.265 specification. The check rejects a level if picSizeInSamples * frameRate exceeds the level's MaxLumaSr limit. Signed-off-by: Srinath Kumarapuram <[email protected]>
e9599e9 to
78befa7
Compare
|
That's great news I will have a deeper look a bit later. |
| } | ||
|
|
||
| return profileTierLevel; | ||
| return (levelIdc <= h265EncodeCapabilities.maxLevelIdc); |
There was a problem hiding this comment.
I've been running some tests for 10-bit support and needed the changes in this PR to run the test encode_h265_main10_profile. While testing this PR on radv (RX7600/VCN 4.0, Mesa 26.2), DetermineLevelTier() fails because radv reports maxLevelIdc = 0 for H265 encode. In Mesa, there are static tables that say level 6.1, but it seems they are overwritten by the kernel's AMDGPU_INFO_VIDEO_CAPS_ENCODE max_level, which is hardcoded to 0.
@lolzballs: do you know where maxLevelIdc should come from? a constant pre-determined by the hardware or queried at runtime (kernel via ioctl)?
There was a problem hiding this comment.
Thanks, https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/43584 should fix it.
Description
This series of commits refactors the code related to the determination of the codec profile and level (and tier, as applicable) for all codecs, to avoid redundant calculations / storage and to provide a single point early in the encoder initialization stage where these values are decided.
Another effect of this refactoring is to use the proper codec profile when creating the video profile used for queries of capabilities and recommended settings. Previously, the video profile constructed for these purposes used a default codec profile, which may not be representative of (or may not support) the coding tools requested.
Type of change
Bug fixes and refactoring
Tests
NVIDIA L2 / NVIDIA 580.94.17 / Ubuntu 24.04.3 LTS
Total Tests: 70
Passed: 50
Crashed: 0
Failed: 0
Not Supported: 2
Skipped: 18 (in skip list)
Success Rate: 100.0%
Additional Details (optional)
Prior to this series of commits, running the test framework would show 1 unsupported test case, and 51 passing test cases. The unsupported test case was
encode_h265_main10_profile, which is actually supported by the NVIDIA driver. The list of passing tests includedencode_av1_high_profileandencode_av1_professional_profilealthough these profiles are not supported.With the current changes,
encode_av1_high_profileandencode_av1_professional_profileare correctly marked as unsupported andencode_h265_main10_profileexecutes successfully.