Doc: Add a section about handling MIDI dynamics with multiple voices

Doc: Add a section about handling MIDI dynamics with multiple voices

As MIDI dynamics of every Voice are by default independent of
dynamic changes occurring in all other Voices, writing LilyPond
input using multiple voices may require some additional
considerations to be taken into account to keep in full control
of MIDI volume for MIDI output.

(While users interested only in typeset output can safely - and
probably do - ignore these considerations, users who also wish to
generate MIDI output can easily remain unaware of the issues
with MIDI dynamics that ignoring them may cause, because the need
for such considerations is not raised directly anywhere in the
documentation.  See, for example, the full disussion thread
<http://lists.gnu.org/archive/html/lilypond-user/2014-05/msg00361.html>
for comments from both groups of users, and the false assumptions
that could be made about MIDI dynamics in output generated from
multiple voices when relating it to the typeset output.)

This change in the documentation tries to describe in more detail
how MIDI dynamics interact with voices, and, now that the last
pieces of source code logic (which had become less and less
effective over time, and had not been complete even when they
were first introduced) of signaling warnings about cases
where the MIDI output might not fully match the expected one
have been removed as a result of the code changes related to
Issue 3945, lists some simple guidelines to help users keep in
control of MIDI volume with multiple voices.

The change consists of a forward reference to a new MIDI
subsection in the beginning of the section about single-staff
polyphonic notation (where the syntax of defining Voices is
described), and a new MIDI subsection which illustrates the
default interaction of MIDI dynamics and voices by way of an
example.

[Carl, 2016-07-05 20:58:03]

This is too wordy for the notation reference.  We need to have a minimum number
of words in the NR, and instead show things by examples.

It should just have examples that show the problem and the solution.  Since we
can't see the MIDI output in the manual, there should be a statement of what
happens in the MIDI.

Thanks,

Carl

https://codereview.appspot.com/302930043/diff/1/Documentation/notation/input....
File Documentation/notation/input.itely (right):

https://codereview.appspot.com/302930043/diff/1/Documentation/notation/input....
Documentation/notation/input.itely:3022: @node MIDI dynamics and voices
This section is too tutorial in language for the notation reference.  Such a
section should be added to the Learning Manual, if it is needed.

Otherwise, this section should be greatly reduced, to just show the code that
introduces the problem and alternative code that can avoid the problem.

https://codereview.appspot.com/302930043/diff/1/Documentation/notation/simult...
File Documentation/notation/simultaneous.itely (right):

https://codereview.appspot.com/302930043/diff/1/Documentation/notation/simult...
Documentation/notation/simultaneous.itely:410: This section presents the ways to
express single-staff
This should actually just be a SeeAlso, as you have listed below.  We don't need
this paragraph in the notation reference.

The Notation Reference is not a tutorial; it needs to be as brief as possible.

[ht, 2016-07-08 21:25:26]

condense the additions, improve accuracy about named voices

[ht, 2016-07-08 22:48:10]

Patch set 2 is an attempt to condense the additions to the documentation.
The main changes are:


* There's now only a "see also" reference to the MIDI section in the section
  on single-staff polyphonic notation.  I still think that a warning about
  MIDI considerations in a more prominent place in this section might be
  useful since the examples about typesetting Voices (both in the
  Learning Manual and the Notation Reference) encourage rather free use of
  temporary Voices to help produce the intended typeset output, without
  alerting the reader to the fact that organizing the voices may benefit
  from some additional planning if the input is going to be used also for
  generating MIDI output.

  I originally learned this lesson the hard way only after several years
  of using LilyPond, having first taken the LM and NR examples as the
  best practices on how to organize the input using temporary Voices for
  any kind of output, then sometimes scratching my head wondering why the
  MIDI output did not always quite match the typeset output (with sudden
  changes in MIDI volume in places where there were no dynamic changes in
  the typeset output), and eventually (after learning more about contexts
  and the behavior of the Dynamic_performer) realizing that LilyPond
  works just as it's supposed to, but it's only my own understanding about
  the behavior of voices in MIDI that had been wrong all along.

  The reason I started this documentation task was to try to help other
  users not make the same mistakes that I did.  Had I been better aware
  of the behavior of voices in MIDI, I could've saved a lot of work
  that I had to spend refactoring the LilyPond files that I had managed
  to produce before realizing the mistake.


* The first patch set's description about the MIDI dynamic assignment
  rules for Voices was slightly inaccurate since, as a matter of fact,
  Voices with identical names *do* share their dynamic changes.  Also
  the example has been extended to demonstrate how this sharing works.


* The MIDI dynamic level assignment for notes in the example is now
  described in comments in the example itself instead of a separate
  list to save some space.


* The ways to control the MIDI dynamic level at Voice boundaries are
  not expanded to example code since omitting dynamic marks from
  typeset output, and using tags, is (in my opinion) better described
  elsewhere in the NR.


* The note about moving Dynamic_performer between contexts has been
  removed since this is not a robust way to help make a group of
  Voices share their dynamics in the general case.  In my experiments
  on the updated example, I observed that this method does not work
  with named Voices at all, and for unnamed Voices it appears to work
  only by accident.  (In other words, the only simple and reliable way
  to control MIDI dynamics at Voice boundaries is to use explicit
  dynamic marks, which is still mentioned in the text.)

[Trevor Daniels, 2016-07-11 07:42:36]

Much improved over the first attempt, thanks.  LGTM.

Trevor

[Dan Eble, 2016-07-11 13:34:45]

https://codereview.appspot.com/302930043/diff/20001/Documentation/notation/in...
File Documentation/notation/input.itely (right):

https://codereview.appspot.com/302930043/diff/20001/Documentation/notation/in...
Documentation/notation/input.itely:3028: value 0.71 in the available MIDI volume
range between 0 and 1.
Is it necessary to be this specific?  If so, is there a way we can pull the
default value out of the C++ code into the documentation so that it will not get
out of sync?

[Carl, 2016-07-11 16:39:01]

Heikki, this is so much better than your first patch.  Thanks for working on it!

As I mention in the comments, I think the current behavior of changing dynamics
in identically-named but distinct voices is flawed, so I think it should be a
known issue, not in the main documentation.  But I like the main documentation
entry about the default value; I think it belongs right where it is and is a
great addition to our documentation.

Thanks,

Carl

https://codereview.appspot.com/302930043/diff/20001/Documentation/notation/in...
File Documentation/notation/input.itely (right):

https://codereview.appspot.com/302930043/diff/20001/Documentation/notation/in...
Documentation/notation/input.itely:3028: value 0.71 in the available MIDI volume
range between 0 and 1.
I think that it's useful to know what the default value is.  But I also think we
should have a comment that indicates it is defined in scm/midi.scm  It would
probably also be useful to give the name of the variable that holds the default
value, because the user can specify any desired default value by changing the
variable.

And it might be useful to show how to use default-dynamic-absolute-volume to set
the default value to one of the dynamic levels (it's currently between mf and
f).

https://codereview.appspot.com/302930043/diff/20001/Documentation/notation/in...
Documentation/notation/input.itely:3030: identically named voices in the same
@code{Staff}.
I hadn't realized this feature of MIDI.  I believe this behavior to be a bug. 
Dynamic changes should only apply to a specific voice, and as is made clear in
http://lilypond.org/doc/v2.18/Documentation/notation/creating-and-referencing...
,
\new will always create a new context.  The fact that a new context is created
is demonstrated in the first note of the new voice A getting the default value,
rather than the pppp value.

I believe it is a flaw in the MIDI performer to allow any Voice in a Staff with
the same name to share MIDI dynamics.

In your code, if you change the second A voice name to AA, the c1 on line 3052
would have the dynamic level of  pppp.

I appreciate your effort to document the fact that a default dynamic level
exists.  I think that is important.  I also appreciate your effort to document
the current behavior of LilyPond, but I don't think we should do it, because I
think it's a bug and we shouldn't document bug behavior in the main body of the
manual (I don't think it's intended behavior; at least I wouldn't be in favor of
that behavior).

I think that the behavior indicated in your example should be documented as a
Known Issue, which means we recognize it as a bug.  And we should have an issue
as well (MIDI volume levels leak between identically named voices).

[Dan Eble, 2016-07-11 23:41:18]

https://codereview.appspot.com/302930043/diff/20001/Documentation/notation/in...
File Documentation/notation/input.itely (right):

https://codereview.appspot.com/302930043/diff/20001/Documentation/notation/in...
Documentation/notation/input.itely:3028: value 0.71 in the available MIDI volume
range between 0 and 1.
On 2016/07/11 16:39:01, Carl wrote:
> I think that it's useful to know what the default value is.  But I also think
we
> should have a comment that indicates it is defined in scm/midi.scm

But I've seen the default hard-coded in C++.

And I think it would make more sense for a default to be a musical dynamic level
(mf), not a number; that would be an easy enhancement to make, but also an easy
documentation change when it comes to it.

https://codereview.appspot.com/302930043/diff/20001/Documentation/notation/in...
Documentation/notation/input.itely:3030: identically named voices in the same
@code{Staff}.
On 2016/07/11 16:39:01, Carl wrote:
> I believe it is a flaw in the MIDI performer to allow any Voice in a Staff
with
> the same name to share MIDI dynamics.

I believe I have a patch for this already, but it depends on my dynamics changes
that are currently in review, so I haven't published it yet.  (Thanks for
pointing this out, because until now the only motivation I had was "this doesn't
make sense--let's clean it up" and now I have a test case.)

