Skip to content

Commit

Permalink
Only cap overflow if it's actually a danger.
Browse files Browse the repository at this point in the history
In the normal case (when we're not in danger of wasting bits
 because we have not used enough), soft_limit is negative.
As a result, log_soft_limit was being set to -1.
Normally this is fine, since we then compare it against
 (log_scale_pixels - log_q_exp), the log of the estimated number of
 bits that will be used, which is usually positive.
However, when the number of pixels is very small (e.g., for a 1x1
 video), it is possible for (log_scale_pixels - log_q_exp) to be
 negative, and we will try to enforce the limit anyway.
This results in very bad things (integer overflows) when we try.

This commit also corrects a parentheses error that was introduced
 when porting the original implementation to rav1e (the division by
 margin should happen before the multiply, which can also cause
 overflows).
  • Loading branch information
Timothy B. Terriberry authored and YaLTeR committed Sep 3, 2019
1 parent d512dc8 commit 6b9f362
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 15 deletions.
60 changes: 60 additions & 0 deletions src/api/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1602,3 +1602,63 @@ fn rdo_lookahead_frames_overflow() {
let res: Result<Context<u8>, _> = config.new_context();
assert!(res.is_err());
}

#[test]
fn log_q_exp_overflow() {
let config = Config {
enc: EncoderConfig {
width: 1,
height: 1,
bit_depth: 8,
chroma_sampling: ChromaSampling::Cs420,
chroma_sample_position: ChromaSamplePosition::Unknown,
pixel_range: PixelRange::Limited,
color_description: None,
mastering_display: None,
content_light: None,
still_picture: false,
time_base: Rational { num: 1, den: 25 },
min_key_frame_interval: 12,
max_key_frame_interval: 240,
reservoir_frame_delay: None,
low_latency: false,
quantizer: 100,
min_quantizer: 64,
bitrate: 1,
tune: Tune::Psychovisual,
tile_cols: 0,
tile_rows: 0,
tiles: 0,
rdo_lookahead_frames: 40,
speed_settings: SpeedSettings {
min_block_size: BlockSize::BLOCK_64X64,
multiref: false,
fast_deblock: true,
reduced_tx_set: true,
tx_domain_distortion: true,
tx_domain_rate: false,
encode_bottomup: false,
rdo_tx_decision: false,
prediction_modes: PredictionModesSetting::Simple,
include_near_mvs: false,
no_scene_detection: true,
diamond_me: true,
cdef: true,
quantizer_rdo: false,
use_satd_subpel: false,
},
show_psnr: false,
train_rdo: false,
},
threads: 1,
};

let mut ctx: Context<u8> = config.new_context().unwrap();
for _ in 0..2 {
ctx.send_frame(ctx.new_frame()).unwrap();
}
ctx.flush();

ctx.receive_packet().unwrap();
let _ = ctx.receive_packet();
}
32 changes: 17 additions & 15 deletions src/rate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1004,21 +1004,23 @@ impl RCState {
// We want to use at least this many bits next frame.
let soft_limit = self.reservoir_fullness + self.bits_per_tu
- (self.reservoir_max - margin);
let log_soft_limit = blog64(soft_limit);
// If we're predicting we won't use that many bits...
// TODO: When using frame re-ordering, we should include the rate
// for all of the frames in the current TU.
// When there is more than one frame, there will be no direct
// solution for the required adjustment, however.
let log_scale_pixels = log_cur_scale + self.log_npixels;
let exp = self.exp[fti] as i64;
let mut log_q_exp = ((log_q + 32) >> 6) * exp;
if log_scale_pixels - log_q_exp < log_soft_limit {
// Scale the adjustment based on how far into the margin we are.
log_q_exp += ((log_scale_pixels - log_soft_limit - log_q_exp) >> 32)
* (margin.min(soft_limit) << 32)
/ margin;
log_q = ((log_q_exp + (exp >> 1)) / exp) << 6;
if soft_limit > 0 {
let log_soft_limit = blog64(soft_limit);
// If we're predicting we won't use that many bits...
// TODO: When using frame re-ordering, we should include the rate
// for all of the frames in the current TU.
// When there is more than one frame, there will be no direct
// solution for the required adjustment, however.
let log_scale_pixels = log_cur_scale + self.log_npixels;
let exp = self.exp[fti] as i64;
let mut log_q_exp = ((log_q + 32) >> 6) * exp;
if log_scale_pixels - log_q_exp < log_soft_limit {
// Scale the adjustment based on how far into the margin we are.
log_q_exp += ((log_scale_pixels - log_soft_limit - log_q_exp)
>> 32)
* ((margin.min(soft_limit) << 32) / margin);
log_q = ((log_q_exp + (exp >> 1)) / exp) << 6;
}
}
}
// We just checked we don't overflow the reservoir next frame, now
Expand Down

0 comments on commit 6b9f362

Please sign in to comment.