Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in

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

Can't Edit
Can't Publish+Mail
Start Review
3 years, 10 months ago by Dan Eble
3 years, 10 months ago


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.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+26 lines, -24 lines) Patch
A input/regression/midi/dynamic-initial.ly View 1 chunk +15 lines, -0 lines 0 comments Download
M input/regression/typography-demo.ly View 1 chunk +0 lines, -1 line 0 comments Download
M lily/dynamic-performer.cc View 5 chunks +11 lines, -23 lines 0 comments Download


Total messages: 4
Dan Eble
(2nd try after reconfiguring git-cl)
3 years, 10 months ago (2016-06-04 21:07:13 UTC) #1
Dan Eble
3 years, 10 months ago (2016-06-04 21:16:32 UTC) #2
I understand from this patch that you are in favor of removing the warning altogether, ...
3 years, 10 months ago (2016-06-05 21:41:55 UTC) #3
Dan Eble
3 years, 10 months ago (2016-06-05 23:47:45 UTC) #4
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.)
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b