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

Loading a file opens the file with write permissions #19

Open
rohanlean opened this issue Nov 19, 2021 · 19 comments
Open

Loading a file opens the file with write permissions #19

rohanlean opened this issue Nov 19, 2021 · 19 comments
Milestone

Comments

@rohanlean
Copy link

When selecting "Load file" from the menu, the file is opened with write permissions. I see no good reason for this, and because of my setup it has caused me a bit of a bother until I figured out what was going on.

@paulfd paulfd added the bug label Nov 19, 2021
@paulfd
Copy link
Member

paulfd commented Nov 19, 2021

Hi! Thanks for the report. I just tested this and this opens ok. In fact, from what I see and without getting too technical, the file loading function we use should open the file for reading only, so there might be something else at play.

Can you detail the version, setup, and problems you're experiencing?

@rohanlean
Copy link
Author

Hey, and thanks for looking into this. Apologies if I have made some wrong assumptions. In more detail my problem was as follows:

I am using the flatpak version of Ardour and sfizz, and I had given the Ardour flatpak read-only permissions to an sfz virtual instrument. The file selection dialog of sfizz is a portal, i.e. it can see all files, even those that Ardour/sfizz cannot normally see or access. When selecting a file that the flatpak has no access to, the portal copies that file and returns the copy to the process. When selecting a file that the process does have sufficient access to, then supposedly no copy is made and the path is return as-is.

The behaviour I observed was that sfizz loaded the instrument, but did not load or play any samples. That behaviour seems consistent with sfizz opening an out of context copy of the sfz file. When I gave the Ardour flatpak write access to the directory with the virtual instrument, then the instrument loaded and played properly. I concluded that sfizz requested write permissions and that the file portal therefore made a copy.

The other possibility I see is that the portal does not handle the case of read-only files appropriately and makes a copy on read-only access even if the process has read permissions. Now that I think about it, perhaps the portal is only queried for a path and does not even know about the intentions? That would probably be hard to fix...

@paulfd
Copy link
Member

paulfd commented Nov 20, 2021

OK, so my test was way too simple :) I did not even know that sfizz had a flatpak version. Did you get this and Ardour from Flathub?

What I am afraid of is the following, and this is also a problem on Android for example: these kind of portals only give access to 1 file, as selected by the user. The issue is that we have other files here, the samples and possibly other included sfz files. This is probably the root cause of the problem...

@rohanlean
Copy link
Author

I did not even know that sfizz had a flatpak version. Did you get this and Ardour from Flathub?

Yes. org.ardour.Ardour and org.freedesktop.LinuxAudio.Plugins.sfizz, runtime version 21.08. I have also removed some permissions from the Ardour sandbox with flatpak override [--user]. For reproduction it is perhaps convenient to remove read-write access to the home folder in particular. I also used flatpak override to give Ardour (read) access to the virtual instrument folder.

Looking at the portal documentation, it seems that unfortunately one cannot communicate to the file chooser that the file will be opened read-only, so the portal will presumably always make a copy if the flatpak has only read access. There is the option of opening a whole folder. But that is not really a straightforward solution for sfizz either, because there can be alternative choices of sfz file in that folder.

Compounding the issue is that sfizz uses a CNewFileSelector from VSTGUI instead of talking to the portal directly.

With my better understanding of the issue I am no longer sure if a fix is worth your time. I myself can certainly live with giving Ardour write permission to the sfz virtual instruments.

@alexlarsson did I miss something? Perhaps a read-only option for the file chooser portal would be nice even independently of this issue. There too I am not sure if it is worth the effort and added complexity, though.

@paulfd
Copy link
Member

paulfd commented Nov 20, 2021

Looking at the portal documentation, it seems that unfortunately one cannot communicate to the file chooser that the file will be opened read-only, so the portal will presumably always make a copy if the flatpak has only read access. There is the option of opening a whole folder. But that is not really a straightforward solution for sfizz either, because there can be alternative choices of sfz file in that folder.

Android has the same issue; in the end you need to go through some hoops to make SFZs work there. In fact there are a number of cases I encountered in my life with (sensible) sidecar files that were hindered by the portal principle. I am not sure what the best solution would be in this case. Ideally the portal should ask for a file with a provision for all other files and subdirectories, but one could argue that it kind of defeats the portal idea...

@rohanlean
Copy link
Author

Ideally the portal should ask for a file with a provision for all other files and subdirectories

That would not even suffice though, because the sfz files can (and sometimes do) reference parent directories.

Ironically this whole issue would be moot if the file chooser dialogue did not use a portal at all. As the portal behaviour is never sensible in the case of sfizz that might even be a reasonable solution. But I do not know if and how one can achieve it, given that the portal is opened by a library that you are using.

@paulfd
Copy link
Member

paulfd commented Nov 20, 2021

Cross-platform/cross-distribution file choosing is harder than it looks 😄 But in a Flatpak application (or any kind of container) can you really bypass the portal? I'd think this is the whole point of Flatpaks.

@rohanlean
Copy link
Author

can you really bypass the portal?

Well, the flatpak has its own view of the file system that can overlap with the host filesystem. If you run a "plain" file chooser in the flatpak, it will see what the sandboxed process sees. In my case that view would have included the virtual instrument folder. The portal is opened by the process talking to some dbus service. Some file choosers just do that automatically.

@paulfd
Copy link
Member

paulfd commented Nov 20, 2021

