Bar width can be set manually

[mwgg]

Added "Bar width" option to the preferences window. When set to 0, width is adjusted according to the number of bars specified. If bar width is set to something other than 0, number of bars is adjusted dynamically to fit into the drawing area.

For some reason, when gaps between bars are disabled and bar width is > 1, the resulting bars are often narrower than the drawing area. When gaps are enabled, the picture is fine at any bar width setting. When bar width is 1 and gaps are disabled, everything is fine as well.

[cboxdoerfer]

Thx for the patch :)

Haven't tested the code yet but I found a few issues:

The source of the problem when the bar width is > 1 is probably here: https://github.com/cboxdoerfer/ddb_musical_spectrum/blob/master/spectrum.c#L359

When gaps are disabled bar width is always reduced by one so the spectrum fills only barw-1/barw of the window (barw = 2: first half, barw = 3: first 2/3, ...).

in line 359: if (CONFIG_GAPS || CONFIG_BAR_W > 1) instead of if (CONFIG_GAPS)

should fix that.

Next issue: https://github.com/cboxdoerfer/ddb_musical_spectrum/pull/6/files#diff-a604b5ecbfc03323766770deca4c2746R266

we don't need to calculate the frequency table on every draw call, only if the size of the window changed. This saves quite a few cpu cycles. Fix should be straight forward by remembering previous dimensions.

Next issue: Is there a reason why you increased the default bar number to 131 or was this just by accident?

[mwgg]

The source of the problem when the bar width is > 1 is probably here

You are right, must have missed that. Fixed now.

we don't need to calculate the frequency table on every draw call

Now calculated if previous observed width is different.

default bar number to 131

I have actually just changed it to 132. It's used as a base number for caclulations of frequency table and note for the tooltip, and allows to expand the frequency range up to B10. Any smaller values cut off top of the frequency range. It's now adjusted to include the full range of *notes[].

Sorry for the silly mistakes.

[cboxdoerfer]

I have actually just changed it to 132. It's used as a base number for caclulations of frequency table and note for the tooltip, and allows to expand the frequency range up to B10. Any smaller values cut off top of the frequency range. It's now adjusted to include the full range of *notes[].

Ok, that's fine. So far I only cared about the frequencies up to ~22 kHz, since I only listen to music with 44.1 kHz samplerate, but I guess using the full note range might be useful for some users which have music with higher samplerates. In the future I'd like to make the frequency/note range configurable anyway.

Sorry for the silly mistakes.

No problem. ;)

I'll have a closer look at the code later that day and then do some testing, if everything works we can merge.

[cboxdoerfer]

So tried the code, found two bugs: If the width of the window is small enough the bar number becomes 0 which leads to arithmetic error because of division by 0 in https://github.com/cboxdoerfer/ddb_musical_spectrum/blob/master/spectrum.c#L360

To fix that we should ensure that the bar number never becomes 0.

(Edit: this should fix that:

CONFIG_NUM_BARS = MAX (1, a.width/added_bar_w); instead of https://github.com/cboxdoerfer/ddb_musical_spectrum/pull/6/files#diff-2d6a4a738ceb8495048f4b03daa2f4cfR137 )

Second bug: you override the CONFIG_BAR_NUM when a fixed bar width is enabled, but the config should only be changed by the user. I suggest to add another variable to store the calculated bar number and do a check in the draw call, something like that:

if (CONFIG_BAR_W > 0) bar_num = calculated_bar_num; else bar_num = CONFIG_BAR_NUM;

So we don't need to override CONFIG_BAR_NUM

[cboxdoerfer]

Fixed those bugs on my local branch and some other minor issues. Everything else seems to work fine, so I guess we can merge. :)

And thx again for the patch. ;)