chore(apu): improve accuracy of Channel 1

src/apu.rs

@@ -7,7 +7,7 @@ use types::ch3::Volume as Ch3Volume;
 use types::ch4::{CounterWidth, Frequency as Ch4Frequency, PolynomialCounter};
 use types::common::{EnvelopeDirection, FrequencyHigh, SoundDuty, VolumeEnvelope};
 use types::fs::{FrameSequencer, State as FSState};
-use types::{ChannelControl, SoundOutput};
+use types::{ChannelControl, NRx4, SoundOutput};
 
 pub mod gen;
 mod types;
@@ -27,7 +27,7 @@ pub struct Apu {
     /// Noise
     ch4: Channel4,
 
-    sequencer: FrameSequencer,
+    fs: FrameSequencer,
     div_prev: Option<u16>,
 
     prod: Option<SampleProducer<f32>>,
@@ -69,20 +69,20 @@ impl BusIo for Apu {
             0x11 if self.ctrl.enabled => self.ch1.set_duty(byte),
             0x12 if self.ctrl.enabled => self.ch1.set_envelope(byte),
             0x13 if self.ctrl.enabled => self.ch1.set_freq_lo(byte),
-            0x14 if self.ctrl.enabled => self.ch1.set_freq_hi(&self.sequencer, byte),
+            0x14 if self.ctrl.enabled => self.ch1.set_freq_hi(&self.fs, byte),
             0x16 if self.ctrl.enabled => self.ch2.set_duty(byte),
             0x17 if self.ctrl.enabled => self.ch2.set_envelope(byte),
             0x18 if self.ctrl.enabled => self.ch2.set_freq_lo(byte),
-            0x19 if self.ctrl.enabled => self.ch2.set_freq_hi(byte),
+            0x19 if self.ctrl.enabled => self.ch2.set_freq_hi(&self.fs, byte),
             0x1A if self.ctrl.enabled => self.ch3.set_dac_enabled(byte),
             0x1B if self.ctrl.enabled => self.ch3.set_len(byte),
             0x1C if self.ctrl.enabled => self.ch3.set_volume(byte),
             0x1D if self.ctrl.enabled => self.ch3.set_freq_lo(byte),
-            0x1E if self.ctrl.enabled => self.ch3.set_freq_hi(byte),
+            0x1E if self.ctrl.enabled => self.ch3.set_freq_hi(&self.fs, byte),
             0x20 if self.ctrl.enabled => self.ch4.set_len(byte),
             0x21 if self.ctrl.enabled => self.ch4.set_envelope(byte),
             0x22 if self.ctrl.enabled => self.ch4.set_poly(byte),
-            0x23 if self.ctrl.enabled => self.ch4.set_frequency(byte),
+            0x23 if self.ctrl.enabled => self.ch4.set_frequency(&self.fs, byte),
             0x24 if self.ctrl.enabled => self.ctrl.set_channel(byte),
             0x25 if self.ctrl.enabled => self.ctrl.set_output(byte),
             0x26 => self.set_status(byte),
@@ -101,7 +101,7 @@ impl Apu {
         if self.is_falling_edge(12, div) {
             use FSState::*;
 
-            match self.sequencer.state() {
+            match self.fs.state() {
                 Length => self.clock_length(),
                 LengthAndSweep => {
                     self.clock_length();
@@ -111,7 +111,7 @@ impl Apu {
                 Nothing => {}
             }
 
-            self.sequencer.next();
+            self.fs.next();
         }
 
         self.div_prev = Some(div);
@@ -172,7 +172,7 @@ impl Apu {
 
         if self.ctrl.enabled {
             // Frame Sequencer reset to Step 0
-            self.sequencer.reset();
+            self.fs.reset();
 
             // Square Duty units are reset to first step
             self.ch1.duty_pos = 0;
@@ -225,8 +225,8 @@ impl Apu {
         self.ch4.enabled = Default::default();
     }
 
-    fn process_length(freq_hi: &FrequencyHigh, counter: &mut u16, enabled: &mut bool) {
+    fn process_length(freq: &impl NRx4, counter: &mut u16, enabled: &mut bool) {
-        if freq_hi.length_enable() && *counter > 0 {
+        if freq.length_enable() && *counter > 0 {
             *counter -= 1;
 
             // Check in this scope ensures (only) the above subtraction
@@ -237,18 +237,6 @@ impl Apu {
         }
     }
 
-    fn ch4_process_length(freq: &Ch4Frequency, length_timer: &mut u16, enabled: &mut bool) {
-        if freq.length_enable() && *length_timer > 0 {
-            *length_timer -= 1;
-
-            // Check in this scope ensures (only) the above subtraction
-            // made length_timer 0
-            if *length_timer == 0 {
-                *enabled = false;
-            }
-        }
-    }
-
     fn clock_length(&mut self) {
         Self::process_length(
             &self.ch1.freq_hi,
@@ -258,19 +246,19 @@ impl Apu {
 
         Self::process_length(
             &self.ch2.freq_hi,
-            &mut self.ch2.length_timer,
+            &mut self.ch2.length_counter,
             &mut self.ch2.enabled,
         );
 
         Self::process_length(
             &self.ch3.freq_hi,
-            &mut self.ch3.length_timer,
+            &mut self.ch3.length_counter,
             &mut self.ch3.enabled,
         );
 
-        Self::ch4_process_length(
+        Self::process_length(
             &self.ch4.freq,
-            &mut self.ch4.length_timer,
+            &mut self.ch4.length_counter,
             &mut self.ch4.enabled,
         );
     }
@@ -287,7 +275,7 @@ impl Apu {
             if self.ch1.sweep_enabled && period != 0 {
                 let new_freq = self.ch1.calc_sweep_freq();
 
-                if new_freq <= 2047 && self.ch1.sweep.shift_count() != 0 {
+                if new_freq <= 0x7FF && self.ch1.sweep.shift_count() != 0 {
                     self.ch1.set_frequency(new_freq);
                     self.ch1.shadow_freq = new_freq;
 
@@ -485,39 +473,29 @@ impl Channel1 {
 
     /// 0xFF14 | NR14 - Channel 1 Frequency high
     pub(crate) fn set_freq_hi(&mut self, fs: &FrameSequencer, byte: u8) {
-        let prev_le = self.freq_hi.length_enable();
+        let mut new_freq: FrequencyHigh = byte.into();
-        self.freq_hi = byte.into();
-
-        if !fs.next_clocks_length() {
-            if !prev_le && self.freq_hi.length_enable() && self.length_counter != 0 {
-                self.length_counter -= 1;
-
-                if self.length_counter == 0 && !self.freq_hi.initial() {
-                    self.enabled = false;
-                }
-            }
-        }
 
         // If this bit is set, a trigger event occurs
-        if self.freq_hi.initial() {
+        if new_freq.trigger() {
-            if self.is_dac_enabled() {
+            self.enabled = true;
-                self.enabled = true;
-            }
 
-            // Length behaviour during trigger event
             if self.length_counter == 0 {
-                self.length_counter = if self.freq_hi.length_enable() && !fs.next_clocks_length() {
+                self.length_counter = 64;
-                    63
+
-                } else {
+                // length_counter was 0 so length evidently wasn't enabled
-                    64
+                self.freq_hi.set_length_enable(false);
-                };
             };
 
-            // Envelope Behaviour during trigger event
+            // reload freq_timer but last two bits are unmodified
+            self.freq_timer = obscure::square::freq_timer_reload(self.freq_timer, self.frequency());
+
+            // Volume Envelope loaded w/ period
             self.period_timer = self.envelope.period();
+
+            // Channel Volume reloaded
             self.current_volume = self.envelope.init_vol();
 
-            // Sweep behaviour during trigger event
+            // Channel 1 Sweep Behaviour
             let sweep_period = self.sweep.period();
             let sweep_shift = self.sweep.shift_count();
 
@@ -529,7 +507,19 @@ impl Channel1 {
             if sweep_shift != 0 {
                 let _ = self.calc_sweep_freq();
             }
+
+            self.enabled = self.is_dac_enabled();
         }
+
+        obscure::nrx4::length_update(
+            &mut new_freq,
+            fs,
+            &mut self.length_counter,
+            &mut self.enabled,
+            self.freq_hi.length_enable(),
+        );
+
+        self.freq_hi = new_freq;
     }
 
     fn tick(&mut self) {
@@ -603,7 +593,7 @@ pub(crate) struct Channel2 {
     current_volume: u8,
 
     // Length Functionality
-    length_timer: u16,
+    length_counter: u16,
 
     freq_timer: u16,
     duty_pos: u8,
@@ -620,7 +610,7 @@ impl Channel2 {
     /// 0xFF16 | NR21 - Channel 2 Sound length / Wave Pattern Duty
     pub(crate) fn set_duty(&mut self, byte: u8) {
         self.duty = byte.into();
-        self.length_timer = 64 - self.duty.sound_length() as u16;
+        self.length_counter = 64 - self.duty.sound_length() as u16;
     }
 
     /// 0xFF17 | NR22 - Channel 2 Volume ENvelope
@@ -648,22 +638,36 @@ impl Channel2 {
     }
 
     /// 0xFF19 | NR24 - Channel 2 Frequency high
-    pub(crate) fn set_freq_hi(&mut self, byte: u8) {
+    pub(crate) fn set_freq_hi(&mut self, fs: &FrameSequencer, byte: u8) {
+        let prev_le = self.freq_hi.length_enable();
         self.freq_hi = byte.into();
 
-        if self.freq_hi.initial() {
+        obscure::nrx4::length_update(
-            // Envelope behaviour during trigger event
+            &mut self.freq_hi,
+            fs,
+            &mut self.length_counter,
+            &mut self.enabled,
+            prev_le,
+        );
+
+        if self.freq_hi.trigger() {
+            self.enabled = true;
+
+            // Reload length counter if need be
+            if self.length_counter == 0 {
+                self.length_counter = 64;
+            }
+
+            // reload frequency timer
+            self.freq_timer = obscure::square::freq_timer_reload(self.freq_timer, self.frequency());
+
+            // reload envelope
             self.period_timer = self.envelope.period();
+
+            // reload volume
             self.current_volume = self.envelope.init_vol();
 
-            // Length behaviour during trigger event
+            self.enabled = self.is_dac_enabled();
-            if self.length_timer == 0 {
-                self.length_timer = 64;
-            }
-
-            if self.is_dac_enabled() {
-                self.enabled = true;
-            }
         }
     }
 
@@ -714,7 +718,7 @@ pub(crate) struct Channel3 {
     wave_ram: [u8; WAVE_PATTERN_RAM_LEN],
 
     // Length Functionality
-    length_timer: u16,
+    length_counter: u16,
 
     freq_timer: u16,
     offset: u8,
@@ -760,7 +764,7 @@ impl Channel3 {
     /// 0xFF1B | NR31 - Sound Length
     pub(crate) fn set_len(&mut self, byte: u8) {
         self.len = byte;
-        self.length_timer = 256 - self.len as u16;
+        self.length_counter = 256 - self.len as u16;
     }
 
     /// 0xFF1C | NR32 - Channel 3 Volume
@@ -792,18 +796,32 @@ impl Channel3 {
     }
 
     /// 0xFF1E | NR34 - Channel 3 Frequency high
-    pub(crate) fn set_freq_hi(&mut self, byte: u8) {
+    pub(crate) fn set_freq_hi(&mut self, fs: &FrameSequencer, byte: u8) {
+        let prev_le = self.freq_hi.length_enable();
         self.freq_hi = byte.into();
 
-        if self.freq_hi.initial() {
+        obscure::nrx4::length_update(
+            &mut self.freq_hi,
+            fs,
+            &mut self.length_counter,
+            &mut self.enabled,
+            prev_le,
+        );
+
+        if self.freq_hi.trigger() {
+            self.enabled = true;
+
             // Length behaviour during trigger event
-            if self.length_timer == 0 {
+            if self.length_counter == 0 {
-                self.length_timer = 256;
+                self.length_counter = 256;
             }
 
-            if self.dac_enabled {
+            self.freq_timer = (2048 - self.frequency()) * 2;
-                self.enabled = true;
+
-            }
+            // reset wave channel's ptr into wave RAM
+            self.offset = 0;
+
+            self.enabled = self.dac_enabled;
         }
     }
 
@@ -862,7 +880,7 @@ pub(crate) struct Channel4 {
     current_volume: u8,
 
     // Length Functionality
-    length_timer: u16,
+    length_counter: u16,
 
     /// Linear Feedback Shift Register (15-bit)
     lf_shift: u16,
@@ -876,7 +894,7 @@ impl Channel4 {
     /// 0xFF20 | NR41 - Channel 4 Sound Length
     pub(crate) fn set_len(&mut self, byte: u8) {
         self.len = byte & 0x3F;
-        self.length_timer = 64 - self.len as u16;
+        self.length_counter = 64 - self.len as u16;
     }
 
     /// 0xFF21 | NR42 - Channel 4 Volume Envelope
@@ -909,25 +927,37 @@ impl Channel4 {
     }
 
     /// 0xFF23 | NR44 - Channel 4 Counter / Consecutive Selector and Restart
-    pub(crate) fn set_frequency(&mut self, byte: u8) {
+    pub(crate) fn set_frequency(&mut self, fs: &FrameSequencer, byte: u8) {
+        let prev_le = self.freq.length_enable();
         self.freq = byte.into();
 
-        if self.freq.initial() {
+        obscure::nrx4::length_update(
-            // Envelope behaviour during trigger event
+            &mut self.freq,
+            fs,
+            &mut self.length_counter,
+            &mut self.enabled,
+            prev_le,
+        );
+
+        if self.freq.trigger() {
+            self.enabled = true;
+
+            if self.length_counter == 0 {
+                self.length_counter = 64;
+            }
+
+            // FIXME: Frequency Timer reloaded?
+
+            // reload envelope
             self.period_timer = self.envelope.period();
+
+            //reload volume
             self.current_volume = self.envelope.init_vol();
 
-            // Length behaviour during trigger event
+            // LFSR reset
-            if self.length_timer == 0 {
-                self.length_timer = 64;
-            }
-
-            // LFSR behaviour during trigger event
             self.lf_shift = 0x7FFF;
 
-            if self.is_dac_enabled() {
+            self.enabled = self.is_dac_enabled();
-                self.enabled = true;
-            }
         }
     }
 
@@ -972,3 +1002,39 @@ impl Channel4 {
         code << 4
     }
 }
+
+mod obscure {
+    pub(super) mod square {
+        pub(crate) fn freq_timer_reload(freq_timer: u16, frequency: u16) -> u16 {
+            (freq_timer & 0x0003) | (((2048 - frequency) * 4) & 0xFFFC)
+        }
+    }
+
+    pub(super) mod nrx4 {
+        use super::super::{FrameSequencer, NRx4};
+
+        /// Implements the obscure behaviour when writing to NRX4 under certain
+        /// conditions
+        ///
+        /// # Arguments
+        /// * `freq_hi` - mutable reference to a channel's frequency high register
+        /// * `fs` - reference to the APU's frame sequencer
+        /// * `counter` - mutable reference to a channel's internal enabled flag
+        /// * `prev_le` - what length_enable was before NRx4 was written with a new value
+        pub(crate) fn length_update(
+            freq: &mut impl NRx4,
+            fs: &FrameSequencer,
+            counter: &mut u16,
+            enabled: &mut bool,
+            prev_le: bool,
+        ) {
+            if !fs.next_clocks_length() && !prev_le && freq.length_enable() && *counter != 0 {
+                *counter -= 1;
+
+                if *counter == 0 && !freq.trigger() {
+                    *enabled = false;
+                }
+            }
+        }
+    }
+}

src/apu/types.rs

@@ -1,5 +1,11 @@
 use bitfield::bitfield;
 
+pub(crate) trait NRx4 {
+    fn trigger(&self) -> bool;
+    fn length_enable(&self) -> bool;
+    fn set_length_enable(&mut self, value: bool);
+}
+
 pub(crate) mod ch1 {
     use super::bitfield;
 
@@ -103,7 +109,7 @@ pub(super) mod ch3 {
 }
 
 pub(super) mod ch4 {
-    use super::bitfield;
+    use super::{bitfield, NRx4};
 
     bitfield! {
         pub struct PolynomialCounter(u8);
@@ -177,17 +183,21 @@ pub(super) mod ch4 {
     bitfield! {
         pub struct Frequency(u8);
         impl Debug;
-        _initial, _: 7;
+        _trigger, _: 7;
-        _length_enable, _: 6;
+        _length_enable, _set_length_enable: 6;
     }
 
-    impl Frequency {
+    impl NRx4 for Frequency {
-        pub(crate) fn length_enable(&self) -> bool {
+        fn trigger(&self) -> bool {
+            self._trigger()
+        }
+
+        fn length_enable(&self) -> bool {
             self._length_enable()
         }
 
-        pub(crate) fn initial(&self) -> bool {
+        fn set_length_enable(&mut self, value: bool) {
-            self._initial()
+            self._set_length_enable(value);
         }
     }
 
@@ -218,24 +228,28 @@ pub(super) mod ch4 {
 }
 
 pub(super) mod common {
-    use super::bitfield;
+    use super::{bitfield, NRx4};
 
     bitfield! {
         pub struct FrequencyHigh(u8);
         impl Debug;
-        _initial, _: 7;
+        _trigger, _: 7;
-        _length_enable, _: 6;
+        _length_enable, _set_length_enable: 6;
         pub freq_bits, set_freq_bits: 2, 0;
     }
 
-    impl FrequencyHigh {
+    impl NRx4 for FrequencyHigh {
-        pub(crate) fn initial(&self) -> bool {
+        fn trigger(&self) -> bool {
-            self._initial()
+            self._trigger()
         }
 
-        pub(crate) fn length_enable(&self) -> bool {
+        fn length_enable(&self) -> bool {
             self._length_enable()
         }
+
+        fn set_length_enable(&mut self, value: bool) {
+            self._set_length_enable(value);
+        }
     }
 
     impl Copy for FrequencyHigh {}
@@ -557,22 +571,10 @@ pub(super) mod fs {
             };
         }
 
-        fn peek(&self) -> State {
-            use State::*;
-
-            match (self.step + 1) % 8 {
-                1 | 3 | 5 => Nothing,
-                0 | 4 => Length,
-                2 | 6 => LengthAndSweep,
-                7 => Envelope,
-                num => unreachable!("Step {} is invalid for the Frame Sequencer", num),
-            }
-        }
-
         pub(crate) fn next_clocks_length(&self) -> bool {
             use State::*;
 
-            match self.peek() {
+            match self.state {
                 Length | LengthAndSweep => true,
                 Nothing | Envelope => false,
             }