Small pitch bends correct and tested.

Small pitch bends correct and tested.

Pitch bends are not rounded to cents.

Pitch bends of full precision centered on zero.

[Janek Warchol, 2011-06-25 20:13:04]

Graham Breed wrote:

I've found and patched two problems with the pitch bend
tuning code:

1) The pitch isn't always rounded to the nearest equally
tempered value, so the result looks strange in a sequencer,
and artifacts caused by the pitch bends are more severe.

2) The tuning is rounded to cents before the pitch bends
are calculated.  It may not be a big deal, but there's no
need for it, and the code's simpler calculating the bends
directly.

I don't think either of these are registered issues.  As
I'm new with Git, the two issues are mixed in the two
patches.  Problem (1) is fixed in get_semitone_pitch().
It's just a question of rounding off the result.
Everything else is for problem (2).

[cpkc_shaw.ca, 2011-06-25 22:50:17]

On 11-06-25 02:13 PM, lemniskata.bernoullego@gmail.com wrote:
> Graham Breed wrote:
>
> I've found and patched two problems with the pitch bend
> tuning code:
>
> 1) The pitch isn't always rounded to the nearest equally
> tempered value, so the result looks strange in a sequencer,
> and artifacts caused by the pitch bends are more severe.
>
> 2) The tuning is rounded to cents before the pitch bends
> are calculated.  It may not be a big deal, but there's no
> need for it, and the code's simpler calculating the bends
> directly.
>
> I don't think either of these are registered issues.  As
> I'm new with Git, the two issues are mixed in the two
> patches.  Problem (1) is fixed in get_semitone_pitch().
> It's just a question of rounding off the result.
> Everything else is for problem (2).


Thanks for this, Graham!  I've added this to the tracker as issue 1711.  
Can you also provide a test file which would demonstrate the problem? I 
recognise this may not be all that easy to do, but it would help in 
verifying the fix.

Colin Campbell
Bug Squad

-- 
The human race has one really effective weapon, and that is laughter.
-- Mark Twain

  

[Keith, 2011-06-26 06:52:37]

Looks, and sounds, good to me.

[gbreed, 2011-06-26 09:38:57]

Colin Campbell <cpkc@shaw.ca> wrote:

> Thanks for this, Graham!  I've added this to the tracker
> as issue 1711. Can you also provide a test file which
> would demonstrate the problem? I recognise this may not
> be all that easy to do, but it would help in verifying
> the fix.

Here's an example based on one Richard Sabey sent me.

In 2.14.1, the sixth tone sharp g comes out as a flattened
g sharp.  The pitch bend message is 0xe0 0x71 0x2a and the
note-on 0x90 0x38 0x5a.  Without pitch bends, g the
note-on is 0x90 0x37 0x5a, so g is 0x37 not 0x38. The third
byte of the pitch bend should be between 0x30 and 0x50 if
they're minimized.

A one semitone pitch bend is 0x1000.  The f is flat by a
f is flat by a sixth tone or a third of a semitone, so the
LSB of the bend should be (-0x1000/-3)&0x7f or (0x4000 -
0x2000/6)&0x7f, which is either 0x2a or 0x2b depending on
how you round it.  With 2.1.14, it's 0x39 (0xe0 0x39 0x35)
and with the patch it's 0xe0 0x2b 0x35.


                          Graham

[Neil Puttock, 2011-06-26 13:13:50]

LGTM.

http://codereview.appspot.com/4654063/diff/1/lily/midi-item.cc
File lily/midi-item.cc (right):

http://codereview.appspot.com/4654063/diff/1/lily/midi-item.cc#newcode202
lily/midi-item.cc:202: + audio_->transposing_.tone_pitch ()) * Rational (2));
indent:

