From d8f41bc2c9baf459dc8d7a72f24d6f58aba905c1 Mon Sep 17 00:00:00 2001 From: "se.cherkasov" Date: Sat, 14 Mar 2026 17:35:35 +0300 Subject: [PATCH] fix(apu): correct frame counter timing, add LP filter, mute aliased triangle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix frame counter running at 2× speed: clock_frame_counter now skips odd CPU cycles (APU cycle = CPU/2), so envelope, sweep, and length counters tick at the correct rate. Fixes sweep-driven whistle in Megaman II. - Switch audio sampling to per-CPU-cycle granularity in run_until_frame_complete_with_audio to eliminate square-wave harmonic aliasing caused by sampling only once per instruction. - Add IIR one-pole low-pass filter (~14 kHz) to AudioMixer to smooth abrupt level transitions (crackling) introduced by correct envelope timing. - Mute triangle channel when timer_period < 2 (≥27 kHz), which aliases into the audible range at 48 kHz. Real NES RC circuit removes these ultrasonics; emulator must suppress them explicitly. - Update all APU bus tests to use correct (doubled) CPU cycle counts. --- src/native_core/apu/api.rs | 6 +++++- src/native_core/apu/timing.rs | 3 +++ src/native_core/bus/tests/apu.rs | 24 ++++++++++++------------ src/runtime/audio.rs | 19 +++++++++++++++++-- src/runtime/core.rs | 12 ++++++++++-- 5 files changed, 47 insertions(+), 17 deletions(-) diff --git a/src/native_core/apu/api.rs b/src/native_core/apu/api.rs index bf09c1f..0116f4e 100644 --- a/src/native_core/apu/api.rs +++ b/src/native_core/apu/api.rs @@ -331,9 +331,13 @@ impl Apu { }; let triangle = { + // Timer period < 2 produces ultrasonic output (~28-56 kHz) that aliases + // to audible frequencies when sampled at 48 kHz. Real hardware filters + // this via the RC output stage; mute here to match that behaviour. let active = (self.channel_enable_mask & 0x04) != 0 && self.length_counters[2] > 0 - && self.triangle_linear_counter > 0; + && self.triangle_linear_counter > 0 + && self.triangle_timer_period() >= 2; if active { TRIANGLE_SEQUENCE[self.triangle_step as usize & 0x1F] } else { diff --git a/src/native_core/apu/timing.rs b/src/native_core/apu/timing.rs index a0f9644..d9da2f0 100644 --- a/src/native_core/apu/timing.rs +++ b/src/native_core/apu/timing.rs @@ -14,6 +14,9 @@ impl Apu { status } pub(crate) fn clock_frame_counter(&mut self) { + if self.cpu_cycle_parity { + return; + } let seq_len = if self.frame_mode_5step { APU_FRAME_SEQ_5_STEP_CYCLES } else { diff --git a/src/native_core/bus/tests/apu.rs b/src/native_core/bus/tests/apu.rs index 9f99f25..8365d65 100644 --- a/src/native_core/bus/tests/apu.rs +++ b/src/native_core/bus/tests/apu.rs @@ -5,7 +5,7 @@ fn apu_frame_irq_asserts_in_4_step_mode() { let mut bus = NativeBus::new(Box::new(StubMapper)); bus.write(0x4017, 0x00); // 4-step, IRQ enabled - for _ in 0..14_918u32 { + for _ in 0..29_832u32 { bus.clock_cpu(1); } @@ -17,7 +17,7 @@ fn reading_4015_clears_apu_frame_irq_flag() { let mut bus = NativeBus::new(Box::new(StubMapper)); bus.write(0x4017, 0x00); // 4-step, IRQ enabled - for _ in 0..14_918u32 { + for _ in 0..29_832u32 { bus.clock_cpu(1); } @@ -30,7 +30,7 @@ fn reading_4015_clears_apu_frame_irq_flag() { fn apu_frame_irq_inhibit_bit_disables_irq_and_clears_pending() { let mut bus = NativeBus::new(Box::new(StubMapper)); bus.write(0x4017, 0x00); // 4-step, IRQ enabled - for _ in 0..14_918u32 { + for _ in 0..29_832u32 { bus.clock_cpu(1); } assert!(bus.poll_irq()); @@ -46,13 +46,13 @@ fn apu_frame_irq_inhibit_bit_disables_irq_and_clears_pending() { fn writing_4015_does_not_acknowledge_apu_frame_irq() { let mut bus = NativeBus::new(Box::new(StubMapper)); bus.write(0x4017, 0x00); // 4-step, IRQ enabled - for _ in 0..14_918u32 { + for _ in 0..29_832u32 { bus.clock_cpu(1); } assert!(bus.poll_irq(), "frame IRQ must be pending"); // Recreate pending frame IRQ and ensure $4015 write does not clear it. - for _ in 0..14_918u32 { + for _ in 0..29_832u32 { bus.clock_cpu(1); } bus.write(0x4015, 0x00); @@ -183,11 +183,11 @@ fn apu_length_counter_decrements_on_half_frame_when_not_halted() { bus.write(0x4003, 0x18); // length index 3 => value 2 assert_eq!(bus.apu.length_counters[0], 2); - for _ in 0..7_457u32 { + for _ in 0..14_913u32 { bus.clock_cpu(1); } assert_eq!(bus.apu.length_counters[0], 1); - for _ in 0..7_458u32 { + for _ in 0..14_916u32 { bus.clock_cpu(1); } assert_eq!(bus.apu.length_counters[0], 0); @@ -218,13 +218,13 @@ fn quarter_frame_clocks_triangle_linear_counter() { bus.write(0x4008, 0x05); // control=0, reload value=5 bus.write(0x400B, 0x00); // set reload flag - for _ in 0..3_729u32 { + for _ in 0..7_457u32 { bus.clock_cpu(1); } assert_eq!(bus.apu.triangle_linear_counter, 5); assert!(!bus.apu.triangle_linear_reload_flag); - for _ in 0..3_728u32 { + for _ in 0..7_456u32 { bus.clock_cpu(1); } assert_eq!(bus.apu.triangle_linear_counter, 4); @@ -238,7 +238,7 @@ fn quarter_frame_envelope_start_reloads_decay() { bus.write(0x4003, 0x00); // start envelope assert_ne!(bus.apu.envelope_start_flags & 0x01, 0); - for _ in 0..3_729u32 { + for _ in 0..7_457u32 { bus.clock_cpu(1); } assert_eq!(bus.apu.envelope_decay[0], 15); @@ -253,7 +253,7 @@ fn sweep_half_frame_updates_pulse_timer_period() { bus.write(0x4003, 0x02); // timer high => period 0x200 bus.write(0x4001, 0x82); // enable, period=1, negate=0, shift=2 - for _ in 0..7_457u32 { + for _ in 0..14_913u32 { bus.clock_cpu(1); } assert_eq!(bus.apu.read(0x4002), 0x80); @@ -267,7 +267,7 @@ fn sweep_negative_pulse1_uses_ones_complement() { bus.write(0x4003, 0x02); bus.write(0x4001, 0x8A); // enable, period=1, negate=1, shift=2 - for _ in 0..7_457u32 { + for _ in 0..14_913u32 { bus.clock_cpu(1); } assert_eq!(bus.apu.read(0x4002), 0x7F); diff --git a/src/runtime/audio.rs b/src/runtime/audio.rs index 349ec80..01256b4 100644 --- a/src/runtime/audio.rs +++ b/src/runtime/audio.rs @@ -7,16 +7,23 @@ pub struct AudioMixer { samples_per_cpu_cycle: f64, sample_accumulator: f64, last_output_sample: f32, + // One-pole IIR low-pass filter state (approximates NES ~14 kHz RC filter). + // Coefficient: a = exp(-2π * fc / fs). At fc=14000, fs=48000: a ≈ 0.160 + lp_coeff: f32, + lp_state: f32, } impl AudioMixer { pub fn new(sample_rate: u32, mode: VideoMode) -> Self { let cpu_hz = mode.cpu_hz(); + let lp_coeff = (-2.0 * std::f64::consts::PI * 14_000.0 / sample_rate as f64).exp() as f32; Self { sample_rate, samples_per_cpu_cycle: sample_rate as f64 / cpu_hz, sample_accumulator: 0.0, last_output_sample: 0.0, + lp_coeff, + lp_state: 0.0, } } @@ -27,6 +34,7 @@ impl AudioMixer { pub fn reset(&mut self) { self.sample_accumulator = 0.0; self.last_output_sample = 0.0; + self.lp_state = 0.0; } pub fn push_cycles(&mut self, cpu_cycles: u32, channels: ChannelOutputs, out: &mut Vec) { @@ -45,13 +53,20 @@ impl AudioMixer { } let start = self.last_output_sample; + let a = self.lp_coeff; + let b = 1.0 - a; if samples == 1 { - out.push(sample); + let s = a * self.lp_state + b * sample; + self.lp_state = s; + out.push(s); } else { let denom = samples as f32; for idx in 0..samples { let t = (idx + 1) as f32 / denom; - out.push(start + (sample - start) * t); + let interp = start + (sample - start) * t; + let s = a * self.lp_state + b * interp; + self.lp_state = s; + out.push(s); } } self.last_output_sample = sample; diff --git a/src/runtime/core.rs b/src/runtime/core.rs index 90cbbb4..685d442 100644 --- a/src/runtime/core.rs +++ b/src/runtime/core.rs @@ -108,8 +108,16 @@ impl NesRuntime { ) -> Result<(), RuntimeError> { self.bus.begin_frame(); while !self.bus.take_frame_complete() { - let cycles = self.step_instruction()?; - mixer.push_cycles(cycles, self.bus.apu_channel_outputs(), out_samples); + self.bus.set_joypad_buttons(self.buttons); + let cpu_cycles = self.cpu.step(&mut self.bus).map_err(RuntimeError::Cpu)?; + // Sample APU output once per CPU cycle for better audio resolution. + // OAM DMA cycles (triggered inside cpu.step) are captured in the + // first take_cpu_cycles_since_poll call of this instruction. + for _ in 0..cpu_cycles { + self.bus.clock_cpu(1); + let actual = self.bus.take_cpu_cycles_since_poll(); + mixer.push_cycles(actual, self.bus.apu_channel_outputs(), out_samples); + } } self.frame_number = self.frame_number.saturating_add(1); Ok(())