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

Issue 277700043: Do not output CC#7 events in MIDI on dynamic changes

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 3 months ago by ht
Modified:
8 years, 3 months ago
Reviewers:
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Do not output CC#7 events in MIDI on dynamic changes As volume changes for MIDI are encoded as note velocities, it is not necessary to add "silent" Audio_dynamic elements to Audio_staffs to be processed by Midi_walker. (Every "silent" Audio_dynamic event will only get converted to a Midi_dynamic event outputted as a CC#7 event with fixed volume 100 in MIDI, see Midi_dynamic::to_string. This behavior is not consistent with the handling of other MIDI controls on which LilyPond will not enforce any "default" values.) As Staff_performer::get_audio_staff has the side effect of creating a new Audio_staff for a voice if it does not yet exist, accessing the current voice's associated Audio_staff was moved before the handling of Audio_dynamic elements. Based on the revision history, not emitting any CC#7 events in MIDI appears to have been the original intention when encoding dynamic changes as note velocities (93b7a6ff072d73dcdd41da59cd18da8aa8d8e8cb), but apparently the initial implementation (of passing "silent" Audio_dynamic events to Midi_walker, and ignoring them only at the output stage) caused problems with MIDI timing (#1593), possibly because the skipped events might have interfered with the logic for tracking the delta times between the MIDI events that would be actually outputted. To fix #1593, commit f114e3c33f9c37c39c7a3fedf66ca5a074785118 conservatively restored the emission of CC#7 events in MIDI. This commit tries to avoid the issues with MIDI timing by pruning all Audio_dynamic events already from the input given to Midi_walker, so that Midi_walker sees no events, the skipping of which would confuse the measurement of delta times at the output stage. This modification (intentionally) changes the MIDI regression test results by removing all CC#7 events on dynamic changes from the MIDI output.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -3 lines) Patch
M lily/staff-performer.cc View 2 chunks +4 lines, -3 lines 0 comments Download

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