((audio_
  + audio

http://codereview.appspot.com/4654063/diff/1/lily/midi-item.cc#newcode203
lily/midi-item.cc:203: return (tune > 0)? int (tune + 0.5): int(tune - 0.5);
int (rint (tune));

http://codereview.appspot.com/4654063/diff/1/lily/midi-item.cc#newcode216
lily/midi-item.cc:216: finetune = PITCH_WHEEL_CENTER + get_fine_tuning();
get_fine_tuning ();

[Carl, 2011-07-05 05:19:45]

LGTM, with Neil's comments taken

Carl

[Janek Warchol, 2011-07-10 21:48:34]

Graham,

I'm worried that i don't hear anything from you since 2 weeks! 
Do you experience any problems with fixing code style issues pointed to by Neil?
If you are very busy, i can fix them myself, but i'd prefer that you fix them
and send me the collection of all patches, up-to-date (this will minimize the
risk of messing something up).  If you encounter any problems with git, i'll
gladly help.

cheers,
Janek

On 2011/06/26 13:13:50, Neil Puttock wrote:
> LGTM.
> 
> http://codereview.appspot.com/4654063/diff/1/lily/midi-item.cc
> File lily/midi-item.cc (right):
> 
> http://codereview.appspot.com/4654063/diff/1/lily/midi-item.cc#newcode202
> lily/midi-item.cc:202: + audio_->transposing_.tone_pitch ()) * Rational (2));
> indent:
> 
> ((audio_
>   + audio
> 
> http://codereview.appspot.com/4654063/diff/1/lily/midi-item.cc#newcode203
> lily/midi-item.cc:203: return (tune > 0)? int (tune + 0.5): int(tune - 0.5);
> int (rint (tune));
> 
> http://codereview.appspot.com/4654063/diff/1/lily/midi-item.cc#newcode216
> lily/midi-item.cc:216: finetune = PITCH_WHEEL_CENTER + get_fine_tuning();
> get_fine_tuning ();

[gbreed, 2011-07-11 09:55:19]

lemniskata.bernoullego@gmail.com wrote:
> Graham,
> 
> I'm worried that i don't hear anything from you since 2
> weeks! Do you experience any problems with fixing code
> style issues pointed to by Neil?

Is that me?  I'm supposed to do something?  Here's a patch


           Graham

[Janek Warchol, 2011-07-11 18:02:39]

2011/7/11 Graham Breed <gbreed@gmail.com>:
> Is that me?  I'm supposed to do something?  Here's a patch

Perfect!

Mike: please push this.
Colin: sorry for overlooking that the countdown was over.  We
definately need an integrated patch/issue tracker!

cheers,
Janek

[Neil Puttock, 2011-07-11 19:14:43]

2011/7/11 Janek Warchoł <lemniskata.bernoullego@gmail.com>:
> 2011/7/11 Graham Breed <gbreed@gmail.com>:
>> Is that me?  I'm supposed to do something?  Here's a patch
>
> Perfect!
>
> Mike: please push this.

Please don't push without adding a commit message.

Thanks,
Neil

[Janek Warchol, 2011-07-11 19:52:32]

2011/7/11 Neil Puttock <n.puttock@gmail.com>:
> 2011/7/11 Janek Warchoł <lemniskata.bernoullego@gmail.com>:
>> 2011/7/11 Graham Breed <gbreed@gmail.com>:
>>> Is that me?  I'm supposed to do something?  Here's a patch
>>
>> Perfect!
>>
>> Mike: please push this.
>
> Please don't push without adding a commit message.

Sorry, i didn't notice.
Attached is a nicely described patch.

cheers,
Janek

[Neil Puttock, 2011-07-11 20:37:56]

2011/7/11 Janek Warchoł <lemniskata.bernoullego@gmail.com>:

> Sorry, i didn't notice.
> Attached is a nicely described patch.

Thanks, pushed to master.

Cheers,
Neil

[Janek Warchol, 2011-07-11 20:44:27]

2011/7/11 Neil Puttock <n.puttock@gmail.com>:
> Thanks, pushed to master.

Thanks, issue closed.

cheers,
Janek