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

Macro naming dialog does not appear on Ardour 6.2 #2455

Closed
xard-dev opened this issue Aug 8, 2020 · 29 comments
Closed

Macro naming dialog does not appear on Ardour 6.2 #2455

xard-dev opened this issue Aug 8, 2020 · 29 comments
Labels
Bug Report Item submitted using the Bug Report template Host Specific Issues related to specific host(s) or host features Linux Issues which only occur on Linux
Milestone

Comments

@xard-dev
Copy link

xard-dev commented Aug 8, 2020

Describe the bug
Macro naming dialog does appear when Rename is activated on Ardour 6.2. The macro name is changed from "-" (dash) to nothing.

Please let us know your surge version
Built from git main commit 9cec9d8

  • Surge Version: 1.7.1
  • Plugin type: VST2, LV2
  • Bits: 64

To Reproduce

  1. add Surge (LV2 or VST2) instance to Ardour track
  2. open UI
  3. secondary click any macro field to get context menu
  4. click Rename

Expected behavior
A naming dialog pops up where the macro name can be defined.

Screenshots
Example of expected result which works under Carla and all versions of Jalv but not on Ardour:
image

Desktop (please complete the following information):

  • OS: Ubuntu 20.20
  • Host Ardour
  • Version 6.2

Additional context
This feature is working on other tested hosts which include Jalv gtk2, gtk3, qt5 and Carla 2.1.

When the dialog is called Ardour 6.2 logs following error:

zenity: symbol lookup error: /lib/x86_64-linux-gnu/libsecret-1.so.0: undefined symbol: g_input_stream_read_all_finish
@xard-dev xard-dev added the Bug Report Item submitted using the Bug Report template label Aug 8, 2020
@xard-dev
Copy link
Author

xard-dev commented Aug 8, 2020

I think I kind of know how Ardour devs will respond to a problem like this...

However the PopupEditorDialg.cpp code should probably not pass an empty value in case the dialog crashes.

@xard-dev xard-dev changed the title Macro naming dialog does appear on Ardour 6.2 Macro naming dialog does not appear on Ardour 6.2 Aug 8, 2020
@baconpaul
Copy link
Collaborator

Yeah I mean Paul basically told me on CDM that if we use any OS level APIs in linux we are making a mistake (check out our comments on the OB-Xa article there). Why is zenity crashing though? Does it work on the command line on your system?

One day we should reimplement these pop ups in vstgui.
;

@xard-dev
Copy link
Author

xard-dev commented Aug 8, 2020

Techincally it's not crashing but not launching at all which makes the popen close on error where the zenity is called from.

As far as I understand (correct me if I'm wrong) the popen tries to run distro shipped zenity under Ardour environment as sub process and once a library call gets redirected outside of the Ardour provided library set things like this start happening.

@baconpaul
Copy link
Collaborator

Right. So lets define crashing as “you type zenity and zenity doesn’t start” :)

If you run zenity from the command line does it work?

@baconpaul
Copy link
Collaborator

Oh I guess it does since you say it works in jalv and so on. Sigh.

@baconpaul baconpaul added Linux Issues which only occur on Linux Host Specific Issues related to specific host(s) or host features labels Aug 8, 2020
@xard-dev
Copy link
Author

xard-dev commented Aug 8, 2020