[ht, 2016-07-12 07:22:16]

On 2016/07/11 16:39:01, Carl wrote:
> I think the current behavior of changing dynamics
> in identically-named but distinct voices is flawed, so I think it should be a
> known issue, not in the main documentation.

I agree that bugs should not be documented as the rules about the behavior. 
However, can it be definitely said in this case whether this is actually a bug? 
Without any specification available to compare the implementation with, the
answer to this question could differ between people, each of whom has their own
expectations about how they'd like things to work.

For example, I'd say that the initialization of a new Dynamic_performer at the
start of every new Voice, which implicitly "resets" the MIDI volume level to the
default level, could equally well be considered a bug (from the point of view of
a *user* who'd prefer worrying as little as possible about how contexts
internally work) simply because this makes it so much more difficult to write
single-staff polyphony which would produce MIDI output "matching" the typeset
output, even though the behavior makes perfect sense from an implementation
point of view.  (It is probably apparent at this point that this is the
viewpoint that I had in mind when I decided to write the patch to the
documentation.)  It could even be argued that sharing dynamics between
identically named Voices provides a workaround for this user problem, since by
using identically named Voices the shared dynamics do not need to be duplicated
in each separate Voice in the LilyPond input.

However, you could well be right that this sharing of dynamics between
identically named voices currently works only by accident (since the
initialization of a new Voice will still reset the dynamic level), just like
moving Dynamic_performer to the Staff context to make Voices within the Staff
share their dynamics works only for Voices without an explicit name.

I suspect all of this behavior is due to the Staff_performer logic, which tries
to distribute notes and dynamics into separate classes by the names of their
associated Voices - maybe this should be done using some other criterion (such
as an internal Voice ID, unique to each Voice instance).

> I think that it's useful to know what the default value is.  But I also think
we
> should have a comment that indicates it is defined in scm/midi.scm  It would
> probably also be useful to give the name of the variable that holds the
default
> value, because the user can specify any desired default value by changing the
> variable.

As Dan already commented, the default value is hard-coded in the C++ code - it
doesn't come from any Scheme-level definition.  I fully agree that instead of
using a hard-coded default value which does not match any of those defined in
scm/midi.scm, the Dynamic_performer implementation should simply be changed to
query for the default value from the default absolute volume map using one of
the keys defined in scm/midi.scm (and I agree that "mf" is probably a reasonable
choice).

> I think that the behavior indicated in your example should be documented as a
> Known Issue, which means we recognize it as a bug.  And we should have an
issue
> as well (MIDI volume levels leak between identically named voices).

So just to confirm, in your opinion the rule about how dynamics are (or are not)
shared between voices are better described just as a known issue?  I guess it
will then make sense to also simplify the example as well to no longer
demonstrate this corner case.

[dan_faithful.be, 2016-07-12 13:38:16]

> I suspect all of this behavior is due to the Staff_performer logic,
> which tries to distribute notes and dynamics into separate classes by
> the names of their associated Voices - maybe this should be done using
> some other criterion (such as an internal Voice ID, unique to each Voice
> instance).

As I mentioned, I already have a private branch that removes the mess from
Staff_performer and lets Dynamic_performer link notes to their dynamic.  I
haven’t tested it with your example, but I expect it to work more sensibly. 
Dynamics shouldn’t affect notes outside of the dynamic performer’s context.

> implementation should simply be changed to query for the default value
> from the default absolute volume map using one of the keys defined in
> scm/midi.scm (and I agree that "mf" is probably a reasonable choice).

If this plan is accepted, you could conscientiously document “mf” as the default
now.

I volunteer to make the change.  If someone would create a ticket and assign it
to me, it would help me remember.  
— 
Dan

[pkx166h, 2016-07-19 07:03:42]

On 2016/07/12 13:38:16, dan_faithful.be wrote:
> > I suspect all of this behavior is due to the Staff_performer logic,
> > which tries to distribute notes and dynamics into separate classes by
> > the names of their associated Voices - maybe this should be done using
> > some other criterion (such as an internal Voice ID, unique to each Voice
> > instance).
> 
> As I mentioned, I already have a private branch that removes the mess from
> Staff_performer and lets Dynamic_performer link notes to their dynamic.  I
> haven’t tested it with your example, but I expect it to work more sensibly. 
> Dynamics shouldn’t affect notes outside of the dynamic performer’s context.
> 
> > implementation should simply be changed to query for the default value
> > from the default absolute volume map using one of the keys defined in
> > scm/midi.scm (and I agree that "mf" is probably a reasonable choice).
> 
> If this plan is accepted, you could conscientiously document “mf” as the
default
> now.
> 
> I volunteer to make the change.  If someone would create a ticket and assign
it
> to me, it would help me remember.  
> — 
> Dan
> 

https://sourceforge.net/p/testlilyissues/issues/4936/

Thanks Dan.