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

Community Presets implementation #2445

Open
Digitalone1 opened this issue Jul 13, 2023 · 153 comments
Open

Community Presets implementation #2445

Digitalone1 opened this issue Jul 13, 2023 · 153 comments

Comments

@Digitalone1
Copy link
Contributor

Digitalone1 commented Jul 13, 2023

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.
@wwmm
Copy link
Owner

wwmm commented Jul 13, 2023

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.

@Digitalone1
Copy link
Contributor Author

Digitalone1 commented Jul 14, 2023

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.

@wwmm
Copy link
Owner

wwmm commented Jul 14, 2023

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".

@Digitalone1
Copy link
Contributor Author

Digitalone1 commented Jul 14, 2023

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.

@wwmm
Copy link
Owner

wwmm commented Jul 14, 2023

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.

@Digitalone1
Copy link
Contributor Author

Digitalone1 commented Jul 16, 2023

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

So what we have to do is:

  • Removing support for system folders inside the preset menu (quite easy);
  • 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?);
  • 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?)

@wwmm
Copy link
Owner

wwmm commented Jul 16, 2023

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
Copy link
Owner

wwmm commented Jul 16, 2023

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 ab8c4fe

@wwmm
Copy link
Owner

wwmm commented Jul 16, 2023

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

@wwmm
Copy link
Owner

wwmm commented Jul 16, 2023

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
Copy link
Owner

wwmm commented Jul 16, 2023

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.

@vchernin
Copy link
Contributor

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.

"--env=LV2_PATH=/app/lib/lv2:/app/extensions/Plugins/lv2"

"org.freedesktop.LinuxAudio.Plugins.Calf": {

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.

@wwmm
Copy link
Owner

wwmm commented Jul 16, 2023

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 ab8c4fe already be putting things in a place visible to Flatpak?

@vchernin
Copy link
Contributor

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 ab8c4fe 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.

@wwmm
Copy link
Owner

wwmm commented Jul 16, 2023

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 wwmm pinned this issue Jul 16, 2023
@vchernin
Copy link
Contributor

vchernin commented Jul 16, 2023

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 #1771 (comment)

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.

@wwmm
Copy link
Owner

wwmm commented Jul 16, 2023

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
Copy link
Owner

wwmm commented Jul 16, 2023

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.

@Digitalone1
Copy link
Contributor Author

Digitalone1 commented Jul 16, 2023

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
Copy link
Contributor Author

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.

@wwmm
Copy link
Owner

wwmm commented Jul 16, 2023

What do you think about this scheme?

It seems good.

@vchernin
Copy link
Contributor

vchernin commented Jul 16, 2023

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.

@Digitalone1
Copy link
Contributor Author

@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.

@vchernin
Copy link
Contributor

vchernin commented Jul 16, 2023

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.

@wwmm
Copy link
Owner

wwmm commented Jul 16, 2023

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.

@vchernin
Copy link
Contributor

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.

@wwmm
Copy link
Owner

wwmm commented Jul 16, 2023

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.

@Digitalone1
Copy link
Contributor Author

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).

@wwmm
Copy link
Owner

wwmm commented Apr 9, 2024

Can we look inside the preset from PresetsManager to search for irs/model filenames and copy them?

Hum... It is probably better to make the community preset import routine to also copy the impulse/rnnoise files to the local folder if it detects the preset contains a reference to these files. THis way there won't be the need to change things in presets_manager.

There's also the other issue that if we start to use the filename rather than the path, the existing presets are deprecated (and not working anymore, except if we make some workaround...)

Hum... At this moment the key in the presets is named kernel-path. Maybe we could add a new key like kernel-file-name and still try to read the path key for backwards compatibility. Newer presets would only set the one about the file name. But we could still check if the one about the path is there to still load old presets.

@Digitalone1
Copy link
Contributor Author

Digitalone1 commented Apr 9, 2024

Hum... It is probably better to make the community preset import routine to also copy the impulse/rnnoise files to the local folder if it detects the preset contains a reference to these files. THis way there won't be the need to change things in presets_manager.

