Re: Add Staff.midiCC context property, refactor handling of MIDI control changes (issue 284280043 by address@hidden)

[ht . lilypond . development]

On 2016/01/21 15:21:22, dak wrote:
> I am not happy about the increasing abuse of context properties for
not actually
> setting context properties but rather sending Midi messages.  The
previous
> interfaces at least actually set a queryable property.  This one takes
a
> pair/value pair instead and only sets a property in the Midi
controller but not
> in the context and one cannot actually query this property again.

I understand that the behavior introduced by the patch is likely
stretching the meaning of a "context property" too far if one considers
all the technical implications.

(As my level of expertise with LilyPond internals is at a level where I
hardly feel like knowing enough about the technical differences between
grob and context properties to consider myself qualified to even
speaking
of them in any of the documentation I've written about the MIDI
enhancements :-) [my experiences with these differences are limited to
once
trying and failing to access values of context properties in music
functions, only to conclude that context properties "probably just don't
work that way"], I've learned to treat context properties myself mostly
as "write-only" things in everyday use.  I understand that the situation
could be different for advanced users and developers writing new music
iterators [I hope that's the correct term].  I guess these advanced
users
are those who will suffer most from breaking the consistency of how
context properties work.)

At a great risk of not exactly understanding what you meant by "This
one [...] only sets a property in the Midi controller but not in the
context and one cannot actually query this property again": isn't it
still the case that the context property will have the list of the most
recent \set as its value at any given point?  In that case, would it
be possible to implement setting the value of the context property as
updating entries in an alist (and generating the corresponding MIDI
control changes as a side effect) without removing any existing
entries, so that querying the value of the context property would
return the list of all settings in effect (as this list of settings
would probably best reflect also the state of the controllers on a
MIDI device)?  Of course, changing the interpretation of what it means
to set the value of a context property, even if it were technically
possible, is not much better for consistency (and there would be no
way to remove entries in the alist without extending the syntax, for
example, with a special controller "value", with the question of
how the removal of an entry should be reflected in MIDI).

Of course, also the conflict in the interaction between midiCC and
the previously introduced MIDI context properties is even more apparent
when one considers setting and reading the values of the properties
which would be settable in MIDI in multiple ways after this change.

> I think that this is not a good idea.  Suggestions how we could rather
do this?
> Possible mechanisms would include Midi-specific music events, or
getting the
> equivalent of override/revert for Midi (mobs then instead of grobs).
Of course,
> if we had mobs, it would make sense to map a _lot_ of the current Midi
> generation through them eventually because it would make programming
the Midi
> generation much more transparent.

All I've wished to have when implementing these MIDI enhancements is
only some way of translating syntactic constructs in LilyPond input
into MIDI events for MIDI output.  Because the Staff.midiInstrument
property already existed at the time I started, context properties
ended up being "chosen" as the interface also for the enhancements
because this appeared to keep the extensions syntactically consistent
with what was already there (and saved me at that point from having
to learn how to extend LilyPond input syntax...) - when one considers
only the syntax, it's hard to beat the intuitiveness of just saying
"\set midiXXX = ..." in LilyPond input.  In view of the above goal,
the midiCC "property" was then just extending the mechanisms of
handling MIDI control changes to one possible conclusion.

I don't object to defining a new syntax for expressing "arbitrary"
MIDI control changes in LilyPond input if this is necessary to keep
the interpretation of context properties consistent - the only
limitation that I have is just that I don't possess the technical
knowledge on how to start right away with such a task.  (I also
probably have a problem of not ever having fully appreciated the
value that the different syntaxes used for working with grob vs.
context properties bring to the user writing LilyPond input -
although I'm sure there's many technical reasons as to why this
difference exists, as a user I'd sometimes wish I were able to "just"
modify [and even query for] properties of things using a consistent
syntax without having to care about what kind of things they are,
other than that they have the property that I'm interested in
accessing.  With this view, I probably wouldn't like to have yet
another syntax for expressing MIDI changes in LilyPond input,
either...)

Nevertheless, given the suggested choices, I'd probably go for just
having a separate event for handling the control changes instead of
introducing mobs, which sounds like it could need a lot of work
without much benefit for most users - the number of people asking
questions about tweaking MIDI output in LilyPond is tiny in
comparison with the number of people asking questions about tweaking
the typeset output (and I don't believe that this is because
LilyPond doesn't have mobs).

The only question I have if the syntax is changed is whether it's
then still possible to have values of MIDI controllers initialized in
a \with block of a Staff using that syntax.  (This concern is
based only on the current implementation of Staff_performer, which
just makes get_property calls to query the initial values of
MIDI-related context properties, to pre-initialize each new
Audio_staff with audio items that set the initial values in MIDI
output.)


https://codereview.appspot.com/284280043/