Skip to content

Commit cded54e

Browse files
committed
Fix input callback buffer resizing
This is related to #90. I have an issue on the iOS simulator with `kAudio_ParamError` validation errors complaining about mismatches in the buffer sizes. I have identified an issue around `buffer.mDataByteSize` not matching the input frames parameter. ```rust // render_callback.rs # 638 let ptr = buffer.mData as *mut u8; let len = buffer.mDataByteSize as usize; let cap = len; let mut vec = Vec::from_raw_parts(ptr, len, cap); match len.cmp(&data_byte_size) { Ordering::Greater => { vec.truncate(data_byte_size); } Ordering::Less => { vec.reserve_exact(data_byte_size - len); } Ordering::Equal => {} } mem::forget(vec); ``` Additionally, the`Vec::truncate` and `Vec::reserve_exact` don't seem will do good things. * `Vec::truncate` will only run the destructors for the extra entries; since it's a `Vec<u8>` it should noop. I believe this can be deleted * `Vec::reserve_exact` would de-allocate the buffer, which would cause use after free somewhere when `mData` is used On this same file `mData: data.as_mut_ptr() as *mut _,` the audio buffer pointer is set to the internal `Vec` pointer. That pointer will be deallocated on reserve/resize if the Vec is under capacity. `Vec::from_raw_parts` `cap` is reset to `len` every time, which doesn't seem right. If the buffer shrinks then grows (which is what seems to happen for me using the iOS simulator), then this block is always allocating a new buffer (while the one actually in use is dangling). When `Vec::from_raw_parts(ptr, len, cap).reserve(10000)` is called, that `mData` pointer is now dangling. This commit modifies this block so that _if the buffers are resized_: * `mData` pointer is updated to point at a valid memory location * `mDataByteSize` is updated with the current frame count * Capacity is preserved between runs so that the buffer isn't re-allocated often - Note, this depends on a single `buffer_capacity`, so this code assumes there's only 1 buffer to resize
1 parent a1930b4 commit cded54e

File tree

1 file changed

+15
-17
lines changed

1 file changed

+15
-17
lines changed

src/audio_unit/render_callback.rs

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use super::audio_format::LinearPcmFlags;
22
use super::{AudioUnit, Element, Scope};
33
use crate::error::{self, Error};
4-
use std::cmp::Ordering;
54
use std::mem;
65
use std::os::raw::c_void;
76
use std::slice;
@@ -580,12 +579,14 @@ impl AudioUnit {
580579

581580
let data_byte_size = buffer_frame_size * sample_bytes as u32 * n_channels;
582581
let mut data = vec![0u8; data_byte_size as usize];
582+
let mut buffer_capacity = data_byte_size as usize;
583583
let audio_buffer = sys::AudioBuffer {
584584
mDataByteSize: data_byte_size,
585585
mNumberChannels: n_channels,
586586
mData: data.as_mut_ptr() as *mut _,
587587
};
588588
// Relieve ownership of the `Vec` until we're ready to drop the `AudioBufferList`.
589+
// TODO: This leaks the len & capacity fields, since only the buffer pointer is released
589590
mem::forget(data);
590591

591592
let audio_buffer_list = Box::new(sys::AudioBufferList {
@@ -627,24 +628,21 @@ impl AudioUnit {
627628
let n_channels = stream_format.channels;
628629
let data_byte_size =
629630
in_number_frames as usize * sample_bytes * n_channels as usize;
630-
let ptr = (*audio_buffer_list_ptr).mBuffers.as_ptr() as *const sys::AudioBuffer;
631+
let ptr = (*audio_buffer_list_ptr).mBuffers.as_ptr() as *mut sys::AudioBuffer;
631632
let len = (*audio_buffer_list_ptr).mNumberBuffers as usize;
632633

633-
let buffers: &[sys::AudioBuffer] = slice::from_raw_parts(ptr, len);
634-
for &buffer in buffers {
635-
let ptr = buffer.mData as *mut u8;
636-
let len = buffer.mDataByteSize as usize;
637-
let cap = len;
638-
let mut vec = Vec::from_raw_parts(ptr, len, cap);
639-
match len.cmp(&data_byte_size) {
640-
Ordering::Greater => {
641-
vec.truncate(data_byte_size);
642-
}
643-
Ordering::Less => {
644-
vec.reserve_exact(data_byte_size - len);
645-
}
646-
Ordering::Equal => {}
647-
}
634+
let buffers: &mut [sys::AudioBuffer] = slice::from_raw_parts_mut(ptr, len);
635+
let old_capacity = buffer_capacity;
636+
for buffer in buffers {
637+
let current_len = buffer.mDataByteSize as usize;
638+
let audio_buffer_ptr = buffer.mData as *mut u8;
639+
let mut vec: Vec<u8> =
640+
Vec::from_raw_parts(audio_buffer_ptr, current_len, old_capacity);
641+
vec.resize(data_byte_size, 0u8);
642+
643+
buffer_capacity = vec.capacity();
644+
buffer.mData = vec.as_mut_ptr() as *mut _;
645+
buffer.mDataByteSize = data_byte_size as u32;
648646
mem::forget(vec);
649647
}
650648
}

0 commit comments

Comments
 (0)