ref: 36be4f7f06906dec1d7c66166bcb290b3105bed2
parent: 6f5189c044cf52171822f6df1ad6ae4a3bcaf2c4
author: John Koleszar <jkoleszar@google.com>
date: Wed Mar 2 12:02:44 EST 2011
Fix drastic undershoot in long form content When the modified_error_left accumulator exceeds INT_MAX, an incorrect cast to int resulted in a negative value, causing the rate control to allocate no bits to that keyframe group, leading to severe undershoot and subsequent poor quality. This error was exposed by the recent change to the rolling target and actual spend accumulators in commit 305be4e4 which fixed them to actually calculate the average value rather than be re-initialized on every frame to the average per-frame bitrate. When this bug was triggered, the target bitrate could be 0, so the rolling target becomes small, which causes the undershoot. The code prior to 305be4e4 did not exhibit this behavior because the rolling target was always set to a reasonable value and was independent of the actual target bitrate. With this patch, the actual target bitrate is calculated correctly, and the rate control tracks as expected. This cast was likely added to silence a compiler warning on a comparison between a double (modified_error_left) and an int (0). Instead, this patch removes the cast and changes the comparison to be against 0.0, which should prevent the warning from reoccuring. This fixes issue #289. Special thanks to gnafu for his efforts in reporting and debugging this fix. Change-Id: Ie5cc1a7b516c578a76c3a50c892a6f04a11621fe
--- a/vp8/encoder/firstpass.c
+++ b/vp8/encoder/firstpass.c
@@ -2494,7 +2494,7 @@
}
// Calculate the number of bits that should be assigned to the kf group.
- if ((cpi->bits_left > 0) && ((int)cpi->modified_error_left > 0))
+ if ((cpi->bits_left > 0) && (cpi->modified_error_left > 0.0))
{// Max for a single normal frame (not key frame)
int max_bits = frame_max_bits(cpi);