Yes it does work (that's how I got the screenshot) but the difference is that all those applications are distro shipped and the Ardour is the official standalone release.

@xard-dev
Copy link
Author

xard-dev commented Aug 8, 2020

I can reproduce this easily from command line by setting the library environment to Ardour provided one:

LD_LIBRARY_PATH=$(/opt/Ardour-6.2.0/lib) zenity
zenity: symbol lookup error: /lib/x86_64-linux-gnu/libsecret-1.so.0: undefined symbol: g_input_stream_read_all_finish

@baconpaul
Copy link
Collaborator

Yeah I see that now sorry. Ardour must be replacing some key library in the environment LDPATH which makes zenity not work.

So the solution here if you are committed to using Ardour is to rewrite those dialogs to use vstgui and not zenity; or in the popes command unset the LD_LIBRARY_PATH from Ardour specific stuff and set it back to the OS default; or maybe try building Ardour yourself.

@baconpaul
Copy link
Collaborator

Yup. Looks like Ardour has an environment which is incompatible with sub-processes on Ubuntu 20

@xard-dev
Copy link
Author

xard-dev commented Aug 8, 2020

And what I was trying to say is that the function calling popen should probably NOPE out if the popen command exists to error.

@baconpaul
Copy link
Collaborator

By “NOPE” you mean “leave the value unchanged”? Sure I would merge that! Or you mean replace the string with “ERROR”? That may have more negative consequences.

https://cdm.link/2020/08/ob-xd-2-0-open-oberheim-synth/

There’s that conversation with Ardour devs. Sigh.

@xard-dev
Copy link
Author

xard-dev commented Aug 8, 2020

I think leaving the value as is like "-" minus / dash in the context of macro widget would be good indication that it's not set

@baconpaul
Copy link
Collaborator

Yeah right so if popes fails keep the old value. I would be finer with that change!

@baconpaul
Copy link
Collaborator

But of course zenity breaking will also impact all the other functions - especially the file/open dialogs - in Ardour which have no obvious substitute.

@xard-dev
Copy link
Author

xard-dev commented Aug 8, 2020

Okay, that is no good.

I'm not aware of all the places where the zenity is being used.

@baconpaul
Copy link
Collaborator

Zenity is used for

  • error messages
  • a few typein pop ups
  • file open dialog

The first two we could rewrite in vstgui and not have a fork.

The third we could also rewrite using vstgui built ins but the vstgui Linux file open spawns zenity just like we do

@xard-dev
Copy link
Author

xard-dev commented Aug 9, 2020

Addition to those Surge also uses xdg-open used to display HTML and XML.

I did a little test where I replaced zenity binary with a script which calls it indirectly and unsets the LD_LIBRARY_PATH (which is not a good idea on top of all this) but it works. Eliminating the environment would have serious side effects but if limited to ardour workaround it might just work.

@baconpaul
Copy link
Collaborator

Ardour also does that. So perhaps a way to get the Ardour devs attention is to point out that their XDG-open call on Ubuntu 20 doesn’t work?

We could definitely parameterize the zenity command name (it is used 4 or 5 places so we could just make a global var instead of the string) and then if the host is Ardour use some other string at initialization time? And that string would be I guess LD_LIBRARY_PATH=. zenity

@xard-dev
Copy link
Author

xard-dev commented Aug 9, 2020

Does Ardour do XDG-open calls?

@baconpaul
Copy link
Collaborator

paul@ubuntu:~/ldev/ardour$ grep -ri xdg-open .
./libs/pbd/openuri.cc:  std::string command = "xdg-open ";
./libs/pbd/openuri.cc:          ::execlp ("xdg-open", "xdg-open", s.c_str(), (char*)NULL);

#657

@tank-trax
Copy link
Collaborator

Are you sure the popup is isn't opening but is appearing as the lowest element or layer underneath? I find that I often have to move the DAW/Host and Surge GUI away to find the popup which was lurking behind everything.

Maybe something to make sure that the popup is indeed the upper most element and can be seen.

@xard-dev
Copy link
Author

xard-dev commented Aug 9, 2020

@baconpaul Good point. Curiously enough the xdg-open called from ardour does work!

@tank-trax That was my first thought but as soon as I couldn't find any windows I started looking terminal. There's quite verbose description in this thread about what has been tested in this thread.

@baconpaul
Copy link
Collaborator

I wonder what magic they do that lets it work there

@baconpaul
Copy link
Collaborator

From that sfizz it seems the magic is 'unset LD_LIBRARY_PATH before you run zenity'
not clear that that would work in any other setting though
so 🤷 not sure what to do here.

@baconpaul baconpaul added this to the Currently Unscheduled milestone Sep 21, 2020
@mvf
Copy link
Collaborator

mvf commented Sep 21, 2020

I suggested a fix to Ardour upstream. (Issue Link)

@baconpaul
Copy link
Collaborator

Yeah if you dig down the threads it's clear that the ardour devs think that's the plugins job not theirs. But maybe they will change their mind.

@baconpaul
Copy link
Collaborator

baconpaul commented Sep 23, 2020

So ardour is obviously never fixing the issue which we perceive as a bug, since they do not perceive it is a bug.

So for now zenity won't work in ardour.

While there are solutions to that problem of various forms, for this particular rename dialog, to remove some very ancient windows code I just pushed a change to move it away from zenity to a built in vstgui-in-our-editor dialog. @xard-dev if you build from head of main you should be able to rename macros in ardour. (I tested linux vst3 in the juce host and it worked fine).

newtypein

I'll leave this issue open in case we end up feeling like doing an 'am i in ardour trying to run zenity so i need to hack my ld library path because that's what ardour needs' fix in the future. But the material problem you experienced is accidentally solved by the change I just pushed!

@xard-dev
Copy link
Author

xard-dev commented Sep 25, 2020

As I finally had some time I compiled the git main commit and this dialog option is in.

I confirm that it works on Ardour6 and does what is expected for the purpose:

  1. tried all possible widgets with rename widget to see the dialog placement which was fine
  2. text selection works with usual shortcuts
  3. keyboard layout and special characters work fine
  4. enter closes the dialog as expected

I think this pretty much closes the this ticket as the macros can be now renamed when using Ardour which was the main point of this ticket. 👍

One small issue would be that trying to select text with mouse closes the dialog but it's a small thing.

Thank you for going this far for a simple thing like this!

Update: Main branch was at commit ef8c17d.

@xard-dev
Copy link
Author

A built-in dialog was implemented which works inside the plugin and now macros can be renamed on Ardour 6.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Report Item submitted using the Bug Report template Host Specific Issues related to specific host(s) or host features Linux Issues which only occur on Linux
Projects
None yet
Development

No branches or pull requests

5 participants