sdatkinson / NeuralAmpModelerPlugin

Plugin for Neural Amp Modeler
MIT License
1.98k stars 132 forks source link

[BUG] White noise when resampling 48K model to 96K in DAW #398

Closed 2-dor closed 11 months ago

2-dor commented 11 months ago

Noticeable increase in apparent noise floor when using a 48k .nam model in a 96k project.

To reproduce:

sdatkinson commented 11 months ago

I'm seeing this too.

Looping a DI that's just my guitar with volume knob all the way off, and a super-gainy model (expecting 48k) and the noise gate disabled, I get these integrated LUFS:

44.1 kHz: -44.0 LUFS 48 kHz: -44.0 LUFS 88.2 kHz: -43.7 LUFS 96 kHz: -40.5 LUFS

Interesting that it really takes off with 96k with +3.5 dB.

I also don't see any difference when playing. Looping a DI with some strumming, all 4 sample rates converge to -17.6 LUFS. That's kind of interesting.

Out of curiosity, I benchmarked it against Tonocracy's NAM player running the same model. Theirs also sees the noise floor go up, but less (only +0.7 dB).

With a 20 kHz LPF like this...

image

...the difference disappears.

So it seems to me that the culprit is how the highest-frequency signals are being handled by the resampling algorithm.

There are a few options:

  1. Leave it alone and allow the user to do high-frequency treatment as they'd prefer (maximum flexibility, more user error)
  2. Add that LPF into the code (it "just works" but more modification of the signal than what a "NAM purist" might want? Technically we're already resampling, so that's out the window?)
  3. Perhaps the resampler can be tuned? I looked at the source code for the iPlug2 component I'm using but I don't see the parameters exposed in a way that suggests they were meant to be played with? (No initialization arguments or public methods). cc @olilarkin, lmk if I'm missing something here? e.g. what's up with the magic number A=4? I see its mathematical interpretation [thanks for the Wikipedia link!] but I'm interested by the fact that the article says a=2 or 3 is typical. Without going through the math, I'd guess higher values have better anti-aliasing properties but more latency?
sdatkinson commented 11 months ago

A few other observations:

Another thought: I can already imagine having to answer this question again and again. I'm not very keen on option (1) in my comment above. I certainly could do something like a first-order LPF at the Nyquist before feeding the signal into the resampler (only on the resampling path). Anything to make this less noticeable would probably be good.


I'm also wondering whether I should consider this a must-fix for this release or if it'd be okay to get it out there and patch it in the next one if I can get it tuned up. My bet is that the majority of users will benefit from this because they're using 44.1. For them, this isn't an issue. @2-dor, feel free to weigh in.

2-dor commented 11 months ago

A few other observations:

  • This shouldn't affect the noise gate--the resampling only happens around the NAM; the rest of the DSP is done in the external sample rate.
  • Something like the Bertom Denoiser, cutting highs, also makes things work fine...

Another thought: I can already imagine having to answer this question again and again. I'm not very keen on option (1) in my comment above. I certainly could do something like a first-order LPF at the Nyquist before feeding the signal into the resampler (only on the resampling path). Anything to make this less noticeable would probably be good.

I'm also wondering whether I should consider this a must-fix for this release or if it'd be okay to get it out there and patch it in the next one if I can get it tuned up. My bet is that the majority of users will benefit from this because they're using 44.1. For them, this isn't an issue. @2-dor, feel free to weigh in.

I think getting it out is priority so no biggie.

olilarkin commented 11 months ago

There may well be a problem with the resampler when going from 96 to 48, will have to test it in isolation.

On Tue, 5 Dec 2023 at 06:33, 2-dor @.***> wrote:

A few other observations:

  • This shouldn't affect the noise gate--the resampling only happens around the NAM; the rest of the DSP is done in the external sample rate.
  • Something like the Bertom Denoiser https://www.bertomaudio.com/denoiser-classic.html, cutting highs, also makes things work fine...

Another thought: I can already imagine having to answer this question again and again. I'm not very keen on option (1) in my comment above. I certainly could do something like a first-order LPF at the Nyquist before feeding the signal into the resampler (only on the resampling path). Anything to make this less noticeable would probably be good.

I'm also wondering whether I should consider this a must-fix for this release or if it'd be okay to get it out there and patch it in the next one if I can get it tuned up. My bet is that the majority of users will benefit from this because they're using 44.1. For them, this isn't an issue. @2-dor https://github.com/2-dor, feel free to weigh in.

I think getting it out is priority so no biggie.

