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

Small additions and new patches! #136

Closed
wants to merge 1 commit into from
Closed

Conversation

reaby
Copy link

@reaby reaby commented Jun 3, 2024

Adds peak volume to vu metter and change color to red when audio clips.
Changed build-in presets to support subfolders.
Added new presets and removed some not-so-great sounding ones, tried to gain-stage the presets.

Changed build-in presets to support subfolders, added new presets and removed some not-so-great sounding ones, tried to gain-stage the presets alittle.
@vsariola
Copy link
Owner

vsariola commented Jun 3, 2024

Thanks!

I see these are three separate things, so please rewrite git history to make them into three separate pull requests / commits, so I can review them more easily & merge them one by one. I don't have time to test these tonight, but I already have some questions:

  1. Changing to red is a good idea. But there was already the line indicating peak volume, what was wrong with that?
  2. This seems like a good idea; I need to check the implementation.
  3. The presets are mostly the old 4klang presets. I'm ok with adjusting the gains, but I'm not quite sure about deleting presets (almost all snares are gone). While you are chasing great sound, some of these may have come about because they are very little bytes, so it's not always just about the sound. Also, I see lots of renaming of the presets; I'd like to have such minor tweaking in a separate commit.

The presets adjustment could be pull request with four commits. For example: first commit renaming all existing presets into better names; second commit adding new presets; third commit tweaking the gains; and fourth commit deleting the presets that you think are bad. These commits are in the order of agreeableness: I might agree the merge the first three, but not the fourth. Having them in four clean commits helps the review process.

@vsariola
Copy link
Owner

Any chance of cleaning up this pull request? I might look into it to get the gain staged instruments, but that "79 files changed" is a bit intimidating, without more context.

@vsariola vsariola closed this in ae21766 Feb 25, 2025
@vsariola
Copy link
Owner

I put the presets in & closed this pull request for now. Please open other pull requests if you still want those UI tweaks (but some things may have changed since then, so the code may not be directly compatible with latest master).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants