From 57b776194136326ea18489d7179b421c4847f070 Mon Sep 17 00:00:00 2001 From: Andrea Ciliberti Date: Tue, 3 Jan 2023 20:37:46 +0100 Subject: [PATCH] Moved safety measures to WaveInfo rather than Ndsp --- ctru-rs/examples/audio_filters.rs | 24 +++++--------- ctru-rs/src/services/ndsp/mod.rs | 53 ++++++++++++++++++++++--------- ctru-rs/src/services/ndsp/wave.rs | 21 +++++++++--- 3 files changed, 62 insertions(+), 36 deletions(-) diff --git a/ctru-rs/examples/audio_filters.rs b/ctru-rs/examples/audio_filters.rs index dbd9b2a..683f293 100644 --- a/ctru-rs/examples/audio_filters.rs +++ b/ctru-rs/examples/audio_filters.rs @@ -24,13 +24,13 @@ fn fill_buffer(audio_data: &mut [u8], frequency: f32) { let mut i = 0.; for chunk in formatted_data { // This is a simple sine wave, with a frequency of `frequency` Hz, and an amplitude 30% of maximum. - let sample: f32 = (frequency * (i / SAMPLE_RATE as f32) * 2. * PI).sin(); - let amplitude = 0.3 * i16::MAX as f32; + let sample: f32 = (frequency * (i / SAMPLE_RATE as f32) * 2. * PI).sin(); + let amplitude = 0.3 * i16::MAX as f32; // This operation is safe, since we are writing to a slice of exactly 16 bits let chunk_ptr: &mut i16 = unsafe { std::mem::transmute(chunk.as_mut_ptr()) }; // Stereo samples are interleaved: left and right channels. - *chunk_ptr = (sample*amplitude) as i16; + *chunk_ptr = (sample * amplitude) as i16; i += 1.; } @@ -116,10 +116,10 @@ fn main() { let mut update_params = false; if keys_down.intersects(KeyPad::KEY_LEFT) { - let wraps; + let wraps; (filter, wraps) = filter.overflowing_sub(1); - if wraps { + if wraps { filter = filter_names.len() - 1; } @@ -150,22 +150,19 @@ fn main() { } let current: &mut WaveInfo; - let other: &mut WaveInfo; if altern { current = &mut wave_info1; - other = &mut wave_info2; } else { current = &mut wave_info2; - other = &mut wave_info1; } let status = current.get_status(); if let WaveStatus::Done = status { - altern = !altern; + fill_buffer(current.get_mut_wavebuffer().get_mut_data(), NOTEFREQ[note]); + channel_zero.queue_wave(current); - fill_buffer(other.get_mut_wavebuffer().get_mut_data(), NOTEFREQ[note]); - channel_zero.queue_wave(other); + altern = !altern; } // Flush and swap framebuffers @@ -175,9 +172,4 @@ fn main() { //Wait for VBlank gfx.wait_for_vblank(); } - - // Ndsp *has* to be dropped before the WaveInfos, - // otherwise the status won't be flagged properly and the program will panic. - // TODO: Find a way to get away with this using implicitness. - drop(ndsp); } diff --git a/ctru-rs/src/services/ndsp/mod.rs b/ctru-rs/src/services/ndsp/mod.rs index f56912f..f299ce2 100644 --- a/ctru-rs/src/services/ndsp/mod.rs +++ b/ctru-rs/src/services/ndsp/mod.rs @@ -1,7 +1,11 @@ pub mod wave; +use wave::{WaveInfo, WaveStatus}; use crate::error::ResultCode; -use wave::WaveInfo; +use crate::services::ServiceReference; + +use once_cell::sync::Lazy; +use std::sync::Mutex; #[derive(Copy, Clone, Debug)] #[repr(u32)] @@ -35,14 +39,29 @@ pub struct Channel { id: i32, } +static NDSP_ACTIVE: Lazy> = Lazy::new(|| Mutex::new(0)); + #[non_exhaustive] -pub struct Ndsp(()); +pub struct Ndsp { + _service_handler: ServiceReference, +} impl Ndsp { pub fn init() -> crate::Result { - ResultCode(unsafe { ctru_sys::ndspInit() })?; + let _service_handler = ServiceReference::new( + &NDSP_ACTIVE, + false, + || { + ResultCode(unsafe { ctru_sys::ndspInit() })?; - Ok(Self(())) + Ok(()) + }, + || unsafe { + ctru_sys::ndspExit(); + }, + )?; + + Ok(Self { _service_handler }) } /// Return the representation of the specified channel. @@ -66,7 +85,7 @@ impl Ndsp { // All channel operations are thread-safe thanks to `libctru`'s use of thread locks. // As such, there is no need to hold channels to ensure correct mutability. -// With this prospective in mind, this struct looks more like a dummy than an actually functional block. +// As such, this struct is more of a dummy than an actually functional block. impl Channel { /// Reset the channel pub fn reset(&self) { @@ -132,15 +151,23 @@ impl Channel { unsafe { ctru_sys::ndspChnWaveBufClear(self.id) }; } - /// Add a wave buffer to the channel's queue. - /// Note: if there are no other buffers in queue, playback for this buffer will start. + /// Add a wave buffer to the channel's queue. + /// If there are no other buffers in queue, playback for this buffer will start. /// - /// # Beware + /// # Warning /// /// `libctru` expects the user to manually keep the info data (in this case [WaveInfo]) alive during playback. - /// Furthermore, there are no checks to see if the used memory is still valid. All responsibility of handling this data is left to the user. - pub fn queue_wave(&self, buffer: &mut WaveInfo) { - unsafe { ctru_sys::ndspChnWaveBufAdd(self.id, &mut buffer.raw_data) }; + /// To ensure safety, checks within [WaveInfo] will clear the whole channel queue if any queued [WaveInfo] is dropped prematurely. + pub fn queue_wave(&self, wave: &mut WaveInfo) { + // TODO: Return an error for already queued/used WaveInfos. + match wave.get_status() { + WaveStatus::Playing | WaveStatus::Queued => return, + _ => (), + } + + wave.set_channel(self.id); + + unsafe { ctru_sys::ndspChnWaveBufAdd(self.id, &mut wave.raw_data) }; } // FILTERS @@ -191,9 +218,5 @@ impl Drop for Ndsp { for i in 0..24 { self.channel(i).unwrap().clear_queue(); } - - unsafe { - ctru_sys::ndspExit(); - } } } diff --git a/ctru-rs/src/services/ndsp/wave.rs b/ctru-rs/src/services/ndsp/wave.rs index 1329964..0e9b43d 100644 --- a/ctru-rs/src/services/ndsp/wave.rs +++ b/ctru-rs/src/services/ndsp/wave.rs @@ -1,3 +1,5 @@ +use ctru_sys::ndspChnWaveBufClear; + use super::AudioFormat; use crate::linear::LinearAllocator; @@ -17,6 +19,7 @@ pub struct WaveInfo<'b> { buffer: &'b mut WaveBuffer, // Holding the data with the raw format is necessary since `libctru` will access it. pub(crate) raw_data: ctru_sys::ndspWaveBuf, + played_on_channel: Option, } #[derive(Copy, Clone, Debug)] @@ -53,7 +56,7 @@ impl WaveBuffer { ctru_sys::DSP_FlushDataCache(data.as_ptr().cast(), data.len().try_into().unwrap()); } - Ok(WaveBuffer { + Ok(Self { data, audio_format, nsamples, @@ -91,7 +94,7 @@ impl<'b> WaveInfo<'b> { next: std::ptr::null_mut(), }; - Self { buffer, raw_data } + Self { buffer, raw_data, played_on_channel: None,} } pub fn get_mut_wavebuffer(&mut self) -> &mut WaveBuffer { @@ -101,6 +104,10 @@ impl<'b> WaveInfo<'b> { pub fn get_status(&self) -> WaveStatus { self.raw_data.status.try_into().unwrap() } + + pub(crate) fn set_channel(&mut self, id: i32) { + self.played_on_channel = Some(id as u8) + } } impl TryFrom for WaveStatus { @@ -131,11 +138,15 @@ impl Drop for WaveBuffer { impl<'b> Drop for WaveInfo<'b> { fn drop(&mut self) { - // This was the only way I found I could check for improper drops of WaveInfos. - // It should be enough to warn users of the danger. + // This was the only way I found I could check for improper drops of `WaveInfos`. + // A panic was considered, but it would cause issues with drop order against `Ndsp`. match self.get_status() { WaveStatus::Free | WaveStatus::Done => (), - _ => panic!("WaveInfo struct has been dropped before finishing use. This could cause Undefined Behaviour."), + // If the status flag is "unfinished" + _ => { + // The unwrap is safe, since it must have a value in the case the status is "unfinished". + unsafe { ndspChnWaveBufClear(self.played_on_channel.unwrap().into() )}; + } } } }