Guitarix release 0.36.0
Moderators: raboof, MattKingUSA, khz
-
- Established Member
- Posts: 37
- Joined: Mon Sep 04, 2017 9:30 pm
Re: Guitarix release 0.36.0
I just tried building Guitarix 0.36.1 and creating a package for it for gentoo. During which I found some things in the wscript/waf configure options that don't seem to be working the way I expected them to.
I noticed there are two options "--no-ladspa" and "--no-new-ladspa", what's the difference between them?
The checks for some external dependencies don't fail when those external dependencies are not installed. For example if I don't use --includeresampler or --includeconvolver and I run ./waf configure it will not fail when I don't have zita-resampler or zita-convolver installed. @tramp Would you be open to patch that changes this?
How do I build LADPSA plugins only? It seems the options to target what to actually build are incomplete and/or too specific. There's an --lv2-only option, which is being used with a negation (not lv2-only) to force checks that don't relate to lv2 at all, i.e. if jack or gtk or ladspa is installed.
IMHO it would be much nicer to define during configure what to build instead of what not to build.
I.e. have a --standalone (that defaults to True) and a --lv2 and a --ladspa configure flag that enables building that specific version of guitarix and is used to check it's own relevant dependencies.
@tramp What do you think?
I noticed there are two options "--no-ladspa" and "--no-new-ladspa", what's the difference between them?
The checks for some external dependencies don't fail when those external dependencies are not installed. For example if I don't use --includeresampler or --includeconvolver and I run ./waf configure it will not fail when I don't have zita-resampler or zita-convolver installed. @tramp Would you be open to patch that changes this?
How do I build LADPSA plugins only? It seems the options to target what to actually build are incomplete and/or too specific. There's an --lv2-only option, which is being used with a negation (not lv2-only) to force checks that don't relate to lv2 at all, i.e. if jack or gtk or ladspa is installed.
IMHO it would be much nicer to define during configure what to build instead of what not to build.
I.e. have a --standalone (that defaults to True) and a --lv2 and a --ladspa configure flag that enables building that specific version of guitarix and is used to check it's own relevant dependencies.
@tramp What do you think?
-
- Established Member
- Posts: 2348
- Joined: Mon Jul 01, 2013 8:13 am
- Has thanked: 9 times
- Been thanked: 468 times
Re: Guitarix release 0.36.0
--no-ladspa exclude the very old guitarix ladspa plugins from the build, I'll exclude them on default within the next circle.aaahaaap wrote:I noticed there are two options "--no-ladspa" and "--no-new-ladspa", what's the difference between them?
--no-new-ladspa exclude the newer ladspa plugin which provide the full guitarix engine as 2 ladspa plugins. This one is more a prov of concept and not been meant to be used in production. It may well be that we'll remove it completely as well.
This is intentional, as we provide zita-resampler and zita-convolver within the source. The --includeresampler and --includeconvolver build flags been meant to use the internal versions even when a usable version is found on the system.aaahaaap wrote:The checks for some external dependencies don't fail when those external dependencies are not installed. For example if I don't use --includeresampler or --includeconvolver and I run ./waf configure it will not fail when I don't have zita-resampler or zita-convolver installed. @tramp Would you be open to patch that changes this?
When non are found, there is no reason to let the configure step fail, as we provide the needed sources. So, sorry, I ain't want to change this behave.
You ain't. As I pointed out above, support for building the ladspa plugs will be drooped sooner or later. I know some people use the newer guitarix ladspa plug, that's why it is still there, but, . . .aaahaaap wrote:How do I build LADPSA plugins only?
Nope, I think it will be good to have the flags set to the defaults we are expected to build the software. Anyone is free to set the flags to his own need, but I ain't want to disable, for example the LV2 plugin build by default. If you ain't want them, you've to disable them.aaahaaap wrote: It seems the options to target what to actually build are incomplete and/or too specific. There's an --lv2-only option, which is being used with a negation (not lv2-only) to force checks that don't relate to lv2 at all, i.e. if jack or gtk or ladspa is installed.
IMHO it would be much nicer to define during configure what to build instead of what not to build.
I.e. have a --standalone (that defaults to True) and a --lv2 and a --ladspa configure flag that enables building that specific version of guitarix and is used to check it's own relevant dependencies.
@tramp What do you think?
Most users been expected to just run ./waf configure && ./waf build && ./waf install
to get the main application and the plugs.
Other then that, the --lv2-only flag is needed to build the LV2 plugs within the MOD plugin builder environment.
On the road again.
-
- Established Member
- Posts: 37
- Joined: Mon Sep 04, 2017 9:30 pm
Re: Guitarix release 0.36.0
Thanks for the response/info!
Removing them would be nice(r), since that would prevent some confusion and maybe users that use them assuming they are usable?
There can of course still be a default for one of these two be set in the wscript, but when a user asks to use the external ones and they are not detected imho that should fail the configure step and report that to the user.
What I meant was to have explicit options for what the user wants to build, be it the standalone application or LV2 plugins (or LADSPA plugins for that matter).
That way the necessary checks for dependencies can be done based on simple logic (standalone enabled? Needs jack, etc. LV2 enabled? Needs liblv2, etc) and they can fail and report so to the user in case any of these dependencies are not found.
With this config there can still be sane defaults, so for example by default the standalone application and LV2 are enabled, whilst LADSPA is disabled. And the user can choose to override this by enabling/disabled what he/she wants.
Hope this clarifies it a bit.
OK, it sounds like LADSPA is actually not really a supported format. I think I'll either just disable both of them by default or remove the option all together.tramp wrote:--no-ladspa exclude the very old guitarix ladspa plugins from the build, I'll exclude them on default within the next circle.aaahaaap wrote:I noticed there are two options "--no-ladspa" and "--no-new-ladspa", what's the difference between them?
--no-new-ladspa exclude the newer ladspa plugin which provide the full guitarix engine as 2 ladspa plugins. This one is more a prov of concept and not been meant to be used in production. It may well be that we'll remove it completely as well.
You ain't. As I pointed out above, support for building the ladspa plugs will be drooped sooner or later. I know some people use the newer guitarix ladspa plug, that's why it is still there, but, . . .aaahaaap wrote:How do I build LADPSA plugins only?
Removing them would be nice(r), since that would prevent some confusion and maybe users that use them assuming they are usable?
Maybe I didn't express myself well enough, but I was looking for the same behavior for the external dependencies as for the internal ones. I.e. an explicit choice.tramp wrote:This is intentional, as we provide zita-resampler and zita-convolver within the source. The --includeresampler and --includeconvolver build flags been meant to use the internal versions even when a usable version is found on the system.aaahaaap wrote:The checks for some external dependencies don't fail when those external dependencies are not installed. For example if I don't use --includeresampler or --includeconvolver and I run ./waf configure it will not fail when I don't have zita-resampler or zita-convolver installed. @tramp Would you be open to patch that changes this?
When non are found, there is no reason to let the configure step fail, as we provide the needed sources. So, sorry, I ain't want to change this behave.
There can of course still be a default for one of these two be set in the wscript, but when a user asks to use the external ones and they are not detected imho that should fail the configure step and report that to the user.
This is kind of similar to the external/internal zita dependencies, so again I might've expressed myself poorly. I'm not suggesting to change the default config but merely how this config is defined/achieved.tramp wrote:Nope, I think it will be good to have the flags set to the defaults we are expected to build the software. Anyone is free to set the flags to his own need, but I ain't want to disable, for example the LV2 plugin build by default. If you ain't want them, you've to disable them.aaahaaap wrote: It seems the options to target what to actually build are incomplete and/or too specific. There's an --lv2-only option, which is being used with a negation (not lv2-only) to force checks that don't relate to lv2 at all, i.e. if jack or gtk or ladspa is installed.
IMHO it would be much nicer to define during configure what to build instead of what not to build.
I.e. have a --standalone (that defaults to True) and a --lv2 and a --ladspa configure flag that enables building that specific version of guitarix and is used to check it's own relevant dependencies.
@tramp What do you think?
Most users been expected to just run ./waf configure && ./waf build && ./waf install
to get the main application and the plugs.
Other then that, the --lv2-only flag is needed to build the LV2 plugs within the MOD plugin builder environment.
What I meant was to have explicit options for what the user wants to build, be it the standalone application or LV2 plugins (or LADSPA plugins for that matter).
That way the necessary checks for dependencies can be done based on simple logic (standalone enabled? Needs jack, etc. LV2 enabled? Needs liblv2, etc) and they can fail and report so to the user in case any of these dependencies are not found.
With this config there can still be sane defaults, so for example by default the standalone application and LV2 are enabled, whilst LADSPA is disabled. And the user can choose to override this by enabling/disabled what he/she wants.
Hope this clarifies it a bit.
-
- Established Member
- Posts: 2348
- Joined: Mon Jul 01, 2013 8:13 am
- Has thanked: 9 times
- Been thanked: 468 times
Re: Guitarix release 0.36.0
aaahaaap wrote:@tramp Would you be open to patch that changes this?
At least it sounds reasonable what you said now.aaahaaap wrote:Maybe I didn't express myself well enough,
If you provide a patch here:
https://sourceforge.net/p/guitarix/patc ... rce=navbar
I'll look at it, and if it turns out to be more clear in the configure steps, I'll accept it.
regards
hermann
On the road again.
-
- Established Member
- Posts: 37
- Joined: Mon Sep 04, 2017 9:30 pm
Re: Guitarix release 0.36.0
I'll give it a try!tramp wrote: At least it sounds reasonable what you said now.
If you provide a patch here:
https://sourceforge.net/p/guitarix/patc ... rce=navbar
I'll look at it, and if it turns out to be more clear in the configure steps, I'll accept it.
regards
hermann
-
- Established Member
- Posts: 37
- Joined: Mon Sep 04, 2017 9:30 pm
Re: Guitarix release 0.36.0
I'm still working on this, will take a bit longer unfortunately, WAF has got to be one the worst build tools there is and the useless docs are not really helping either I have some time off the next two weeks so I should be able to figure it out during that timeaaahaaap wrote:I'll give it a try!tramp wrote: At least it sounds reasonable what you said now.
If you provide a patch here:
https://sourceforge.net/p/guitarix/patc ... rce=navbar
I'll look at it, and if it turns out to be more clear in the configure steps, I'll accept it.
regards
hermann
-
- Established Member
- Posts: 37
- Joined: Mon Sep 04, 2017 9:30 pm
Re: Guitarix release 0.36.0
Hi Hermann,tramp wrote:aaahaaap wrote:@tramp Would you be open to patch that changes this?At least it sounds reasonable what you said now.aaahaaap wrote:Maybe I didn't express myself well enough,
If you provide a patch here:
https://sourceforge.net/p/guitarix/patc ... rce=navbar
I'll look at it, and if it turns out to be more clear in the configure steps, I'll accept it.
regards
hermann
First of all, happy new year!
It took a bit longer, but can you have a look at the changes I have so far? I've tried to make the commits as isolated as possible. https://github.com/simonvanderveldt/gui ... nk/wscript
The following is now in place:
- Fail when user requests to use external zita-resampler/zita-convolver and they are not installed
- Most of the no-<option> options have been replaced by regular <option> options. This enables a lot clearer logic in the wscript.
For example no more "if not lv2_only" but a clear "if standalone" to conditionally check the standalone application's dependencies.
- This also means it's now possible for the user to clearly choose which formats he/she wants to be built: "standalone", "lv2", "ladspa" and "new-ladspa"
- LV2 sub-options (GUI and MOD GUI) are disabled when LV2 is disabled
Based on your comments I've only made small changes to the current defaults:
- Building of LADSPA plugins is disabled by default (but can be enabled by the user)
- Building of new LADSPA plugins is disabled by default (but can be enabled by the user)
- Building of MOD LV2 plugin UI's is disabled by default (but can be enabled by the user)
I'd be very interested in what you think of it.
IMHO it's already a lot easier to read/understand the logic and it's a lot easier for the user to choose what to build + that choice is now properly honored as well
Also I ran into some things I'm not sure are correct/have some questions about:
- lilv seems to be needed for the standalone application and for the LADSPA plugins. Is that correct?
https://github.com/simonvanderveldt/gui ... cript#L684
- When is the Roboto font needed? I guess only for the standalone application and for LV2 GUI's?
- In src/gx_head's wscript "ladspaplugin.cpp" and "ladspaback.cpp" are always included which means ladspa and rdf need to be installed even when not building LADSPA plugins. Is this correct? I image it might be for using LADSPA plugins in the standalone application?
- The no-sse option is honored only when LV2 plugins are being built, I don't think that's necessary, right? Could just always be honored.
https://github.com/simonvanderveldt/gui ... lify-logic
- There's one plugin (jcm800pre) in src/plugins that has LV2 stuff in it. All other LV2 plugins live in src/LV2 so I think something went wrong there.
- In some places having the build work depends on order of the code in the wscript instead of dependencies being defined. For example the new LADSPA build needs to happen after one or more of the dirs in the lines above it are built otherwise it fails. It would be cleaner if these dependencies are properly defined.
- I haven't added a check that at least 1 of "standalone", "lv2", "ladspa" and "new-ladspa" should be chosen. Do you think that's necessary?
[edit] I have checked the majority of build combinations:
- Building everything works
- Building just the standalone application works
- Building just LV2 plugins works
- Building just LADSPA or new LADSPA plugins doesn't work. They seem to require several other items to be built/copied first (src/faust, src/plugins and others). These dependencies are currently not defined (and I don't really know them). Is this something you'd like to be fixed?
Regards,
Simon
P.S I have never used sourceforge before, so don't know (yet) how to create a patch. Will try to do so when I get the OK from you
Last edited by simonvanderveldt on Tue Jan 02, 2018 12:16 pm, edited 7 times in total.
-
- Established Member
- Posts: 2348
- Joined: Mon Jul 01, 2013 8:13 am
- Has thanked: 9 times
- Been thanked: 468 times
Re: Guitarix release 0.36.0
Hi
Thanks for that.
I've a short look and it looks good so far.
Unfortunately I'm away for 14 day's now, and can't check it out for real.
I'll comment on your github page when I'm back, so that we could clear it for future use.
regards
hermann
Thanks for that.
I've a short look and it looks good so far.
Unfortunately I'm away for 14 day's now, and can't check it out for real.
I'll comment on your github page when I'm back, so that we could clear it for future use.
regards
hermann
On the road again.
-
- Established Member
- Posts: 37
- Joined: Mon Sep 04, 2017 9:30 pm
Re: Guitarix release 0.36.0
Thanks for the quick look. And no problem, looking forward to the feedback in a couple of weekstramp wrote:Hi
Thanks for that.
I've a short look and it looks good so far.
Unfortunately I'm away for 14 day's now, and can't check it out for real.
I'll comment on your github page when I'm back, so that we could clear it for future use.
regards
hermann
In the meantime I'll continue to try different build config combinations.
-
- Established Member
- Posts: 37
- Joined: Mon Sep 04, 2017 9:30 pm
Re: Guitarix release 0.36.0
Hi Hermann,tramp wrote:Hi
Thanks for that.
I've a short look and it looks good so far.
Unfortunately I'm away for 14 day's now, and can't check it out for real.
I'll comment on your github page when I'm back, so that we could clear it for future use.
regards
hermann
Have you had the chance to take a quick look? If you have any feedback, just let me know.
-
- Established Member
- Posts: 2348
- Joined: Mon Jul 01, 2013 8:13 am
- Has thanked: 9 times
- Been thanked: 468 times
Re: Guitarix release 0.36.0
Hi Simon
First, many thanks for your contribution.
Configure options become indeed more clear now.
Even if I used to use it like it was, I could see the benefit.
I've pushed your changes to our git repository.
https://sourceforge.net/p/guitarix/git/ci/master/tree/
For the record, I've created a patch from your branch with:
to get a patch with all history information
and apply it to our master with:
so in the git history you are the author of this commits as it should to be,
and I've signed them up.
If you've more changes to do, let me know and I'll check it out, . .
regards
hermann
First, many thanks for your contribution.
Configure options become indeed more clear now.
Even if I used to use it like it was, I could see the benefit.
I've pushed your changes to our git repository.
https://sourceforge.net/p/guitarix/git/ci/master/tree/
For the record, I've created a patch from your branch with:
Code: Select all
git format-patch master --stdout > wscript-simplify-logic.patch
and apply it to our master with:
Code: Select all
git am --signoff < wscript-simplify-logic.patch
and I've signed them up.
If you've more changes to do, let me know and I'll check it out, . .
regards
hermann
On the road again.
-
- Established Member
- Posts: 37
- Joined: Mon Sep 04, 2017 9:30 pm
Re: Guitarix release 0.36.0
Hi Hermann,tramp wrote:Hi Simon
First, many thanks for your contribution.
Configure options become indeed more clear now.
Even if I used to use it like it was, I could see the benefit.
I've pushed your changes to our git repository.
https://sourceforge.net/p/guitarix/git/ci/master/tree/
...
so in the git history you are the author of this commits as it should to be,
and I've signed them up.
If you've more changes to do, let me know and I'll check it out, . .
regards
hermann
That's great! And thanks for being so careful about the attribution. Appreciated! I would've been totally OK without is as well,I'm already happy I can help
If you have time, can you have a look at the questions I posted? Just trying to clear some stuff up and make sure everything is defined and working as it should. See below for the list:
aaahaaap wrote:Also I ran into some things I'm not sure are correct/have some questions about:
- lilv seems to be needed for the standalone application and for the LADSPA plugins. Is that correct?
https://github.com/simonvanderveldt/gui ... cript#L684
- When is the Roboto font needed? I guess only for the standalone application and for LV2 GUI's?
- In src/gx_head's wscript "ladspaplugin.cpp" and "ladspaback.cpp" are always included which means ladspa and rdf need to be installed even when not building LADSPA plugins. Is this correct? I image it might be for using LADSPA plugins in the standalone application?
- The no-sse option is honored only when LV2 plugins are being built, I don't think that's necessary, right? Could just always be honored?
https://github.com/simonvanderveldt/gui ... lify-logic
- There's one plugin (jcm800pre) in src/plugins that has LV2 stuff in it. All other LV2 plugins live in src/LV2 so I think something went wrong there?
- I haven't added a check that at least 1 of "standalone", "lv2", "ladspa" and "new-ladspa" should be chosen. Do you think that's necessary?
- Building just LADSPA or new LADSPA plugins without building the standalone application doesn't work. They seem to require several other items to be built/copied first (src/faust, src/plugins and others). These dependencies are currently not defined (and I don't really know them). Is this something you'd like to be fixed? Or would it be better to just start removing the LADSPA stuff alltogether?
-
- Established Member
- Posts: 2348
- Joined: Mon Jul 01, 2013 8:13 am
- Has thanked: 9 times
- Been thanked: 468 times
Re: Guitarix release 0.36.0
Hi
Yes sure, forgotten about this.
For the standalone app the Roboto font is chosen because it best fit in the layout of the interface.
Hope that answer so far your questions,
regards
hermann
Yes sure, forgotten about this.
Yes, lilv is needed for the standalone and the new ladspa plug, not for the older ones.aaahaaap wrote:Also I ran into some things I'm not sure are correct/have some questions about:
- lilv seems to be needed for the standalone application and for the LADSPA plugins. Is that correct?
https://github.com/simonvanderveldt/gui ... cript#L684
Roboto font is used only in the stand alone app, the LV2 UI's use what ever you've installed.aaahaaap wrote: - When is the Roboto font needed? I guess only for the standalone application and for LV2 GUI's?
For the standalone app the Roboto font is chosen because it best fit in the layout of the interface.
Yes, ladspa and rdf is needed by both, indeed the standalone need it to support ladspa plugins.aaahaaap wrote: - In src/gx_head's wscript "ladspaplugin.cpp" and "ladspaback.cpp" are always included which means ladspa and rdf need to be installed even when not building LADSPA plugins. Is this correct? I image it might be for using LADSPA plugins in the standalone application?
The no-sse is special for the LV2 plugs, to simplify the build of the LV2 plugs for the MOD. Otherwise, you could just overwrite the --cxxflags-release flag, when you would disable sse support, but honestly, that is strongly not recommended. So better keep it just for the LV2 plugs.aaahaaap wrote: - The no-sse option is honored only when LV2 plugins are being built, I don't think that's necessary, right? Could just always be honored?
https://github.com/simonvanderveldt/gui ... lify-logic
No, nothing wrong with that. The jcm800pre is generated by the ampsim toolkit, it could produce guitarix plugins, LV2 plugins, or plugins which support both API's. The jcm800pre support both, that's all.aaahaaap wrote: - There's one plugin (jcm800pre) in src/plugins that has LV2 stuff in it. All other LV2 plugins live in src/LV2 so I think something went wrong there?
Maybe we just add a info output, if the standalone app will be build.aaahaaap wrote: - I haven't added a check that at least 1 of "standalone", "lv2", "ladspa" and "new-ladspa" should be chosen. Do you think that's necessary?
Well, the new ladspa requires indeed the build of the standalone first. But I guess it's time to let it go slowly out. So no special addition for it to the waf script any more.aaahaaap wrote: - Building just LADSPA or new LADSPA plugins without building the standalone application doesn't work. They seem to require several other items to be built/copied first (src/faust, src/plugins and others). These dependencies are currently not defined (and I don't really know them). Is this something you'd like to be fixed? Or would it be better to just start removing the LADSPA stuff alltogether?
Hope that answer so far your questions,
regards
hermann
On the road again.
-
- Established Member
- Posts: 37
- Joined: Mon Sep 04, 2017 9:30 pm
Re: Guitarix release 0.36.0
This definitely answers all my questions Thanks again!tramp wrote:Hi
...
Hope that answer so far your questions,
regards
hermann
-
- Established Member
- Posts: 564
- Joined: Thu Mar 12, 2015 8:41 am
- Has thanked: 44 times
- Been thanked: 8 times
Re: Guitarix release 0.36.0
Hi Guys,
Thanks for streamlining those build flags.
I have just updated the AUR pkgbuild and it does seems to reduce the size of the package by half (80MB -> 40MB).
According to the build log, everything seems to be there (LV2+GUI, new LADSPA), but I didn't have a chance to fully test yet.
So just to double check, here's what I went with:
Did I miss something?
Thanks for streamlining those build flags.
I have just updated the AUR pkgbuild and it does seems to reduce the size of the package by half (80MB -> 40MB).
According to the build log, everything seems to be there (LV2+GUI, new LADSPA), but I didn't have a chance to fully test yet.
So just to double check, here's what I went with:
Code: Select all
python2 waf configure --prefix=/usr \
--cxxflags-release \
--optimize \
--new-ladspa \
--install-roboto-font