Skip to content

Commit

Permalink
Merge pull request #789 from Thinkofname/audio-fixes
Browse files Browse the repository at this point in the history
Audio safety fixes
  • Loading branch information
Cobrand authored Oct 9, 2018
2 parents 7351450 + a0734fd commit 2b533d9
Showing 1 changed file with 36 additions and 42 deletions.
78 changes: 36 additions & 42 deletions src/sdl2/audio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ use std::ops::{Deref, DerefMut};
use std::path::Path;
use std::marker::PhantomData;
use std::mem;
use std::mem::transmute;
use std::ptr;

use AudioSubsystem;
Expand Down Expand Up @@ -209,12 +208,16 @@ pub enum AudioStatus {
impl FromPrimitive for AudioStatus {
fn from_i64(n: i64) -> Option<AudioStatus> {
use self::AudioStatus::*;
let n = n as u32;

Some( match unsafe { transmute::<u32, sys::SDL_AudioStatus>(n) } {
sys::SDL_AudioStatus::SDL_AUDIO_STOPPED => Stopped,
sys::SDL_AudioStatus::SDL_AUDIO_PLAYING => Playing,
sys::SDL_AudioStatus::SDL_AUDIO_PAUSED => Paused,
const STOPPED: i64 = sys::SDL_AudioStatus::SDL_AUDIO_STOPPED as i64;
const PLAYING: i64 = sys::SDL_AudioStatus::SDL_AUDIO_PLAYING as i64;
const PAUSED: i64 = sys::SDL_AudioStatus::SDL_AUDIO_PAUSED as i64;

Some(match n {
STOPPED => Stopped,
PLAYING => Playing,
PAUSED => Paused,
_ => return None,
})
}

Expand Down Expand Up @@ -373,13 +376,15 @@ extern "C" fn audio_callback_marshall<CB: AudioCallback>
use std::slice::from_raw_parts_mut;
use std::mem::size_of;
unsafe {
let cb_userdata: &mut CB = &mut *(userdata as *mut CB);
let cb_userdata: &mut Option<CB> = &mut *(userdata as *mut _);
let buf: &mut [CB::Channel] = from_raw_parts_mut(
stream as *mut CB::Channel,
len as usize / size_of::<CB::Channel>()
);

cb_userdata.callback(buf);
if let Some(cb) = cb_userdata {
cb.callback(buf);
}
}
}

Expand All @@ -394,14 +399,13 @@ pub struct AudioSpecDesired {
}

impl AudioSpecDesired {
fn convert_to_ll<CB, F, C, S>(freq: F, channels: C, samples: S, userdata: *mut CB) -> sys::SDL_AudioSpec
fn convert_to_ll<CB, F, C, S>(freq: F, channels: C, samples: S, userdata: *mut Option<CB>) -> sys::SDL_AudioSpec
where
CB: AudioCallback,
F: Into<Option<i32>>,
C: Into<Option<u8>>,
S: Into<Option<u16>>,
{
use std::mem::transmute;

let freq = freq.into();
let channels = channels.into();
Expand All @@ -413,22 +417,20 @@ impl AudioSpecDesired {

// A value of 0 means "fallback" or "default".

unsafe {
sys::SDL_AudioSpec {
freq: freq.unwrap_or(0),
format: <CB::Channel as AudioFormatNum>::audio_format().to_ll(),
channels: channels.unwrap_or(0),
silence: 0,
samples: samples.unwrap_or(0),
padding: 0,
size: 0,
callback: Some(audio_callback_marshall::<CB>
as extern "C" fn
(arg1: *mut c_void,
arg2: *mut uint8_t,
arg3: c_int)),
userdata: transmute(userdata)
}
sys::SDL_AudioSpec {
freq: freq.unwrap_or(0),
format: <CB::Channel as AudioFormatNum>::audio_format().to_ll(),
channels: channels.unwrap_or(0),
silence: 0,
samples: samples.unwrap_or(0),
padding: 0,
size: 0,
callback: Some(audio_callback_marshall::<CB>
as extern "C" fn
(arg1: *mut c_void,
arg2: *mut uint8_t,
arg3: c_int)),
userdata: userdata as *mut _,
}
}

Expand Down Expand Up @@ -596,7 +598,7 @@ pub struct AudioDevice<CB: AudioCallback> {
device_id: AudioDeviceID,
spec: AudioSpec,
/// Store the callback to keep it alive for the entire duration of `AudioDevice`.
userdata: Box<CB>
userdata: Box<Option<CB>>
}

impl<CB: AudioCallback> AudioDevice<CB> {
Expand All @@ -607,14 +609,8 @@ impl<CB: AudioCallback> AudioDevice<CB> {
D: Into<Option<&'a str>>,
{

// SDL_OpenAudioDevice needs a userdata pointer, but we can't initialize the
// callback without the obtained AudioSpec.
// Create an uninitialized box that will be initialized after SDL_OpenAudioDevice.
let userdata: *mut CB = unsafe {
let b: Box<CB> = Box::new(mem::uninitialized());
mem::transmute(b)
};
let desired = AudioSpecDesired::convert_to_ll(spec.freq, spec.channels, spec.samples, userdata);
let mut userdata: Box<Option<CB>> = Box::new(None);
let desired = AudioSpecDesired::convert_to_ll(spec.freq, spec.channels, spec.samples, &mut *userdata);

let mut obtained = unsafe { mem::uninitialized::<sys::SDL_AudioSpec>() };
unsafe {
Expand All @@ -636,10 +632,8 @@ impl<CB: AudioCallback> AudioDevice<CB> {
id => {
let device_id = AudioDeviceID::PlaybackDevice(id);
let spec = AudioSpec::convert_from_ll(obtained);
let mut userdata: Box<CB> = mem::transmute(userdata);

let garbage = mem::replace(&mut userdata as &mut CB, get_callback(spec));
mem::forget(garbage);
*userdata = Some(get_callback(spec));

Ok(AudioDevice {
subsystem: a.clone(),
Expand Down Expand Up @@ -713,7 +707,7 @@ impl<CB: AudioCallback> AudioDevice<CB> {
/// but the callback data will be dropped.
pub fn close_and_get_callback(self) -> CB {
drop(self.device_id);
*self.userdata
self.userdata.expect("Missing callback")
}
}

Expand All @@ -725,11 +719,11 @@ pub struct AudioDeviceLockGuard<'a, CB> where CB: AudioCallback, CB: 'a {

impl<'a, CB: AudioCallback> Deref for AudioDeviceLockGuard<'a, CB> {
type Target = CB;
fn deref(&self) -> &CB { &self.device.userdata }
fn deref(&self) -> &CB { (*self.device.userdata).as_ref().expect("Missing callback") }
}

impl<'a, CB: AudioCallback> DerefMut for AudioDeviceLockGuard<'a, CB> {
fn deref_mut(&mut self) -> &mut CB { &mut self.device.userdata }
fn deref_mut(&mut self) -> &mut CB { (*self.device.userdata).as_mut().expect("Missing callback") }
}

impl<'a, CB: AudioCallback> Drop for AudioDeviceLockGuard<'a, CB> {
Expand Down Expand Up @@ -835,7 +829,7 @@ mod test {
assert_eq!(new_buffer.len(), new_buffer_expected.len(), "capacity must be exactly equal to twice the original vec size");

// // this has been commented, see https://discourse.libsdl.org/t/change-of-behavior-in-audiocvt-sdl-convertaudio-from-2-0-5-to-2-0-6/24682
// // to maybe re-enable it someday
// // to maybe re-enable it someday
// assert_eq!(new_buffer, new_buffer_expected);
}
}

0 comments on commit 2b533d9

Please sign in to comment.