diff --git a/changelog.md b/changelog.md index 5b44271..abe0338 100644 --- a/changelog.md +++ b/changelog.md @@ -6,8 +6,8 @@ Next **Compatibility**: - * Ensures compatibility with Rust 1.36.0 through 1.72.1. This bumps the minimum - supported Rust version from 1.16 to 1.36. + * Ensures compatibility with Rust 1.40.0 through 1.72.1. This bumps the minimum + supported Rust version from 1.16 to 1.40. 3.5.0 ----- diff --git a/rust-toolchain.toml b/rust-toolchain.toml index da9c1dd..072ab04 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -2,4 +2,4 @@ # Note, this is the Minimum Supported Rust Version. We build with this by # default to ensure we don't accidentally break it, but of course you should be # able to build with more recent versions. -channel = "1.36.0" +channel = "1.40.0" diff --git a/src/read.rs b/src/read.rs index 0dc7c57..d424225 100644 --- a/src/read.rs +++ b/src/read.rs @@ -14,7 +14,6 @@ use std::cmp; use std::fs; use std::io; use std::marker; -use std::mem; use std::path; use super::{Error, Result, Sample, SampleFormat, WavSpec, WavSpecEx}; @@ -29,8 +28,8 @@ pub trait ReadExt: io::Read { // TODO: There is an RFC proposing a method like this for the standard library. fn read_into(&mut self, buf: &mut [u8]) -> io::Result<()>; - /// Reads `n` bytes and returns them in a vector. - fn read_bytes(&mut self, n: usize) -> io::Result>; + /// Reads 4 bytes and returns them in an array. + fn read_4_bytes(&mut self) -> io::Result<[u8; 4]>; /// Skip over `n` bytes. fn skip_bytes(&mut self, n: usize) -> io::Result<()>; @@ -110,13 +109,8 @@ impl ReadExt for R } #[inline(always)] - fn read_bytes(&mut self, n: usize) -> io::Result> { - // We allocate a runtime fixed size buffer, and we are going to read - // into it, so zeroing or filling the buffer is a waste. This method - // is safe, because the contents of the buffer are only exposed when - // they have been overwritten completely by the read. - let mut buf = Vec::with_capacity(n); - unsafe { buf.set_len(n); } + fn read_4_bytes(&mut self) -> io::Result<[u8; 4]> { + let mut buf = [0_u8; 4]; try!(self.read_into(&mut buf[..])); Ok(buf) } @@ -192,8 +186,10 @@ impl ReadExt for R } #[inline(always)] - fn read_le_f32(&mut self) -> io::Result { - self.read_le_u32().map(|u| unsafe { mem::transmute(u) }) + fn read_le_f32(&mut self) -> io::Result { + let mut buf = [0u8; 4]; + try!(self.read_into(&mut buf)); + Ok(f32::from_le_bytes(buf)) } } @@ -273,14 +269,14 @@ pub fn read_wave_header(reader: &mut R) -> Result { // into it is more cumbersome, but also avoids a heap allocation. Is // the compiler smart enough to avoid the heap allocation anyway? I // would not expect it to be. - if b"RIFF" != &try!(reader.read_bytes(4))[..] { + if b"RIFF" != &try!(reader.read_4_bytes())[..] { return Err(Error::FormatError("no RIFF tag found")); } let file_len = try!(reader.read_le_u32()); // Next four bytes indicate the file type, which should be WAVE. - if b"WAVE" != &try!(reader.read_bytes(4))[..] { + if b"WAVE" != &try!(reader.read_4_bytes())[..] { return Err(Error::FormatError("no WAVE tag found")); } diff --git a/src/write.rs b/src/write.rs index c585546..743c45a 100644 --- a/src/write.rs +++ b/src/write.rs @@ -14,6 +14,7 @@ use std::fs; use std::io; use std::mem; use std::io::{Seek, Write}; +use std::mem::MaybeUninit; use std::path; use super::{Error, Result, Sample, SampleFormat, WavSpec, WavSpecEx}; use ::read; @@ -114,7 +115,7 @@ impl WriteExt for W #[inline(always)] fn write_le_f32(&mut self, x: f32) -> io::Result<()> { - let u = unsafe { mem::transmute(x) }; + let u = unsafe { mem::transmute::(x) }; self.write_le_u32(u) } } @@ -178,7 +179,7 @@ pub struct WavWriter /// The buffer for the sample writer, which is recycled throughout calls to /// avoid allocating frequently. - sample_writer_buffer: Vec, + sample_writer_buffer: Vec>, /// The offset of the length field of the data chunk. /// @@ -466,7 +467,7 @@ impl WavWriter // We need a bigger buffer. There is no point in growing the old // one, as we are going to overwrite the samples anyway, so just // allocate a new one. - let mut new_buffer = Vec::with_capacity(num_bytes); + let mut new_buffer = Vec::>::with_capacity(num_bytes); // The potentially garbage memory here will not be exposed: the // buffer is only exposed when flushing, but `flush()` asserts that @@ -737,7 +738,7 @@ pub struct SampleWriter16<'parent, W> where W: io::Write + io::Seek + 'parent { writer: &'parent mut W, /// The internal buffer that samples are written to before they are flushed. - buffer: &'parent mut [u8], + buffer: &'parent mut [MaybeUninit], /// Reference to the `data_bytes_written` field of the writer. data_bytes_written: &'parent mut u32, @@ -762,33 +763,20 @@ impl<'parent, W: io::Write + io::Seek> SampleWriter16<'parent, W> { /// Note that nothing is actually written until `flush()` is called. #[inline(always)] pub fn write_sample(&mut self, sample: S) { - assert!((self.index as usize) <= self.buffer.len() - 2, + assert!((self.index as usize) + 2 <= self.buffer.len(), "Trying to write more samples than reserved for the sample writer."); - let s = sample.as_i16() as u16; - - // Write the sample in little endian to the buffer. - self.buffer[self.index as usize] = s as u8; - self.buffer[self.index as usize + 1] = (s >> 8) as u8; - - self.index += 2; + // SAFETY: We performed the bounds check in the above assertion. + unsafe { self.write_sample_unchecked(sample) }; } - #[cfg(target_arch = "x86_64")] unsafe fn write_u16_le_unchecked(&mut self, value: u16) { - // x86_64 is little endian, so we do not need to shuffle bytes around; - // we can just store the 16-bit integer in the buffer directly. - let ptr: *mut u16 = mem::transmute(self.buffer.get_unchecked_mut(self.index as usize)); - *ptr = value; - } - - #[cfg(not(target_arch = "x86_64"))] - unsafe fn write_u16_le_unchecked(&mut self, value: u16) { - // Write a sample in little-endian to the buffer, independent of the - // endianness of the architecture we are running on. - let idx = self.index as usize; - *self.buffer.get_unchecked_mut(idx) = value as u8; - *self.buffer.get_unchecked_mut(idx + 1) = (value >> 8) as u8; + // On little endian machines the compiler produces assembly code + // that merges the following two lines into a single instruction. + *self.buffer.get_unchecked_mut(self.index as usize) = MaybeUninit::new(value as u8); + self.buffer.get_unchecked_mut(self.index as usize).assume_init(); + *self.buffer.get_unchecked_mut(self.index as usize + 1) = MaybeUninit::new((value >> 8) as u8); + self.buffer.get_unchecked_mut(self.index as usize + 1).assume_init(); } /// Like `write_sample()`, but does not perform a bounds check when writing @@ -813,7 +801,15 @@ impl<'parent, W: io::Write + io::Seek> SampleWriter16<'parent, W> { panic!("Insufficient samples written to the sample writer."); } - try!(self.writer.write_all(&self.buffer)); + // SAFETY: casting `self.buffer` to a `*const [MaybeUninit]` is safe since the caller guarantees that + // `self.buffer` is initialized, and `MaybeUninit` is guaranteed to have the same layout as `u8`. + // The pointer obtained is valid since it refers to memory owned by `self.buffer` which is a + // reference and thus guaranteed to be valid for reads. + // This is copied from the nightly implementation for slice_assume_init_ref. + let slice = unsafe { &*(self.buffer as *const [MaybeUninit] as *const [u8]) }; + + try!(self.writer.write_all(slice)); + *self.data_bytes_written += self.buffer.len() as u32; Ok(()) }