Moved safety measures to WaveInfo rather than Ndsp

ctru-rs/examples/audio_filters.rs

@@ -30,7 +30,7 @@ fn fill_buffer(audio_data: &mut [u8], frequency: 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.;
     }
@@ -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]);
+            altern = !altern;
-            channel_zero.queue_wave(other);
         }
 
         // 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);
 }

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<Mutex<usize>> = Lazy::new(|| Mutex::new(0));
+
 #[non_exhaustive]
-pub struct Ndsp(());
+pub struct Ndsp {
+    _service_handler: ServiceReference,
+}
 
 impl Ndsp {
     pub fn init() -> crate::Result<Self> {
+        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) {
@@ -133,14 +152,22 @@ impl Channel {
     }
 
     /// Add a wave buffer to the channel's queue. 
-    /// Note: if there are no other buffers in queue, playback for this buffer will start.
+    /// 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.
+    /// To ensure safety, checks within [WaveInfo] will clear the whole channel queue if any queued [WaveInfo] is dropped prematurely.
-    pub fn queue_wave(&self, buffer: &mut WaveInfo) {
+    pub fn queue_wave(&self, wave: &mut WaveInfo) {
-        unsafe { ctru_sys::ndspChnWaveBufAdd(self.id, &mut buffer.raw_data) };
+        // 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();
-        }
     }
 }

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<u8>,
 }
 
 #[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<u8> 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.
+        // 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.
+        // 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() )};
+            }
         }
     }
 }