OK I see. In this case you wouldn't want to ask the portal then, although people unaware of Flatpaks and such would probably be very surprised. This doesn't seem to be a zenity option, but we might add it in the future. For the time being, I think I will just remove the bug from the next release target but keep it around to remember.

By the way, if you use the LV2 plugin and Ardour's generic GUI (which includes a file chooser with the LV2 plugin but not VST3), what happens in this case?

@rohanlean
Copy link
Author

By the way, if you use the LV2 plugin and Ardour's generic GUI (which includes a file chooser with the LV2 plugin but not VST3), what happens in this case?

Oh, I did not know about that generic GUI trick. Because Ardour uses the deprecated GTK2, which does not support portals, this actually does it. 😄

@falkTX
Copy link

falkTX commented Nov 20, 2021

By the way, if you use the LV2 plugin and Ardour's generic GUI (which includes a file chooser with the LV2 plugin but not VST3), what happens in this case?

Oh, I did not know about that generic GUI trick. Because Ardour uses the deprecated GTK2, which does not support portals, this actually does it. smile

You can use desktop portal dbus stuff for the plugin UI without a problem.
I implemented it in DPF a couple of days ago, see HAVE_DBUS stuff in https://github.com/DISTRHO/DPF/blob/develop/distrho/extra/FileBrowserDialog.cpp
DPF license is ISC but feel free to use that code in any way, getting plugin Uis to behave proper would be very nice to see.
One day we will be able to say goodbye to the zenity/kdialog hacks.

@rohanlean
Copy link
Author

The best solution for the flatpak case would perhaps be to let the user choose both a folder -- with a portal -- and an sfz file from that folder (restricting the file chooser to the folder). That way even users who are clueless about flatpak can load instruments. A minor downside is that the host flatpak and all plugins running within it will gain unnecessary write access to the instrument. That could be fixed by the file portal allowing the application to ask for read-only access, and presenting the distinction to the user.

@paulfd
Copy link
Member

paulfd commented Nov 20, 2021

@falkTX yeah I had this noted down somewhere, I'll probably try to integrate this into VSTGUI in our own fork, and possibly upstream if this is OK with you. However, it'd probably be cool to have a way to choose whether or not to use the portal at runtime.

@rohanlean Meh, not entirely convinced about this solution 😄 Detecting that we're running within the jail and informing the user of the proper steps he should take to ensure good behavior would be better in my eyes. I know for example AppImages set some global variables you can query to check your own jail, I'm sure Flatpaks have something like this.

@rohanlean
Copy link
Author

Detecting that we're running within the jail and informing the user of the proper steps he should take to ensure good behavior would be better in my eyes.

Hmm, that does seem like a lot more work for the user than selecting folder and sfz file in succession. They would have to type some commands into a host shell, and then restart the plugin host. Sfizz cannot even determine what exactly the commands should be.

You could use the two-step selection only when running inside of a flatpak, although I am not sure if the minor convenience of not having to select a folder first trumps consistency across platforms.

@falkTX
Copy link

falkTX commented Nov 20, 2021

yeah I had this noted down somewhere, I'll probably try to integrate this into VSTGUI in our own fork, and possibly upstream if this is OK with you. However, it'd probably be cool to have a way to choose whether or not to use the portal at runtime.

All fine, target is to allow better plugins here so do as you see best.
I do not have a use for vstgui, so I wont do that myself.

@paulfd
Copy link
Member

paulfd commented Nov 22, 2021

You could use the two-step selection only when running inside of a flatpak, although I am not sure if the minor convenience of not having to select a folder first trumps consistency across platforms.

Maybe the most user-friendly way when a jail is detected is then is to warn the user on opening that we're going to need a particular process (i.e. select a directory first). We may even offer the option of finding a file within the jail or outside it at this point.

@rohanlean
Copy link
Author

Maybe the most user-friendly way when a jail is detected is then is to warn the user on opening that we're going to need a particular process (i.e. select a directory first). We may even offer the option of finding a file within the jail or outside it at this point.

That seems to cover all the bases. The important thing for me would be that, once you already know what is going on, you do not lose time but can immediately get to selecting in whichever way you want.

I cannot really comment on what the experience would be like for someone who is ignorant of the sandbox. I think usually the aim is to keep it completely transparent to casual users. On the other hand audio plugin users seem to be used to far more daunting interfaces. I am new to this and I am often overwhelmed when first opening a new plugin. 😀

@paulfd
Copy link
Member

paulfd commented Nov 22, 2021

Let's see, I think this is a minor annoyance in the grand scheme of things. We could even cheat and ask the user for a file, but actually ask the portal for the directory (seems a bit weird but heh). Another question is: do authorizations persist if you close and reopen Ardour for example (i.e. I give access to a folder through the portal one day, does the access persist if I reopen Ardour another day?). If not this is going to be a larger headache.

@rohanlean
Copy link
Author

Let's see, I think this is a minor annoyance in the grand scheme of things.

Agreed.

We could even cheat and ask the user for a file, but actually ask the portal for the directory

That does not work, for multiple reasons. First I do not see how you want to determine the directory. Remember that it is not necessarily the directory that contains the sfz file. But more importantly you

  • cannot determine the directory after asking for a file, because the path you get is unrelated to the original path;
  • cannot ask the portal for access to a specific folder. You can only ask for access to a folder, that the user then has to select.

do authorizations persist if you close and reopen Ardour for example

Yes.

@redtide redtide transferred this issue from sfztools/sfizz May 8, 2023
@redtide redtide added this to the 1.3.0 milestone May 17, 2023
@redtide redtide removed the bug label Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants