View Issue Details

IDProjectCategoryView StatusLast Update
0009191ardourbugspublic2023-06-17 19:57
Reportermpk Assigned Tox42  
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
PlatformRedhatOSLinuxOS Version(any)
Product Version7.2 
Summary0009191: Ctrl-drag region in snap mode creates glued copy
DescriptionCopying a region using ctrl-drag while in snap mode creates a copy which is marked as "Glued to Bars and Beats". Note that the session option "Glue new regions to bars and beats" is not selected.

The new region's length property (in the saved xml) is a mix of both audio time and beat time, e.g.: length="a273568446480@b2818560" with the distance (length) portion expressed in audio time and the position portion expressed in beat time.

Subsequently it is not possible to unglue this region due to the logic in Editor::toggle_region_lock_style and Region::set_position_time_domain.

1. Editor::toggle_region_lock_style checks region->position_time_domain then calls Region::set_position_time_domain if the requested time domain differs. In this case the new region's position is different, so:

2. Region::set_position_time_domain checks _length.val().time_domain(). But in this case the new region's length is already in AudioTime so nothing happens further.
Steps To Reproduce1. In a new session, import an audio region.
2. Enable snap mode
3. Ctrl-drag the region to copy it.
4. The new region has the musical notes icon denoting "Glued to Bars and Beats".
5. Attempt to unglue the region (Region > Position > Glued to Bars and Beats) - nothing happens.
Additional InformationNote that dragging a region or source from the Editor List in snap mode has the same problem.
TagsNo tags attached.

Activities

x42

2023-01-05 18:04

administrator   ~0027162

Thanks for that. There are various issues with "a...@b..."

It should not be possible to mix time domains, and I wondered how users managed to do this :)

mpk

2023-01-05 18:24

reporter   ~0027163

I used this patch to fix my session which had a...@b... regions with some brute force. But I don't really understand what's meant to happen here.
debug-toggle-glue.patch (3,536 bytes)   
commit bee6a3c0ae9d03cba86121c798571fb317353ab6
Author: Mark Knoop <mark@markknoop.com>
Date:   Thu Jan 5 17:09:01 2023 +0000

    Change logic to fix partially glued regions

diff --git a/gtk2_ardour/editor_ops.cc b/gtk2_ardour/editor_ops.cc
index 00a7da8b78..1ed261d357 100644
--- a/gtk2_ardour/editor_ops.cc
+++ b/gtk2_ardour/editor_ops.cc
@@ -6550,6 +6550,12 @@ Editor::toggle_region_lock_style ()
 	for (RegionSelection::iterator i = rs.begin(); i != rs.end(); ++i) {
 		(*i)->region()->clear_changes ();
 		Temporal::TimeDomain const td = ((*i)->region()->position_time_domain() == Temporal::AudioTime && !cmi->get_inconsistent()) ? Temporal::BeatTime : Temporal::AudioTime;
+		cerr << "==================================================================" << endl;
+		if (td == Temporal::AudioTime) {
+			cerr << "Editor::toggle_region_lock_style : Region time domain set to Audio Time" << endl;
+		} else {
+			cerr << "Editor::toggle_region_lock_style : Region time domain set to Beat Time" << endl;
+		}
 		(*i)->region()->set_position_time_domain (td);
 		_session->add_command (new StatefulDiffCommand ((*i)->region()));
 	}
