debugging FbWr/FbRd, the buffer is not read correctly

src/dsp/mod.rs

@@ -14,6 +14,8 @@ mod node_test;
 mod node_tseq;
 #[allow(non_upper_case_globals)]
 mod node_sampl;
+#[allow(non_upper_case_globals)]
+mod node_fbwr_fbrd;
 
 pub mod tracker;
 mod satom;
@@ -35,6 +37,8 @@ use node_out::Out;
 use node_test::Test;
 use node_tseq::TSeq;
 use node_sampl::Sampl;
+use node_fbwr_fbrd::FbWr;
+use node_fbwr_fbrd::FbRd;
 
 pub const MIDI_MAX_FREQ : f32 = 13289.75;
 
@@ -303,6 +307,11 @@ macro_rules! node_list {
              // | | |    |       defa/lt_/value
              // | | |    |       |  |   /
                {2 0 mono setting(0) 0  1},
+            fbwr => FbWr UIType::Generic UICategory::IOUtil
+               (0  inp   n_id      d_id      -1.0, 1.0, 0.0),
+            fbrd => FbRd UIType::Generic UICategory::IOUtil
+               (0  atv   n_id      d_id      -1.0, 1.0, 1.0)
+               [0 sig],
             test => Test UIType::Generic UICategory::IOUtil
                (0 f     n_id      d_id       0.0, 1.0, 0.5)
                {1 0 s    setting(0) 0  10},
@@ -321,7 +330,7 @@ pub mod labels {
     }
 
     pub mod Out {
-        pub const mono : [&'static str; 2] = ["Mono", "Stereo"];
+        pub const mono : [&'static str; 2] = ["Stereo", "Mono"];
     }
 
     pub mod Amp {
@@ -1099,7 +1108,7 @@ pub fn node_factory(node_id: NodeId) -> Option<(Node, NodeInfo)> {
         ) => {
             match node_id {
                 $(NodeId::$variant(_) => Some((
-                    Node::$variant { node: $variant::new() },
+                    Node::$variant { node: $variant::new(&node_id) },
                     NodeInfo::from_node_id(node_id),
                 )),)+
                 _ => None,

src/dsp/node_amp.rs

@@ -3,7 +3,7 @@
 // See README.md and COPYING for details.
 
 use crate::nodes::{NodeAudioContext, NodeExecContext};
-use crate::dsp::{SAtom, ProcBuf, DspNode, LedPhaseVals};
+use crate::dsp::{NodeId, SAtom, ProcBuf, DspNode, LedPhaseVals};
 
 /// A simple amplifier
 #[derive(Debug, Clone)]
@@ -11,7 +11,7 @@ pub struct Amp {
 }
 
 impl Amp {
-    pub fn new() -> Self {
+    pub fn new(_nid: &NodeId) -> Self {
         Self {
         }
     }

src/dsp/node_out.rs

@@ -3,7 +3,7 @@
 // See README.md and COPYING for details.
 
 use crate::nodes::{NodeAudioContext, NodeExecContext};
-use crate::dsp::{SAtom, ProcBuf, inp, at, DspNode, LedPhaseVals};
+use crate::dsp::{NodeId, SAtom, ProcBuf, inp, at, DspNode, LedPhaseVals};
 
 /// The (stereo) output port of the plugin
 #[derive(Debug, Clone)]
@@ -14,7 +14,7 @@ pub struct Out {
 }
 
 impl Out {
-    pub fn new() -> Self {
+    pub fn new(_nid: &NodeId) -> Self {
         Self {
             input:  [0.0; 2],
         }

src/dsp/node_sampl.rs

@@ -3,7 +3,7 @@
 // See README.md and COPYING for details.
 
 use crate::nodes::{NodeAudioContext, NodeExecContext};
-use crate::dsp::{SAtom, ProcBuf, DspNode, LedPhaseVals};
+use crate::dsp::{NodeId, SAtom, ProcBuf, DspNode, LedPhaseVals};
 use crate::dsp::{out, at, inp, denorm}; //, inp, denorm, denorm_v, inp_dir, at};
 use super::helpers::Trigger;
 
@@ -19,7 +19,7 @@ pub struct Sampl {
 }
 
 impl Sampl {
-    pub fn new() -> Self {
+    pub fn new(_nid: &NodeId) -> Self {
         Self {
             phase:          0.0,
             srate:          44100.0,

src/dsp/node_sin.rs

@@ -3,7 +3,7 @@
 // See README.md and COPYING for details.
 
 use crate::nodes::{NodeAudioContext, NodeExecContext};
-use crate::dsp::{SAtom, ProcBuf, denorm, out, inp, DspNode, LedPhaseVals};
+use crate::dsp::{NodeId, SAtom, ProcBuf, denorm, out, inp, DspNode, LedPhaseVals};
 use crate::dsp::helpers::fast_sin;
 
 
@@ -19,7 +19,7 @@ pub struct Sin {
 const TWOPI : f32 = 2.0 * std::f32::consts::PI;
 
 impl Sin {
-    pub fn new() -> Self {
+    pub fn new(_nid: &NodeId) -> Self {
         Self {
             srate: 44100.0,
             phase: 0.0,

src/dsp/node_test.rs

@@ -3,7 +3,7 @@
 // See README.md and COPYING for details.
 
 use crate::nodes::{NodeAudioContext, NodeExecContext};
-use crate::dsp::{SAtom, ProcBuf, GraphFun, GraphAtomData, DspNode, LedPhaseVals};
+use crate::dsp::{NodeId, SAtom, ProcBuf, GraphFun, GraphAtomData, DspNode, LedPhaseVals};
 
 /// A simple amplifier
 #[derive(Debug, Clone)]
@@ -11,7 +11,7 @@ pub struct Test {
 }
 
 impl Test {
-    pub fn new() -> Self {
+    pub fn new(_nid: &NodeId) -> Self {
         Self {
         }
     }

src/dsp/node_tseq.rs

@@ -4,7 +4,7 @@
 
 use crate::nodes::{NodeAudioContext, NodeExecContext};
 use crate::dsp::helpers::TriggerClock;
-use crate::dsp::{SAtom, ProcBuf, DspNode, LedPhaseVals};
+use crate::dsp::{NodeId, SAtom, ProcBuf, DspNode, LedPhaseVals};
 use crate::dsp::tracker::TrackerBackend;
 
 use crate::dsp::MAX_BLOCK_SIZE;
@@ -18,11 +18,11 @@ pub struct TSeq {
 }
 
 impl Clone for TSeq {
-    fn clone(&self) -> Self { Self::new() }
+    fn clone(&self) -> Self { Self::new(&NodeId::Nop) }
 }
 
 impl TSeq {
-    pub fn new() -> Self {
+    pub fn new(_nid: &NodeId) -> Self {
         Self {
             backend:       None,
             srate:         48000.0,

src/nodes/node_exec.rs

@@ -82,20 +82,34 @@ pub trait NodeAudioContext {
     fn input(&mut self, channel: usize, frame: usize) -> f32;
 }
 
-/// Implements a trivial delay buffer for the feedback nodes
+/// Implements a trivial buffer for the feedback nodes
 /// FbWr and FbRd.
-pub struct FeedbackDelay {
+///
+/// Note that the previous audio period or even the first one ever may
+/// produce less than 64 samples. This means we need to keep track
+/// how many samples were actually written to the feedback buffer!
+/// See also `sample_count` field.
+pub struct FeedbackBuffer {
+    /// The feedback buffer that holds the samples of the previous period.
     buffer:         [f32; MAX_FB_DELAY_SIZE],
+    /// The write pointer.
     write_ptr:      usize,
+    /// Read pointer, is always behind write_ptr by an initial amount
     read_ptr:       usize,
+    /// The number of samples written into the buffer (to prevent overreads)
+    /// We need to keep track of the number of samples actually written into
+    /// the delay because the first or previous periods may have produced
+    /// not enough samples.
+    sample_count:   usize,
 }
 
-impl FeedbackDelay {
+impl FeedbackBuffer {
     pub fn new() -> Self {
         Self {
             buffer:         [0.0; MAX_FB_DELAY_SIZE],
             write_ptr:      0,
             read_ptr:       (64 + MAX_FB_DELAY_SIZE) % MAX_FB_DELAY_SIZE,
+            sample_count:   0,
         }
     }
 
@@ -106,6 +120,7 @@ impl FeedbackDelay {
     pub fn set_sample_rate(&mut self, sr: f32) {
         self.buffer            = [0.0; MAX_FB_DELAY_SIZE];
         self.write_ptr         = 0;
+        self.sample_count      = 0;
         // The delay sample count maximum is defined by MAX_FB_DELAY_SRATE,
         // after that the feedback delays become shorter than they should be
         // and things won't sound the same at sample rate
@@ -125,26 +140,35 @@ impl FeedbackDelay {
     #[inline]
     pub fn write(&mut self, s: f32) {
         self.write_ptr = (self.write_ptr + 1) % MAX_FB_DELAY_SIZE;
+        self.sample_count += 1;
         self.buffer[self.write_ptr] = s;
+        println!("WRITE[{}]={}", self.write_ptr, s);
     }
 
     #[inline]
     pub fn read(&mut self) -> f32 {
+        if self.sample_count > 0 {
+            self.sample_count - 1;
             self.read_ptr = (self.read_ptr + 1) % MAX_FB_DELAY_SIZE;
-        self.buffer[self.read_ptr]
+            let s = self.buffer[self.read_ptr];
+            println!("READ[{}]={}", self.read_ptr, s);
+            s
+        } else {
+            0.0
+        }
     }
 }
 
 /// Contains global state that all nodes can access.
 /// This is used for instance to implement the feedbackd delay nodes.
 pub struct NodeExecContext {
-    pub feedback_delay_buffers:     Vec<FeedbackDelay>,
+    pub feedback_delay_buffers:     Vec<FeedbackBuffer>,
 }
 
 impl NodeExecContext {
     fn new() -> Self {
         let mut fbdb = vec![];
-        fbdb.resize_with(MAX_ALLOCATED_NODES, || FeedbackDelay::new());
+        fbdb.resize_with(MAX_ALLOCATED_NODES, || FeedbackBuffer::new());
         Self {
             feedback_delay_buffers: fbdb,
         }

tests/basics.rs

@@ -916,3 +916,46 @@ fn check_matrix_output_feedback() {
     assert_float_eq!(fo_amp.1, 0.11627);
 }
 
+#[test]
+fn check_matrix_node_feedback() {
+    let (node_conf, mut node_exec) = new_node_engine();
+    let mut matrix = Matrix::new(node_conf, 7, 7);
+
+    let sin  = NodeId::Sin(0);
+    let sin2 = NodeId::Sin(1);
+    let wr   = NodeId::FbWr(0);
+    let rd   = NodeId::FbRd(0);
+    let wr2  = NodeId::FbWr(1);
+    let rd2  = NodeId::FbRd(1);
+    let out  = NodeId::Out(0);
+    matrix.place(0, 0, Cell::empty(sin)
+                       .out(None, None, sin.out("sig")));
+    matrix.place(0, 1, Cell::empty(wr)
+                       .input(wr.inp("inp"), None, None));
+    matrix.place(1, 0, Cell::empty(rd)
+                       .out(None, None, rd.out("sig")));
+    matrix.place(1, 1, Cell::empty(out)
+                       .input(out.inp("ch1"), None, None));
+
+    matrix.place(0, 2, Cell::empty(sin2)
+                       .out(None, None, sin2.out("sig")));
+    matrix.place(0, 3, Cell::empty(wr2)
+                       .input(wr2.inp("inp"), None, None));
+    matrix.place(1, 2, Cell::empty(rd2)
+                       .out(None, None, rd2.out("sig")));
+    matrix.place(1, 3, Cell::empty(out)
+                       .input(out.inp("ch2"), None, None));
+    matrix.sync().unwrap();
+
+    let freq_param = sin2.inp_param("freq").unwrap();
+    matrix.set_param(
+        freq_param,
+        SAtom::param(freq_param.norm(880.0)));
+
+    let (out_l, out_r) = run_for_ms(&mut node_exec, 5.0);
+
+    println!("L: {:?}", out_l);
+    println!("R: {:?}", out_r);
+
+    assert!(false);
+}