lmms
lmms copied to clipboard
LMMS
LV2 Buf Size - powerOf2BlockLength
MOD Devices build hardware effect units based on open source software. They have a selection of pitch shifter based effects that need powerOf2BlockLength. https://github.com/moddevices/mod-pitchshifter
In order to support this we would, as I understand it, need to just limit the buffer size to a multiple of two. I don't see this as a problem. Of the values available today only 32, 64, 128, 256, 512, 1024, 2048 and 4096 would remain and the m_bufferSizeSlider code would be simplified.
I don't know what the flip side of the change would be. Could there be plugins out there who demand that they are free to not follow the multiple of two scheme? I doubt it but I don't know for sure.
Discussion on Discord summed up in comment below.
To test this quickly all you need to do is set the buffer size to one of the selection above and apply this one line hack.
--- a/src/core/lv2/Lv2Manager.cpp
+++ b/src/core/lv2/Lv2Manager.cpp
@@ -150,6 +150,7 @@ Lv2Manager::Lv2Manager() :
m_supportedFeatureURIs.insert(LV2_BUF_SIZE__boundedBlockLength);
// block length is only changed initially in AudioEngine CTOR
m_supportedFeatureURIs.insert(LV2_BUF_SIZE__fixedBlockLength);
+ m_supportedFeatureURIs.insert(LV2_BUF_SIZE__powerOf2BlockLength);
auto supportOpt = [this](Lv2UridCache::Id id)
{
mod-pitchshift plugins: Drop Capo Super Capo 2Voices Super Whammy Harmonizer HarmonizerCS Harmonizer2 (three audio outputs and thus not supported by lmms)
From the developers: These effects are supposed to work well with the following values of Frames/Period: 64, 128, 256, 512
mod-cabsim-IR-loader (also needs gui to be usable) https://github.com/moddevices/mod-cabsim-IR-loader
Here is a fork of mod-pitchshifter with the required feature of powerOf2BlockLength removed. You need to make sure your number of frames is as specified above or lmms will crash. https://github.com/ycollet/mod-gxpitchshifter
@JohannesLorenz Also, the plugin above has an issue with negative numbers. I think you need to add one lcd segment for the minus sign.
@zonkmachine A more simple alternative: Let LMMS check if the current buffer size is a power of 2, and if yes, support the feature. This works, because LMMS can not resize the buffer size after LMMS has been started.
A disadvantage of this would be that users who use e.g. 192 as a buffer size might not understand why those plugins are not shown. But it's at least better than nothing, I guess.
+1 for the LCD widget, I'll fix this.
A disadvantage of this would be that users who use e.g. 192 as a buffer size might not understand why those plugins are not shown.
For this reason I didn't mention it. Maybe a very clear message on startup could help though.
Suggested approaches for implementing this as taken from discord/dev-only on 08/20/2022, thread starting around here:
- Limit buffer sizes in LMMS to power of 2 (will this be an issue?)
- Only allow these Lv2 plugins if the buffer size is a multiple of 2. However, users with other buffer sizes might wonder why those plugins are missing.
- Like (2), but bring a dialogue whenever the buffer size is being changed to non-power-of-2.
- Split the buffer to be a power of 2, ex. process 192 as 128+64 or 64+64+64
In response to alternative 3, @Spekular 's "I think a text label in the settings menu would be a pretty good place for this info. Either one that only appears on non-power-of-two sizes ("Buffer size is not a power of two, some LV2 plugins cannot be loaded") or one that's always there (previous text but add "If" to the beginning)" was the most upvoted comment.
Summed up
This suggests that the frame size options of today should be kept and that a message should be implemented in the settings menu to inform the user of the effects on plugins the setting has. If the buffer is a power of 2 value, powerOf2BlockLength is set.
For reference: This issue will be fixed in #6484 . The PR already contains the logic, but a message is yet to implement.
mod-pitchshifter doesn't work with a 32 frames buffer.
We could add another category: Next to the "plugins blacklisted", we could introduce "plugins blacklisted with buffersize <= 32". And we could display another message in the buffersize selection window: "Some plugins do not work with a buffersize <= 32 and will be disabled.". Does this make sense?
@JohannesLorenz An oddity. If I save a project on the lv2-worker branch with plugins from the selection above (capo, etc.), I can now open it in on master and it will work (given that the buffer size is a power of to...).
@JohannesLorenz An oddity. If I save a project on the lv2-worker branch with plugins from the selection above (capo, etc.), I can now open it in on master and it will work (given that the buffer size is a power of to...).
Yes, I think we do these "Does LMMS support all plugin's required features?" tests not when loading a savefile (as opposed to displaying plugins to open). Should that be implemented? Though, I would usually expect that this only happens during development of Lv2 plugins (so only to you and me). But it can also happen if you load a savefile from a newer LMMS version in an older LMMS - not sure if this scenario happens often.
Should that be implemented?
If it's most an issue during development, then probably not.
But it can also happen if you load a savefile from a newer LMMS version in an older LMMS - not sure if this scenario happens often.
There is a popup message in the bottom of the screen in this case but it's not a big note and goes away rather quickly.
mod-pitchshifter doesn't work with a 32 frames buffer.
Just to find the right value: Which is the smalles power of two where it does work? 64? 128? 256?
Also, can you please give me the complete URL?
Just to find the right value: Which is the smalles power of two where it does work? 64? 128? 256?
64, no upper limit.
Also, can you please give me the complete URL?
http://moddevices.com/plugins/mod-devel/2Voices http://moddevices.com/plugins/mod-devel/Capo http://moddevices.com/plugins/mod-devel/Drop http://moddevices.com/plugins/mod-devel/Harmonizer http://moddevices.com/plugins/mod-devel/Harmonizer2 http://moddevices.com/plugins/mod-devel/HarmonizerCS http://moddevices.com/plugins/mod-devel/SuperCapo http://moddevices.com/plugins/mod-devel/SuperWhammy
http://moddevices.com/plugins/mod-devel/GX2Voices http://moddevices.com/plugins/mod-devel/GXCapo http://moddevices.com/plugins/mod-devel/GXDrop http://moddevices.com/plugins/mod-devel/GXHarmonizer http://moddevices.com/plugins/mod-devel/GXHarmonizer2 http://moddevices.com/plugins/mod-devel/GXHarmonizerCS http://moddevices.com/plugins/mod-devel/GXSuperCapo http://moddevices.com/plugins/mod-devel/GXSuperWhammy
I appended the list and added blacklisting those plugins if the buffersize is 32 or smaller. Also, I added warning labels for the buffersize slider. It's appended to the lv2 worker PR (#worker). @zonkmachine would be cool if you could test it!
As this is already part of the Lv2 Worker PR, suggesting to close this. Is this OK, @zonkmachine ?
As this is already part of the Lv2 Worker PR, suggesting to close this. Is this OK, @zonkmachine ?
But why, it hasn't been merged? Just add a closes #6492 in the worker PR. Also, it's still got this issue that isn't fixed. Although I think my fix of using the config value would work.
But why, it hasn't been merged?
I usually try to reduce items on my TODO list, so many PR-issue-duplicates. Though I agree that this also has disadvantages - The issue still helps user seeing that it is not fixed.
Also, it's still got this issue that isn't fixed
Good point, sorry, forgot this one. Yes, then this issue will persist, even after the Lv2 Worker PR merge.
Also, it's still got this issue that isn't fixed
Good point, sorry, forgot this one. Yes, then this issue will persist, even after the Lv2 Worker PR merge.
May i suggest something. How about closing this PR for now and rework it after Lv2 worker PR gets merged. "The issue" can get a seperate issue rather than a comment in the worker PR (as a reminder)
How about closing this PR ...
How about not closing unimplemented ideas/issues? Especially since I'm still around and actively monitor and update issues that I have created. @JohannesLorenz can choose to drop the powerOf2 stuff but leaving it in with a possible crash to be fixed later when the fix is a possible simple one is unnecessary.