easyeffects icon indicating copy to clipboard operation
easyeffects copied to clipboard

Community Presets implementation

Open Digitalone1 opened this issue 11 months ago • 155 comments

After reading #1771, I wanted to test presets placed in a system config folder.

In addition to the common local config ~/.config/easyeffects, EE looks into system folders given by g_get_system_config_dirs (appending input/output directories): https://github.com/wwmm/easyeffects/blob/master/src/presets_manager.cpp#L34-L38

And also into etc/easyeffects/input and etc/easyeffects/output: https://github.com/wwmm/easyeffects/blob/master/src/presets_manager.cpp#L41-L42

  • The first issue I encountered is that if I copy a preset into a system folder, the new preset name is not shown into the menu. But if I shut down and relaunch the application, the new presets are listed inside the menu. It seems system folders are scanned only at the application startup. @wwmm can you confirm?

  • Another issue is that the menu shows the delete button for a system preset, but this does not work because the application does not have root privileges to erase the related file.

System folders could be useful in the future if someone wants to distribute community presets in a separate package. But the issues above should be addressed.

  • If the presets are installed when EE is running, they should be immediately visible, not requiring to restart the application.
  • System presets should net be removable from the preset menu because they are tied to the external package, so the should disappear only if the user uninstalls the package. Besides the remove button does not work for them and this is confusing for the user. Therefore we should remove the delete button for system preset entries.
  • We should test what happens for double presets (files with the same name) placed in both user and system folders. I suppose the ones in user config should take the precedence.

Digitalone1 avatar Jul 13 '23 19:07 Digitalone1

It seems system folders are scanned only at the application startup. @wwmm can you confirm?

Yes. At this moment we listen to changes only in the user folder and not in the system https://github.com/wwmm/easyeffects/blob/cf37bb3b9c692850b93f299eaeeaff874f4f75ab/src/presets_manager.cpp#L65C13-L65C13. The problem is that while the user config is in only one place the system preset path can have multiple locations. I am not sure if creating a folder monitor for each possible path is a good approach or even a good idea from a performance point of view.

System presets should net be removable from the preset menu because they are tied to the external package, so the should disappear only if the user uninstalls the package. Besides the remove button does not work for them and this is confusing for the user. Therefore we should remove the delete button for system preset entries.

That is why I think that community or system presets should be tied to the import dialog. This way the workflow forces the user to import everything to a single directory where we can actually delete files. The import dialog could point to a system path by default. And in this system path the community presets would be located for people interested in them.

We should test what happens for double presets (files with the same name) placed in both user and system folders. I suppose the ones in user config should take the precedence.

I am not sure about what would happen. We remove duplicated names from the list. So it will depend on the order erase is removing the duplicated values.

wwmm avatar Jul 13 '23 23:07 wwmm

That is why I think that community or system presets should be tied to the import dialog. This way the workflow forces the user to import everything to a single directory where we can actually delete files. The import dialog could point to a system path by default. And in this system path the community presets would be located for people interested in them.

Can you explain this method more in depth? I don't get it fully.

Anyway, aside of the community presets, if we still want to support system folders, we still need to hide the remove button because the file cannot be deleted.

If this is troublesome to do, then we should remove system folders.

About the scanning, we could just support \etc\easyeffects since the others seem to be unused anyway. I see lots of softwares installing system configuration files in a custom folder under etc, not specific subfolder or other locations.

Digitalone1 avatar Jul 14 '23 10:07 Digitalone1

Can you explain this method more in depth? I don't get it fully

It isn't really a method because all we would be doing would be to set the default path of the import dialog window to the system presets folder and nothing else. At this moment we do not set any default path to that dialog. Let's imagine that package maintainers start to put community presets inside /etc/easyeffects/presets. If the import dialog window starts there by default people will see what is there and choose to import one if they want to do so.

Anyway, aside of the community presets, if we still want to support system folders, we still need to hide the remove button because the file cannot be deleted.

Sure. But I am not sure if these system folders are being used by many people. Even after all these years I can't remember cases where people were actually using presets in system folders.

About the scanning, we could just support \etc\easyeffects since the others seem to be unused anyway. I see lots of softwares installing system configuration files in a custom folder under etc, not specific subfolder or other locations.

I think that the difficulty here is that for presets subfolders make sense. And the new / changed / removed monitoring probably does not work recursively. So the monitoring would still be "broken".

wwmm avatar Jul 14 '23 12:07 wwmm

It's a simple idea and it might work. The only issue is that if I usually download presets in a local folder (i.e. the Download directory), then having the import dialog to start always from the system folder it's a bit of an hassle.

