brummer10 / guitarix

guitarix virtual versatile amplification for Jack/Linux
234 stars 24 forks source link

NSM: in headless mode, don't announce with :optional-gui: #128

Closed grammoboy2 closed 1 year ago

grammoboy2 commented 1 year ago

Is headless mode supposed to work with NSM? I guess it should, might be even very useful when working with network sessions, but Guitarix only announces with :optional-gui: as far as I can see, while in headless mode it shouldn't most likely.

Headless mode currently needs a cli argument: --nogui, but this can be fixed in the same way it's done in non-mixer-noui I guess. Then it also works in Non-session-manager and Agordejo.

https://github.com/linuxaudio/non/blob/master/mixer/src/main.C#L221 https://github.com/linuxaudio/non/blob/master/mixer/wscript#L106

brummer10 commented 1 year ago

This looks to me more like a workaround than a solution. There are a couple more cli args which may be useful also when running under NSM. Unfortunately NSM decide to not support cli args. That is the culprit. In my opinion the correct way to fix this is that NSM allow those (Like Ray-session already done), instead force every app to implement workarounds for this flaw. I wouldn't add a symlink for every possible useful command-line arg guitarix support in the build system. So, sorry, this is a wont-fix for me.

grammoboy2 commented 1 year ago

I did think about this, but I don't think it's a flaw in NSM. It keeps the usage of NSM simple, compatible and it makes it possible to support the :switch: feature, where application don't have to restart (with a other argument). Why don't people complain when they can't add arguments to plugins, when adding one into Ardour? Imagine people could. It makes things far more complex, something which linuxaudio is already famous for.

You don't have to add a symlink for every possible useful command line arg, that's not true. Most of them could be stored in the guitarix project file (like mute) and be restored when under NSM. Others could be environment variables, like rpcport, rpchost and display. There are not many things that are easier to support in code then a simple getenv I think.

Which arguments guitarix provides could not be handled in one of these mentioned ways you think?

brummer10 commented 1 year ago

You are on the wrong track here. A plugin is a library loaded by a host to "run" inside the host. Even "run" is not true, it's just that the host load the library and calling functions from it. For library's it is simply not possible to give command-line args when load them.

Beside the --nogui arg there is the --onlygui which you'll need to control the ui-less engine. And, one of the most overseen options ( by newbies) is the --name arg. Those allow you to set a name for the guitarix instance. Most (semi) professional guitarix users makes heavy use of it, as it allow you to run multiple instances with given names, for example -n lead -n rhythm -n bass I guess you'll imagine how that will simplify your workflow. Environment variables been used in guitarix as well, downside in this scenario is that they'll be true for each instance, so that wouldn't work.

For handling command-line args in NSM there could simply be added a new field --args, for example, those could contain arguments for the app to start. If the NSM server support it, it could forward the args to the client, if not, it could simply ignore them. Checkout how gdb handle this. If done carefully it didn't harm older NSM sessions or older NSM servers.

grammoboy2 commented 1 year ago

You are on the wrong track here. A plugin is a library loaded by a host to "run" inside the host. Even "run" is not true, it's just that the host load the library and calling functions from it. For library's it is simply not possible to give command-line args when load them.

But still those plugins provide enough functionality by themselves, so that no one is missing the option to add arguments apparently. So the same could be true for NSM clients. For users this host/plugin technology limitations has the advantage that it keeps it simple for the user (and developer).

Beside the --nogui arg there is the --onlygui which you'll need to control the ui-less engine.

Could be a symlink like '-noui'.

And, one of the most overseen options ( by newbies) is the --name arg. Those allow you to set a name for the guitarix instance. Most (semi) professional guitarix users makes heavy use of it, as it allow you to run multiple instances with given names, for example -n lead -n rhythm -n bass I guess you'll imagine how that will simplify your workflow.

This is a different topic I think. This should be solved by using NSM labels and/ or JACK metadata (pretty-names).

Environment variables been used in guitarix as well, downside in this scenario is that they'll be true for each instance, so that wouldn't work.

This is not useful for all settings, but rpcport, rpchost, display and --log-terminal, are useful for specific situations. They could only be set on the server, or just in the terminal/ environment before the NSM server is started. Then the NSM client inherits the same environment.

For handling command-line args in NSM there could simply be added a new field --args, for example, those could contain arguments for the app to start. If the NSM server support it, it could forward the args to the client, if not, it could simply ignore them. Checkout how gdb handle this. If done carefully it didn't harm older NSM sessions or older NSM servers.

First there must be a convincing reason to provide this. Your examples didn't convince me so far. I still don't see a convincing example where functionality can only be handled when using arguments. I'll reconsider them though. For now I only see that it would make things more complex for the user and the developer (supporting :switch:, more complexity to NSM server and GUI, less compatibility, more complexity for user). You say that RaySession is supporting it, but that's not exactly true. It provides a way to use it, but with a message that it's not supported by NSM.

To be clear, I'm not saying Guitarix should remove the argument options, but it should be provide ways to store it when used with NSM.

