Re: Fix MIDI output for Kievan (issue 6193043)

2012-05-07 Thread graham
LGTM http://codereview.appspot.com/6193043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Fix MIDI output for Kievan (issue 6193043)

2012-05-05 Thread benko . pal
hi Aleksandr, I've uploaded a patch that gets at Vaticana and Mensural as well. It seems that Beam_performer would not be necessary for the two, but I don't know about Tie_performer and Slur_performer. It would probably be good for someone who knows more than me about Gregorian chant to

Re: Fix MIDI output for Kievan (issue 6193043)

2012-05-05 Thread aleksandr . andreev
On 2012/05/05 16:58:22, benko.pal wrote: thank you very much! please add Beam_performer too to ensure consistency with Voice (may be needed when doing ancient and modern edition from a common source). Added Beam_performer for Vaticana and Mensural based on this comment.

Re: Fix MIDI output for Kievan (issue 6193043)

2012-05-04 Thread aleksandr . andreev
For whatever reason, \alias Staff and \alias Voice do not bring in the performers. If I comment out the line \consists Note_performer, I get no notes in the MIDI file, only the appropriate number of rests. I think this is also why Vaticana does not appear to work properly with MIDI. Try this

Re: Fix MIDI output for Kievan (issue 6193043)

2012-05-04 Thread dak
http://codereview.appspot.com/6193043/diff/1/ly/performer-init.ly File ly/performer-init.ly (right): http://codereview.appspot.com/6193043/diff/1/ly/performer-init.ly#newcode88 ly/performer-init.ly:88: \consists Dynamic_performer On 2012/05/04 02:59:06, Carl wrote: Again, don't the performers

Re: Fix MIDI output for Kievan (issue 6193043)

2012-05-04 Thread aleksandr . andreev
On 2012/05/04 15:34:17, dak wrote: Aliases are only relevant for properties, I think. The initial values get inherited, and any use of context property setting commands in the music or context mods will accept changes for Voice into this context. But translators are independent. Sorry,

Re: Fix MIDI output for Kievan (issue 6193043)

2012-05-04 Thread dak
On 2012/05/04 16:48:11, aleksandr.andreev wrote: On 2012/05/04 15:34:17, dak wrote: Aliases are only relevant for properties, I think. The initial values get inherited, and any use of context property setting commands in the music or context mods will accept changes for Voice into this

Re: Fix MIDI output for Kievan (issue 6193043)

2012-05-04 Thread Carl . D . Sorensen
On 2012/05/04 16:48:11, aleksandr.andreev wrote: Sorry, I'm a little green. Does this mean I need to change something in this patch? No. David was explaining why the \alias didn't cause the performers to be brought in automatically. My comment was invalid. Your patch is good. Thanks,

Re: Fix MIDI output for Kievan (issue 6193043)

2012-05-04 Thread Benkő Pál
hi Aleksandr, 2012/5/4 aleksandr.andr...@gmail.com: For whatever reason, \alias Staff and \alias Voice do not bring in the performers. If I comment out the line \consists Note_performer, I get no notes in the MIDI file, only the appropriate number of rests. I think this is also why Vaticana

Re: Fix MIDI output for Kievan (issue 6193043)

2012-05-04 Thread aleksandr . andreev
On 2012/05/04 19:33:06, benko.pal wrote: would you fix Vaticana and Mensural too? I've uploaded a patch that gets at Vaticana and Mensural as well. It seems that Beam_performer would not be necessary for the two, but I don't know about Tie_performer and Slur_performer. It would probably be

Fix MIDI output for Kievan (issue 6193043)

2012-05-03 Thread aleksandr . andreev
Reviewers: , Message: Here is a patch that attempts to fix issues with MIDI playback of Kievan notation. Description: Fix MIDI output for Kievan Adding KievanStaff and KievanVoice for MIDI output of Kievan notation. Please review this at http://codereview.appspot.com/6193043/ Affected files:

Re: Fix MIDI output for Kievan (issue 6193043)

2012-05-03 Thread Carl . D . Sorensen
Looks like it works, but I think it can (and should) be simplified. If it can't, then LGTM. http://codereview.appspot.com/6193043/diff/1/ly/performer-init.ly File ly/performer-init.ly (right): http://codereview.appspot.com/6193043/diff/1/ly/performer-init.ly#newcode49 ly/performer-init.ly:49: