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

Issue 296570043: Midi_walker::do_start_note: skip ignored notes in stop_note_queue (Closed)

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

Description

Midi_walker::do_start_note: skip ignored notes in stop_note_queue For each semitone pitch value, stop_note_queue is likely supposed to contain at most one Midi_note event with its "ignore_" flag set to false, and the comparisons between notes of equal semitone pitch to be always done between the input note and this unique queued note that is not (yet) being ignored. If notes which are already being ignored are not skipped in the loop, the task of raising the "ignore_" flags for note events of equal semitone pitch (overlapping in time) which stop before the maximum stopping time of these notes may, due to breaking out of the loop, fail to work if there are three or more simultaneous notes of equal semitone pitch, leading to the emission of premature "note off" events for this pitch, as demonstrated, for example, in <http://lists.gnu.org/archive/html/bug-lilypond/2016-06/msg00042.html>.

Patch Set 1 #

Patch Set 2 : add regression test #

Patch Set 3 : clarify description of regression test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -2 lines) Patch
A input/regression/midi/midi-overlapping-notes.ly View 1 2 1 chunk +22 lines, -0 lines 0 comments Download
M lily/midi-walker.cc View 1 chunk +3 lines, -2 lines 0 comments Download

Messages

Total messages: 4
ht
Midi_walker::do_start_note: skip ignored notes in stop_note_queue For each semitone pitch value, stop_note_queue is likely supposed ...
3 years, 2 months ago (2016-06-26 15:40:57 UTC) #1
Dan Eble
LGTM. If this didn't change any regression test results and you don't want it to ...
3 years, 2 months ago (2016-06-26 17:14:54 UTC) #2
ht
add regression test
3 years, 2 months ago (2016-07-02 09:03:53 UTC) #3
ht
3 years, 2 months ago (2016-07-02 09:10:04 UTC) #4
clarify description of regression test
Sign in to reply to this message.

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