Re: Issue 4048: Improve crescendo performance with unspecified dynamics (issue 302910043 by nine.fierce.ball...@gmail.com)

[nine . fierce . ballads]

Heikki, thanks for taking the time to review this patch.


What are the main user-level justifications for increasing the

complexity of the

Dynamic_performer in this way (in terms of the amount of code)?


It addresses the original request in the ticket and other cases noted in
response.

It also improves an example I saw elsewhere recently (I don't remember
where, but I think it was yours), something like mf < ... < ... < ... <
... f.  The volume no longer saturates early.


User acceptance of the changes could
suffer, especially if the new heuristics could change the MIDI output

for (and

in the worst case "break") existing scores.


The regression tests show no differences.


The acceptance of the new rules could possibly be helped by adding

also some

user-level documentation about the behavior of the new

Dynamic_performer.   (I'd
probably accept *any* documented Dynamic_performer implementation over
the
existing one.)

Well, the level of detail in the documentation should depend on the use
case for MIDI output.  I've been told very clearly that "It can serve as
a proofhearing aid or a practice aid.  It is not intended to serve as a
substitute for a player when recording.  For
that, LilyPond produces sheet music fit for running through a human"
(http://lists.gnu.org/archive/html/lilypond-devel/2013-11/msg00194.html).

For a proofhearing or practice aid, I don't see that any more needs to
be said than that Lilypond chooses the target dynamics of (de)crescendi
when they are not specified.  A user who cares for a specific dynamic
doesn't need any knowledge of the performer's heuristics; he needs only
to specify the dynamic.  A user who doesn't care, doesn't care.


Are there user-visible changes or improvements that should be
mentioned in the Changes document?


I don't think that these minor changes to MIDI output in cases where the
user didn't care enough to specify a dynamic are much to brag about.


https://codereview.appspot.com/302910043/diff/40001/lily/dynamic-performer.cc
File lily/dynamic-performer.cc (right):

https://codereview.appspot.com/302910043/diff/40001/lily/dynamic-performer.cc#newcode53
lily/dynamic-performer.cc:53: Real look_up_absolute_volume(SCM
dynamicString,
On 2016/07/09 18:16:50, ht wrote:

missing space after open parenthesis


Thank you.  I've run fixcc.py on dynamic-performer.cc and audio-item.cc
and amended my commits, though I first reverted some changes not related
to my work.

https://codereview.appspot.com/302910043/diff/40001/lily/dynamic-performer.cc#newcode101
lily/dynamic-performer.cc:101: Real max_target_vol)
On 2016/07/09 18:16:50, ht wrote:

Assuming that equalizer settings do not change between dynamic spans

added to

the queue, this function appears to be called always with the same

values for

min_target_vol  and  max_target_vol.  Under this assumption, is it

necessary to

have these values passed every time as function parameters (removing

them could

simplify the interface)?


Assuming that context properties do not change in the course of the
performance does not seem very lilypondish.  It is true that I have not
tried to handle the possibility rigorously, but I thought it better to
write this interface this way.  It's small potatoes compared to the
rest.


is it correct that the function just
overwrites the values of the queue's member variables (that is, would

the

function actually need to keep track of the minimum and maximum volume

over all

UnfinishedSpans that are added to the queue)?


Correct, and I think handling a change in equalizer settings at
arbitrary moments during crescendi would greatly complicate this
performer.  I didn't think long about it before deciding it was not
worth my time.  Maybe it is worth someone's time, but I doubt it.  If it
matters to you, I suppose we could document it as a known issue.

https://codereview.appspot.com/302910043/diff/40001/lily/dynamic-performer.cc#newcode156
lily/dynamic-performer.cc:156: depart_dir_ = next_grow_dir;
On 2016/07/09 18:16:50, ht wrote:

This looks like a redundant assignment due to the preceding condition.


Yes.  I appreciate your attention to detail.

https://codereview.appspot.com/302910043/diff/40001/lily/dynamic-performer.cc#newcode217
lily/dynamic-performer.cc:217: // Return a volume which is reasonably
distant from the given volumes in the
On 2016/07/09 18:16:50, ht wrote:

I guess that the "given volumes" mean only  start_vol  and  end_vol,

not

min_vol  and  max_vol  as these just define the available volume

range.  If this

is correct, I think it would be useful to state this more explicitly

here.

Yes.  I added max and min parameters after documenting the function.
I'll update the comment.


Also, aren't  min_vol  and  max_vol  again essentially fixed values

(provided

that equalization settings do not change)?


Yes, but I've made what I consider a reasonable effort to handle changes
to equalization settings: a solution that does not place them with the
highest 

Re: Doc: Add a section about handling MIDI dynamics with multiple voices (issue 302930043 by ht.lilypond.developm...@gmail.com)

[nine . fierce . ballads]

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

https://codereview.appspot.com/302930043/diff/20001/Documentation/notation/input.itely#newcode3028
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/input.itely#newcode3030
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.)

https://codereview.appspot.com/302930043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Doc: Add a section about handling MIDI dynamics with multiple voices (issue 302930043 by ht.lilypond.developm...@gmail.com)

[Carl . D . Sorensen]

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/input.itely
File Documentation/notation/input.itely (right):

https://codereview.appspot.com/302930043/diff/20001/Documentation/notation/input.itely#newcode3028
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/input.itely#newcode3030
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-contexts
,
\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  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  .

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).

https://codereview.appspot.com/302930043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Reitveld email

[David Kastrup]

Dan Eble  writes:

> I didn’t receive any email about a comment made two days ago on one of
> my patches (https://codereview.appspot.com/302910043
> ).  I don’t see any options
> in the Reitveld settings related to notifying me about feedback by
> email.  Am I blind?  TIA.

Spam folder maybe?

-- 
David Kastrup

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Reitveld email

[Dan Eble]

I didn’t receive any email about a comment made two days ago on one of my 
patches (https://codereview.appspot.com/302910043 
).  I don’t see any options in the 
Reitveld settings related to notifying me about feedback by email.  Am I blind? 
 TIA.
— 
Dan

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Leave of absence

[tisimst]

On Mon, Jul 11, 2016 at 2:05 AM, Thomas Morley-2 [via Lilypond] <
ml-node+s1069038n192541...@n5.nabble.com> wrote:

> Hi all,
>
> I'll be on vacations for the next two weeks, doing camping in Normandie.
> Thus completely without net-access.
>
> Cheers,
>   Harm
>

A well-deserved vacation. Enjoy living off the grid! Thanks for all you do,
Harm.

--
Abraham Lee




--
View this message in context: 
http://lilypond.1069038.n5.nabble.com/Leave-of-absence-tp192541p192559.html
Sent from the Dev mailing list archive at Nabble.com.
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Doc: Add a section about handling MIDI dynamics with multiple voices (issue 302930043 by ht.lilypond.developm...@gmail.com)

[nine . fierce . ballads]

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

https://codereview.appspot.com/302930043/diff/20001/Documentation/notation/input.itely#newcode3028
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?

https://codereview.appspot.com/302930043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Leave of absence

[David Kastrup]

Thomas Morley  writes:

> Hi all,
>
> I'll be on vacations for the next two weeks, doing camping in Normandie.
> Thus completely without net-access.

Here's to good weather!

-- 
David Kastrup

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Leave of absence

[Urs Liska]

Have fun.

I just read they have 29 degrees Celsius during summer: 14 in the
morning and 15 in the afternoon ;-)


Am 11.07.2016 um 10:05 schrieb Thomas Morley:
> Hi all,
>
> I'll be on vacations for the next two weeks, doing camping in Normandie.
> Thus completely without net-access.
>
> Cheers,
>   Harm
>
> ___
> lilypond-devel mailing list
> lilypond-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/lilypond-devel


___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Leave of absence

[Thomas Morley]

Hi all,

I'll be on vacations for the next two weeks, doing camping in Normandie.
Thus completely without net-access.

Cheers,
  Harm

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Doc: Add a section about handling MIDI dynamics with multiple voices (issue 302930043 by ht.lilypond.developm...@gmail.com)

[tdanielsmusic]

Much improved over the first attempt, thanks.  LGTM.

Trevor


https://codereview.appspot.com/302930043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel