From 36be4f7f06906dec1d7c66166bcb290b3105bed2 Mon Sep 17 00:00:00 2001 From: John Koleszar Date: Wed, 2 Mar 2011 17:02:44 -0500 Subject: [PATCH] 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 --- vp8/encoder/firstpass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vp8/encoder/firstpass.c b/vp8/encoder/firstpass.c index 964e68ab0..e0b2a8a45 100644 --- a/vp8/encoder/firstpass.c +++ b/vp8/encoder/firstpass.c @@ -2494,7 +2494,7 @@ void vp8_find_next_key_frame(VP8_COMP *cpi, FIRSTPASS_STATS *this_frame) } // 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);