I'm using the NAM code in another context (ie: not using IPlug2). I've currently got my own hacked up version, but it would be nice to have the core code be more easily isolated from the specific plugin context.

To this end:

The dsp code is currently using "iplug::sample" for the sample format. The neural network code is all float-based. Maybe it should require I/O as float?

The dsp code references parameters, but they don't seem to actually be used anywhere (unless I'm missing something?). It also takes input/output gain, but they seem to be hardcoded to 1.0 - the actual gain adjustments seem to be done independently in the plugin (which I think makes sense). Maybe the dsp gain code, the parameters, or both could be removed?

Look into this. Doesn't sound right.

See http://eigen.tuxfamily.org/dox-3.2/group__TopicStructHavingEigenMembers.html and act accordingly.

the root cause is that the skipping and remainder code part has a bug:
const int skipChars = extraBytes / 4

should be:
const int skipChars = extraBytes / 4 * 4; // truncate to dword size

Have a fix here:
#63

Now that it's in AudioDSPTools.

This line fails to compile:

In file included from ../src/NAM/NeuralAmpModelerCore/NAM/lstm.cpp:5:
../src/NAM/NeuralAmpModelerCore/NAM/lstm.h:26:84: error: no member named 'lastN' in namespace 'Eigen::placeholders'
   26 |   Eigen::VectorXf get_hidden_state() const { return this->_xh(Eigen::placeholders::lastN(this->_get_hidden_size())); };
      |                                                               ~~~~~~~~~~~~~~~~~~~~~^

The latest eigen-3.4.0 is used.

What version of Eigen are you using that it is able to build with?

Instead of the current looking for a negative sample rate to imply that the model doesn't know what it expects, let's add this to the API explicitly, similarly to how the model loudness is handled.

get_dsp() only supports models whose version is 0.5.0 and 0.5.1. It should support any model whose major version is zero and minor version is 5; patch version match shouldn't be necessary for basic functionality.

If not fixed, a fix on this will be requied every tim the patch version bumps on neural-amp-modeler--not fun!

I haven't properly checked if this is the case here, but using Eigen types as function parameters can result in unwanted temporary copies in some cases. The Eigen::Ref<> class exists to work around this. Further information here: http://eigen.tuxfamily.org/dox/TopicFunctionTakingEigenTypes.html#TopicUsingRefClass

I wondered if this might be one lever for optimizing the NAMCore code

https://github.com/sdatkinson/NeuralAmpModelerCore/blob/885a535d29022138f3efcbfd73aed4743f750b68/NAM/wavenet.h#L102C6-L102C6

Ope!

Get rid of TARGET_DSP_LOUDNESS.

Downstream projects will take care of this themselves.

Along with the other cleanup of the core interfaces, I think it makes sense to consider making the model processing mono (rather than multi-channel).

There isn't any internal support for more than a single channel, and it would make the interface cleaner.

This Issue is for models to expose a getter, .GetExpectedSampleRate(), so that interfacing code can understand what the ML models implicitly expect to receive.