And how can we detect if the preset contains references to these files?

@Digitalone1
Copy link
Contributor Author

Never mind. I started to make a refactoring and it seems doable. I think it's best to keep both kernel-path and kernel-name since various parts of the code rely on kernel-path. We can just save only the kernel-name inside the preset and keep kernel-path in gsettings since we derive it starting from the kernel-name and use it in various part of the code rather than constructing it every time...

@Digitalone1
Copy link
Contributor Author

Digitalone1 commented Apr 10, 2024

Unfortunately I couldn't retain the kernel-path. I switched the gsettings key to kernel-name (and model-name).

With the latest changes the community presets containing Convolver and/or RNNoise can be tested and correctly imported. I had to set the autoselect property as false for the RNNoise list so that when a new model is tested from the community preset, no item in the list is selected. This allows the user to reselect the standard model (or their local models) later.

In practice the presets now saves only the filename without extension. When an irs/model changes, the filename is searched, first locally, then in system folders if not found locally. If not found at all, the plugins goes in passthough (or fall back to the standard model for RNNoise).

I didn't have time to make a backward compatibility for previous presets. I don't know where it's better to do it. Maybe in the preset reading stage extracting the name from the path and saving to kernel-name. @wwmm what do you think?

@vchernin When you have time, please make a Flatpak test package including also convolver irs and rnnoise models. I updated the guidelines, take a look at it. Thanks.

@wwmm
Copy link
Owner

wwmm commented Apr 10, 2024

Maybe in the preset reading stage extracting the name from the path and saving to kernel-name. @wwmm what do you think?

I think it is fine. In the past something similar happened when we had to change the format of the preset file. It should be enough to do the parsing only in the reading phase. When writing the new format will be used anyway.

@Digitalone1
Copy link
Contributor Author

Waiting for a real package to try, especially on Flathub, I'd like to point out that it's better to test it before making a new release.

@vchernin
Copy link
Contributor

vchernin commented Apr 22, 2024

I've pushed backwards compatible packages changes to support the flatpak presets on flathub flathub/com.github.wwmm.easyeffects@6e302cd.

@Digitalone1 If you'd like, I can send a pr to flathub to create a package for your preset repo to be the first one. I don't want it to be added to flathub until #3088 is merged, but it would be good to do it sooner rather than later.

@Digitalone1
Copy link
Contributor Author

I've pushed backwards compatible packages changes to support the flatpak presets on flathub flathub/com.github.wwmm.easyeffects@6e302cd.

@Digitalone1 If you'd like, I can send a pr to flathub to create a package for your preset repo to be the first one. I don't want it to be added to flathub until #3088 is merged, but it would be good to do it sooner rather than later.

It's a good idea, but I have only one preset which is not enough for the test. We need something with multiple presets and additional files.

Looking the presets listed in the Wiki, I suggest to mix my preset with JackHack's, since one of them rely on a irs file which gives us the opportunity to test the backward compatibility to the previous presets.

So the new Flatpak extension could have the following output presets:

  • JackHack96 Advanced Auto Gain.json
  • JackHack96 Bass Boosted.json
  • JackHack96 Bass Enhancing + Perfect EQ.json
  • JackHack96 Boosted.json
  • JackHack96 Loudness+Autogain.json
  • JackHack96 Perfect EQ.json
  • Digitalone1 LoudnessEqualizer.json

Plus the additional Convolver irs (only the following since all the others in JackHack repo are not used):

I don't know how to name this package. @vchernin pick a name you think it's appropriate.

@vchernin I'd like to test the latest changes on Flatpak. In the last weeks I started to migrate some apps on Flatpak, so I have now mixed packages from upstream Arch and Flatpak. But if I install EasyEffects, it's the previous release. How can I test the master branch? I'd like to keep EasyEffects Arch version, but at least I want to try the community presets on Flatpak version before the new release (then after a first try, I will remove it).

@wwmm
Copy link
Owner