You've closed this issue, but the issue is not about providing arguments (important and interesting topic for sure, thanks for providing your pov), but about announcing to NSM with the :optional-gui: capability while in headless mode. This is also a issue when NSM would accept arguments.

brummer10 commented 1 year ago

You've closed this issue, but the issue is not about providing arguments (important and interesting topic for sure, thanks for providing your pov), but about announcing to NSM with the :optional-gui: capability while in headless mode. This is also a issue when NSM would accept arguments.

There is no way to run guitarix headless under NSM for now. And Yes, I've added the label "wontfix" some posts ago, hence I closed the issue as we didn't come any further here. You've given your arguments and I've give mine. I'm not willing to implement such a workaround as you suggested in your opening post, even after reading all your arguments (and think about them), so, it seems this issue will remain unresolved. If NSM ever supports command line args you are welcome to come back with this issue and I'll re-check the optional-gui announcement.

grammoboy2 commented 1 year ago

If guitarix was handling NSM functionality in headless mode, which I don't see why it shouldn't, then it would be easy for me to fix that symlink for my usecase. Actually I think it could be a nice usecase for a NSM network session.

But indeed, looking at the code a bit closer, it doesn't handle NSM at all when in headless mode. That would be much harder for me to fix in the right way, would better be done by the developer who knows the code and the coding most likely. Then I'll fix that symlink myself. ;)

https://github.com/brummer10/guitarix/blob/2d3d89cf922f3e63111414a55050d78b493661fb/trunk/src/gx_head/gui/gx_main.cpp#L697

https://github.com/brummer10/guitarix/blob/master/trunk/src/gx_head/gui/gx_main.cpp#L787

https://github.com/brummer10/guitarix/blob/2d3d89cf922f3e63111414a55050d78b493661fb/trunk/src/gx_head/gui/gx_main.cpp#L898

https://github.com/brummer10/guitarix/blob/2d3d89cf922f3e63111414a55050d78b493661fb/trunk/src/gx_head/gui/gx_main.cpp#L980

grammoboy2 commented 1 year ago

Beside the --nogui arg there is the --onlygui which you'll need to control the ui-less engine.

Does --onlygui save it's state actually? If not, it could be used in NSM-Proxy or similar.

brummer10 commented 1 year ago

Does --onlygui save it's state actually?

No, it's a runtime option.

If not, it could be used in NSM-Proxy or similar.

Isn't the same true for --nogui As with a proxy we didn't need any NSM support at all?

grammoboy2 commented 1 year ago

Isn't the same true for --nogui As with a proxy we didn't need any NSM support at all?

No, because --nogui does save it's state (and so benefits from proper reply, error handling, user experience and such via NSM).

grammoboy2 commented 1 year ago

Ok, so I tested this remote setup of Guitarix, assuming that it would show me that particular example where I had to admit that arguments would be needed if you want to launch multiple instances of Guitarix.

But man, that avahi system seems to works wonderful. The only problem for NSM I see now, would be that it shouldn't pick a default port, but a random one, to be able to use multiple instances. Which can then be chosen in the guitarix -G version on the other host (running in NSM-Proxy for example).

Note, that nothing in the client API of NSM prevents a server to implement arguments, like Raysession did. There are just very good reasons to not support this. I agree with this from a user and a developer perspective. The RaySession developer also agrees with this taken from what I read from him, but his take on this is less strict, so it's possible but not recommended in RaySession.

Anyway, impressive job. I feel you would really shoot yourself in the foot if you don't make it possible to use such setup with NSM (network sessions). The only 'symlink binary' that would be needed, would be guitarix-noui, Others could use it in Raysession. If Guitarix announces without :optional-gui: in headless mode, then the NSM gui could make it visible when the headless version is used (besides labels, pretty names, jack metadata and such). There is no technical limitation in NSM that prevents Guitarix to support this.

brummer10 commented 1 year ago

Thanks for testing it and give feedback. Nice to hear you like how it works. However, as pointed out earlier, even when you've the ability to run a headless instance under NSM, you can't give it a name, so user experience will only reflect the NSM limitations instead reflect what guitarix could do . That is a showstopper for me. All this works very well without using NSM for that. So, I've no motivation to do any further work on that for the time being. Anyway, as it is open source, patches been welcome and reviewed open minded here.

grammoboy2 commented 1 year ago

Nsm clients can send a label. Agordejo and RaySession are capable of renaming the NSM client. Jack clients can be renamed in Qjackctl. Jack metadata pretty name implementations around NSM will probably arise as well. I don't see a limitation for this in the NSM api currently.

One thing about arguments. You say that the symlink solution of non-mixer is a workaround as being inferior. Arguments are also a workaround to be able to use the same code in a slightly different manner. Not much of a difference I think. Just smart Unix thinkering imo.

I looked at the code, it's too complex for me at the moment to do it in a reasonable amount of time with a reasonable quality level. For you it would be not so hard I think. All code is already there, the architecture needs improvement to handle both situations, this will also improve Guitarix most likely.

Anyway, I got some valuable insights from the discussion. Have a nice evening.