— Reply to this email directly, view it on GitHub https://github.com/sdatkinson/NeuralAmpModelerPlugin/issues/398#issuecomment-1840043844, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFACLSW3MH5QEJ3PX5NULLYH2W2RAVCNFSM6AAAAABAGP6DCWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBQGA2DGOBUGQ . You are receiving this because you were mentioned.Message ID: @.***>

sdatkinson commented 11 months ago

Looking into it a little more, I have the sense that it would be appropriate for the choice of A to depend on what sample rates the Lanczos sampler is resampling between. However, I'm not sure of the details yet.

olilarkin commented 11 months ago

I did a test with a sine sweep and i don't see any problems with the noise floor...

https://github.com/sdatkinson/NeuralAmpModelerPlugin/assets/655662/52871881-3ad9-4729-84ef-81367f004f4e

you can do the same with IPlugEffect...

#pragma once

#include "IPlug_include_in_plug_hdr.h"
#include "NonIntegerResampler.h"

const int kNumPresets = 1;

enum EParams
{
  kGain = 0,
  kNumParams
};

using namespace iplug;

class IPlugEffect final : public Plugin
{
public:
  IPlugEffect(const InstanceInfo& info);
  void ProcessBlock(sample** inputs, sample** outputs, int nFrames) override;
  void OnReset() override;
  NonIntegerResampler<> mNonIntegerResampler;
};
#include "IPlugEffect.h"
#include "IPlug_include_in_plug_src.h"

IPlugEffect::IPlugEffect(const InstanceInfo& info)
: Plugin(info, MakeConfig(kNumParams, kNumPresets))
, mNonIntegerResampler(48000, ESRCMode::kLancsoz)
{
  GetParam(kGain)->InitDouble("Gain", 0., 0., 100.0, 0.01, "%");
}

void IPlugEffect::OnReset()
{
  mNonIntegerResampler.Reset(GetSampleRate(), GetBlockSize());
}

void IPlugEffect::ProcessBlock(sample** inputs, sample** outputs, int nFrames)
{
  const double gain = GetParam(kGain)->Value() / 100.;
  const int nChans = NOutChansConnected();

  mNonIntegerResampler.ProcessBlock(inputs, outputs, nFrames,
  [nChans, gain](sample** inputs, sample** outputs, int nFrames){
    for (int s = 0; s < nFrames; s++) {
      for (int c = 0; c < nChans; c++) {
        outputs[c][s] = inputs[c][s] * gain;
      }
    }
  });

}
DomMcsweeney commented 11 months ago

@olilarkin - Here's the difference between 48khz and 96khz. I loaded up NAM and an amp model (any will do) and armed the track. I recorded the 'output' twice, once at 48khz, next at 96khz. The noise from the audio interface is being recorded through NAM.... but nothing is plugged into the audio interface. The input and output controls on the NAM plugin are at maximum to exaggerate. It's roughly 6db higher at 96khz.

https://github.com/sdatkinson/NeuralAmpModelerPlugin/assets/86134812/c9d2e814-fc67-4daa-8f61-10f5fc635683

olilarkin commented 11 months ago

could someone share a .nam where i can easily reproduce the problem? I just tried it with a guitar here and i am not noticing much of a difference in the volume of the noise when i switch the host samplerate between 48 and 96k.

2-dor commented 11 months ago

could someone share a .nam where i can easily reproduce the problem? I just tried it with a guitar here and i am not noticing much of a difference in the volume of the noise when i switch the host samplerate between 48 and 96k.

https://tonehunt.org/2dor/9115b091-7858-4f69-ae42-a324edc3b527

This should cover it. It's 1 DI profile and 2 that have cooked in IRs

DomMcsweeney commented 11 months ago

To be fair, when no plugins are loaded on a track in reaper, and when you change sample rates - the noise floor of an armed track with instrument input selected (but with nothing physically connected to audio interface) - is louder at 96khz than at 48khz. image

(I had to boost the track level by about 70db to exaggerate the result, but it seems to be consistent regardless of any plugins present on the track)

olilarkin commented 11 months ago

I think setting A=12 is a big improvement for 44.1khz -> 48khz as well as 96khz->48khz. I've reworked the iPlug2 resampling classes to default to this value. I've also renamed the class... https://github.com/iPlug2/iPlug2/commit/4bfc3e15d67f0c7c930aab71d1bf0e551362a1c4

@sdatkinson if you define IPLUG_SIMDE you can get some extra performance too since i think you were using the scalar version before