Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Add sample rate warning for non 48kHz #182

Closed
mikeoliphant opened this issue Apr 10, 2023 · 5 comments · Fixed by #331
Closed

[FEATURE] Add sample rate warning for non 48kHz #182

mikeoliphant opened this issue Apr 10, 2023 · 5 comments · Fixed by #331
Assignees
Labels
enhancement New feature or request priority:low Low priority issues

Comments

@mikeoliphant
Copy link
Contributor

It might be helpful to have some sort of warning in the plugin UI if the plugin is operating at a sample rate that doesn't match the model. Not sure what, exactly - maybe a small, but highly visible (red?) indicator that shows an explanation popup when clicked?

Even if/when we get resampling implemented in the plugin, it would still probably be useful for the user to know it is happening.

@mikeoliphant mikeoliphant added enhancement New feature or request priority:low Low priority issues unread This issue is new and hasn't been seen by the maintainers yet urgent:no labels Apr 10, 2023
@sdatkinson
Copy link
Owner

Agreed again :) Plan is to get the data sample rate into the model file first so that it's available over here for that reason (but one could safely assume 48k for models before that point since non-48k I think still yells at you if you try!)

@sdatkinson sdatkinson removed the unread This issue is new and hasn't been seen by the maintainers yet label Apr 11, 2023
@mikeoliphant
Copy link
Contributor Author

Yeah, we're going to have to assume 48k by default for all of the existing models out there anyway.

I think there are probably a bunch of people out there running the plugin at 44.1 and not knowing there is anything wrong. I did that myself initially.

@jan-kowalski
Copy link

jan-kowalski commented Apr 12, 2023

But how will such a warning work when oversampling can be set in Reaper for any plug-in, such as NAM? Then the project in Reaper can have 44.1kHz sample rate, but NAM itself will work in oversampling. Will then also such a warning appear?

@mikeoliphant
Copy link
Contributor Author

But how will such a warning work when oversampling can be set in Reaper for any plug-in,

It does not make sense to use Reaper oversampling with NAM - using a sample rate higher than that used to train the model will make things worse, not better.

In any case, if you did turn on oversampling in Reaper, then the plugin would see the oversampled sample rate. So if you were running at 48K, and enabled 2x oversampling, the plugin would see a 96k sample rate - and should complain (unless the model happened to be trained at 96k).

@sdatkinson
Copy link
Owner

Adding this to the checklist for next plugin version, though I'm hoping I can implement #59 instead, which will make this one obsolete 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority:low Low priority issues
Projects
None yet
3 participants