LMMS
Enable LADSPA plugins on MSVC attempt 2
Succeeds #6758, which I messed up after a force push. Luckily, I had an almost identical branch so just merged that branch in.
@Rossmaxx I'm not sure if you're interested, but sometimes to reduce the number of commits on a PR, I'll create a pull request against my own fork, squash and merge that PULL request, and then hard-reset and cherry-pick that commit it to the upstream (e.g., "this") PR. (Technically, you could do the same thing with a squash, but in my experience, squashing can cause a lot of grief when so many commits span such a long period of time).
(You don't need to do this, I just thought it might be helpful)
@Rossmaxx I'm not sure if you're interested, but sometimes to reduce the number of commits on a PR, I'll create a pull request against my own fork, squash and merge that PULL request, and then hard-reset and cherry-pick that commit it to the upstream (e.g., "this") PR. (Technically, you could do the same thing with a squash, but in my experience, squashing can cause a lot of grief when so many commits span such a long period of time).
(You don't need to do this, I just thought it might be helpful)
Could you not just do a 'Squash and Merge' on this PR when it gets merged?
Could you not just do a 'Squash and Merge' on this PR when it gets merged?
Yes, but in my experience, each time a commit hits master and/or this branch, it increases the chance of a new merge conflict. @Rossmaxx has been presumably fixing these as they occur, but I wanted to at least mention it (this branch has 108 commits as of today). 🍻
@tresf thanks for the tips but I ain't planning to squash commits for now in this PR. I'll do it in the future for upcoming prs.
@Rossmaxx
I'm not sure why the macOS build fails. It's saying that the array size of U data[(1U<<IntBits)+1] is not a constant expression, but it actually is. EDIT: Maybe the "array size" it's referring to is the array size in the calling code, which is just a float* instead of float[N]. Maybe this is just one of those weird cases that some compilers accept but others don't.
A possible workaround is this: In src/calf/fixed_point.h in the veal repo, replace all four instances of U data[(1U<<IntBits)+1] with U* data.
The downside is that the parameters won't specify the size of the arrays they expect anymore, but that shouldn't be necessary.
Are we ready to merge this?
@tresf is doubtful about the veal changes. Though everything else is done, other than one comment by @Veratil which can be done in a follow up pr.
@tresf is doubtful about the veal changes.
I've only asked for basic regression testing. :). If you guys want to yolo it, go ahead. I'm far too busy to help with this portion right now. 🍻
I've only asked for basic regression testing.
I don't really know what to test for over there and I'm busy with college myself. If anyone would like to volunteer, please go ahead.
The CI problem was a temporary Chocolatey outage. I reran the affected jobs.
Are we ready to merge this?
@tresf is doubtful about the veal (Calf LADSPA) changes. Though everything else is done, other than one comment by @Veratil which can be done in a follow up pr.
Sorry for the delay. I've had some time to review the veal changes. They seem to fall into 5 categories:
- UI changes
- MSVC-compat
- New features
- Minor bug fixes/guards
- Refactors
The only significant change was a refactor to orfanidis_eq.h, which seems to have encountered a very large refractor. At a glance, it should only affect the 5, 8, 12, 30 band EQs that Calf ships, and that's only assuming a mistake was made while refactoring.
I think the veal stuff is safe to merge.