diff --git a/libs/ardour/region.cc b/libs/ardour/region.cc
index 62afa7cc5e..8b3a138260 100644
--- a/libs/ardour/region.cc
+++ b/libs/ardour/region.cc
@@ -583,17 +583,28 @@ Region::special_set_position (timepos_t const & pos)
 void
 Region::set_position_time_domain (Temporal::TimeDomain td)
 {
-	if (_length.val().time_domain() != td) {
+	cerr << "  Region::set_position_time_domain" << endl;
 
+	if (position().time_domain() != td) {
+
+		if (td == Temporal::AudioTime) {
+			cerr << "  Region::set_position_time_domain : Region time domain set to Audio Time" << endl;
+		} else {
+			cerr << "  Region::set_position_time_domain : Region time domain set to Beat Time" << endl;
+		}
 		/* _length is a property so we cannot directly call
 		 * ::set_time_domain() on it. Create a temporary timecnt_t,
 		 * change it's time domain, and then assign to _length
 		 */
 
+		cerr << "  Region::set_position_time_domain : old length = " << _length.val().str() << endl;
+
 		timecnt_t t (_length.val());
 		t.set_time_domain (td);
 		_length = t;
 
+		cerr << "  Region::set_position_time_domain : new length = " << _length.val().str() << endl;
+
 		send_change (Properties::time_domain);
 	}
 }
diff --git a/libs/temporal/timeline.cc b/libs/temporal/timeline.cc
index 2c2798e2e1..357373530e 100644
--- a/libs/temporal/timeline.cc
+++ b/libs/temporal/timeline.cc
@@ -155,13 +155,27 @@ timecnt_t::end (TimeDomain return_domain) const
 void
 timecnt_t::set_time_domain (TimeDomain td)
 {
+	std::cerr << "    timecnt_t::set_time_domain" << std::endl;
+
 	if (time_domain() == td) {
-		return;
+		std::cerr << "    timecnt_t::set_time_domain : time domain already set!" << std::endl;
+		//return;
+	}
+
+	if (td == Temporal::AudioTime) {
+		std::cerr << "    timecnt_t::set_time_domain : time domain set to Audio Time" << std::endl;
+	} else {
+		std::cerr << "    timecnt_t::set_time_domain : time domain set to Beat Time" << std::endl;
 	}
 
+	std::cerr << "    timecnt_t::set_time_domain : old position = " << _position.str() << std::endl;
+
 	_position.set_time_domain (td);
 
-	if (_distance.flagged()) {
+	std::cerr << "    timecnt_t::set_time_domain : new position = " << _position.str() << std::endl;
+
+	//if (_distance.flagged()) {
+	if (td == Temporal::AudioTime) {
 		/* beats -> superclock */
 		_distance = int62_t (false, TempoMap::use()->superclock_at (Beats::ticks (magnitude())));
 	} else {
debug-toggle-glue.patch (3,536 bytes)   

x42

2023-01-10 17:43

administrator   ~0027170

I can also create the inverse. ctrl+drag duplicate a music-locked MIDI region using a timecode grid. This results in "b...@a..."

x42

2023-01-10 18:18

administrator   ~0027171

The patch you suggested will allow an audio-region to have a duration specified in beats.
This will cause various issues. Beat granularity is coarser than samples. This can lead to the audio disk-reader trying to read more samples than are present.

An Audio region's length must always be given in samples, and (at this time) a MIDI region's duration in Music-time. Only the position's time domain can change.


--
This can change in the future: When Notes inside a MIDI region can use Audio-time, a MIDI region's duration can also be specified in in Audio time.
Likewise an audio region can to have Music Time duration once Arodur supports time-stretching

mpk

2023-01-10 18:31

reporter   ~0027172

Just to be clear, I didn't intend to suggest that my patch solves the problem! I just used this as a way to recover a session in which some regions had become broken (displayed as "glued", but actually with "a...@b..." lengths).

x42

2023-01-10 19:09

administrator   ~0027173

This should be fixed in Ardour 7.2-91-gf658a4c0b2 -- please test

x42

2023-01-10 19:16

administrator   ~0027174

> Just to be clear, I didn't intend to suggest that my patch solves the problem!

It was very helpful to further track down the actual issue. Thanks for that! The `timecnt_t::set_time_domain` part is still somewhat relevant.
I was thinking out loud and leaving a comment in case someone else investigates the issue. -- There are still various edge-cases related to this.

mpk

2023-01-11 09:28

reporter   ~0027176

Thanks, looks like this is working!

Issue History

Date Modified Username Field Change
2023-01-05 16:43 mpk New Issue
2023-01-05 18:04 x42 Note Added: 0027162
2023-01-05 18:24 mpk Note Added: 0027163
2023-01-05 18:24 mpk File Added: debug-toggle-glue.patch
2023-01-10 17:43 x42 Note Added: 0027170
2023-01-10 18:18 x42 Note Added: 0027171
2023-01-10 18:31 mpk Note Added: 0027172
2023-01-10 19:09 x42 Assigned To => x42
2023-01-10 19:09 x42 Status new => feedback
2023-01-10 19:09 x42 Note Added: 0027173
2023-01-10 19:16 x42 Note Added: 0027174
2023-01-11 09:28 mpk Note Added: 0027176
2023-01-11 09:28 mpk Status feedback => assigned
2023-06-17 19:57 x42 Status assigned => resolved
2023-06-17 19:57 x42 Resolution open => fixed