wwmm commented Apr 22, 2024

How can I test the master branch?

I think you can download one of the artifacts generated by our github actions and install it manually through the command line. An user did this in the past if I am not mistaken.

@vchernin
Copy link
Contributor

vchernin commented Apr 22, 2024

Looking the presets listed in the Wiki, I suggest to mix my preset with JackHack's, since one of them rely on a irs file which gives us the opportunity to test the backward compatibility to the previous presets.

I considered making a preset package mixing both preset repos together, but I decided it was strange and just made 2 preset packages, one for your LoudnessEqualizer and one for all JackHack96's presets.

To test it locally indeed you can first install the devel build from github actions. https://github.com/wwmm/easyeffects/actions/runs/8786350683/artifacts/1436075826. Then you can build the preset packages using flatpak-builder with the flathub repo branches I have:

git clone --branch=io.github.wwmm.easyeffects.Presets.JackHack96 https://github.com/vchernin/flathub flathub-JackHack96
flatpak-builder build-dir-JackHack96 --user --install flathub-JackHack96/io.github.wwmm.easyeffects.Presets.JackHack96.json --force-clean --install-deps-from flathub

git clone --branch=io.github.wwmm.easyeffects.Presets.LoudnessEqualizer https://github.com/vchernin/flathub flathub-LoudnessEqualizer
flatpak-builder build-dir-LoudnessEqualizer --user --install flathub-LoudnessEqualizer/io.github.wwmm.easyeffects.Presets.LoudnessEqualizer.json --force-clean --install-deps-from flathub

After installing the packages you may need to restart Easy Effects for the presets to show up.

@vchernin
Copy link
Contributor

@Digitalone1 how are IRS files supposed to work within the UI? Output and input presets are clearly working on my machine, I can see them in the top left menu. But I cannot figure out where the IRS should show up in the convolver. The IRS file is definitely present at the correct location.

If it is supposed to show up in this menu it is missing.
image

@vchernin
Copy link
Contributor

how are IRS files supposed to work within the UI? Output and input presets are clearly working on my machine, I can see them in the top left menu. But I cannot figure out where the IRS should show up in the convolver. The IRS file is definitely present at the correct location.

If it is supposed to show up in this menu it is missing.

Oh, now I realize the IRS files are installed with the preset, nevermind.

@vchernin
Copy link
Contributor

vchernin commented Apr 22, 2024

Hm, following the instructions above when trying to import JackHack96 Bass Boosted.json I see:

(easyeffects:2): easyeffects-WARNING **: 15:42:50.414: 	presets_manager.cpp:850	[json.exception.out_of_range.403] key 'kernel-name' not found

(easyeffects:2): easyeffects-WARNING **: 15:42:50.414: 	presets_manager.cpp:922	can't import addons for the community preset: /app/extensions/Presets/output/JackHack96/Bass Boosted.json; import stage aborted, please reload the community preset list

(easyeffects:2): easyeffects-WARNING **: 15:42:50.414: 	presets_manager.cpp:925	if the issue goes on, contact the maintainer of the community package

Which sounds like because this is an older preset and only using kernel-path? https://github.com/JackHack96/EasyEffects-Presets/blob/8a4499d34e6c461ecda548f077d49ddf8ba73e12/Bass%20Boosted.json#L17

@Digitalone1
Copy link
Contributor Author

@vchernin It seems /app/extensions/Presets path should be added also in other parts of the code. Try to look inside the Convolver and the RNNoise classes.

@Digitalone1
Copy link
Contributor Author

@vchernin I managed to install the artifact, but I can't for the extension:

error: org.freedesktop.Sdk/x86_64/23.08 not installed
Failed to init: Unable to find sdk org.freedesktop.Sdk//23.08 version stable

It does not seem really straightforward. Please, test it yourself.

@vchernin
Copy link
Contributor

It seems as of 35b69a7 the "try community preset" button works (which I did not test before), but the import button still does not work with the same error.

I managed to install the artifact, but I can't for the extension:

