Issue 3945: fix (De)crescendo with unspecified starting volume in MIDI

This review combines two commits.  I would normally upload them as separate patch sets to ease review, but I had some difficulty because it's been a while since my last contribution.

I.  Issue 3945: fix (De)crescendo with unspecified starting volume in MIDI

    The approach is to remove the block of code that issued the warning
    and created a default dynamic.  There seems to be another block further
    down that creates a default dynamic anyway.

II. Dynamic_performer: eliminate an unnecessary variable
    Use -1 to represent an unknown volume.

    This is just clean-up.

[Dan Eble, 2016-06-04 21:07:13]

(2nd try after reconfiguring git-cl)

[ht, 2016-06-05 21:41:55]

I understand from this patch that you are in favor of removing the warning
altogether, which - I agree - works very inconsistently in current versions (as
demonstrated by the examples in the issues about the warning).  As you yourself
have noted on a discussion attached to issue 4104, however, the behavior used to
be different in LilyPond 2.16...


It appears that the changes in behavior (the introduction of the warning as part
of issue 2707, and the changes that made the behavior to I what think is still
current, see issue 3234) are a result of code changes by developers with
opposite views about how serious a problem it is to start a new Voice without
explicitly specifying an explicit MIDI volume for the first note inside the
Voice.

As I am to blame for originally adding the warning to the code, let me explain
here why I did it :-)

During my initial learning phase on using LilyPond I spent a lot of time
transcribing music with LilyPond in order to produce MIDIs (for my choir
practice).  However, on more than one occasion, after successfully finishing a
piece (with orchestral parts and everything, including all expressive marks for
the typeset output, and keyboard parts written in a happy and carefree way using
lots of temporary Voices and simultaneous music without really understanding the
MIDI implications of using \\ between simultaneous music expressions), rendering
the LilyPond input to MIDI gave surprising and even disappointing results - at
the boundaries of Voices, there could be sudden jumps in the MIDI volume without
any explicit dynamic markings in LilyPond input.

After learning more about LilyPond contexts, I eventually came to realize that
the observed behavior was actually not a bug, but was caused by LilyPond's
default context settings where every new Voice context has its own
Dynamic_performer (which does not inherit any properties - such as the "current"
note velocity level - from its syntactically enclosing Voice context, since
Voice contexts, even if syntactically nested, do not nest in the default
LilyPond context hierarchy).  Therefore, a Voice context syntactically enclosed
within another Voice context will have its own Dynamic_performer with the
initial note velocity in MIDI possibly set to something different from what
could be expected by looking at the typeset output, where Voice contexts are not
distinguishable in an unambiguous way in the general case.

This realization then led to a lot of additional work on refactoring all of the
write-only LilyPond code that I had managed to produce to get rid of the
temporary Voice contexts (or in some cases move the Dynamic_performer from the
Voice context to the Staff context), and a rule of thumb to make sure that every
Voice context always begins with an explicit MIDI dynamic.  To try to help
myself (and others) avoid making the mistake of creating new Voices without an
explicit initial MIDI dynamic in the future, I thought it useful to add a
warning to the code to alert about cases where the generated MIDI output might
fail to match the user's expectations (similar to the warnings that had already
been given about "Impossible (de)crescendos in MIDI").  (However, at this point
I can't explain at all why I then decided to focus only on (de)crescendos - I
think it's just as bad to begin a new Voice with just a single note without a
dynamic indication if one cares so much about MIDI volume.)

It appears that the author of the patch for issue 3234, after having apparently
discovered the same above feature of Voice contexts, approached the handling of
initial MIDI velocity in new Voice contexts from a different angle - instead of
giving warnings about cases where the MIDI velocity might get "reset to the
default value" upon the creation of a Dynamic_performer (which could result in
MIDI volume changes failing to match the user's expectations, so the user should
probably check them in the source), the goal of the patch appears to
specifically have been to make the Dynamic_performer silently use default
velocity values in more scenarios than it used to.  I believe this patch is the
reason for the change in behavior between 2.16 and 2.18.


After following the discussion on the mailing lists in the years after writing
the patch, I've since come to the conclusion that many people end up being just
confused by this warning, and most users do not see - or even care about - the
subtleties of keeping in full control of the MIDI volume when crossing Voice
boundaries.  Even though I'd hoped there to have been more users like myself who
find these kind of details important, not caring about such details is of course
understandable: after all, it's not likely to be possible for any computer
program that only follows simple fixed rules to produce a good interpretation of
the dynamic markings of an arbitrary piece of typeset music automatically
anyway.  Since I have personally also learned to always take extra precautions
with Voices if I need to care about MIDI velocity, I won't mind if the warning
gets removed from the code.

(With the warning removed, what I'd nevertheless like to have is a warning in
the documentation about the implications of creating temporary Voices on MIDI
velocity, maybe already in the section where temporary voices are introduced. 
Writing such a patch for documentation has been for several years on my to-do
list, but I just never seem to find the time to write one...)

[Dan Eble, 2016-06-05 23:47:45]

HT, thanks for your review and your detailed response.

> (or in some cases move the Dynamic_performer from the Voice context to the
Staff context)

I understand what you are saying.  I often use the part combiner, which lays
waste to the organization of musical parts within Lilypond Voice contexts.  Not
only does it create problems with MIDI dynamics, but also with lyrics (which
NullVoice helps work around) and voice-based accidental rules.  My current
work-around involves creating invisible Voice contexts to hold the complete part
and moving the MIDI performers there.

(That reminds me to try Trevor's (?) vocal templates and see if there are any
improvements I could contribute.)