Re: [Audacity-devel] Change Pitch crash

[Vaughan J. <va...@au...>]

On 6/25/2013 9:29 AM, Steve the Fiddle wrote:
> On 25 June 2013 00:05, Steve the Fiddle <ste...@gm...> wrote:
>> The new layout looks good and the control updating seems to be working correctly
>
> I spoke too soon.
> There is still a miscount when updating negative octave numbers less than -1.
>
> The easiest way to reproduce:
> Generate a default tone (A 440).
> scroll through the "from octave" control so that it counts down.
> The "to octave" control follows the count until -1.
> When "from octave" changes to -2, "to octave" remains at -1 but the
> "to frequency" changes to what should be octave -2.
>
> The problem also occurs when updating the "from octave" by changing
> the "from frequency", so this needs fixing even if we decide to update
> the semitones change rather than the "to pitch".
>
> Not sure if this is the proper way to do it, but it works here:
>
> int PitchOctave(const double dMIDInote)
> {
>     double dRound = (dMIDInote < 0.0) ? -0.5 : 0.5;
>     int iMIDInote = ((int)(dMIDInote + dRound));
>     return (floor(((float)(iMIDInote - 12)) / 12.0));
> }

Can't it be done more simply than converting float to int, then casting 
the int to float?

I don't really understand the reasoning behind this, so please talk it 
through. I think the key is using floor() but don't see what all the 
other change is for.

And btw, since floor returns a float, I think this generates a warning 
in some compiers, that the float is being truncated, or that the return 
type is a mismatch with the fn declaration.


>
> There is also some inaccuracy when calculating the note names, for
> example, if 3.99 Hz is entered as the "from frequency" the "from
> pitch" should be a "C" not "C#". This looks like another rounding
> error because newFromMIDInote looks to be accurate. I don't think that
> the rounding is correct in "unsigned int PitchIndex(const double
> dMIDInote)" when dMIDInote is negative. Should it be something like:
>
> unsigned int PitchIndex(const double dMIDInote)
> {
>     // MIDI numbers can be negative.
>     int nPitchIndex = (dMIDInote > 0.0)?
>                          ((int)(dMIDInote + 0.5) % 12) :
>                          ((int)(dMIDInote - 0.5) % 12);

Ah, yes, as I mentioned, this code was originally written for non-neg 
MIDI numbers, and needs this further adjustment. But I used the same 
code as for dRound, because I thin it's more readable -- and they should 
be consistent because it's the same test as in PitchOctave. Thanks for 
the fix. Committed.

Thanks,
V