Thanks, Heikki; this looks very good. Panning will help people to proof-read complex scores by listening to the MIDI output. I added an item to the bug tracker to add some documentation when this is done.
Did you consider adding the functionality to Staff_performer, rather than the separate Midi_effect_performer ? They both control settings that apply to a MIDI channel. If we move one _performer to the Voice context and not the other, the results seem more potentially confusing, than potentially useful. https://codereview.appspot.com/14434043/diff/8001/lily/midi-item.cc File lily/midi-item.cc (right): https://codereview.appspot.com/14434043/diff/8001/lily/midi-item.cc#newcode367 lily/midi-item.cc:367: // Audio_control_function_value_change::Control. Is it appropriate, then, to make this array a public static member of Audio_control_function_value_change and still access it from here? (I find the spaghetti of object-oriented obfuscation in the MIDI-output code to be overwhelming, so I'm happy that you seem to find your way through it.) https://codereview.appspot.com/14434043/diff/8001/lily/midi-item.cc#newcode398 lily/midi-item.cc:398: Implementing fine as well as coarse seems to be more trouble than it is worth. https://codereview.appspot.com/14434043/diff/8001/lily/staff-performer.cc File lily/staff-performer.cc (right): https://codereview.appspot.com/14434043/diff/8001/lily/staff-performer.cc#newcode71 lily/staff-performer.cc:71: Control_function_value_map control_function_value_map_; Better to move up a few lines, outside of the group of mappings indexed by voice names, and to define directly without typedef so it can be compared with the others. https://codereview.appspot.com/14434043/diff/8001/lily/staff-performer.cc#newcode231 lily/staff-performer.cc:231: if (!ins.second && ai->value_ == value) Why bother removing duplicate settings of midiPanPosition, etc. ? The bookkeeping is complicated, and a duplicate setting would come from a second explicit \set Staff.midiPanPosition, so the user might expect to see the duplicate control-change-event. Users often have applications that are surprising; someone might use the MIDI output as input to another program that does not remember control settings properly. https://codereview.appspot.com/14434043/diff/8001/ly/performer-init.ly File ly/performer-init.ly (right): https://codereview.appspot.com/14434043/diff/8001/ly/performer-init.ly#newcode200 ly/performer-init.ly:200: instrumentName = #"bright acoustic" Answering your earlier question on the bug-tracker, I would suggest simply omitting this initialization. The current state with the erroneous name has no effect, and people have not complained about a missing initial programChange. If you fix it, and start sending an initial programChange, someone might have a problem. https://codereview.appspot.com/14434043/diff/8001/scm/define-context-properties.scm File scm/define-context-properties.scm (right): https://codereview.appspot.com/14434043/diff/8001/scm/define-context-properties.scm#newcode435 scm/define-context-properties.scm:435: (midiChannelMapping ,symbol? "How to map MIDI channels: per @code{instrument} (default), @code{staff} or @code{voice}.") Oops. I changed the default to be 'staff, so that it works with the default arrangement of performers in the Staff-like contexts, but forgot to change this line of documentation. I'll fix it later if you don't. I hope that didn't cause confusion. https://codereview.appspot.com/14434043/diff/8001/scm/define-context-properties.scm#newcode438 scm/define-context-properties.scm:438: @code{midiChannelMapping}). Ranges from@tie{}@w{-1} to@tie{}1, I would skip the text in "(...)" because "MIDI channel associated with the context" says it all. https://codereview.appspot.com/14434043/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel