easyeffects icon indicating copy to clipboard operation
easyeffects copied to clipboard

Feature: Sort equalizer bands by frequency, and/or manually reorder?

Open nyanpasu64 opened this issue 3 years ago • 6 comments
trafficstars

EasyEffects Version

6.2.8

What package are you using?

Flatpak (Flathub)

Distribution

Void Linux

Describe the bug

When adjusting a complex equalization preset, I often like to create fine-tuned bands at specific frequencies and widths, rather than purely adjusting the amplitude of preset bands. As a result, over time I often end up with bands out of frequency order, making it hard to see at a glance whether low or high frequencies are brought up or down.

Expected Behavior

Ideally there would be a button to sort bands in frequency order (after adding bands), and perhaps even a way to move undesired bands to the end of the list (either setting the frequency to 20 KHz then sorting, or drag-and-drop bands to reorder them) so you can decrease the band count.

Debug Log

Debug Log
Paste your log here

Additional Information

No response

nyanpasu64 avatar Aug 06 '22 22:08 nyanpasu64

This will require some serious refactoring in the equalizer window code. For performance reasons I already intended to actually use a listview container instead of a GtkBox. But I have no idea about when I will be able to focus on this. With a listview handling the bands sorting may become possible. Doing this with a GtkBox holding the band widgets will be a pain.

wwmm avatar Aug 06 '22 22:08 wwmm

Is it possible to sort the underlying data structure holding bands (as if you exported to JSON and sorted the bands there), then rebuild the UI from the sorted bands (as if you loaded the preset from disk)?

I looked into sorting the JSON presets by hand, but it seemed like a nontrivial amount of work I'd have to write a JSON-parsing program for, rather than quickly sorting by hand in a text editor (the JSON contains band0, band1, then band10 through band19, then band2 and so on, plus you duplicate bands for left and right channels). I may get around to it at some point, I hope, but I'm not super well versed on JSON libraries.

nyanpasu64 avatar Aug 06 '22 22:08 nyanpasu64

We have an APO format import feature. Could you do it in APO format? Its syntax is not too hard to write.

Maybe from the documentation is seems hard:

https://sourceforge.net/p/equalizerapo/wiki/Configuration%20reference/

But if you download an APO preset and look inside it, the format is pretty straightforward.

https://sourceforge.net/projects/peace-equalizer-apo-extension/files/Configurations%20%28presets%29/

Digitalone1 avatar Aug 07 '22 08:08 Digitalone1

plus you duplicate bands for left and right channels).

It isn't really duplicated. We use an equalizer that is capable of applying different settings for each channel. But unless you enable the split channels mode you only have to worry about what it is in the left channel. When split channels is disabled EE will map the values from the left channel to the right channel internally.

I think that the workaround suggested by @Digitalone1 will be simpler to do but in case you want to edit the json files directly you will have to rename the bands section accordingly. For example after iterating through the bands and finding which one has the smallest frequency rename the band section name to band0. Once you find the next smallest frequency rename its band section to band1 and so on. As EE code is right now it creates the widgets in the band index order.

wwmm avatar Aug 07 '22 14:08 wwmm

As a new user, I did find the current UX for this a bit frustrating. I had been tweaking the bands, and later decided I would like to remove a band, but I couldn't find a way to delete a specific one, only decrement from the right..

So a quick fix, I'd just move the band I didn't want to the end maybe (not that this would be intuitive since I would have thought the bands were always sorted by frequency), but that wasn't possible via UI either.

polarathene avatar Aug 27 '22 23:08 polarathene

but I couldn't find a way to delete a specific one

That is something that should be possible to do. But we definitely have to be careful. Under the hoods we use the equalizer from Linux Studio Plugins. This equalizer has a fixed number of bands and each of then has its own string id for each parameter. When you "create a band" what is actually happening is that we bind the widgets to a given key in our gsettings database. And the same is done from the plugin to the database. When deleting bands in the middle we have to somehow keep track of what is going on so we do not bind multiple widgets to the same band. Doable... But a little tricky to handle this.

wwmm avatar Aug 27 '22 23:08 wwmm

Also autoeq imported files are not sorted by freq so It is a little bit counterintuitive to adapt a bit high or low frequencies searching for the right column.

bhack avatar Oct 04 '22 13:10 bhack

Also autoeq imported files are not sorted by freq so It is a little bit counterintuitive to adapt a bit high or low frequencies searching for the right column.

Autoeq? As far as I know we have only import for APO and GraphicEq.

Anyway, can't you just sort the config file (APO or GraphicEq)? LSP equalizer is bound to gsettings key, so it's difficult from code side to sort the bands.

Digitalone1 avatar Oct 04 '22 13:10 Digitalone1

Autoeq? As far as I know we have only import for APO and GraphicEq.

Yes If you see they are not sorted: https://github.com/jaakkopasanen/AutoEq/tree/master/results

I think that we can sort it at https://github.com/wwmm/easyeffects/blob/782ffe0078e357591d2d713e218e4516a213aa2b/src/equalizer_ui.cpp#L303

bhack avatar Oct 04 '22 13:10 bhack

It's hard to sort them there because we scan the file line by line.

It's easier if you edit the file manually.

Digitalone1 avatar Oct 04 '22 13:10 Digitalone1

It's hard to sort them there because we scan the file line by line.

I don't understand what is the problem... You need only to add the operator to your APO_band struct:

#include <iostream>
#include <numbers>
#include <vector>

using namespace std;

struct APO_Band {
  std::string type;
  float freq = 1000.0F;
  float gain = 0.0F;
  float quality = (1.0F / std::numbers::sqrt2_v<float>);
    bool operator<(const APO_Band& a) const
    {
        return freq < a.freq;
    }
};


int main() {
    std::vector<struct APO_Band> bands;
    sort( bands.begin(), bands.end());

}

bhack avatar Oct 04 '22 14:10 bhack

Nice. Since we try to adhere to C++20, maybe it's better to overload <=>

Then add the sort after the empty check.

  eq_file.close();

  if (bands.empty()) {
    return false;
  }

  std::sort(bands.begin(), bands.end());

Digitalone1 avatar Oct 04 '22 14:10 Digitalone1

I suppose I'd use a stable sort to keep different filter types and Q values with the same corner/center frequency in their original order.

nyanpasu64 avatar Oct 04 '22 14:10 nyanpasu64

An example (if I intended it right):

auto operator<=>(const APO_Band& a) -> int {
  if (freq < a.freq) {
    return -1
  }

  if (freq > a.freq) {
    return 1;
  }

  return 0;
}

Digitalone1 avatar Oct 04 '22 14:10 Digitalone1

In c++20 you can also just use this without changing the struct:

std::ranges::sort(bands, {}, &APO_Band::freq);

bhack avatar Oct 04 '22 15:10 bhack

Okay, can you test it and make the pull request?

Just be careful to place the sort after the empty check.

Digitalone1 avatar Oct 04 '22 16:10 Digitalone1

Okay, can you test it and make the pull request?

I would like but you don't have a clear CONTRIBUTING.md to setup the dev environment. It will be probably require more time to figure out how to setup the dev-env that this single line.

bhack avatar Oct 04 '22 16:10 bhack

I would like but you don't have a clear CONTRIBUTING.md to setup the dev environment. It will be probably require more time to figure out how to setup the dev-env that this single line.

It could be more clearly indicated, but this wiki page has all the instructions to build EasyEffects. I recommend gnome builder from flathub for the most straightforward experience.

vchernin avatar Oct 04 '22 16:10 vchernin

I would like but you don't have a clear CONTRIBUTING.md to setup the dev environment.

Usually what I do once inside the source code folder is

  1. glib-compile-schemas data/schemas/
  2. meson _build
  3. cd _build
  4. ninja
  5. cd src
  6. source ../../util/environmental_variables.sh
  7. If everything went well ./easyeffects should show EasyEffects.

wwmm avatar Oct 04 '22 16:10 wwmm

Does glib-compile-schemas install all the dependencies?

bhack avatar Oct 04 '22 17:10 bhack

Does glib-compile-schemas install all the dependencies?

No. It doesn't install any dependency. We use GSettings for our database and it needs its xml schemas to be compiled to a binary file. Usually this is done in a system folder by the package management tool. But as I run in a custom folder while developing I have to do this myself. As well as set some environmental variables. What is done by environmental_variables.sh

wwmm avatar Oct 04 '22 17:10 wwmm

Isn't better to just add a Dockerfile and eventually a .devcontainer in the repository? It will make the contribution supereasy..

bhack avatar Oct 04 '22 17:10 bhack

For compiling dependencies, take a look at the PKGBUILD.

'meson' 'itstool' 'appstream-glib' 'git'

Digitalone1 avatar Oct 04 '22 17:10 Digitalone1

Isn't better to just add a Dockerfile and eventually a .devcontainer in the repository? It will make the contribution supereasy..

Assuming someone is willing to do this and making sure it works. As I do not use Docker I do not have any idea about how to do it. And I also do not have the time this is probably going to require to be put in place and maintained.

wwmm avatar Oct 04 '22 17:10 wwmm

I don’t really see a point of a docker container setup when in terms of practical usability Flatpak is probably easier (and already maintained here), since Flatpak is ultimately just a container/sandbox under the hood like docker except focused on gui apps. The whole point is to be able to build with as little manual hassle with dependencies and build system commands as possible, which is very easy in gnome builder, so I think that is best for new contributors.

But if someone wants a dev container file if they prefer docker/podman I don’t see why not.

vchernin avatar Oct 04 '22 17:10 vchernin

I don’t really see a point of a docker container setup when in terms of practical usability Flatpak is probably easier (and already maintained here), since Flatpak is ultimately just a container/sandbox under the hood like docker except focused on gui apps. The whole point is to be able to build with as little manual hassle with dependencies and build system commands as possible like with gnome builder, so I think that is best for new contributors.

The main difference is that you need to compile all the dependencies instead to have they OS pre-installed "-dev" (like in Dockerfile). So for occasional PRs it is a quite heavy overhead.

What we need to do with the flathub Gtkbuild after compilation to run it?

bhack avatar Oct 04 '22 17:10 bhack

So for occasional PRs it is a quite heavy overhead.

Yeah that is true, although it is at least alleviated with a local build cache. Buildstream (used for flatpak runtimes) has a concept of a cache that can be shared from "upstream" so you don’t need to build everything. But that doesn’t exist for flatpak builder unfortunately.

What we need to do with the flathub Gtkbuild after compilation to run it?

Flathub gtk build? If you mean to build EasyEffects you must first ensure the plugins are installed, then build, then run.

flatpak install flathub org.freedesktop.LinuxAudio.Plugins.LSP//22.08 org.freedesktop.LinuxAudio.Plugins.ZamPlugins//22.08 org.freedesktop.LinuxAudio.Plugins.MDA//22.08 -y
flatpak-builder build-dir --user --install util/flatpak/com.github.wwmm.easyeffects.Devel.json --force-clean --ccache
flatpak run com.github.wwmm.easyeffects.Devel

vchernin avatar Oct 04 '22 17:10 vchernin

It seems something is missing: lv2_wrapper.cpp:63 Could not find the plugin: http://lsp-plug.in/plugins/lv2/para_equalizer_x32_lr

bhack avatar Oct 04 '22 17:10 bhack

It seems something is missing: lv2_wrapper.cpp:63 Could not find the plugin: http://lsp-plug.in/plugins/lv2/para_equalizer_x32_lr

The equalizer needs the Linux Studio Plugins package. At least on Arch Linux it is named lsp-plugins.

wwmm avatar Oct 04 '22 17:10 wwmm

I'am running the master prepared flatpack:

flatpak run com.github.wwmm.easyeffects.Devel

bhack avatar Oct 04 '22 18:10 bhack