encode: codec profile and level fixes - #171
Conversation
srinathkr-nv
changed the title
49f442f to
4fba129
Compare
srinathkr-nv
changed the title
srinathkr-nv
changed the title
|
@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. |
e9599e9 to
78befa7
Compare
|
That's great news I will have a deeper look a bit later. |
|
@srinathkr-nv could you please review the history of commits and shorten a bit ? See my comment #171 (comment) |
dabrain34
left a comment
dabrain34
left a comment
There was a problem hiding this comment.
please squash some commits
3a82d42 to
7212773
Compare
|
I have now folded four of the original commits which touched the the H.264 level determination code into a single commit. The commits a409e1f, 5886e91, c8863a2 and 1827c29 fundamentally re-architect the code involving profile and level determination and should be kept as they are. That leaves the remaining two commits in this PR, which are intentionally kept separate because it is awkward to squash them into any of the other commits (and they have a very specific focus area). |
|
We dont use Regarding the history, the 4 patches does not exist anymore, so I dont know what are the patches you'd like to keep. I would squash
And then clarify a bit the commit message that we are improving the level selection and then the profile selection. |
| profileIdc = STD_VIDEO_H264_PROFILE_IDC_BASELINE; | ||
|
|
||
| // Upgrade to MAIN profile if using B-frames or CABAC entropy coding | ||
| if ((gopStructure.GetConsecutiveBFrameCount() > 0) || (entropyCodingMode == ENTROPY_CODING_MODE_CABAC)) |
There was a problem hiding this comment.
by default GetConsecutiveBFrameCount will be initialized to CONSECUTIVE_B_FRAME_COUNT_MAX_VALUE which is 255. So I would first validate that you want BFrameCount with the command line in order to select the right profile according to this. Otherwise it will never be STD_VIDEO_H264_PROFILE_IDC_BASELINE
you could for example add the line
const uint8_t bFrameCount = gopStructure.GetConsecutiveBFrameCount();
const bool wantsBFrames = (bFrameCount != CONSECUTIVE_B_FRAME_COUNT_MAX_VALUE) && (bFrameCount > 0);
And use wantsBFrames to validate that b-frame has been selected
There was a problem hiding this comment.
I will see if there's a better way to do this; if not, I'll apply your suggestion to the code.
| if ((tuningMode == VK_VIDEO_ENCODE_TUNING_MODE_LOSSLESS_KHR) || | ||
| (input.chromaSubsampling == VK_VIDEO_CHROMA_SUBSAMPLING_444_BIT_KHR)) { | ||
| minProfile = STD_VIDEO_H264_PROFILE_IDC_HIGH_444_PREDICTIVE; | ||
| } else if (gopStructure.GetConsecutiveBFrameCount() > 0) { |
There was a problem hiding this comment.
you should validate that b-frame has been validated by command line argument and not CONSECUTIVE_B_FRAME_COUNT_MAX_VALUE
| StdVideoH264ProfileIdc minProfile = STD_VIDEO_H264_PROFILE_IDC_BASELINE; | ||
| if ((tuningMode == VK_VIDEO_ENCODE_TUNING_MODE_LOSSLESS_KHR) || | ||
| (input.chromaSubsampling == VK_VIDEO_CHROMA_SUBSAMPLING_444_BIT_KHR)) { | ||
| minProfile = STD_VIDEO_H264_PROFILE_IDC_HIGH_444_PREDICTIVE; |
There was a problem hiding this comment.
Here STD_VIDEO_H264_PROFILE_IDC_HIGH should be selected if
f ((encodeBitDepthLuma > 8) ||
(encodeChromaSubsampling != VK_VIDEO_CHROMA_SUBSAMPLING_420_BIT_KHR))
There was a problem hiding this comment.
The HIGH profile for H.264 does not support bit-depth > 8 and a chroma subsampling other than 4:2:0 and 4:0:0. The correct profile for those cases would be the HIGH_10 and HIGH_422 profiles, but correctly using those profiles will require additional changes beyond just the profile- and level-related fixes which I'm making here.
The current formatting of the level limits table makes it hard to read and compare against the source for the values within (i.e. table A-1 of the H.264 specification). Further, we have a "prog" column in this table whose entries 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, but these values are not used anywhere in the code as interlaced encoding is not supported (and therefore frame_mbs_only_flag = 1 always). This change adds some whitespace to align the columns of this table and make it more readable, while removing the unused "prog" column. When editing the table, it was noticed that no entries were present for levels 6.0, 6.1, 6.2 defined in table A-1. These entries have been added now, and they allow removing some workarounds where the level was previously artificially limited to level 5.2 as the maximum supported level. Finally, this change also fixes a bug in the level determination code in EncoderConfigH264::DetermineLevel(). This code uses a fixed multiplier equal to 1200 for the MaxBR and MaxCPB entries in the level limits table, taken from table A-1 of the H.264 specification, but this multiplier is to be used only for the Baseline, Constrained Baseline, Main and Extended profiles. Profiles greater than these use a larger multiplier, as specified in table A-2 of the H.264 specification. Due to the use of a smaller multiplier, 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 multiplier dependent on the codec profile.
…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.
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.
…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.
…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.
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.
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.
7212773 to
20ee9f7
Compare
4 participants
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.