Skip to content

/claxon forked from ruuda/claxon

Permalink
Browse files

Rewrite passing a slice with uninitialized memory into appending to a… 

… preallocated Vec with length 0. This rules out the possibility of reading from uninitialized memory. Just one problem: this fails some decoding tests. I've probably messed up some math somewhere.
  • Loading branch information...
@Shnatsel
Shnatsel committed Sep 15, 2018
1 parent 4607031 commit 73c7e9a4c9795cdab0af10cdb7909b48bf8204c9
Unified Split
Showing with 75 additions and 96 deletions.
  1. +26 −50 src/frame.rs
  2. +49 −46 src/subframe.rs
View file
76 src/frame.rs
@@ -609,39 +609,19 @@ pub struct FrameReader<R: ReadBytes> {
pub type FrameResult = Result<Option<Block>>;

/// A function to expand the length of a buffer, or replace the buffer altogether,
/// so it can hold at least `new_len` elements. The contents of the buffer can
/// be anything, it is assumed they will be overwritten anyway.
///
/// To use this function safely, the caller must overwrite all `new_len` bytes.
unsafe fn ensure_buffer_len(mut buffer: Vec<i32>, new_len: usize) -> Vec<i32> {
if buffer.len() < new_len {
// Previous data will be overwritten, so instead of resizing the
// vector if it is too small, we might as well allocate a new one.
if buffer.capacity() < new_len {
buffer = Vec::with_capacity(new_len);
}

// We are going to fill the buffer anyway, so there is no point in
// initializing it with default values. This does mean that there could
// be garbage in the buffer, therefore this function is unsafe.
buffer.set_len(new_len);
/// so it can hold at least `new_capacity` elements without reallocating.
/// The length of returned buffer is guaranteed to be zero.
fn ensure_buffer_capacity(mut buffer: Vec<i32>, new_capacity: usize) -> Vec<i32> {
// Previous data will be overwritten, so instead of resizing the
// vector if it is too small, we might as well allocate a new one.
if buffer.capacity() < new_capacity {
buffer = Vec::with_capacity(new_capacity);
} else {
buffer.truncate(new_len);
buffer.truncate(0);
}
buffer
}

#[test]
fn ensure_buffer_len_returns_buffer_with_new_len() {
for capacity in 0..10 {
for new_len in 0..10 {
let buffer = Vec::with_capacity(capacity);
let resized = unsafe { ensure_buffer_len(buffer, new_len) };
assert_eq!(resized.len(), new_len);
}
}
}

impl<R: ReadBytes> FrameReader<R> {
/// Creates a new frame reader that will yield at least one element.
pub fn new(input: R) -> FrameReader<R> {
@@ -675,10 +655,8 @@ impl<R: ReadBytes> FrameReader<R> {
// decoded.
let total_samples = header.channels() as usize * header.block_size as usize;

// Ensure the buffer is the right size to hold all samples, potentially
// allocating a new buffer without initializing the memory. From here
// on, we must be careful to overwrite each byte in the buffer.
buffer = unsafe { ensure_buffer_len(buffer, total_samples) };
// Ensure the buffer is the right capacity to hold all samples
buffer = ensure_buffer_capacity(buffer, total_samples);

let bps = match header.bits_per_sample {
Some(x) => x,
@@ -696,44 +674,42 @@ impl<R: ReadBytes> FrameReader<R> {
// we need a bitstream. Then we can decode subframes from the bitstream.
{
let mut bitstream = Bitstream::new(&mut crc_input);
let bs = header.block_size as usize;
let bs = header.block_size;

match header.channel_assignment {
ChannelAssignment::Independent(n_ch) => {
for ch in 0..n_ch as usize {
try!(subframe::decode(&mut bitstream,
bps,
&mut buffer[ch * bs..(ch + 1) * bs]));
for _ch in 0..n_ch {
try!(subframe::decode(&mut bitstream, bps, &mut buffer, bs));
}
debug_assert_eq!(buffer.len(), bs as usize * n_ch as usize);
}
ChannelAssignment::LeftSideStereo => {
// The side channel has one extra bit per sample.
try!(subframe::decode(&mut bitstream, bps, &mut buffer[..bs]));
try!(subframe::decode(&mut bitstream,
bps + 1,
&mut buffer[bs..bs * 2]));
try!(subframe::decode(&mut bitstream, bps, &mut buffer, bs));
try!(subframe::decode(&mut bitstream, bps + 1, &mut buffer, bs));
debug_assert_eq!(buffer.len(), bs as usize * 2);

// Then decode the side channel into the right channel.
decode_left_side(&mut buffer[..bs * 2]);
decode_left_side(&mut buffer.as_mut_slice());
}
ChannelAssignment::RightSideStereo => {
// The side channel has one extra bit per sample.
try!(subframe::decode(&mut bitstream, bps + 1, &mut buffer[..bs]));
try!(subframe::decode(&mut bitstream, bps, &mut buffer[bs..bs * 2]));
try!(subframe::decode(&mut bitstream, bps + 1, &mut buffer, bs));
try!(subframe::decode(&mut bitstream, bps, &mut buffer, bs));
debug_assert_eq!(buffer.len(), bs as usize * 2);

// Then decode the side channel into the left channel.
decode_right_side(&mut buffer[..bs * 2]);
decode_right_side(buffer.as_mut_slice());
}
ChannelAssignment::MidSideStereo => {
// Decode mid as the first channel, then side with one
// extra bitp per sample.
try!(subframe::decode(&mut bitstream, bps, &mut buffer[..bs]));
try!(subframe::decode(&mut bitstream,
bps + 1,
&mut buffer[bs..bs * 2]));
try!(subframe::decode(&mut bitstream, bps, &mut buffer, bs));
try!(subframe::decode(&mut bitstream, bps + 1, &mut buffer, bs));
debug_assert_eq!(buffer.len(), bs as usize * 2);

// Then decode mid-side channel into left-right.
decode_mid_side(&mut buffer[..bs * 2]);
decode_mid_side(&mut buffer.as_mut_slice());
}
}

View file
95 src/subframe.rs
@@ -183,7 +183,8 @@ fn verify_rice_to_signed() {
/// It is assumed that the length of the buffer is the block size.
pub fn decode<R: ReadBytes>(input: &mut Bitstream<R>,
bps: u32,
buffer: &mut [i32])
buffer: &mut Vec<i32>,
bs: u16)
-> Result<()> {
// The sample type i32 should be wide enough to accomodate for all bits of
// the stream, but this can be verified at a higher level than here. Still,
@@ -204,10 +205,10 @@ pub fn decode<R: ReadBytes>(input: &mut Bitstream<R>,
let sf_bps = bps - header.wasted_bits_per_sample;

match header.sf_type {
SubframeType::Constant => try!(decode_constant(input, sf_bps, buffer)),
SubframeType::Verbatim => try!(decode_verbatim(input, sf_bps, buffer)),
SubframeType::Fixed(ord) => try!(decode_fixed(input, sf_bps, ord as u32, buffer)),
SubframeType::Lpc(ord) => try!(decode_lpc(input, sf_bps, ord as u32, buffer)),
SubframeType::Constant => try!(decode_constant(input, sf_bps, buffer, bs)),
SubframeType::Verbatim => try!(decode_verbatim(input, sf_bps, buffer, bs)),
SubframeType::Fixed(ord) => try!(decode_fixed(input, sf_bps, u16::from(ord), buffer, bs)),
SubframeType::Lpc(ord) => try!(decode_lpc(input, sf_bps, u16::from(ord), buffer, bs)),
}

// Finally, everything must be shifted by 'wasted bits per sample' to
@@ -235,7 +236,8 @@ enum RicePartitionType {

fn decode_residual<R: ReadBytes>(input: &mut Bitstream<R>,
block_size: u16,
buffer: &mut [i32])
buffer: &mut Vec<i32>,
out_len: u16)
-> Result<()> {
// Residual starts with two bits of coding method.
let partition_type = match try!(input.read_leq_u8(2)) {
@@ -268,7 +270,7 @@ fn decode_residual<R: ReadBytes>(input: &mut Bitstream<R>,
// equivalent but more expensive.
debug_assert_eq!(n_partitions * n_samples_per_partition as u32, block_size as u32);

let n_warm_up = block_size - buffer.len() as u16;
let n_warm_up = block_size - out_len;

// The partition size must be at least as big as the number of warm-up
// samples, otherwise the size of the first partition is negative.
@@ -279,22 +281,16 @@ fn decode_residual<R: ReadBytes>(input: &mut Bitstream<R>,
// Finally decode the partitions themselves.
match partition_type {
RicePartitionType::Rice => {
let mut start = 0;
let mut len = n_samples_per_partition - n_warm_up;
for _ in 0..n_partitions {
let slice = &mut buffer[start..start + len as usize];
try!(decode_rice_partition(input, slice));
start = start + len as usize;
try!(decode_rice_partition(input, buffer, len));
len = n_samples_per_partition;
}
}
RicePartitionType::Rice2 => {
let mut start = 0;
let mut len = n_samples_per_partition - n_warm_up;
for _ in 0..n_partitions {
let slice = &mut buffer[start..start + len as usize];
try!(decode_rice2_partition(input, slice));
start = start + len as usize;
try!(decode_rice2_partition(input, buffer, len));
len = n_samples_per_partition;
}
}
@@ -308,7 +304,8 @@ fn decode_residual<R: ReadBytes>(input: &mut Bitstream<R>,
// function into decode_residual.
#[inline(always)]
fn decode_rice_partition<R: ReadBytes>(input: &mut Bitstream<R>,
buffer: &mut [i32])
buffer: &mut Vec<i32>,
len: u16)
-> Result<()> {
// A Rice partition (not Rice2), starts with a 4-bit Rice parameter.
let rice_param = try!(input.read_leq_u8(4)) as u32;
@@ -334,16 +331,16 @@ fn decode_rice_partition<R: ReadBytes>(input: &mut Bitstream<R>,
// (measured from real-world FLAC files).

if rice_param <= 8 {
for sample in buffer.iter_mut() {
for _ in 0..len {
let q = try!(input.read_unary());
let r = try!(input.read_leq_u8(rice_param)) as u32;
*sample = rice_to_signed((q << rice_param) | r);
buffer.push(rice_to_signed((q << rice_param) | r));
}
} else {
for sample in buffer.iter_mut() {
for _ in 0..len {
let q = try!(input.read_unary());
let r = try!(input.read_gt_u8_leq_u16(rice_param));
*sample = rice_to_signed((q << rice_param) | r);
buffer.push(rice_to_signed((q << rice_param) | r));
}
}

@@ -356,7 +353,8 @@ fn decode_rice_partition<R: ReadBytes>(input: &mut Bitstream<R>,
#[inline(never)]
#[cold]
fn decode_rice2_partition<R: ReadBytes>(input: &mut Bitstream<R>,
buffer: &mut [i32])
buffer: &mut Vec<i32>,
len: u16)
-> Result<()> {
// A Rice2 partition, starts with a 5-bit Rice parameter.
let rice_param = try!(input.read_leq_u8(5)) as u32;
@@ -366,37 +364,41 @@ fn decode_rice2_partition<R: ReadBytes>(input: &mut Bitstream<R>,
return Err(Error::Unsupported("unencoded binary is not yet implemented"))
}

for sample in buffer.iter_mut() {
for _ in 0..len {
// First part of the sample is the quotient, unary encoded.
let q = try!(input.read_unary());

// Next is the remainder, in rice_param bits. Because at this
// point rice_param is at most 30, we can safely read into a u32.
let r = try!(input.read_leq_u32(rice_param));
*sample = rice_to_signed((q << rice_param) | r);
buffer.push(rice_to_signed((q << rice_param) | r));
}

Ok(())
}

fn decode_constant<R: ReadBytes>(input: &mut Bitstream<R>,
bps: u32,
buffer: &mut [i32])
buffer: &mut Vec<i32>,
bs: u16)
-> Result<()> {
let sample_u32 = try!(input.read_leq_u32(bps));
let sample = extend_sign_u32(sample_u32, bps);

for s in buffer {
*s = sample;
for _ in 0..bs {
buffer.push(sample);
}
// TODO: compare performance with iterator version:
//buffer.extend(iter::repeat(sample).take(bs));

Ok(())
}

#[cold]
fn decode_verbatim<R: ReadBytes>(input: &mut Bitstream<R>,
bps: u32,
buffer: &mut [i32])
buffer: &mut Vec<i32>,
bs: u16)
-> Result<()> {

// This function must not be called for a sample wider than the sample type.
@@ -407,14 +409,17 @@ fn decode_verbatim<R: ReadBytes>(input: &mut Bitstream<R>,
debug_assert!(bps <= 32);

// A verbatim block stores samples without encoding whatsoever.
for s in buffer {
*s = extend_sign_u32(try!(input.read_leq_u32(bps)), bps);
for _ in 0..bs {
buffer.push(extend_sign_u32(try!(input.read_leq_u32(bps)), bps));
}
// TODO: This would have been a whole lot easier if input would've been
// a proper iterator instead of repeatedly calling a function with `try!`
// which is basically the same thing, but ad-hoc and poorly optimizable

Ok(())
}

fn predict_fixed(order: u32, buffer: &mut [i32]) -> Result<()> {
fn predict_fixed(order: u16, buffer: &mut [i32]) -> Result<()> {
// When this is called during decoding, the order as read from the subframe
// header has already been verified, so it is safe to assume that
// 0 <= order <= 4. Still, it is good to state that assumption explicitly.
@@ -491,26 +496,25 @@ fn verify_predict_fixed() {

fn decode_fixed<R: ReadBytes>(input: &mut Bitstream<R>,
bps: u32,
order: u32,
buffer: &mut [i32])
order: u16,
buffer: &mut Vec<i32>,
bs: u16)
-> Result<()> {
// The length of the buffer which is passed in, is the length of the block.
// Thus, the number of warm-up samples must not exceed that length.
if buffer.len() < order as usize {
if bs < order {
return fmt_err("invalid fixed subframe, order is larger than block size")
}

// There are order * bits per sample unencoded warm-up sample bits.
try!(decode_verbatim(input, bps, &mut buffer[..order as usize]));
try!(decode_verbatim(input, bps, buffer, order));

// Next up is the residual. We decode into the buffer directly, the
// predictor contributions will be added in a second pass. The first
// `order` samples have been decoded already, so continue after that.
try!(decode_residual(input,
buffer.len() as u16,
&mut buffer[order as usize..]));
try!(decode_residual(input, bs, buffer, bs - order));

try!(predict_fixed(order, buffer));
try!(predict_fixed(order, buffer.as_mut_slice()));

Ok(())
}
@@ -604,21 +608,22 @@ fn verify_predict_lpc() {

fn decode_lpc<R: ReadBytes>(input: &mut Bitstream<R>,
bps: u32,
order: u32,
buffer: &mut [i32])
order: u16,
buffer: &mut Vec<i32>,
bs: u16)
-> Result<()> {
// The order minus one fits in 5 bits, so the order is at most 32.
debug_assert!(order <= 32);

// On the frame decoding level it is ensured that the buffer is large
// enough. If it can't even fit the warm-up samples, then there is a frame
// smaller than its lpc order, which is invalid.
if buffer.len() < order as usize {
if bs < order {
return fmt_err("invalid LPC subframe, lpc order is larger than block size")
}

// There are order * bits per sample unencoded warm-up sample bits.
try!(decode_verbatim(input, bps, &mut buffer[..order as usize]));
try!(decode_verbatim(input, bps, buffer, order));

// Next are four bits quantised linear predictor coefficient precision - 1.
let qlp_precision = try!(input.read_leq_u8(4)) as u32 + 1;
@@ -657,11 +662,9 @@ fn decode_lpc<R: ReadBytes>(input: &mut Bitstream<R>,
// Next up is the residual. We decode it into the buffer directly, the
// predictor contributions will be added in a second pass. The first
// `order` samples have been decoded already, so continue after that.
try!(decode_residual(input,
buffer.len() as u16,
&mut buffer[order as usize..]));
try!(decode_residual(input, bs, buffer, bs - order));

try!(predict_lpc(&coefficients[..order as usize], qlp_shift, buffer));
try!(predict_lpc(&coefficients[..order as usize], qlp_shift, buffer.as_mut_slice()));

Ok(())
}

0 comments on commit 73c7e9a

Please sign in to comment.
You can’t perform that action at this time.