-
-
Notifications
You must be signed in to change notification settings - Fork 269
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
Comments
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.
That is why I think that community or system presets should be tied to the
I am not sure about what would happen. We remove duplicated names from the list. So it will depend on the order |
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 |
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
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.
I think that the difficulty here is that for presets subfolders make sense. And the |
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 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 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. |
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.
At least on GNOME it shows the
Yes. But I suggest
This should be possible. If I remember well the file dialog has a multiple files selection mode. |
I could work on this in the next weeks if I manage to find time. So what we have to do is:
|
I will take a look at the documentation. But thinking about the path again we probably should use |
Inside the build time generated header |
We just have to see how to use gtk_file_dialog_open_multiple instead of the |
It seems that we just have to set https://docs.gtk.org/gtk4/property.FileDialog.initial-folder.html. But I did not test it. |
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 The best reference I have is currently the flatpak build finds plugins at
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. |
You are right. Now that I think about it inside
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.
Shouldn't ab8c4fe already be putting things in a place visible to Flatpak? |
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?
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. |
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.
Oh... Ok. |
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)
From my earlier comment:
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. |
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 |
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. |
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
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 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:
This way there will be no conflict because every package should have its own subfolder. A package can install additional subfolders, i.e.:
What do you think about this scheme? Anyway, Easyeffects package will install the |
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.
Sure. Just pick some from here. I'd say @p-chan5, @JackHack96 and mine which seems more generic. |
It seems good. |
Yes
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.
Something worth noting is mixing package installed presets and user installed presets in a 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 The rest of the proposal I quite like and I think it should work well. |
@vchernin I agree not adding presets at build time. You said
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.
Ok, so we would only recommend to install directories in
I didn't know that. So we can't use
Sure. I was only suggesting that users could move their own presets to |
I am not sure what you mean by this?
I think to some degree using 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: 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 |
EasyEffects will still use |
I am fine either way, I don't think having So just to decide so we don't need to revisit this, should it be When I have time I will prototype the flatpak setup and make sure there isn't any unexpected weirdness. |
Yes. Inside this folder package maintainers will create subfolders to organize the presets as commented before by @Digitalone1. |
That we won't recommend users to install files in |
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. |
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. |
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:
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). |
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. |
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 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. |
@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. |
Oh, now I realize the IRS files are installed with the preset, nevermind. |
Hm, following the instructions above when trying to import JackHack96
Which sounds like because this is an older preset and only using |
@vchernin It seems |
@vchernin I managed to install the artifact, but I can't for the extension:
It does not seem really straightforward. Please, test it yourself. |
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.
This error was because a dependency was missing, I modified the |
@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? |
@vchernin Maybe I understood which is the issue. The backward compatibility for 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 Install this preset (maybe updating the repository), and it should work. |
That seems reasonable enough. Although would we want to have both In the meantime for testing I pushed bass boosted with
Indeed trying a preset and then saving it to a file works and produces 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. |
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. |
@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? |
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 For Flatpaks installed for a singular user they will be installed to So e.g. for a user installation |
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. |
Yes @vchernin, it would be very appreciated. |
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. |
@wwmm It's almost two months community presets have been implemented. Is there something else stopping a new release? Many things changed in the last months, it would be a good opportunity to push them to regular users. |
No special reason. But I think it would be good to try to fix #3188 before doing it. I will test today's commit for a few days and make a new release by the end of this week. |
I think this can be finally closed. @wwmm you can unpin it. |
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 byg_get_system_config_dirs
(appending input/output directories): https://github.com/wwmm/easyeffects/blob/master/src/presets_manager.cpp#L34-L38And also into
etc/easyeffects/input
andetc/easyeffects/output
: https://github.com/wwmm/easyeffects/blob/master/src/presets_manager.cpp#L41-L42The 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.
The text was updated successfully, but these errors were encountered: