wwmm / easyeffects

Limiter, compressor, convolver, equalizer and auto volume and many other plugins for PipeWire applications
GNU General Public License v3.0
6.12k stars 264 forks source link

app crashes when we uploads APO or reset the eq #3193

Closed Ayush583-Aswal closed 1 week ago

Ayush583-Aswal commented 1 week ago

EasyEffects Version

7.1.6

What package are you using?

Flatpak (Flathub)

Distribution

Ubuntu 24.04 LTS

Describe the bug

when i upload APO to an Eq the app crashes and force quit/wait option comes. This is also the case for , when we are resetting the eq filters. this error was coming in terminal: (easyeffects:2): GLib-GIO-WARNING **: 21:46:40.879: g_settings_set_value: value for key 'band9-frequency' in schema 'com.github.wwmm.easyeffects.equalizer.channel' is outside of valid range

where band9 filter was something like this : 'Filter 9: Off PK Fc 0 Hz Gain 0 dB Q 0'

Expected Behavior

the app should be able to switch the APO filters and reset the filters without getting force quit alert. like we can change the eq settings easily in wavelet(on android) without getting any error and it should function smoothly

Debug Log

Debug Log
``` Paste your log here ```

Additional Information

No response

wwmm commented 1 week ago

Attach here on github the preset you tried to load. The last time I tested the APO import it was fine. But i is not a feature I have a reason to use. So maybe there was a regression somewhere.

Digitalone1 commented 1 week ago

where band9 filter was something like this : 'Filter 9: Off PK Fc 0 Hz Gain 0 dB Q 0'

Indeed 0 Hz frequency is not valid (10 is minimum). But I'd like the app won't crash on out of bounds values. I didn't know gsettinsg does not have an internal check and it crashes directly...

wwmm commented 1 week ago

where band9 filter was something like this : 'Filter 9: Off PK Fc 0 Hz Gain 0 dB Q 0'

Indeed 0 Hz frequency is not valid (10 is minimum). But I'd like the app won't crash on out of bounds values. I didn't know gsettinsg does not have an internal check and it crashes directly...

Hum... I wonder if the crash was really inside gsettings. In this bug report there is only a warning coming from it.

Digitalone1 commented 1 week ago

I think it's in gsettinsg, but it does not provide an easy way to check the key range.

We have an unused util for that, but https://docs.gtk.org/gio/method.SettingsSchemaKey.get_range.html says it's better to not use it in normal programs.

Then there's https://docs.gtk.org/gio/method.SettingsSchemaKey.range_check.html but dealing with GVariant is very annoying.

Maybe the easiest way is to get the ranges from Gtk widgets. We already do it in UI helpers along with std::clamp().

Digitalone1 commented 1 week ago

Here we are: https://github.com/wwmm/easyeffects/blob/master/src/ui_helpers.cpp#L159

This is just straightforward. Since the code is in eq UI, it should be easy to access those spinbuttons. And there's no need to make tricky db/linear conversions since we are dealing with frequencies.

Ayush583-Aswal commented 1 week ago

Attach here on github the preset you tried to load. The last time I tested the APO import it was fine. But i is not a feature I have a reason to use. So maybe there was a regression somewhere.

Truthear-dita.txt Screenshot from 2024-06-19 23-49-50

Ayush583-Aswal commented 1 week ago

attached the preset text file too

Digitalone1 commented 1 week ago

@Ayush583-Aswal Filters from 7 to 10 are just useless/placeholders. You can delete them from the preset file.

wwmm commented 1 week ago

Attach here on github the preset you tried to load. The last time I tested the APO import it was fine. But i is not a feature I have a reason to use. So maybe there was a regression somewhere.

Truthear-dita.txt Screenshot from 2024-06-19 23-49-50

I tested this APO preset now and although the outside of valid range warnings were shown there was no crash or notification window about the process not responding. The equalizer worked. I wonder why in @Ayush583-Aswal system the window is not responding...

Digitalone1 commented 1 week ago

I tested this APO preset now and although the outside of valid range warnings were shown there was no crash or notification window about the process not responding. The equalizer worked. I wonder why in @Ayush583-Aswal system the window is not responding...

Same to me, but we are on master branch, many things have changed from the latest release.

Anyway I don't like those warnings and it would be better to handle those range checks from our side. I was looking into it and we can't access spinbuttons/scales from equalizer_ui because they are in the band_box_ui, so I'm afraid we should add GSettingsSchemaKey to make those checks before applying the key values.

Besides, I see there's the new band_width control but we don't set it when the APO is imported/exported. Maybe we should do it.

And importing some test APO presets I have on my system, the BP type is not applied, but from APO config documentation I see it is the Band Pass filter which is present in the LSP equalizer. I suppose we made the import algorithm when Band Pass was not already implemented.

Anyway, the APO import/export feature needs some updates.

Digitalone1 commented 1 week ago

For the "freeze" during the reset of the Equalizer, this is a know issue on Flatpak. Gsettings engine on Flatpak is not the best and resetting all the Equalizer controls takes a lot of time.

wwmm commented 1 week ago

For the "freeze" during the reset of the Equalizer, this is a know issue on Flatpak. Gsettings engine on Flatpak is not the best and resetting all the Equalizer controls takes a lot of time.

Oh... For some reason I forgot to consider this. It is totally possible.

Digitalone1 commented 1 week ago

@wwmm Since the range checks have been implemented, this could be closed.