This error was because a dependency was missing, I modified the flatpak-builder command above so now it should auto-install dependencies.

@Digitalone1
Copy link
Contributor Author

@vchernin Can't test it now, but it's weird the try function is working while the import not. Are you sure the irs file is loaded with try?

@Digitalone1
Copy link
Contributor Author

Digitalone1 commented Apr 23, 2024

@vchernin Maybe I understood which is the issue. The backward compatibility for kernel-path has been only added in the local preset loading, not in the community importing stage and, as I think about it, it's better we don't add it there since packagers should follow the latest specs.

Please, test with the updated preset. When you import the preset with the Convolver, just save it and you should have it in the local folder with a new format having the updated kernel-name which reports only the file name, not the full path.

Install this preset (maybe updating the repository), and it should work.

@vchernin
Copy link
Contributor

vchernin commented Apr 24, 2024

@vchernin Maybe I understood which is the issue. The backward compatibility for kernel-path has been only added in the local preset loading, not in the community importing stage and, as I think about it, it's better we don't add it there since packagers should follow the latest specs.

That seems reasonable enough. Although would we want to have both kernel-name and kernel-path in preset repos to have backwards compatibility for older easyeffects versions?

In the meantime for testing I pushed bass boosted with kernel-name only to my flathub repo, from the instructions above.

Please, test with the updated preset. When you import the preset with the Convolver, just save it and you should have it in the local folder with a new format having the updated kernel-name which reports only the file name, not the full path.

Indeed trying a preset and then saving it to a file works and produces kernel-name correctly.

I also found the irs directory was not always present which is fixed by #3103.

With both these fixes I can import, and then load a preset with an irs file in flatpak without any issues.

@Digitalone1
Copy link
Contributor Author

That seems reasonable enough. Although would we want to have both kernel-name and kernel-path in preset repos to have backwards compatibility for older easyeffects versions?

It's not fully possible. We can't report the full path in the preset. It's also an issue about privacy. The packager can still have additional presets for older versions, that are not installed in the Flatpak extension.

@Digitalone1
Copy link
Contributor Author

@vchernin I wonder what happens if I install a Flatpak extension and use an EasyEffects regular build made by a distro. I shouldn't see those community presets, right? Presets in Flatpak extensions would be visible only in the Flatpak version. Where exactly are they stored?

@vchernin
Copy link
Contributor

vchernin commented Apr 24, 2024

I wonder what happens if I install a Flatpak extension and use an EasyEffects regular build made by a distro. I shouldn't see those community presets, right? Presets in Flatpak extensions would be visible only in the Flatpak version. Where exactly are they stored?

Indeed you will not see those community presets. Flatpak stores all Flatpak package files in its own directory on the host system, unless the regular EasyEffects is aware of where to look Flatpak packages will not have any effect. Flatpak will not put anything very interesting on the host XDG_DATA_DIRS.

For Flatpaks installed for a singular user they will be installed to ~/.local/share/flatpak. For a system wide install it is /var/lib/flatpak.

So e.g. for a user installation ~/.local/share/flatpak/runtime/io.github.wwmm.easyeffects.Presets.JackHack96/x86_64/stable/active/files shows the files packaged in the extension.

@vchernin
Copy link
Contributor

vchernin commented May 9, 2024

I have not seen any new issues as of #3111. @Digitalone1 if you think this is the right time I can submit your preset package to flathub.

There doesn't need to be a Easy Effects release yet, it would be good to make sure flathub doesn't have any issues with e.g. the naming convention before anything is released.

@Digitalone1
Copy link
Contributor Author

Yes @vchernin, it would be very appreciated.

@Digitalone1
Copy link
Contributor Author

Digitalone1 commented May 10, 2024

So it seems Flathub won't likely accept presets packages. Luckily distro packages exist and they could be made in the future.

If this is the case, I will remove any mention to Flatpak/Flathub since no presets can be found there. Maybe an alternative solution could be creating a custom Flatpak remote, but I don't have the will, nor the knowledge to do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants