The issue:
NAM A2 models incorrectly report `DSP::HasLoudness() == false.
Repro.
std::filesystem::path path { "A2 Test Model - Mesa_Boogie Dual Rectifier Gain 10.nam"};
std::unique_ptr<nam::DSP> namDsp = nam::get_dsp(config_filename);
assert(namDsp->HasLoudness()); // incorrectly returns false..
Discussion
I'm currently using the input/output loudness metadata methods in PiPedal (HasInputLevel(), GetInputLevel(), HasLoudness(), GetLoudness(), HasOUtputLevel(), &c). It was nice to see these added; but calling any of these methods on an A2 model with a top-level slimmable container indicates that the A2 model does not have loudness metadata.
The core problem is that metadata values are defined per submodule in a slimmable container, and the top-level module (the slimmable container, of type nam::container::ContainerModel) does not have metadata. The metadata is, instead, present on each of the submodules.
There's also a minor flaw in initialization and prewarming code for A2 slimmable containers, that's worth considering if changes are made to fix this. As written, nam::container::ContainerModel pre-warms ALL submodules instead of just the active submodule. It doesn't seem like a great idea to have a model that might run with a Slimmable value of 0.5 to pre-warm its larger 1.0 submodule as well. ContainerModel::Reset also resets all submodules, although that's not really as big of an issue as is superfluous prewarming. And, although one can selectively reset and prewarm using SetSlimmableSize, you still have to call ContainerModel::Reset to set the value of sampleRate and maxBufferSize). So you end up resetting all the submodules anyway (and the reset active submodule twice).
proposed fix
The proposed fix, I think, would be to copy metdata settings out of the active submodule into the top-level submodule in the ContainerModel constructor. (which would requires protected setter methods in nam::DSP). When the constructor exits, the active submodule is set to the last (and presumably best) submodule. Add code to copy metadata settings out of the active submodule into the top-level ContainerModel. Also copy metadata settings from the active submodule again, in ContainerModel::SetSlimmableSize(const double val). And perhaps ensure that Reset, prewarm, and ResetAndPrewarm act only on the active submodule. The activation sequence for a client that is using ContainerModel::SetSlimmableSize thus becomes. Modify the documentation of SetSlimmableSize to indicate that it MUST be followed by a call to Reset and prewarm.
std::unique_ptr<nam::DSP) dsp = nam::get_dsp(config_filename);
if (hasSlimmableSize)
{
dsp->SetSlimmableSize(slimmableSize);
}
dsp->ResetAndPrewarm(sampleRate,maxBufferSize);
Offer to pull
I'd be happy to send you a pull request with necessary changes if that would be helpful to you.
The issue:
NAM A2 models incorrectly report `DSP::HasLoudness() == false.
Repro.
Discussion
I'm currently using the input/output loudness metadata methods in PiPedal (HasInputLevel(), GetInputLevel(), HasLoudness(), GetLoudness(), HasOUtputLevel(), &c). It was nice to see these added; but calling any of these methods on an A2 model with a top-level slimmable container indicates that the A2 model does not have loudness metadata.
The core problem is that metadata values are defined per submodule in a slimmable container, and the top-level module (the slimmable container, of type nam::container::ContainerModel) does not have metadata. The metadata is, instead, present on each of the submodules.
There's also a minor flaw in initialization and prewarming code for A2 slimmable containers, that's worth considering if changes are made to fix this. As written,
nam::container::ContainerModelpre-warms ALL submodules instead of just the active submodule. It doesn't seem like a great idea to have a model that might run with a Slimmable value of 0.5 to pre-warm its larger 1.0 submodule as well. ContainerModel::Reset also resets all submodules, although that's not really as big of an issue as is superfluous prewarming. And, although one can selectively reset and prewarm using SetSlimmableSize, you still have to call ContainerModel::Reset to set the value of sampleRate and maxBufferSize). So you end up resetting all the submodules anyway (and the reset active submodule twice).proposed fix
The proposed fix, I think, would be to copy metdata settings out of the active submodule into the top-level submodule in the ContainerModel constructor. (which would requires protected setter methods in nam::DSP). When the constructor exits, the active submodule is set to the last (and presumably best) submodule. Add code to copy metadata settings out of the active submodule into the top-level ContainerModel. Also copy metadata settings from the active submodule again, in ContainerModel::SetSlimmableSize(const double val). And perhaps ensure that Reset, prewarm, and ResetAndPrewarm act only on the active submodule. The activation sequence for a client that is using ContainerModel::SetSlimmableSize thus becomes. Modify the documentation of SetSlimmableSize to indicate that it MUST be followed by a call to Reset and prewarm.
Offer to pull
I'd be happy to send you a pull request with necessary changes if that would be helpful to you.