Call native BrotliEncoderMaxCompressedSize method for BrotliEncoder.GetMaxCompressedLength #108043
Conversation
…etMaxCompressedLength
ghost
added
the
| public const int Quality_Default = 4; | ||
| public const int Quality_Max = 11; | ||
| public const int MaxInputSize = int.MaxValue - 515; // 515 is the max compressed extra bytes | ||
| public const int MaxInputSize = int.MaxValue - 524_166; // 524_166 is the max compressed extra bytes |
There was a problem hiding this comment.
Where does this number come from?
There was a problem hiding this comment.
Before int.MaxValue - 515 = 2_147_483_132 was producing max compressed length, i.e. GetMaxCompressedLength(2_147_483_132)) returned int.MaxValue. Now this produces negative number
After testing with various values now this value equals 524_166 and GetMaxCompressedLength(int.MaxValue - 524_166))producesint.MaxValue`
The comment is not that clear, but do not have a better version
There was a problem hiding this comment.
Rather than having this hardcoded constant, though, couldn't we just call the native method and then throw ArgumentOutOfRangeException if the result was > int.MaxValue? Part of the point of this change was delegating all behavior to the native function, but we're lifting some of the behavior back up into the C# by hardcoding this constant, which in theory could be invalidated the next time we take a Brotli update.
|
/ba-g test failures in |
2 participants
BrotliEncoder.GetMaxCompressedLengthreturns invalid value for some input because managed implementation differs from native Brotli native implementation that updated many years agoAs discussed in the issue we should use the native implementation since it's the source of truth, and this size needs to remain in sync with what the encoder actually does.
Fixes #35142