View Issue Details

IDProjectCategoryView StatusLast Update
0009009ardourbugspublic2022-10-30 19:09
Reporterthebutant Assigned Topaul  
PriorityhighSeveritymajorReproducibilityalways
Status assignedResolutionopen 
PlatformUbuntuOSLinuxOS Version(any)
Product VersionMixbus 8.x 
Summary0009009: After tempo change in the tempo bar, midi regions are offset and behaves unexpectedly.
DescriptionWhen changing tempo with the tempo marker during a project, the midi regions recorded after the tempo change behave unexpectedly and are very difficult to work with.
Modwheel automation is offset, editing midi notes is offset and responds rather wildly.
Steps To Reproduce1/ New project.
2/ Record a midi region with several notes and modwheel automation. This one will work as expected, it's simply there as a reference.
3/ After this region, change tempo in the tempo bar (for instance so that the first part of the project has its default 120 bpm, and the second part has 92 bpm (doesn't matter, but just to make a tempo change).
4/ Record a midi region in this second part, the one with the different tempo. Record the region with several notes and modwheel automation.
5/ You will now see that the modwheel automation for this new midi region is offset, it comes before the notes it relates to. Playback will nevertheless play it correcty, you can see that the modwheel fader to the left (sorry, I'm not sure what it's called) won't correspond with the nodes under the midi region. The modwheel fader to the left acts correctly.
6/ Try to edit several modwheel automation nodes, select by dragging the cursor over. I'm not able to select the ones I wish to select, I only get to select nodes that are placed earlier in the region.
7/ Try to select several midi notes in the region the same way, these are also offset. You'll get to select notes that come earlier in the region, but not the ones you try to select.
8/ Try to select 1 note. You're able to. Try to make it longer or shorter. I will probably be a rather frustating task, as it behaves unexpectedly.
9/ Split the midi region. Drag the two regions apart a little. Mark them with the range tool, right click and "Consolidate." My oh my, what a mess. ):O
TagsNo tags attached.

Relationships

parent of 0009045 closedx42 Note disappears if edit is applied 
parent of 0009050 feedbackpaul Midi editor snapping offset after tempo marker 
Not all the children of this issue are yet resolved or closed.

Activities

paul

2022-10-21 16:42

administrator   ~0026667

Thanks for the excellent bug report. Will try to get to this ASAP.

x42

2022-10-24 02:11

administrator   ~0026681

Last edited: 2022-10-24 02:16

(7) and (8) should be fixed since Ardour 7.0-82-g63c78ebced

The rec-region not showing notes while recording is understood, I attach a patch which explains the issue, however the actual fix will need further work.
Ardour::Region needs an API for `source->natural_position().distance (abspos)`.

--

PS. While recording region->source()->natural_position () is using BeatTime - while region->source_position() is using AudioTime - the value of the latter is not valid until after rec-stop
midi_rec_region_fix.diff (3,414 bytes)   
diff --git a/gtk2_ardour/midi_region_view.cc b/gtk2_ardour/midi_region_view.cc
index 923ed9ab03..cc1037c317 100644
--- a/gtk2_ardour/midi_region_view.cc
+++ b/gtk2_ardour/midi_region_view.cc
@@ -1674,29 +1674,6 @@ MidiRegionView::end_write()
 	_marked_for_velocity.clear();
 }
 
-
-/** Resolve an active MIDI note (while recording).
- */
-void
-MidiRegionView::resolve_note (uint8_t note, Temporal::Beats end_time)
-{
-	if (midi_view()->note_mode() != Sustained) {
-		return;
-	}
-
-	if (_active_notes && _active_notes[note]) {
-		/* Set note length so update_note() works.  Note this is a local note
-		   for recording, not from a model, so we can safely mess with it. */
-		_active_notes[note]->note()->set_length (end_time - _active_notes[note]->note()->time());
-
-		/* End time is relative to the region being recorded. */
-		_active_notes[note]->set_x1 (trackview.editor().time_to_pixel (_region->region_beats_to_region_time (end_time)));
-		_active_notes[note]->set_outline_all ();
-		_active_notes[note] = 0;
-	}
-}
-
-
 /** Extend active notes to rightmost edge of region (if length is changed)
  */
 void
