Repository navigation
rate control: fix int32 overflow in RC_TIMESTAMP_MODE bit budgets - #4063
Open
Nayana-Naik73 wants to merge 1 commit into
Open
Nayana-Naik73 wants to merge 1 commit into
Nayana-Naik73 wants to merge 1 commit into
Conversation
WelsRcFrameDelayJudgeTimeStamp sizes the skip and padding buffers as iSpatialBitrate * 50 in int arithmetic, which overflows above ~42.9 Mbps. RcDecideTargetBitsTimestamp computes iTlayerWeight * kiGopBits in int32 for every P frame, which overflows once the average frame exceeds ~1.07 Mbit (above ~32 Mbps at 30 fps with one temporal layer). Widen both intermediates to int64_t as cisco#3982 did for the other RC modes and add a timestamp-mode counterpart to HighBitrateIdrTargetBits.
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.
In RC_TIMESTAMP_MODE, WelsRcFrameDelayJudgeTimeStamp sizes the skip and padding buffers as iSpatialBitrate * 50 in int, and RcDecideTargetBitsTimestamp computes iTlayerWeight * kiGopBits in int32 on every P frame, so a 1280x720 encode at 60 Mbps (or 1080p30 at 40 Mbps) hits signed overflow that -fsanitize=undefined reports at ratectl.cpp:1448 and :643. #3982 widened the matching math for the other RC modes, which use the int64 iBitRate, but missed the timestamp path; this widens those intermediates to int64_t the same way and adds HighBitrateTimestampTargetBits next to HighBitrateIdrTargetBits, which fails under UBSan before the change and passes after.