search

indiependente / autoEqMac

EqMac preset generator powered by AutoEq.
MIT License
23 stars 1 forks source link

Bug with recomputed AutoEQ results containing extra spaces #24

Closed douglascamata closed 2 years ago

douglascamata commented 2 years ago

Lately some measurements were reprocessed to generate newer results in AutoEq. In this generation, the fixed band EQ presets now have an extra space at the end of every line. Due to this, autoEqMac cannot parse some floats. Example:

❯ autoEqMac
🎧 autoEqMac - EqMac preset generator powered by AutoEq
Please select headphones model:
🎧 >>> HIFIMAN Sundara (2020 revised earpads)
👉 You selected: HIFIMAN Sundara (2020 revised earpads)
2022/09/27 10:58:29 ⛔️ could not find fixed band EQ preset: could not get global EQ preamp data: could not extract global preamp value: strconv.ParseFloat: parsing "-5.5 ": invalid syntax

As seen, there's one extra space after the -5.5 number. It can be see in the raw file of the fixed band EQ here (select all the text content and you will see the extra spaces).

All the numeric strings should be "trimmed" before parsed (with strings.Trim).

I'll try to give a nudge in the AutoEq repository about this, as the extra spaces are also not elegant and often not desirable.

VenturaDelMonte commented 2 years ago

Thanks for reporting this bug @douglascamata, I think it makes sense to fix it also on autoEqMac's side. I ll see if I can do it later today.

douglascamata commented 2 years ago

@VenturaDelMonte thanks! I'll open a PR to fix it on AutoEq's side. I am not sure when it will be merged or even if it'll be accepted at all, as it will trigger a huuuge diff just with space changes. I'll link the PR over there (when I open it) to this issue here for tracking purposes.

indiependente commented 2 years ago

@douglascamata we just release a new version. Please update and try downloading the preset you need again, it should work now!

douglascamata commented 2 years ago

Thanks, guys! You rock 👊

douglascamata commented 2 years ago

Hey @indiependente and @VenturaDelMonte, I can confirm the fix on autoEqMac side works.

But I believe this shouldn't be fixed on the AutoEq repository. I saw that you are parsing each headphone's README to look for preamp. I do not recommend that you do this. The README isn't structured information, it is free text that can change at any time, so the implementation becomes fragile.

My recommendation is that you grab this information from FixedBandEQ text file, like this one: https://github.com/jaakkopasanen/AutoEq/blob/647d0422ac346a5616eaf6ddda0420317121941b/results/oratory1990/harman_over-ear_2018/HIFIMAN%20Sundara%20(Dekoni%20heepskin%20Earpads)/HIFIMAN%20Sundara%20(Dekoni%20heepskin%20Earpads)%20FixedBandEQ.txt. You can be sure that the structure of this file won't be changing anytime soon. It should be stable.

douglascamata commented 2 years ago

When I wrote this issue I thought you were already using the FixedBandEQ file that I linked. I read your source code again and realized you weren't.

douglascamata commented 2 years ago

I will move the conversation to a new issue to avoid talking in this closed one.