all: refactoring audio stuff and using structopt in desktop app

Former-commit-id: 8fb2e158eba5f81bc9fb953bfa6d0f4d9e505a61
Former-commit-id: c436751be80c9517401777ec5060061383d75929

core/benches/performance.rs

@@ -7,9 +7,6 @@ use std::rc::Rc;
 
 use rustboyadvance_core::prelude::*;
 
-struct BenchmarkHardware {}
-impl AudioInterface for BenchmarkHardware {}
-
 fn create_gba() -> GameBoyAdvance {
     // TODO: do I really want this file in my repository ?
     let bios = include_bytes!("roms/normatt_gba_bios.bin");
@@ -22,14 +19,7 @@ fn create_gba() -> GameBoyAdvance {
         .build()
         .unwrap();
 
-    let dummy = Rc::new(RefCell::new(BenchmarkHardware {}));
+    let mut gba = GameBoyAdvance::new(bios.to_vec().into_boxed_slice(), gpak, NullAudio::new());
-
-    let mut gba = GameBoyAdvance::new(
-        bios.to_vec().into_boxed_slice(),
-        gpak,
-        dummy.clone(),
-        dummy.clone(),
-    );
     gba.skip_bios();
     // skip initialization of the ROM to get to a stabilized scene
     for _ in 0..60 {

core/src/cartridge/backup/mod.rs

@@ -1,5 +1,5 @@
-use std::convert::TryFrom;
 use std::fmt;
+use std::str::FromStr;
 
 mod backup_file;
 pub use backup_file::BackupFile;
@@ -16,10 +16,9 @@ pub enum BackupType {
     AutoDetect = 5,
 }
 
-impl TryFrom<&str> for BackupType {
+impl FromStr for BackupType {
-    type Error = String;
+    type Err = String;
-
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
-    fn try_from(s: &str) -> Result<Self, Self::Error> {
         use BackupType::*;
         match s {
             "autodetect" => Ok(AutoDetect),

core/src/gba.rs

@@ -1,5 +1,5 @@
 /// Struct containing everything
-use std::cell::{Cell, RefCell};
+use std::cell::Cell;
 use std::rc::Rc;
 
 use bincode;
@@ -15,7 +15,7 @@ use super::sound::SoundController;
 use super::sysbus::SysBus;
 use super::timer::Timers;
 
-use super::AudioInterface;
+use super::sound::interface::DynAudioInterface;
 
 use arm7tdmi::{self, Arm7tdmiCore};
 use rustboyadvance_utils::Shared;
@@ -26,7 +26,7 @@ pub struct GameBoyAdvance {
     pub io_devs: Shared<IoDevices>,
     pub scheduler: SharedScheduler,
     interrupt_flags: SharedInterruptFlags,
-    pub audio_device: Rc<RefCell<dyn AudioInterface>>,
+    pub audio_interface: DynAudioInterface,
 }
 
 #[derive(Serialize, Deserialize)]
@@ -63,7 +63,7 @@ impl GameBoyAdvance {
     pub fn new(
         bios_rom: Box<[u8]>,
         gamepak: Cartridge,
-        audio_device: Rc<RefCell<dyn AudioInterface>>,
+        audio_interface: DynAudioInterface,
     ) -> GameBoyAdvance {
         // Warn the user if the bios is not the real one
         match check_real_bios(&bios_rom) {
@@ -80,7 +80,7 @@ impl GameBoyAdvance {
         let timers = Timers::new(interrupt_flags.clone());
         let sound_controller = Box::new(SoundController::new(
             &mut scheduler,
-            audio_device.borrow().get_sample_rate() as f32,
+            audio_interface.get_sample_rate() as f32,
         ));
         let io_devs = Shared::new(IoDevices::new(
             intc,
@@ -103,7 +103,7 @@ impl GameBoyAdvance {
             cpu,
             sysbus,
             io_devs,
-            audio_device,
+            audio_interface,
             scheduler,
             interrupt_flags,
         };
@@ -117,7 +117,7 @@ impl GameBoyAdvance {
         savestate: &[u8],
         bios: Box<[u8]>,
         rom: Box<[u8]>,
-        audio_device: Rc<RefCell<dyn AudioInterface>>,
+        audio_interface: DynAudioInterface,
     ) -> bincode::Result<GameBoyAdvance> {
         let decoded: Box<SaveState> = bincode::deserialize_from(savestate)?;
 
@@ -148,7 +148,7 @@ impl GameBoyAdvance {
             sysbus,
             io_devs,
             interrupt_flags: interrupts,
-            audio_device,
+            audio_interface,
             scheduler,
         })
     }
@@ -299,7 +299,7 @@ impl GameBoyAdvance {
                     Some(timers.handle_overflow_event(channel_id, event_time, apu, dmac))
                 }
                 EventType::Gpu(gpu_event) => Some(io.gpu.on_event(gpu_event, &mut *self.sysbus)),
-                EventType::Apu(event) => Some(io.sound.on_event(event, &self.audio_device)),
+                EventType::Apu(event) => Some(io.sound.on_event(event, &mut self.audio_interface)),
             };
             if let Some((new_event, when)) = new_event {
                 // We schedule events added by event handlers relative to the handled event time
@@ -378,21 +378,8 @@ impl GameBoyAdvance {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use std::cell::RefCell;
-    use std::rc::Rc;
 
-    use crate::cartridge::GamepakBuilder;
+    use crate::prelude::*;
-    use arm7tdmi::memory::BusIO;
-
-    struct DummyAudio {}
-
-    impl DummyAudio {
-        fn new() -> DummyAudio {
-            DummyAudio {}
-        }
-    }
-
-    impl AudioInterface for DummyAudio {}
 
     fn make_mock_gba(rom: &[u8]) -> GameBoyAdvance {
         let bios = vec![0; 0x4000].into_boxed_slice();
@@ -402,8 +389,7 @@ mod tests {
             .without_backup_to_file()
             .build()
             .unwrap();
-        let dummy = Rc::new(RefCell::new(DummyAudio::new()));
+        let mut gba = GameBoyAdvance::new(bios, cartridge, NullAudio::new());
-        let mut gba = GameBoyAdvance::new(bios, cartridge, dummy.clone());
         gba.skip_bios();
 
         gba

core/src/lib.rs

@@ -55,26 +55,6 @@ pub(crate) mod overrides;
 #[cfg(feature = "debugger")]
 pub mod debugger;
 
-pub type StereoSample<T> = [T; 2];
-
-pub trait AudioInterface {
-    fn get_sample_rate(&self) -> i32 {
-        44100
-    }
-
-    /// Pushes a stereo sample into the audio device
-    /// Sample should be normilized to siged 16bit values
-    /// Note: It is not guarentied that the sample will be played
-    #[allow(unused_variables)]
-    fn push_sample(&mut self, samples: &[i16]) {}
-}
-
-pub trait InputInterface {
-    fn poll(&mut self) -> u16 {
-        keypad::KEYINPUT_ALL_RELEASED
-    }
-}
-
 #[derive(Debug)]
 pub enum GBAError {
     IO(::std::io::Error),
@@ -122,8 +102,10 @@ pub mod prelude {
     #[cfg(feature = "debugger")]
     pub use super::debugger::Debugger;
     pub use super::gpu::{DISPLAY_HEIGHT, DISPLAY_WIDTH};
+    pub use super::sound::interface::{
+        AudioInterface, DynAudioInterface, NullAudio, SimpleAudioInterface,
+    };
     pub use super::Bus;
-    pub use super::{AudioInterface, StereoSample};
     pub use super::{GBAError, GBAResult, GameBoyAdvance};
     pub use rustboyadvance_utils::{read_bin_file, write_bin_file};
 }

core/src/overrides.rs

@@ -1,5 +1,5 @@
 use std::collections::HashMap;
-use std::convert::TryFrom;
+use std::str::FromStr;
 
 use yaml_rust::YamlLoader;
 
@@ -34,7 +34,7 @@ lazy_static! {
             let game_code = String::from(game["code"].as_str().unwrap());
             let force_rtc = game["rtc"].as_bool().unwrap_or(false);
             let save_type = if let Some(save_type) = game["save_type"].as_str() {
-                match BackupType::try_from(save_type) {
+                match BackupType::from_str(save_type) {
                     Ok(x) => Some(x),
                     _ => panic!("{}: invalid save type {:#}", game_code, save_type),
                 }

core/src/sound/dsp.rs

@@ -1,4 +1,4 @@
-use crate::StereoSample;
+use super::StereoSample;
 
 use serde::{Deserialize, Serialize};
 

core/src/sound/interface.rs

@@ -0,0 +1,63 @@
+use rustboyadvance_utils::audio::{AudioRingBuffer, SampleConsumer, SampleProducer};
+
+pub type StereoSample<T> = [T; 2];
+
+pub trait AudioInterface {
+    fn get_sample_rate(&self) -> i32 {
+        44100
+    }
+
+    /// Pushes stereo sample buffer into the audio device
+    /// Sample should be normilized to siged 16bit values
+    /// Note: It is not guarentied that the sample will be played
+    #[allow(unused_variables)]
+    fn push_sample(&mut self, sample: &StereoSample<i16>) {}
+}
+
+pub struct SimpleAudioInterface {
+    producer: SampleProducer,
+    sample_rate: i32,
+}
+
+impl SimpleAudioInterface {
+    pub fn create_channel(
+        sample_rate: i32,
+        buffer_size: Option<usize>,
+    ) -> (Box<Self>, SampleConsumer) {
+        let (producer, consumer) =
+            AudioRingBuffer::new_with_capacity(buffer_size.unwrap_or(8192)).split();
+        (
+            Box::new(SimpleAudioInterface {
+                producer,
+                sample_rate,
+            }),
+            consumer,
+        )
+    }
+}
+
+impl AudioInterface for SimpleAudioInterface {
+    #[inline]
+    fn get_sample_rate(&self) -> i32 {
+        self.sample_rate
+    }
+
+    #[inline]
+    fn push_sample(&mut self, sample: &StereoSample<i16>) {
+        let _ = self.producer.push(sample[0]);
+        let _ = self.producer.push(sample[1]);
+    }
+}
+
+pub type DynAudioInterface = Box<dyn AudioInterface>;
+
+#[derive(Debug, Default)]
+pub struct NullAudio {}
+
+impl AudioInterface for NullAudio {}
+
+impl NullAudio {
+    pub fn new() -> Box<NullAudio> {
+        Box::new(NullAudio::default())
+    }
+}

core/src/sound/mod.rs

@@ -1,6 +1,3 @@
-use std::cell::RefCell;
-use std::rc::Rc;
-
 use bit::BitIndex;
 use serde::{Deserialize, Serialize};
 
@@ -8,10 +5,10 @@ use super::dma::DmaController;
 use super::iodev::consts::*;
 use super::sched::*;
 
-use crate::{AudioInterface, StereoSample};
-
 mod fifo;
 use fifo::SoundFifo;
+pub mod interface;
+pub use interface::{AudioInterface, DynAudioInterface, StereoSample};
 
 mod dsp;
 use dsp::{CosineResampler, Resampler};
@@ -59,8 +56,6 @@ const REG_FIFO_A_H: u32 = REG_FIFO_A + 2;
 const REG_FIFO_B_L: u32 = REG_FIFO_B;
 const REG_FIFO_B_H: u32 = REG_FIFO_B + 2;
 
-type AudioDeviceRcRefCell = Rc<RefCell<dyn AudioInterface>>;
-
 #[derive(Serialize, Deserialize, Clone, Debug)]
 pub struct SoundController {
     cycles: usize, // cycles count when we last provided a new sample.
@@ -321,7 +316,7 @@ impl SoundController {
     }
 
     #[inline]
-    fn on_sample(&mut self, audio_device: &AudioDeviceRcRefCell) -> FutureEvent {
+    fn on_sample(&mut self, audio_device: &mut DynAudioInterface) -> FutureEvent {
         let mut sample = [0f32, 0f32];
 
         for (channel, out_sample) in sample.iter_mut().enumerate() {
@@ -339,9 +334,8 @@ impl SoundController {
 
         self.resampler.feed(&sample, &mut self.output_buffer);
 
-        let mut audio = audio_device.borrow_mut();
         self.output_buffer.drain(..).for_each(|[left, right]| {
-            audio.push_sample(&[
+            audio_device.push_sample(&[
                 (left.round() as i16) * (std::i16::MAX / 512),
                 (right.round() as i16) * (std::i16::MAX / 512),
             ]);
@@ -352,7 +346,7 @@ impl SoundController {
     pub fn on_event(
         &mut self,
         event: ApuEvent,
-        audio_device: &AudioDeviceRcRefCell,
+        audio_device: &mut DynAudioInterface,
     ) -> FutureEvent {
         match event {
             ApuEvent::Sample => self.on_sample(audio_device),

fps_bench/src/main.rs

@@ -1,21 +1,9 @@
-use std::cell::RefCell;
 use std::env;
 use std::path::Path;
-use std::rc::Rc;
 
 use rustboyadvance_core::prelude::*;
 use rustboyadvance_utils::FpsCounter;
 
-struct DummyAudio {}
-
-impl DummyAudio {
-    fn new() -> DummyAudio {
-        DummyAudio {}
-    }
-}
-
-impl AudioInterface for DummyAudio {}
-
 fn main() {
     if env::args().count() < 3 {
         eprintln!("usage: {} <bios> <rom>", env::args().nth(0).unwrap());
@@ -35,9 +23,7 @@ fn main() {
         .build()
         .unwrap();
 
-    let dummy = Rc::new(RefCell::new(DummyAudio::new()));
+    let mut gba = GameBoyAdvance::new(bios.into_boxed_slice(), gamepak, NullAudio::new());
-
-    let mut gba = GameBoyAdvance::new(bios.into_boxed_slice(), gamepak, dummy.clone());
     gba.skip_bios();
 
     let mut fps_counter = FpsCounter::default();

platform/rustboyadvance-jni/src/audio/mod.rs

@@ -7,23 +7,34 @@ pub mod util {
     use jni::signature::{JavaType, Primitive};
     use jni::JNIEnv;
 
-    pub fn get_sample_rate(env: &JNIEnv, audio_player_obj: JObject) -> i32 {
+    macro_rules! call_audio_player_method {
-        let audio_player_klass = env.get_object_class(audio_player_obj).unwrap();
+        ($env:ident, $audio_player_obj:ident, $method_name:literal, "()I") => {{
-        let mid_get_sample_rate = env
+            let audio_player_klass = $env
-            .get_method_id(audio_player_klass, "getSampleRate", "()I")
+                .get_object_class($audio_player_obj)
-            .expect("failed to get methodID for getSampleRate");
+                .map_err(|e| format!("failed to get class: {:?}", e))?;
-        let result = env
+            let mid_get_sample_rate = $env
+                .get_method_id(audio_player_klass, $method_name, "()I")
+                .map_err(|e| format!("failed to get methodID for {}: {:?}", $method_name, e))?;
+            let result = $env
                 .call_method_unchecked(
-                audio_player_obj,
+                    $audio_player_obj,
                     mid_get_sample_rate,
                     JavaType::Primitive(Primitive::Int),
                     &[],
                 )
-            .unwrap();
+                .map_err(|e| format!("getSampleRate() failed: {:?}", e))?;
-        let sample_rate = match result {
+            match result {
-            JValue::Int(sample_rate) => sample_rate as i32,
+                JValue::Int(sample_rate) => Ok(sample_rate),
-            _ => panic!("bad return value"),
+                value => panic!("bad return value {:?}", value),
-        };
+            }
-        return sample_rate;
+        }};
+    }
+
+    pub fn get_sample_rate(env: &JNIEnv, audio_player_obj: JObject) -> Result<i32, String> {
+        call_audio_player_method!(env, audio_player_obj, "getSampleRate", "()I")
+    }
+
+    pub fn get_sample_count(env: &JNIEnv, audio_player_obj: JObject) -> Result<i32, String> {
+        call_audio_player_method!(env, audio_player_obj, "getSampleCount", "()I")
     }
 }

platform/rustboyadvance-jni/src/audio/thread.rs

@@ -4,7 +4,7 @@ use std::sync::mpsc::{channel, Sender};
 use std::thread;
 use std::thread::JoinHandle;
 
-use rustboyadvance_utils::audio::Consumer;
+use rustboyadvance_utils::audio::SampleConsumer;
 
 use jni::JavaVM;
 
@@ -20,8 +20,11 @@ pub enum AudioThreadCommand {
 pub(crate) fn spawn_audio_worker_thread(
     audio_connector: AudioJNIConnector,
     jvm: JavaVM,
-    mut consumer: Consumer<i16>,
+    mut consumer: SampleConsumer,
-) -> (JoinHandle<AudioJNIConnector>, Sender<AudioThreadCommand>) {
+) -> (
+    JoinHandle<(AudioJNIConnector, SampleConsumer)>,
+    Sender<AudioThreadCommand>,
+) {
     let (tx, rx) = channel();
 
     let handle = thread::spawn(move || {
@@ -58,8 +61,8 @@ pub(crate) fn spawn_audio_worker_thread(
 
         info!("[AudioWorker] terminating");
 
-        // return the audio connector back
+        // return the audio stuff back to main thread
-        audio_connector
+        (audio_connector, consumer)
     });
 
     (handle, tx)

platform/rustboyadvance-jni/src/emulator.rs

@@ -1,10 +1,8 @@
 use rustboyadvance_core::prelude::*;
-use rustboyadvance_utils::audio::{AudioRingBuffer, Producer};
+use rustboyadvance_utils::audio::SampleConsumer;
 // use rustboyadvance_core::util::FpsCounter;
 
-use std::cell::RefCell;
 use std::path::Path;
-use std::rc::Rc;
 use std::sync::{Mutex, MutexGuard};
 use std::time::{Duration, Instant};
 
@@ -15,28 +13,6 @@ use jni::JNIEnv;
 
 use crate::audio::{self, connector::AudioJNIConnector, thread::AudioThreadCommand};
 
-struct AudioDevice {
-    sample_rate: i32,
-    audio_producer: Option<Producer<i16>>,
-}
-
-impl AudioInterface for AudioDevice {
-    fn push_sample(&mut self, samples: &[i16]) {
-        if let Some(prod) = &mut self.audio_producer {
-            for s in samples.iter() {
-                let _ = prod.push(*s);
-            }
-        } else {
-            // The gba is never ran before audio_producer is initialized
-            unreachable!()
-        }
-    }
-
-    fn get_sample_rate(&self) -> i32 {
-        self.sample_rate
-    }
-}
-
 struct Renderer {
     renderer_ref: GlobalRef,
     frame_buffer_ref: GlobalRef,
@@ -145,8 +121,20 @@ impl Default for EmulationState {
     }
 }
 
+fn create_audio(
+    env: &JNIEnv,
+    audio_player_obj: JObject,
+) -> Result<(Box<SimpleAudioInterface>, SampleConsumer), String> {
+    let sample_rate = audio::util::get_sample_rate(env, audio_player_obj)?;
+    let sample_count = audio::util::get_sample_count(env, audio_player_obj)? as usize;
+    Ok(SimpleAudioInterface::create_channel(
+        sample_rate,
+        Some(sample_count * 2),
+    ))
+}
+
 pub struct EmulatorContext {
-    audio_device: Rc<RefCell<AudioDevice>>,
+    audio_consumer: Option<SampleConsumer>,
     renderer: Renderer,
     audio_player_ref: GlobalRef,
     keypad: Keypad,
@@ -188,11 +176,9 @@ impl EmulatorContext {
         let renderer = Renderer::new(env, renderer_obj)?;
 
         info!("Creating GBA Instance");
-        let audio = Rc::new(RefCell::new(AudioDevice {
+        let audio_player_ref = env.new_global_ref(audio_player).unwrap();
-            sample_rate: audio::util::get_sample_rate(env, audio_player),
+        let (audio_device, audio_consumer) = create_audio(env, audio_player_ref.as_obj())?;
-            audio_producer: None,
+        let mut gba = GameBoyAdvance::new(bios, gamepak, audio_device);
-        }));
-        let mut gba = GameBoyAdvance::new(bios, gamepak, audio.clone());
         if skip_bios != 0 {
             info!("skipping bios");
             gba.skip_bios();
@@ -202,14 +188,13 @@ impl EmulatorContext {
         let keypad = Keypad::new(env, keypad_obj);
 
         info!("creating context");
-        let audio_player_ref = env.new_global_ref(audio_player).unwrap();
         let context = EmulatorContext {
             gba,
             keypad,
             renderer,
             audio_player_ref,
             emustate: Mutex::new(EmulationState::default()),
-            audio_device: audio.clone(),
+            audio_consumer: Some(audio_consumer),
         };
         Ok(context)
     }
@@ -236,30 +221,25 @@ impl EmulatorContext {
             .map_err(|e| format!("could not get savestate buffer, error {}", e))?;
 
         let renderer = Renderer::new(env, renderer_obj)?;
-
+        let audio_player_ref = env.new_global_ref(audio_player).unwrap();
-        let audio = Rc::new(RefCell::new(AudioDevice {
+        let (audio_device, audio_consumer) = create_audio(env, audio_player_ref.as_obj())?;
-            sample_rate: audio::util::get_sample_rate(env, audio_player),
+        let gba =
-            audio_producer: None,
+            GameBoyAdvance::from_saved_state(&savestate, bios, rom, audio_device).map_err(|e| {
-        }));
-        let gba = GameBoyAdvance::from_saved_state(&savestate, bios, rom, audio.clone()).map_err(
-            |e| {
                 format!(
                     "failed to create GameBoyAdvance from saved savestate, error {:?}",
                     e
                 )
-            },
+            })?;
-        )?;
 
         let keypad = Keypad::new(env, keypad_obj);
 
-        let audio_player_ref = env.new_global_ref(audio_player).unwrap();
         Ok(EmulatorContext {
             gba,
             keypad,
             renderer,
             audio_player_ref,
             emustate: Mutex::new(EmulationState::default()),
-            audio_device: audio.clone(),
+            audio_consumer: Some(audio_consumer),
         })
     }
 
@@ -285,15 +265,13 @@ impl EmulatorContext {
         // Instanciate an audio player connector
         let audio_connector = AudioJNIConnector::new(env, self.audio_player_ref.as_obj());
 
-        // Create a ringbuffer between the emulator and the audio thread
-        let (prod, cons) = AudioRingBuffer::new_with_capacity(audio_connector.sample_count).split();
-
-        // Store the ringbuffer producer in the emulator
-        self.audio_device.borrow_mut().audio_producer = Some(prod);
-
         // Spawn the audio worker thread, give it the audio connector, jvm and ringbuffer consumer
-        let (audio_thread_handle, audio_thread_tx) =
+        // Note - after this operation `self` no longer has `audio_consumer`
-            audio::thread::spawn_audio_worker_thread(audio_connector, jvm, cons);
+        let (audio_thread_handle, audio_thread_tx) = audio::thread::spawn_audio_worker_thread(
+            audio_connector,
+            jvm,
+            self.audio_consumer.take().unwrap(),
+        );
 
         info!("starting main emulation loop");
 
@@ -302,7 +280,7 @@ impl EmulatorContext {
         'running: loop {
             let emustate = *self.emustate.lock().unwrap();
 
-            let limiter = match emustate {
+            let vsync = match emustate {
                 EmulationState::Initial => unsafe { std::hint::unreachable_unchecked() },
                 EmulationState::Stopped => unsafe { std::hint::unreachable_unchecked() },
                 EmulationState::Pausing => {
@@ -334,7 +312,7 @@ impl EmulatorContext {
             //     info!("FPS {}", fps);
             // }
 
-            if limiter {
+            if vsync {
                 let time_passed = start_time.elapsed();
                 let delay = FRAME_TIME.checked_sub(time_passed);
                 match delay {
@@ -350,13 +328,12 @@ impl EmulatorContext {
         audio_thread_tx.send(AudioThreadCommand::Terminate).unwrap(); // we surely have an endpoint, so it will work
         info!("waiting for audio worker to complete");
 
-        let audio_connector = audio_thread_handle.join().unwrap();
+        let (audio_connector, audio_consumer) = audio_thread_handle.join().unwrap();
+        self.audio_consumer.replace(audio_consumer);
         info!("audio worker terminated");
 
         audio_connector.pause(env);
 
-        self.audio_device.borrow_mut().audio_producer = None;
-
         *self.emustate.lock().unwrap() = EmulationState::Stopped;
 
         Ok(())

platform/rustboyadvance-libretro/src/lib.rs

@@ -13,34 +13,18 @@ use unsafe_unwrap::UnsafeUnwrap;
 
 use rustboyadvance_core::keypad::Keys as _GbaButton;
 use rustboyadvance_core::prelude::*;
-use rustboyadvance_utils::audio::AudioRingBuffer;
+use rustboyadvance_utils::audio::SampleConsumer;
 
 use std::ops::Deref;
 use std::path::Path;
 
-use std::cell::RefCell;
 use std::default::Default;
-use std::rc::Rc;
-
-#[derive(Default)]
-struct AudioDevice {
-    audio_ring_buffer: AudioRingBuffer,
-}
-
-impl AudioInterface for AudioDevice {
-    fn push_sample(&mut self, samples: &[i16]) {
-        let prod = self.audio_ring_buffer.producer();
-        for s in samples.iter() {
-            let _ = prod.push(*s);
-        }
-    }
-}
 
 #[derive(Default)]
 struct RustBoyAdvanceCore {
     gba: Option<GameBoyAdvance>,
     game_data: Option<GameData>,
-    audio: Option<Rc<RefCell<AudioDevice>>>,
+    audio_consumer: Option<SampleConsumer>,
 }
 
 #[repr(transparent)]
@@ -119,10 +103,12 @@ impl libretro_backend::Core for RustBoyAdvanceCore {
                     .video(240, 160, 60.0, PixelFormat::ARGB8888)
                     .audio(44100.0);
 
-                let audio = Rc::new(RefCell::new(AudioDevice::default()));
+                let (audio_device, audio_consumer) =
-                let gba = GameBoyAdvance::new(bios.into_boxed_slice(), gamepak, audio.clone());
+                    SimpleAudioInterface::create_channel(44100, None);
 
-                self.audio = Some(audio);
+                let gba = GameBoyAdvance::new(bios.into_boxed_slice(), gamepak, audio_device);
+
+                self.audio_consumer = Some(audio_consumer);
                 self.gba = Some(gba);
                 self.game_data = Some(game_data);
                 LoadGameResult::Success(av_info)
@@ -136,7 +122,6 @@ impl libretro_backend::Core for RustBoyAdvanceCore {
 
         // gba and audio are `Some` after the game is loaded, so avoiding overhead of unwrap
         let gba = unsafe { self.gba.as_mut().unsafe_unwrap() };
-        let audio = unsafe { self.audio.as_mut().unsafe_unwrap() };
 
         let key_state = gba.get_key_state_mut();
         macro_rules! update_controllers {
@@ -165,12 +150,11 @@ impl libretro_backend::Core for RustBoyAdvanceCore {
 
         // upload sound samples
         {
+            let mut audio_consumer = self.audio_consumer.take().unwrap();
             let mut audio_samples = [0; 4096 * 2];
-            let mut audio = audio.borrow_mut();
+            let count = audio_consumer.pop_slice(&mut audio_samples);
-            let consumer = audio.audio_ring_buffer.consumer();
-            let count = consumer.pop_slice(&mut audio_samples);
-
             handle.upload_audio_frame(&audio_samples[..count]);
+            self.audio_consumer.replace(audio_consumer);
         }
     }
 

platform/rustboyadvance-sdl2/Cargo.toml

@@ -10,7 +10,7 @@ rustboyadvance-utils = { path = "../../utils/" }
 sdl2 = { version = "0.33.0", features = ["image"] }
 ringbuf = "0.2.2"
 bytesize = "1.0.0"
-clap = { version = "2.33", features = ["color", "yaml"] }
+structopt = "0.3"
 log = "0.4.8"
 flexi_logger = { version = "0.14", features = ["colors"] }
 bit = "^0.1"

platform/rustboyadvance-sdl2/src/audio.rs

@@ -1,98 +1,63 @@
 use sdl2;
-use sdl2::audio::{AudioCallback, AudioDevice, AudioSpec, AudioSpecDesired};
+use sdl2::audio::{AudioCallback, AudioDevice, AudioFormat, AudioSpec, AudioSpecDesired};
 
-use rustboyadvance_core::{AudioInterface, StereoSample};
+use rustboyadvance_core::prelude::SimpleAudioInterface;
+use rustboyadvance_utils::audio::SampleConsumer;
 
-use ringbuf;
+pub struct GbaAudioCallback {
-use ringbuf::{Consumer, Producer, RingBuffer};
+    consumer: SampleConsumer,
-
+    #[allow(unused)]
-struct GbaAudioCallback {
-    consumer: Consumer<StereoSample<i16>>,
     spec: AudioSpec,
 }
 
-pub struct DummyAudioPlayer {}
-
-impl AudioInterface for DummyAudioPlayer {}
-
-pub struct Sdl2AudioPlayer {
-    _device: AudioDevice<GbaAudioCallback>,
-    producer: Producer<StereoSample<i16>>,
-    freq: i32,
-}
-
 impl AudioCallback for GbaAudioCallback {
     type Channel = i16;
 
     fn callback(&mut self, out_samples: &mut [i16]) {
-        let sample_count = out_samples.len() / 2;
+        let written = self.consumer.pop_slice(out_samples);
-
+        for s in out_samples.iter_mut().skip(written) {
-        for i in 0..sample_count {
+            *s = self.spec.silence as i16;
-            if let Some([left, right]) = self.consumer.pop() {
-                out_samples[2 * i] = left;
-                out_samples[2 * i + 1] = right;
-            } else {
-                out_samples[2 * i] = self.spec.silence as i16;
-                out_samples[2 * i + 1] = self.spec.silence as i16;
-            }
         }
     }
 }
 
-impl AudioInterface for Sdl2AudioPlayer {
+pub fn create_audio_player(
-    fn get_sample_rate(&self) -> i32 {
+    sdl: &sdl2::Sdl,
-        self.freq
+) -> Result<(Box<SimpleAudioInterface>, AudioDevice<GbaAudioCallback>), String> {
-    }
-
-    fn push_sample(&mut self, sample: &[i16]) {
-        #![allow(unused_must_use)]
-        self.producer.push([sample[0], sample[1]]);
-    }
-}
-
-pub fn create_audio_player(sdl: &sdl2::Sdl) -> Sdl2AudioPlayer {
     let desired_spec = AudioSpecDesired {
         freq: Some(44_100),
         channels: Some(2), // stereo
         samples: None,
     };
 
-    let audio_subsystem = sdl.audio().unwrap();
+    let audio_subsystem = sdl.audio()?;
 
     let mut freq = 0;
 
-    let mut producer: Option<Producer<StereoSample<i16>>> = None;
+    let mut gba_audio = None;
 
-    let device = audio_subsystem
+    let device = audio_subsystem.open_playback(None, &desired_spec, |spec| {
-        .open_playback(None, &desired_spec, |spec| {
         info!("Found audio device: {:?}", spec);
         freq = spec.freq;
 
-            // Create a thread-safe SPSC fifo
+        if spec.format != AudioFormat::S16LSB {
-            let ringbuf_size = (spec.samples as usize) * 2;
+            panic!("Unsupported audio format {:?}", spec.format);
-            let rb = RingBuffer::<StereoSample<i16>>::new(ringbuf_size);
-            let (prod, cons) = rb.split();
-
-            // move producer to the outer scope
-            producer = Some(prod);
-
-            GbaAudioCallback {
-                consumer: cons,
-                spec,
         }
-        })
+
-        .unwrap();
+        // Create a thread-safe SPSC fifo
+        let ringbuf_samples_per_channel = (spec.samples as usize) * 2; // we want the ringbuf to hold 2 frames worth of samples
+        let ringbuf_size = (spec.channels as usize) * ringbuf_samples_per_channel;
+        info!("ringbuffer size = {}", ringbuf_size);
+
+        let (audio_device, consumer) =
+            SimpleAudioInterface::create_channel(freq, Some(ringbuf_size));
+        // Move the audio to outer scope
+        gba_audio = Some(audio_device);
+
+        GbaAudioCallback { consumer, spec }
+    })?;
 
     device.resume();
 
-    Sdl2AudioPlayer {
+    Ok((gba_audio.take().unwrap(), device))
-        _device: device,
-        freq,
-        producer: producer.unwrap(),
-    }
-}
-
-pub fn create_dummy_player() -> DummyAudioPlayer {
-    info!("Dummy audio device");
-    DummyAudioPlayer {}
 }

platform/rustboyadvance-sdl2/src/main.rs

@@ -12,21 +12,14 @@ use sdl2::EventPump;
 
 use bytesize;
 use spin_sleep;
-
+use structopt::StructOpt;
-use std::cell::RefCell;
-use std::rc::Rc;
 
 use std::fs;
 use std::io::Cursor;
-use std::path::{Path, PathBuf};
+use std::path::Path;
 use std::process;
 use std::time;
 
-use std::convert::TryFrom;
-
-#[macro_use]
-extern crate clap;
-
 #[macro_use]
 extern crate log;
 use flexi_logger;
@@ -34,12 +27,11 @@ use flexi_logger::*;
 
 mod audio;
 mod input;
+mod options;
 mod video;
 
-use audio::{create_audio_player, create_dummy_player};
 use video::{SCREEN_HEIGHT, SCREEN_WIDTH};
 
-use rustboyadvance_core::cartridge::BackupType;
 use rustboyadvance_core::prelude::*;
 
 use rustboyadvance_utils::FpsCounter;
@@ -50,9 +42,6 @@ const DEFAULT_GDB_SERVER_ADDR: &'static str = "localhost:1337";
 const CANVAS_WIDTH: u32 = SCREEN_WIDTH;
 const CANVAS_HEIGHT: u32 = SCREEN_HEIGHT;
 
-fn get_savestate_path(rom_filename: &Path) -> PathBuf {
-    rom_filename.with_extension("savestate")
-}
 
 /// Waits for the user to drag a rom file to window
 fn wait_for_rom(canvas: &mut WindowCanvas, event_pump: &mut EventPump) -> Result<String, String> {
@@ -101,6 +90,17 @@ fn ask_download_bios() {
     const OPEN_SOURCE_BIOS_URL: &'static str =
         "https://github.com/Nebuleon/ReGBA/raw/master/bios/gba_bios.bin";
     println!("Missing BIOS file. If you don't have the original GBA BIOS, you can download an open-source bios from {}", OPEN_SOURCE_BIOS_URL);
+    std::process::exit(0);
+}
+
+fn load_bios(bios_path: &Path) -> Box<[u8]> {
+    match read_bin_file(bios_path) {
+        Ok(bios) => bios.into_boxed_slice(),
+        _ => {
+            ask_download_bios();
+            unreachable!()
+        }
+    }
 }
 
 fn main() -> Result<(), Box<dyn std::error::Error>> {
@@ -114,24 +114,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         .start()
         .unwrap();
 
-    let mut frame_limiter = true;
+    let opts = options::Options::from_args();
-    let yaml = load_yaml!("cli.yml");
-    let matches = clap::App::from_yaml(yaml).get_matches();
-
-    let bios_path = Path::new(matches.value_of("bios").unwrap_or_default());
-    let bios_bin = match read_bin_file(bios_path) {
-        Ok(bios) => bios.into_boxed_slice(),
-        _ => {
-            ask_download_bios();
-            std::process::exit(0);
-        }
-    };
-
-    let skip_bios = matches.occurrences_of("skip_bios") != 0;
-
-    let debug = matches.occurrences_of("debug") != 0;
-    let silent = matches.occurrences_of("silent") != 0;
-    let with_gdbserver = matches.occurrences_of("with_gdbserver") != 0;
 
     info!("Initializing SDL2 context");
     let sdl_context = sdl2::init().expect("failed to initialize sdl2");
@@ -174,65 +157,27 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         }
     };
 
-    let mut rom_path = match matches.value_of("game_rom") {
-        Some(path) => path.to_string(),
-        _ => {
-            info!("[!] Rom file missing, please drag a rom file into the emulator window...");
-            wait_for_rom(&mut canvas, &mut event_pump)?
-        }
-    };
-
     let mut renderer = video::Renderer::from_canvas(canvas);
-    let audio: Rc<RefCell<dyn AudioInterface>> = if silent {
+    let (audio_interface, mut _sdl_audio_device) = audio::create_audio_player(&sdl_context)?;
-        Rc::new(RefCell::new(create_dummy_player()))
+    let mut rom_name = opts.rom_name();
-    } else {
-        Rc::new(RefCell::new(create_audio_player(&sdl_context)))
-    };
 
-    let mut savestate_path = get_savestate_path(&Path::new(&rom_path));
+    let bios_bin = load_bios(&opts.bios);
 
-    let mut rom_name = Path::new(&rom_path).file_name().unwrap().to_str().unwrap();
+    let mut gba = GameBoyAdvance::new(
+        bios_bin.clone(),
+        opts.cartridge_from_opts()?,
+        audio_interface,
+    );
 
-    let mut builder = GamepakBuilder::new()
+    if opts.skip_bios {
-        .save_type(BackupType::try_from(
-            matches.value_of("save_type").unwrap(),
-        )?)
-        .file(Path::new(&rom_path));
-
-    if matches.occurrences_of("rtc") != 0 {
-        builder = builder.with_rtc();
-    }
-
-    let gamepak = builder.build()?;
-
-    let mut gba = GameBoyAdvance::new(bios_bin.clone(), gamepak, audio.clone());
-
-    if skip_bios {
         gba.skip_bios();
     }
 
-    if debug {
+    if opts.gdbserver {
-        #[cfg(feature = "debugger")]
-        {
-            gba.cpu.set_verbose(true);
-            let mut debugger = Debugger::new();
-            info!("starting debugger...");
-            debugger
-                .repl(&mut gba, matches.value_of("script_file"))
-                .unwrap();
-            info!("ending debugger...");
-            return Ok(());
-        }
-        #[cfg(not(feature = "debugger"))]
-        {
-            panic!("Please compile me with 'debugger' feature");
-        }
-    }
-
-    if with_gdbserver {
         todo!("gdb")
     }
 
+    let mut vsync = true;
     let mut fps_counter = FpsCounter::default();
     let frame_time = time::Duration::new(0, 1_000_000_000u32 / 60);
     'running: loop {
@@ -244,7 +189,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
                     scancode: Some(scancode),
                     ..
                 } => match scancode {
-                    Scancode::Space => frame_limiter = false,
+                    Scancode::Space => vsync = false,
                     k => input::on_keyboard_key_down(&mut gba, k),
                 },
                 Event::KeyUp {
@@ -265,28 +210,28 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
                     Scancode::F5 => {
                         info!("Saving state ...");
                         let save = gba.save_state()?;
-                        write_bin_file(&savestate_path, &save)?;
+                        write_bin_file(&opts.savestate_path(), &save)?;
                         info!(
                             "Saved to {:?} ({})",
-                            savestate_path,
+                            opts.savestate_path(),
                             bytesize::ByteSize::b(save.len() as u64)
                         );
                     }
                     Scancode::F9 => {
-                        if savestate_path.is_file() {
+                        if opts.savestate_path().is_file() {
-                            let save = read_bin_file(&savestate_path)?;
+                            let save = read_bin_file(&opts.savestate_path())?;
-                            info!("Restoring state from {:?}...", savestate_path);
+                            info!("Restoring state from {:?}...", opts.savestate_path());
                             gba.restore_state(&save)?;
                             info!("Restored!");
                         } else {
                             info!("Savestate not created, please create one by pressing F5");
                         }
                     }
-                    Scancode::Space => frame_limiter = true,
+                    Scancode::Space => vsync = true,
                     k => input::on_keyboard_key_up(&mut gba, k),
                 },
                 Event::ControllerButtonDown { button, .. } => match button {
-                    Button::RightStick => frame_limiter = !frame_limiter,
+                    Button::RightStick => vsync = !vsync,
                     b => input::on_controller_button_down(&mut gba, b),
                 },
                 Event::ControllerButtonUp { button, .. } => {
@@ -318,16 +263,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
                 }
                 Event::Quit { .. } => break 'running,
                 Event::DropFile { filename, .. } => {
-                    // load the new rom
+                    todo!("impl DropFile again")
-                    rom_path = filename;
-                    savestate_path = get_savestate_path(&Path::new(&rom_path));
-                    rom_name = Path::new(&rom_path).file_name().unwrap().to_str().unwrap();
-                    let gamepak = GamepakBuilder::new().file(Path::new(&rom_path)).build()?;
-                    let bios_bin = read_bin_file(bios_path).unwrap();
-
-                    // create a new emulator - TODO, export to a function
-                    gba = GameBoyAdvance::new(bios_bin.into_boxed_slice(), gamepak, audio.clone());
-                    gba.skip_bios();
                 }
                 _ => {}
             }
@@ -341,7 +277,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
             renderer.set_window_title(&title);
         }
 
-        if frame_limiter {
+        if vsync {
             let time_passed = start_time.elapsed();
             let delay = frame_time.checked_sub(time_passed);
             match delay {

platform/rustboyadvance-sdl2/src/options.rs

@@ -0,0 +1,64 @@
+use std::path::PathBuf;
+
+use rustboyadvance_core::{
+    cartridge::{BackupType, GamepakBuilder},
+    prelude::Cartridge,
+};
+use structopt::StructOpt;
+
+const SAVE_TYPE_POSSIBLE_VALUES: &[&str] =
+    &["sram", "flash128k", "flash64k", "eeprom", "autodetect"];
+
+#[derive(StructOpt, Debug)]
+#[structopt(name = "rustboyadvance-sdl2")]
+pub struct Options {
+    /// Rom file to emulate, may be a raw dump from a cartridge or a compiled ELF file
+    #[structopt(name = "ROM", parse(from_os_str))]
+    pub rom: PathBuf,
+
+    /// Bios file to use
+    #[structopt(long, parse(from_os_str), default_value = "gba_bios.bin")]
+    pub bios: PathBuf,
+
+    /// Skip running the bios boot animation and jump straight to the ROM
+    #[structopt(long)]
+    pub skip_bios: bool,
+
+    /// Do not output sound
+    #[structopt(long)]
+    pub silent: bool,
+
+    /// Initalize gdbserver and wait for a connection from gdb
+    #[structopt(short = "d", long)]
+    pub gdbserver: bool,
+
+    /// Force emulation of RTC, use for games that have RTC but the emulator fails to detect
+    #[structopt(long)]
+    pub rtc: bool,
+
+    /// Override save type, useful for troublemaking games that fool the auto detection
+    #[structopt(long, default_value = "autodetect", possible_values = SAVE_TYPE_POSSIBLE_VALUES)]
+    pub save_type: BackupType,
+}
+
+type DynError = Box<dyn std::error::Error>;
+
+impl Options {
+    pub fn cartridge_from_opts(&self) -> Result<Cartridge, DynError> {
+        let mut builder = GamepakBuilder::new()
+            .save_type(self.save_type)
+            .file(&self.rom);
+        if self.rtc {
+            builder = builder.with_rtc();
+        }
+        Ok(builder.build()?)
+    }
+
+    pub fn savestate_path(&self) -> PathBuf {
+        self.rom.with_extension("savestate")
+    }
+
+    pub fn rom_name(&self) -> &str {
+        self.rom.file_name().unwrap().to_str().unwrap()
+    }
+}

platform/rustboyadvance-wasm/src/emulator.rs

@@ -1,24 +1,20 @@
-use std::cell::RefCell;
+use rustboyadvance_utils::audio::SampleConsumer;
-use std::rc::Rc;
-
 use wasm_bindgen::prelude::*;
 use wasm_bindgen::Clamped;
 
 use js_sys::Float32Array;
 
-use web_sys::AudioContext;
 use web_sys::CanvasRenderingContext2d;
 
 use rustboyadvance_core::keypad as gba_keypad;
 use rustboyadvance_core::prelude::*;
-use rustboyadvance_utils::audio::AudioRingBuffer;
 
 use bit::BitIndex;
 
 #[wasm_bindgen]
 pub struct Emulator {
     gba: GameBoyAdvance,
-    interface: Rc<RefCell<Interface>>,
+    audio_consumer: SampleConsumer,
     frame: Option<Box<[u8]>>,
 }
 
@@ -33,51 +29,17 @@ fn translate_frame_to_u8(input_fb: &[u32], out_fb: &mut [u8]) {
     }
 }
 
-struct Interface {
-    sample_rate: i32,
-    audio_ctx: AudioContext,
-    audio_ring_buffer: AudioRingBuffer,
-}
-
-impl Drop for Interface {
-    fn drop(&mut self) {
-        let _ = self.audio_ctx.clone();
-    }
-}
-
-impl Interface {
-    fn new(audio_ctx: AudioContext) -> Result<Interface, JsValue> {
-        Ok(Interface {
-            sample_rate: audio_ctx.sample_rate() as i32,
-            audio_ctx: audio_ctx,
-            audio_ring_buffer: Default::default(),
-        })
-    }
-}
-
 fn convert_sample(s: i16) -> f32 {
     (s as f32) / 32767_f32
 }
 
-impl AudioInterface for Interface {
-    fn get_sample_rate(&self) -> i32 {
-        self.sample_rate
-    }
-
-    fn push_sample(&mut self, samples: &[i16]) {
-        let prod = self.audio_ring_buffer.producer();
-        for s in samples.iter() {
-            let _ = prod.push(*s);
-        }
-    }
-}
-
 #[wasm_bindgen]
 impl Emulator {
     #[wasm_bindgen(constructor)]
     pub fn new(bios: &[u8], rom: &[u8]) -> Result<Emulator, JsValue> {
         let audio_ctx = web_sys::AudioContext::new()?;
-        let interface = Rc::new(RefCell::new(Interface::new(audio_ctx)?));
+        let (audio_device, audio_consumer) =
+            SimpleAudioInterface::create_channel(audio_ctx.sample_rate() as i32, None);
 
         let gamepak = GamepakBuilder::new()
             .take_buffer(rom.to_vec().into_boxed_slice())
@@ -85,11 +47,11 @@ impl Emulator {
             .build()
             .unwrap();
 
-        let gba = GameBoyAdvance::new(bios.to_vec().into_boxed_slice(), gamepak, interface.clone());
+        let gba = GameBoyAdvance::new(bios.to_vec().into_boxed_slice(), gamepak, audio_device);
 
         Ok(Emulator {
             gba,
-            interface,
+            audio_consumer,
             frame: Some(vec![0; 240 * 160 * 4].into_boxed_slice()),
         })
     }
@@ -152,12 +114,9 @@ impl Emulator {
         }
     }
 
-    pub fn collect_audio_samples(&self) -> Result<Float32Array, JsValue> {
+    pub fn collect_audio_samples(&mut self) -> Result<Float32Array, JsValue> {
-        let mut interface = self.interface.borrow_mut();
+        let mut samples = Vec::with_capacity(self.audio_consumer.len());
-
+        while let Some(sample) = self.audio_consumer.pop() {
-        let consumer = interface.audio_ring_buffer.consumer();
-        let mut samples = Vec::with_capacity(consumer.len());
-        while let Some(sample) = consumer.pop() {
             samples.push(convert_sample(sample));
         }
 

utils/src/lib.rs

@@ -90,10 +90,12 @@ macro_rules! host_breakpoint {
 
 pub mod audio {
     pub use ringbuf::{Consumer, Producer, RingBuffer};
+    pub type SampleProducer = Producer<i16>;
+    pub type SampleConsumer = Consumer<i16>;
 
     pub struct AudioRingBuffer {
-        prod: Producer<i16>,
+        prod: SampleProducer,
-        cons: Consumer<i16>,
+        cons: SampleConsumer,
     }
 
     impl Default for AudioRingBuffer {
@@ -110,15 +112,15 @@ pub mod audio {
             AudioRingBuffer { prod, cons }
         }
 
-        pub fn producer(&mut self) -> &mut Producer<i16> {
+        pub fn producer(&mut self) -> &mut SampleProducer {
             &mut self.prod
         }
 
-        pub fn consumer(&mut self) -> &mut Consumer<i16> {
+        pub fn consumer(&mut self) -> &mut SampleConsumer {
             &mut self.cons
         }
 
-        pub fn split(self) -> (Producer<i16>, Consumer<i16>) {
+        pub fn split(self) -> (SampleProducer, SampleConsumer) {
             (self.prod, self.cons)
         }
     }