Since get_dsp is part of this repo, we are in charge of deciding what to do if no sample rate is recorded as part of the model (i.e. from .nam files pre-sdatkinson/neural-amp-modeler#284). In this case, I will return -1.0, and this will be defined to mean "We don't know." (NeuralAmpModelerPlugin will take this to mean that it must be the implicit default from earlier, 48kHz; others are free to do as they'd like, though this will likely be what they want to do as well assuming they're building for the same user ecosystem.)

The semantics are also that this is the expected sample rate--there's nothing artistically wrong with disregarding it, and this is an artist's tool at the end of the day 🙂

See: sdatkinson/NeuralAmpModelerPlugin#219

Thought I'd add an issue to track this.

Looking at the code, it seems like if we get rid of the parametric stuff from WaveNet, we can get rid of the "condition" matrix, avoid a copy, and work directly on the input:

Implement Resample, which subclasses DSP. It encapsulates another DSP object, and takes care of the incoming and outgoing resampling required to expose a buffer to the encapsulated object at the sample rate that it expects.

The use case for this is sdatkinson/NeuralAmpModelerPlugin#59. For example, a NAM model that expects a sample rate of 48k may be encapsulated, and the DAW may be running the plugin with a buffer provided at a sample rate of 44.1kHz. Resample will:

• Resample the incoming buffer to 48k
• Provide the resampled buffer to the encapsulated NAM model
• Resample the NAM model's output back to 44.1k.

This will incur a delay due to the bounding samples that are required to interpolate a given sample as well as the integer sample numbers, meaning that a varying number of samples will be outputted by both of the resampling functions.

If the inner and outer sampling rates are equal, then it would be great to bypass any interpolation math and do a straight pass-through.

The purpose of the resampling block is to expose a clean interface around the encapsulated DSP object, and should be orthogonal from the resampling algorithm itself that's used. I'll provide a simple cubic interpolation algorithm, and other developers may swap in more accurate or better-optimized routines as they'd like.

neural-amp-modeler v0.5.1 defines a metadata dict in the model files containing a "loudness" value. Store this (if available) in models.

It would be great to get the fast tanh code out there - either by default, or as a toggle.

It drastically improves performance, and could do a lot to mitigate the high-CPU usage complaints people have.

PR #28 makes it very easy to integrate into the plugin.

See: sdatkinson/NeuralAmpModelerPlugin#296

E.g.

The skip-list is getting long. An allow-list (i.e. idIsNotJunk()) will probably be a simpler implementation and support more formats.

These result in a lot of copied code. Instead, we can ->SetLoudness() once at the end of get_dsp().

Pick up sdatkinson/NeuralAmpModelerPlugin#100

It's ugly being part of the public interface to nam::DSP? Perhaps it can be worked into the end of the process() routine?

The WaveNet implementation has some "anti-pop" code for handling discontinuities in the audio stream when switching models. The other network types could benefit from this as well. Perhaps it could be moved into the underlying dsp code?

I'm happy to make the required code changes.

Currently, the WaveNet model processing code re-sizes vectors and matrices based on the audio buffer size during processing. This is non-ideal for real-time operation. Instead, all sizing operations should be done out-of-band of the processing loop.

In most cases, the current behavior should not cause significant problems. If there is a fixed audio buffer size the resize operations should only happen once. A fixed buffer size is not guaranteed, however - DAWs will sometimes vary the block size.

Extend get_dsp() to accept not only accept file paths but model data. This requires defining a struct to contain the information that is usually loaded in from a model file.

Thanks to #34 it's possible to build some tools automatically 🎉

Next step would be to try running them with some models (LSTM, WaveNet, ConvNet).

Some super-light models should do--the goal I have with this Issue is to be able to assert correct operation--so we can be more confident as we iterate on the core code that we're not breaking models, etc.

Another chunk that we need to ignore.

Pick up sdatkinson/NeuralAmpModelerPlugin#84

Get things out of the global namespace so that things are less likely to collide with downstream uses.

Propose using namcore.

Currently, there are at least two buffer copies (one at input and one at output) that could be avoided by directly using the passed in input/output buffers.

See: sdatkinson/NeuralAmpModelerPlugin#300

Make all models be the same loudness, normalizing out their training data's loudness.

While reviewing #78 I realized that the DSP class's structure might be somewhat simplified by making process() do what _process_core_() does currently. In most cases, the existing _process_core_() implementations could become the implementations for process() directly with minimal modifications (e.g. add three lines to assign the pointers and buffer length before proceeding with the rest).

However, the current _apply_output_level_() is taking care of output level normalization; if we want this to remain part of this lbirary's responsibility, then the current class structure is a nice and DRY solution, but I can also see the justification of pushing this out of DSP and letting plugins using NAM take care of it themselves (getting the information they want via e.g. a new DSP::GetLoudness().

I'm somewhat keen to do the latter (simplify this library and push out the gain adjustment to be the responsibility of the plugin) but want to raise it as an issue here to see if anyone has any strong opinions or thoughts if I'm missing something.

HPF as a prerequisite to resolve sdatkinson/NeuralAmpModelerPlugin#271

LPF, might as well while we're at it.

This is being removed from NeuralAmpModelerCore in an effort to slim down its scope to focus on the NAM-specific DSP and not on general things like biquads :)

It would be great to have an automated build/test setup for the core, triggered by PRs.

It could ensure the code builds without errors, and run a set of test models on test data to make sure the models are producing the expected output.

So that we can ask what it currently expects. Needed to resolve sdatkinson/NeuralAmpModelerPlugin#351 efficiently.

~DSP();
should be:

virtual ~DSP();

It doesn't seem right that there would be a "target" loudness in this library; that sounds like a concern of someone else using it (e.g. NeuralAmpModelerPlugin). Accordingly, it doesn't make sense to assign that target loudness to the attribute on construction if the model data doesn't have one provided. Instead, valid usage might look like first asking whether the model knows its loudness by calling HasLoudness(), then subsequently asking what that loudness is with GetLoudness().

It's not possible here to use e.g. -1 as the fallback value since Loudness is in dB and can be positive or negative.

This would cause a minor version bump.

It might make sense to rename dsp.cpp/.h to nam.cpp/.h and also rename the DSP class.

Same as sdatkinson/neural-amp-modeler#277, though I'm not sure if Sphinx works for C++ (never tried).

In order to make the core NAM network code more portable, it would be good to separate it from the other plugin dsp code (noise gate, EQ, impulse response).

I propose a separate source directory for the actual NN code ("NAM"?) and separation of the existing code in dsp.h (it currently has two DSP class implementations - one for the network code and another for the other dsp functions).

If this seems good to you, I'll put together a quick PR for it.

The pre-warm code might be leaner in a separate method that gets called after model initialization in get_dsp.

Then WaveNet (and I assume ConvNet) can override it to pre-warm over the receptive field and LSTM can do nothing.

Also, it would make it possible to avoid the default pre-warm behavior and do something else if you bypass get_dsp.

Define a method to reset the model. Proposed signature:

virtual void Reset(const double sampleRate, const int maxBlockSize);

This function would be responsible for

• Resizing arrays
• (Potentially?) doing the warm-up calculations. [_I'm not sure about this since get_dsp() doesn't ask about the max block size, so we couldn't know what the max block size is at initialization, necessarily?

Unfortunately, I can't really do anything with the sample rate for the models that are currently supported; however, this isn't typical for effects, and it could be useful for subclasses that can do something with it (e.g. the resampling class that I'm implementing in NeuralAmpModelerPlugin). So, having it as part of the signature may be helpful for extending the method.