@@ -4329,7 +4306,8 @@ MidiRegionView::data_recorded (boost::weak_ptr<MidiSource> w)
 		   we want to convert to beats relative to source start.
 		*/
 
-		Temporal::Beats const time_beats = _region->absolute_time_to_source_beats (timepos_t (ev.time()));
+
+		Temporal::Beats const time_beats = src->natural_position ().distance (timepos_t (ev.time ())).beats ();
 
 		if (ev.type() == MIDI_CMD_NOTE_ON) {
 
@@ -4347,7 +4325,28 @@ MidiRegionView::data_recorded (boost::weak_ptr<MidiSource> w)
 			}
 
 		} else if (ev.type() == MIDI_CMD_NOTE_OFF) {
-			resolve_note (ev.note (), time_beats);
+
+			// XXX WAS resolve_note (ev.note (), time_beats);
+			uint8_t note = ev.note ();
+			Temporal::Beats end_time = time_beats;
+
+			if (_active_notes && _active_notes[note]) {
+				/* Set note length so update_note() works.  Note this is a local note
+					 for recording, not from a model, so we can safely mess with it. */
+				_active_notes[note]->note()->set_length (end_time - _active_notes[note]->note()->time());
+
+				/* End time is relative to the source being recorded. */
+
+				// XXX ideally we'd calculate this based on end_time.
+				// - we first have to get the absolute position of end_time (for tempo-map conversion)
+				// - then calculate the distance from that relative to src->natural_position ()
+				// - and then take the samples() value of that and convert it to pixels
+				//
+				// Much simpler to just use ev.time() which is alredy the absolute position (in sample-time)
+				_active_notes[note]->set_x1 (trackview.editor().sample_to_pixel ((src->natural_position ().distance (timepos_t (ev.time ()))).samples()));
+				_active_notes[note]->set_outline_all ();
+				_active_notes[note] = 0;
+			}
 		}
 
 		back = ev.time ();
diff --git a/gtk2_ardour/midi_region_view.h b/gtk2_ardour/midi_region_view.h
index e4bbba4d00..f5ff024604 100644
--- a/gtk2_ardour/midi_region_view.h
+++ b/gtk2_ardour/midi_region_view.h
@@ -125,7 +125,6 @@ public:
 	GhostRegion* add_ghost (TimeAxisView&);
 
 	NoteBase* add_note(const boost::shared_ptr<NoteType> note, bool visible);
-	void resolve_note(uint8_t note_num, Temporal::Beats end_time);
 
 	void cut_copy_clear (Editing::CutCopyOp);
 	bool paste (Temporal::timepos_t const & pos, const ::Selection& selection, PasteContext& ctx);
midi_rec_region_fix.diff (3,414 bytes)   

x42

2022-10-24 02:33

administrator   ~0026682

PPS. step-entry works nicely across tempo-changes, at Region::region_beats_to_region_time() works correctly in that context

paul

2022-10-28 22:43

administrator   ~0026738

note visibility during capture now fixed in git master. also enhanced with new color for notes during capture.

I believe all items in this report are now fixed, but will wait for confirmation before marking "Resolved"

Issue History

Date Modified Username Field Change
2022-10-19 08:37 thebutant New Issue
2022-10-21 16:42 paul Assigned To => paul
2022-10-21 16:42 paul Status new => assigned
2022-10-21 16:42 paul Note Added: 0026667
2022-10-24 02:11 x42 Note Added: 0026681
2022-10-24 02:11 x42 File Added: midi_rec_region_fix.diff
2022-10-24 02:12 x42 Note Edited: 0026681
2022-10-24 02:16 x42 Note Edited: 0026681
2022-10-24 02:16 x42 Note Edited: 0026681
2022-10-24 02:33 x42 Note Added: 0026682
2022-10-28 14:50 x42 Relationship added parent of 0009045
2022-10-28 22:43 paul Note Added: 0026738
2022-10-30 19:09 x42 Relationship added parent of 0009050