If I remember correctly, it starts from the folder where it has been left the last time it was used, am I correct?

Anyway, if we go on that route, system folders should be removed from the code and /etc/easyeffects would be the folder where packages should install community presets (which I think on Flatpak it would be something under /var/.

Besides we should also allow multiple selections, because if a package installs 20 presets and I'd like to test them all, importing one at a time would be a big pain. Honestly, I don't remember if we already allow multiple selection.

Digitalone1 avatar Jul 14 '23 14:07 Digitalone1

It's a simple idea and it might work. The only issue is that if I usually download presets in a local folder (i.e. the Download directory), then having the import dialog to start always from the system folder it's a bit of an hassle.

Yes. It is annoying. But at least for me it is already annoying because it does not start in the folder I usually copy presets before importing.

It's a simple idea and it might work. The only issue is that if I usually download presets in a local folder (i.e. the Download directory), then having the import dialog to start always from the system folder it's a bit of an hassle.

At least on GNOME it shows the Recent files list by default and not a directory.

Anyway, if we go on that route, system folders should be removed from the code and /etc/easyeffects would be the folder where packages should install community presets (which I think on Flatpak it would be something under /var/.

Yes. But I suggest /etc/easyeffects/presets because maybe in the future the community decides to do something similar for impulse response files.

Besides we should also allow multiple selections, because if a package installs 20 presets and I'd like to test them all, importing one at a time would be a big pain. Honestly, I don't remember if we already allow multiple selection.

This should be possible. If I remember well the file dialog has a multiple files selection mode.

wwmm avatar Jul 14 '23 14:07 wwmm

I could work on this in the next weeks if I manage to find time.

So what we have to do is:

  • [x] Removing support for system folders inside the preset menu (quite easy);
  • [x] Adding multiple selection support for import dialogs related to preset and impulse managers (@wwmm any hint on that?);
  • [ ] Implementing a guideline for packagers to be published here on Github explaining how to provide community presets (where to place them, how to do for avoiding conflicts, etc...);
  • [ ] Let the import dialogs start from the proper directories where community presets should be placed (@wwmm any hint here?);
  • [x] Creating those directories after the installation (otherwise we can't start from them in import dialogs if they don't exist; @wwmm should we implement a post install script for this?)

Digitalone1 avatar Jul 16 '23 13:07 Digitalone1

I will take a look at the documentation. But thinking about the path again we probably should use /usr/share/easyeffects/presets. The /etc path is historically more for system services configuration. For example gnome apps put this kind of files inside /usr/share.

wwmm avatar Jul 16 '23 14:07 wwmm

Creating those directories after the installation (otherwise we can't start from them in import dialogs if they don't exist; @wwmm should we implement a post install script for this?)

I have updated the master branch using a Meson function designed for this task https://github.com/wwmm/easyeffects/commit/ab8c4fe1269468faa296a43de0d31bf767e3270f

wwmm avatar Jul 16 '23 14:07 wwmm

Inside the build time generated header #include "config.h" there will be #define SYSTEM_PRESETS_DIR

wwmm avatar Jul 16 '23 14:07 wwmm

Adding multiple selection support for import dialogs related to preset and impulse managers (@wwmm any hint on that?);

We just have to see how to use gtk_file_dialog_open_multiple instead of the gtk_file_dialog_open we use right now.

wwmm avatar Jul 16 '23 14:07 wwmm

Let the import dialogs start from the proper directories where community presets should be placed (@wwmm any hint here?);

It seems that we just have to set https://docs.gtk.org/gtk4/property.FileDialog.initial-folder.html. But I did not test it.

wwmm avatar Jul 16 '23 14:07 wwmm

To be clear the idea here is these preset packages would be installed with e.g. pacman and then the presets be shown automatically in easyeffects' dialog right? I think this can work well in flatpak but I will need to add some new configuration to the flatpak package.

In flatpak we will need use a different directory. Since it is configurable that isn’t an issue. Flatpak packages normally cannot modify things /usr in the flatpak package or on the host system.

The best reference I have is currently the flatpak build finds plugins at /app/extensions/Plugins/lv2. We would probably want something like /app/extensions/Presets. Unfortunately there isn’t much documentation for how this extension system works but from examples it is easy enough to understand.

https://github.com/wwmm/easyeffects/blob/ab8c4fe1269468faa296a43de0d31bf767e3270f/util/flatpak/com.github.wwmm.easyeffects.Devel.json#L13

https://github.com/wwmm/easyeffects/blob/ab8c4fe1269468faa296a43de0d31bf767e3270f/util/flatpak/com.github.wwmm.easyeffects.Devel.json#L36

On a related note it would probably be good to have at least 1 reference preset package by the time this feature is released? And to also test it.

vchernin avatar Jul 16 '23 14:07 vchernin

Implementing a guideline for packagers to be published here on Github explaining how to provide community presets (where to place them, how to do for avoiding conflicts, etc...);

You are right. Now that I think about it inside easyeffects/presets we will have to create a folder for input and another for output presets.

On a related note it would probably be good to have at least 1 reference preset package by the time this feature is released?

It is fine. But I do not have presets to put there. The ones I have are very specific to my hardware and use cases. Maybe the ones @Digitalone1 already shares in her github page could be installed by default as examples. If I am not mistaken @p-chan5 is also maintaining a github repository with presets. Maybe he is interested in this.

Git has a feature I have never used that allows third party git projects to be fetched as subprojects. Maybe if we look at it carefully there is a way to make Meson directly get the presets from @Digitalone1 and @p-chan5 at build time without the need to insert all of this in EasyEffects source code.

In flatpak we will need use a different directory. Since it is configurable that isn’t an issue. Flatpak packages normally cannot modify things /usr in the flatpak package or on the host system.

Shouldn't https://github.com/wwmm/easyeffects/commit/ab8c4fe1269468faa296a43de0d31bf767e3270f already be putting things in a place visible to Flatpak?

wwmm avatar Jul 16 '23 14:07 wwmm

Git has a feature I never used that allows third party git projects to be fetched as subprojects. Maybe if we look at it carefully there is a way to make Meson directly get the presets

Wouldn’t it be better to not have any presets at build time? And instead make some preset packages recommended pacman dependencies of easyeffects? So users can uninstall them easily, and more importantly it doesn’t affect the build at all?

Shouldn't https://github.com/wwmm/easyeffects/commit/ab8c4fe1269468faa296a43de0d31bf767e3270f already be putting things in a place visible to Flatpak?

Well it is visible to the easyeffects flatpak package, but not to other flatpak packages or future flatpak extension packages. While system packages installed from e.g. pacman could technically modify the flatpak data directory, it is unusual for e.g. pacman to manage files for a flatpak package.

vchernin avatar Jul 16 '23 15:07 vchernin

Wouldn’t it be better to not have any presets at build time? And instead make some preset packages recommended pacman dependencies of easyeffects? So users can uninstall them easily, and more importantly it doesn’t affect the build at all?

It is fine to do this. I thought you were thinking about shipping an example inside EasyEffects package. But your idea is to have third party distribution packages available by the time we release this EE feature. Ok.

Well it is visible to the easyeffects flatpak package, but not to other flatpak packages or future flatpak extension packages. While system packages installed from e.g. pacman could technically modify the flatpak data directory, it is unusual for e.g. pacman to manage files for a flatpak package.

Oh... Ok.

wwmm avatar Jul 16 '23 15:07 wwmm

Should we establish a convention/recommendation on how to group/organize preset packages? Or is this best left to packagers to decide? What if most packages have 10 presets, but one packager decides to split 10 presets into individual packages?

An example from https://github.com/wwmm/easyeffects/issues/1771#issuecomment-1235647058

Should we have:

  • A package containing every preset from the wiki: Arch: easyeffects-all-presets Flatpak: com.github.wwmm.easyeffects.Presets.all

  • One package per presets repo listed in the wiki: Arch: easyeffects-presets-digitalone1 Flatpak: com.github.wwmm.easyeffects.Presets.digitalone1.

  • One package per preset file listed in the wiki: Arch: easyeffects-presets-digitalone1-loudnessequalizer Flatpak: com.github.wwmm.easyeffects.Presets.digitalone1-loudnessequalizer.

From my earlier comment:

On a related note it would probably be good to have at least 1 reference preset package by the time this feature is released?

If anyone would like to make a preset package early as a reference/testing package, I can contribute a flatpak manifest file and submit it to the flathub github when everything is ready.

vchernin avatar Jul 16 '23 15:07 vchernin

Should we establish a convention/recommendation on how to group/organize preset packages?

I would let this decision to the package maintainers. As in the end the user will have to use EE import dialog it does not really matter to us how the files will be inside the system easyeffects/presets directory.

wwmm avatar Jul 16 '23 15:07 wwmm

it does not really matter to us how the files will be inside the system easyeffects/presets directory.

It is probably a good idea to separate input from output presets to make the user life easier. But other than this it should not matter much what is done.

wwmm avatar Jul 16 '23 16:07 wwmm

Well it is visible to the easyeffects flatpak package, but not to other flatpak packages or future flatpak extension packages.

So, if I understood correctly, a flatpak package can install the presets in the proper place where EE flatpak can see them, right?

My proposal for the location. Whether to use /usr or /etc, easyeffects folder will contain:

  • input for input effects
  • output for output effects
  • irs for convolver impulses
  • rnnoise for RNNoise models

Any of the above folders can have custom user files. This is useful for users that may have more user profiles and they want to import presets from a session to another. So they will move the files in the system directory (using root) and will import them in the application running on the other user session. I.e easyeffects/output/my-default-preset.json.

The same folders can have custom directories that will belong to community packages. So If I have a package shipping presets and impulses I will install:

  • easyeffects/output/community-package-name/community-out-preset.json
  • easyeffects/input/community-package-name/community-in-preset.json
  • easyeffects/irs/community-package-name/community-conv-impulse.irs

This way there will be no conflict because every package should have its own subfolder.

A package can install additional subfolders, i.e.:

  • easyeffects/irs/community-package-name/specific-device1/device1-impulse.irs
  • easyeffects/irs/community-package-name/specific-device2/device2-impulse.irs

What do you think about this scheme?

Anyway, Easyeffects package will install the easyeffects directory, but on the removal the directory should be left because it may still contain community presets or user custom presets.

Digitalone1 avatar Jul 16 '23 16:07 Digitalone1

Wouldn’t it be better to not have any presets at build time? And instead make some preset packages recommended pacman dependencies of easyeffects? So users can uninstall them easily, and more importantly it doesn’t affect the build at all?

It's doable, but not all distro will follow. Maybe doing it for Flatpak should be a good idea since its usage will grow on time.

If anyone would like to make a preset package early as a reference/testing package, I can contribute a flatpak manifest file and submit it to the flathub github when everything is ready.

Sure. Just pick some from here. I'd say @p-chan5, @JackHack96 and mine which seems more generic.

Digitalone1 avatar Jul 16 '23 17:07 Digitalone1

What do you think about this scheme?

It seems good.

wwmm avatar Jul 16 '23 17:07 wwmm

So, if I understood correctly, a flatpak package can install the presets in the proper place where EE flatpak can see them, right?

Yes

It's doable, but not all distro will follow. Maybe doing it for Flatpak should be a good idea since its usage will grow on time.

Including any preset packages at build time in my mind is too inconsistent. If we want to encourage preset authors/packagers to make their preset packages easily available they should make a distribution/flatpak package right? But if we have several preset packages added at build time they will likely ask to add their preset to the easyeffects build. So then we need to decide what goes in at build time and what should go in a separate package. Instead, if we don’t include any presets at build time, it means there would be several reference preset packages on distributions making it easier for others to create new packages.

Not to mention build time presets may introduce an annoyance with the flatpak package. I am not sure if preset extensions installed from separate flatpak packages at runtime, and presets installed at build time could be easily combined into one directory. It is something I need to test but if easyeffects never includes presets at build time it avoids this problem.

Any of the above folders can have custom user files. This is useful for users that may have more user profiles and they want to import presets from a session to another. So they will move the files in the system directory (using root) and will import them in the application running on the other user session.

Something worth noting is mixing package installed presets and user installed presets in a /usr directory could cause confusion. Usually /usr should only be modified by the system package manager and so should only contain package installed presets (ignoring /usr/local). On immutable distros like fedora silverblue, /usr is mounted as read only. You can only write to /etc or your home directory in /var.

With these changes can’t users add their own presets to the easyeffects config directory as done today? If they really wanted they can modify /usr, but most users probably shouldn’t. At least it could use a warning.

The rest of the proposal I quite like and I think it should work well.

vchernin avatar Jul 16 '23 18:07 vchernin

@vchernin I agree not adding presets at build time.

You said

And instead make some preset packages recommended pacman dependencies of easyeffects? So users can uninstall them easily, and more importantly it doesn’t affect the build at all?

On this I don't know. Arch packagers should be contacted, but not all distro will follow. If it's doable on Flatpak, it's not an issue.

Something worth noting is mixing package installed presets and user installed presets in a /usr directory could cause confusion.

Ok, so we would only recommend to install directories in easyeffects folder.

On immutable distros like fedora silverblue, /usr is mounted as read only. You can only write to /etc or your home directory in /var.

I didn't know that. So we can't use /usr for community presets anyway.

With these changes can’t users add their own presets to the easyeffects config directory as done today? If they really wanted they can modify /usr, but most users probably shouldn’t. At least it could use a warning.

Sure. I was only suggesting that users could move their own presets to easyeffects/output to make them available on another user session (because files of a specific user can't be accessed from another user session). But it's not mandatory, we could just ignore this use case.

Digitalone1 avatar Jul 16 '23 19:07 Digitalone1

Ok, so we would only recommend to install directories in easyeffects folder.

I am not sure what you mean by this?

I didn't know that. So we can't use /usr for community presets anyway.

I think to some degree using /usr/share/easyeffects for package manager provided presets would be ideal, as that is what that directory is there for. But /usr is read only sometimes. I don't really see a better way besides only using /etc/easyeffects/presets as the directory the import dialog points to (ignoring flatpak for a moment). It works for package managers and for manually copying files not available in a package.

One issue I am realizing I did not think through.. the flatpak extensions directory, where the plugins will be intalled cannot actually be modified by the user. From within the easyeffects flatpak package the directory will be: /app/extensions/Presets and so you will have e.g. /app/extensions/Presets/output/community-package-name/community-out-preset.json. However, this directory is always created at runtime using the flatpak extension packages that are installed. Meaning, users cannot permanently modify this directory without creating a whole flatpak package. Is this a problem?

I think it is a problem we can skip, as flatpaks would be the easiest to publish many packages for (it could be easily scripted). If we need to revise it in the future (and somehow allow multiple directories to host "community presets") it could be done without breaking backwards compatbility, since we will still need to use /app/extensions/Presets regardless.

vchernin avatar Jul 16 '23 20:07 vchernin

I think to some degree using /usr/share/easyeffects for package manager provided presets would be ideal, as that is what that directory is there for. But /usr is read only sometimes. I don't really see a better way besides only using /etc/easyeffects/presets as the directory the import dialog points to (ignoring flatpak for a moment). It works for package managers and for manually copying files not available in a package.

EasyEffects will still use ~/.config/easyeffects/ and its equivalent in the Flatpak side. The only difference is the import dialog pointing by default to the system presets folder. I understand some users may want to put things in the system folder. But I do not see a read only /usr as a problem worth of a solution from our side. Traditionally /etc is more for things like services settings so I think it is better to not put things like presets there.

wwmm avatar Jul 16 '23 21:07 wwmm

EasyEffects will still use ~/.config/easyeffects/ and its equivalent in the Flatpak side. The only difference is the import dialog pointing by default to the system presets folder. I understand some users may want to put things in the system folder. But I do not see a read only /usr as a problem worth of a solution from our side. Traditionally /etc is more for things like services settings so I think it is better to not put things like presets there.

I am fine either way, I don't think having ~/.config/easyeffects/ still available and necessary for some situations should be too confusing.

So just to decide so we don't need to revisit this, should it be /usr/share/easyeffects/presets then? In flatpak it can be /app/extensions/Presets, which is similar to where many audio plugins sit in /app/extensions/Plugins.

When I have time I will prototype the flatpak setup and make sure there isn't any unexpected weirdness.

vchernin avatar Jul 16 '23 21:07 vchernin

So just to decide so we don't need to revisit this, should it be /usr/share/easyeffects/presets then? In flatpak it can be /app/extensions/Presets, which is similar to where many audio plugins sit in /app/extensions/Plugins.

Yes. Inside this folder package maintainers will create subfolders to organize the presets as commented before by @Digitalone1.

wwmm avatar Jul 16 '23 22:07 wwmm

I am not sure what you mean by this?

That we won't recommend users to install files in /usr/share/easyeffects/output, so it should contain only directories related to packages (even if a user can still move files there anyway).

Digitalone1 avatar Jul 17 '23 16:07 Digitalone1

Hi developers. Thank you for considering me in this idea. Sure, I'd like to help with this!

It would be nice if installing these packages were two or three clicks away, this would benefit both new and novice users. Of course, the results will depend on the hardware they have, although taking into account that the Linux audio drivers are "generic" the quality of the speaker, headphones or monitors will be what makes the difference.

I agree with this scheme. Perhaps I would be a little more careful with distributing .iris or .rnnoise files as they can be copyright infringing or in some cases confusing to newcomers.

Regarding the maintenance of this package, I could provide updates that are in sync with my latest release and are compatible with the latest EE stable version.

p-chan5 avatar Jul 18 '23 06:07 p-chan5

Most of the work is done. But, as I said in #2485, even if the initial folder is set, the GTK file dialog starts from home folder, so it's not working as intended.

Digitalone1 avatar Aug 04 '23 06:08 Digitalone1