View Issue Details

IDProjectCategoryView StatusLast Update
0002377ardourbugspublic2020-04-19 20:13
Reporterheiko Assigned Topaul  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version2.1 
Target Version2.8.12 
Summary0002377: moving on meter change around results in unwanted sideeffects on other meter change markers
Descriptionin a project with multiple meter changes (13/4 and 7/8, in my example), moving around meter change markers results in unwanted behavior.

how to reproduce:

- i have a couple of bars in 7/8
- then after that i have some bars in 13/4
- now i move the 13/4 marker into the future some way
[so far everything is still ok]
- then i move the 13/4 marker backwards towards where it used to be
-> and that's not good - it kills lots of markers to the LEFT of where the 13/4 lands

in fact, the "killed" markers turn out to be moved entirely elsewhere.

(the project is about 4 minutes long, and there are meter change markers all the way up to about 4 hours into the project [possibly moved there by ardour's bar length recalculation(?)])
Additional Informationfor reference: this problem was discussed on freenode #ardour on 2008-08-27 between around 0:30 and 1:00am GMT between "heiko" and "oofus".
TagsNo tags attached.

Relationships

related to 0002496 confirmed Moving/creating tempo/meter markers ardour seems to move markers randomly 
related to 0002370 new Multiple tempos in session move around 

Activities

mari8i

2008-09-01 09:04

reporter   ~0005130

I think this is the same bug I reported, it has effect on meters and tempos..

I tried to solve it by my self, but after several hours I didn't find anything..

See http://tracker.ardour.org/view.php?id=2370

2008-09-16 15:40

 

ardour.patch (406 bytes)   
Index: libs/ardour/tempo.cc
===================================================================
--- libs/ardour/tempo.cc	(revision 3733)
+++ libs/ardour/tempo.cc	(working copy)
@@ -660,6 +660,7 @@
 			
 			if (prev) {
 				metric.set_start (prev->start());
+				metric.set_frame (prev->frame());
 			} else {
 				// metric will be at frames=0 bbt=1|1|0 by default
 				// which is correct for our purpose
ardour.patch (406 bytes)   

tokenrove

2008-09-16 15:42

reporter   ~0005138

Hey, this is the same bug I was going to report, too. Note that I have attached a patch which seems to fix the basic problem.

My description follows:

I am recording a lot of music with very long, elaborate click tracks.
There is a bug that has been plaguing me for a long time in using
ardour, for at least a few years, but I could never figure out what
was going on, and so my band just resorted to using a separate audio
track as a click. Still, the advantages of being able to edit in a
beat and bar oriented way caused me to always try, every ardour
release, and see if things would work better. Finally, with 2.5, I
decided to try and track down the bug for myself.

Here are some steps to reproduce it: (the numbers chosen are largely
arbitrary; I'm not sure if it can be made more minimal)

 * Start a new project. Displaying BBT will make the problem clearer.

 * Change the tempo at bar 1 to 130 bpm.

 * Add a meter change (5/4) at bar 2.

 * Add a meter change (4/4) at bar 8.

 * Move the 5/4 meter around, noting that the bar display as you drag
it will say some bizarre combinations of beats and bars. Once you
drop it, the positions of the 4/4 meter will have changed to be
something like 10 bars further than it should be.

This behavior really sucks when you have a ten minute long song with
time changes every few bars...

I didn't have a lot of time to take a look at it, and it was late last
night, but the patch I came up with is attached. I don't know enough
about the ardour internals to know if it's correct, but it seems to
work for me. Comments appreciated.

The way things are calculated in bbt_time_with_metric seem pretty iffy
to me, but at least with the attached change, the meters don't fly off
into infinity.

2008-09-17 19:44

 

ardour-tempo.patch (3,677 bytes)   
Index: libs/ardour/tempo.cc
===================================================================
--- libs/ardour/tempo.cc	(revision 3733)
+++ libs/ardour/tempo.cc	(working copy)
@@ -660,6 +660,7 @@
 			
 			if (prev) {
 				metric.set_start (prev->start());
+				metric.set_frame (prev->frame());
 			} else {
 				// metric will be at frames=0 bbt=1|1|0 by default
 				// which is correct for our purpose
@@ -800,46 +801,44 @@
 {
 	nframes_t frame_diff;
 
-	uint32_t xtra_bars = 0;
-	double xtra_beats = 0;
-	double beats = 0;
-
 	// cerr << "---- BBT time for " << frame << " using metric @ " << metric.frame() << " BBT " << metric.start() << endl;
 
 	const double beats_per_bar = metric.meter().beats_per_bar();
-	const double frames_per_bar = metric.meter().frames_per_bar (metric.tempo(), _frame_rate);
-	const double beat_frames = metric.tempo().frames_per_beat (_frame_rate, metric.meter());
+	const double ticks_per_frame = metric.tempo().frames_per_beat (_frame_rate, metric.meter()) / Meter::ticks_per_beat;
 
 	/* now compute how far beyond that point we actually are. */
 
 	frame_diff = frame - metric.frame();
-	
-	// cerr << "----\tdelta = " << frame_diff << endl;
 
-	xtra_bars = (uint32_t) floor (frame_diff / frames_per_bar);
-	frame_diff -= (uint32_t) floor (xtra_bars * frames_per_bar);
-	xtra_beats = (double) frame_diff / beat_frames;
+        bbt.ticks = metric.start().ticks + (uint32_t)round((double)frame_diff / ticks_per_frame);
+        uint32_t xtra_beats = bbt.ticks / (uint32_t)Meter::ticks_per_beat;
+        bbt.ticks %= (uint32_t)Meter::ticks_per_beat;
 
-	// cerr << "---\tmeaning " << xtra_bars << " xtra bars and " << xtra_beats << " xtra beats\n";
+        bbt.beats = metric.start().beats + xtra_beats - 1; // correction for 1-based counting, see below for matching operation.
+        bbt.bars = metric.start().bars + (uint32_t)floor((double)bbt.beats / beats_per_bar);
+        bbt.beats = (uint32_t)fmod((double)bbt.beats, beats_per_bar);
 
-	/* and set the returned value */
+        /* if we have a fractional number of beats per bar, we see if
+           we're in the last beat (the fractional one).  if so, we
+           round ticks appropriately and bump to the next bar. */
+        double beat_fraction = beats_per_bar - floor(beats_per_bar);
+        /* XXX one problem here is that I'm not sure how to handle
+           fractional beats that don't evenly divide ticks_per_beat.
+           If they aren't handled consistently, I would guess we'll
+           continue to have strange discrepancies occuring.  Perhaps
+           this will also behave badly in the case of meters like
+           0.1/4, but I can't be bothered to test that.
+        */
+        uint32_t ticks_on_last_beat = (uint32_t)floor(Meter::ticks_per_beat * beat_fraction);
+        if(bbt.beats > (uint32_t)floor(beats_per_bar) &&
+           bbt.ticks >= ticks_on_last_beat) {
+          bbt.ticks -= ticks_on_last_beat;
+          bbt.beats = 0;
+          bbt.bars++;
+        }
 
-	/* and correct beat/bar shifts to match the meter.
-	  remember: beat and bar counting is 1-based, 
-	  not zero-based 
-	  also the meter may contain a fraction
-	*/
-	
-	bbt.bars = metric.start().bars + xtra_bars; 
+        bbt.beats++; // correction for 1-based counting, see above for matching operation.
 
-	beats = (double) metric.start().beats + xtra_beats;
-	
-	bbt.bars += (uint32_t) floor(beats/ (beats_per_bar+1) );
-
-	beats = fmod(beats - 1, beats_per_bar )+ 1.0;
-	bbt.ticks = (uint32_t)( round((beats - floor(beats)) *(double) Meter::ticks_per_beat));
-	bbt.beats = (uint32_t) floor(beats);
-	
 	// cerr << "-----\t RETURN " << bbt << endl;
 }
 
ardour-tempo.patch (3,677 bytes)   

tokenrove

2008-09-17 19:45

reporter   ~0005140

Attached another patch which seems to correct the aforementioned bbt_time_with_metric behavior. Heed the warnings therein, though.

kevinc

2008-09-19 05:21

reporter   ~0005141

I just ran into what seems like the same problem. I'm using version 2.4.1. If I move a meter marker with the mouse, then it pretty much corrupts most of the meter markers I've already placed. It also renumbers the measures quite strangely. When zoomed in so that I have several horizontal screens worth of song, when I scroll left or right the measure numbers aren't continuous. When I leave one "page" with a measure number of 30-something, I enter the next "page" and the measure number jumps into the hundreds, e.g. 451. I can place, edit, and remove meter markers just fine and all is well if I don't move one with the mouse. I hope this helps.

tokenrove

2008-09-19 22:53

reporter   ~0005142

kevinc, can you try my second patch above and let me know if it works for you?

paul

2008-10-10 23:18

administrator   ~0005171

tokenrove ... shall i check out both patches or just the second one?

tokenrove

2008-10-11 01:02

reporter   ~0005172

The second one supercedes the first. I've been using it since I posted it, and it's been working quite well for me.

Thanks.

csuwi

2008-10-23 13:10

reporter   ~0005204

I've been plagued by this too so I'm glad someone's had a chance to look at it.


I've tried this patch briefly and I think things are better but there's still odd behaviour (I can't really nail it down right now) when you move between meter changes that aren't at the start of bars. That seems like a nonsensical situation but ardour allows it and then is confused by it.

I'd imagine that to handle meter changes that aren't aligned with bars we should probably have some sort of "reset" event on the timeline which marks the time between the end of the last real bar and the start of the new one as completely freeform time.

I'll continue working with a patched version and note anything concrete I can figure out.

seablade

2008-11-24 11:41

manager   ~0005331

Assigning to Paul to take a look at the patch.

    Seablade

csuwi

2009-01-15 17:50

reporter   ~0005610

Spent another couple of hours being messed around by this again. I decided to put the meter changes in by hand and noticed this:

    <Meter start="1|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="no"/>
    <Meter start="11|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>
    <Meter start="12|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
    <Meter start="49737|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>
    <Meter start="49738|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
    <Meter start="49756|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>
    <Meter start="49757|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
    <Meter start="49810|1|623" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>

Wiping out the > 49000 bar elements and then using 2.7.1 seemed to function correctly, but the default location for the inserted meter is bizarre -- not where you click. Also, can we set the Beats Per Bar to an integer? Perhaps some avant garde stuff uses fractional fractions(!) but surely having it as int/int rather than real/real is a) sufficient and b) what 99% of people actually want?

paul

2009-01-15 20:34

administrator   ~0005612

There is quite a bit of interesting non-western music that can only be properly represented in traditional tempo/meter notation with non-integer beats per bar.

The bad location of the inserted marker is likely caused by a zoom bug that someone filed separately. I don't understand what it was that you noticed in the above XML ...

csuwi

2009-01-16 15:31

reporter   ~0005616

Sorry. Bug-report in haste, clarify at leisure.

I revisited an old session with 2.7.1 and spent a while trying to massage the meter changes into it but things kept going wrong. Inserting a single 3/4 change at around bar 17 resulted in a scatter of 3/4 and 4/4 meter changes seemingly appearing. Deleting and moving as appropriate sometimes worked but often caused other mutations.

In the end I looked at the file and saw that there were spurious meter changes at e.g. 49373 -- way after the end of the track. Removing them by hand in the XML and reloading the session caused it to work perfectly (though I treated it rather gently).

csuwi

2009-01-21 16:56

reporter   ~0005623

I've just have another session (new, from scratch in 2.7.1, currently rev 4422) get totally spaced out by this bug. Three hours of carefully measured and placed time changes suddenly gone AWOL. :(

I've had a look for the "zoom bug" but can't find the one you mean. I'm seeing a problem that if you're not zoomed (and perhaps anchored to the start of the session) then right clicking on the mark or meter bar registers the click somewhere off to the left. Is that what you're referring to?

As for the time signature related stuff, it seems like something loses the plot and gets very confused about the meter. Bar numbers rocket (by a factor of about four at least sometimes) and the changes get scattered throughout the session.

This:
    <Meter start="1|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="no"/>
    <Meter start="17|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>
    <Meter start="19|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
    <Meter start="20|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>
    <Meter start="21|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
    <Meter start="22|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>

Became this:
movable="no"/>
    <Meter start="1|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="no"/>
    <Meter start="17|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>
    <Meter start="41|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
    <Meter start="60|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>
    <Meter start="86|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
    <Meter start="107|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>

Unfortunately I don't have a back up of any of the changes beyond that point.

I'm attaching a screenshot attached showing a point in the session where things go weird (zooming to different levels affected this).

And further to my comment about meter changes in the middle of a bar, they still seem more likely to cause the problem but even with snap-to-bar on I've managed to scramble the session a second time whilst writing this.

Interestingly, the shifting of the meter changes is exactly the same so it's unlikely that it's related to anything like where I click or which meter change I'm moving and to where.

This is really crippling my recording at the moment :(

2009-01-21 17:03

 

meterweirdness.png (53,259 bytes)   
meterweirdness.png (53,259 bytes)   

csuwi

2009-01-21 17:35

reporter   ~0005624

OK, I've reconstructed the session and am attaching the diff for the lines related to the meter changes.
Note that there are seemingly many things I can do which trigger the corruption but it's always exactly the same as this within this session.

2009-01-21 17:35

 

meter-changes.txt (5,950 bytes)   
-    <Tempo start="1|1|0" beats-per-minute="150.000000" note-type="4.000000" movable="no"/>
-    <Meter start="1|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="no"/>
-    <Meter start="17|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>
-    <Meter start="19|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
-    <Meter start="20|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>
-    <Meter start="21|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
-    <Meter start="22|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>
-    <Meter start="23|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
-    <Meter start="40|1|0" note-type="4.000000" beats-per-bar="7.000000" movable="yes"/>
-    <Meter start="42|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
-    <Meter start="58|1|0" note-type="4.000000" beats-per-bar="7.000000" movable="yes"/>
-    <Meter start="60|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
-    <Meter start="68|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>
-    <Meter start="70|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
-    <Meter start="71|1|0" note-type="4.000000" beats-per-bar="7.000000" movable="yes"/>
-    <Meter start="73|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
-    <Meter start="88|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>
-    <Meter start="89|1|0" note-type="4.000000" beats-per-bar="7.000000" movable="yes"/>
-    <Meter start="91|1|0" note-type="4.000000" beats-per-bar="5.000000" movable="yes"/>
-    <Meter start="149|1|0" note-type="4.000000" beats-per-bar="7.000000" movable="yes"/>
-    <Meter start="151|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
-    <Meter start="167|1|0" note-type="4.000000" beats-per-bar="7.000000" movable="yes"/>
-    <Meter start="176|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>
-    <Tempo start="177|1|0" beats-per-minute="130.000000" note-type="4.000000" movable="yes"/>
-    <Meter start="177|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
-    <Meter start="291|1|0" note-type="4.000000" beats-per-bar="5.000000" movable="yes"/>
-    <Meter start="293|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
-    <Meter start="295|1|0" note-type="4.000000" beats-per-bar="5.000000" movable="yes"/>
-    <Meter start="297|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
-    <Meter start="299|1|0" note-type="4.000000" beats-per-bar="5.000000" movable="yes"/>
-    <Meter start="301|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
-    <Meter start="303|1|0" note-type="4.000000" beats-per-bar="5.000000" movable="yes"/>
-    <Tempo start="305|1|0" beats-per-minute="150.000000" note-type="4.000000" movable="yes"/>
+    <Tempo start="1|1|0" beats-per-minute="150.000000" note-type="4.000000" movable="no"/>
+    <Meter start="1|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="no"/>
+    <Meter start="17|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>
+    <Meter start="41|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
+    <Meter start="60|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>
+    <Meter start="86|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
+    <Meter start="107|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>
+    <Meter start="135|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
+    <Meter start="173|1|0" note-type="4.000000" beats-per-bar="7.000000" movable="yes"/>
+    <Meter start="197|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
+    <Meter start="255|1|0" note-type="4.000000" beats-per-bar="7.000000" movable="yes"/>
+    <Meter start="310|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
+    <Meter start="359|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>
+    <Meter start="453|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
+    <Meter start="525|1|0" note-type="4.000000" beats-per-bar="7.000000" movable="yes"/>
+    <Meter start="568|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
+    <Meter start="658|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>
+    <Meter start="779|1|0" note-type="4.000000" beats-per-bar="7.000000" movable="yes"/>
+    <Meter start="833|1|0" note-type="4.000000" beats-per-bar="5.000000" movable="yes"/>
+    <Meter start="967|1|0" note-type="4.000000" beats-per-bar="7.000000" movable="yes"/>
+    <Meter start="1065|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
+    <Meter start="1252|1|0" note-type="4.000000" beats-per-bar="7.000000" movable="yes"/>
+    <Meter start="1368|1|0" note-type="4.000000" beats-per-bar="3.000000" movable="yes"/>
+    <Tempo start="1639|1|0" beats-per-minute="130.000000" note-type="4.000000" movable="yes"/>
+    <Meter start="1874|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
+    <Meter start="2144|1|0" note-type="4.000000" beats-per-bar="5.000000" movable="yes"/>
+    <Meter start="2381|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
+    <Meter start="2655|1|0" note-type="4.000000" beats-per-bar="5.000000" movable="yes"/>
+    <Meter start="2899|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
+    <Meter start="3234|1|0" note-type="4.000000" beats-per-bar="5.000000" movable="yes"/>
+    <Meter start="3526|1|0" note-type="4.000000" beats-per-bar="4.000000" movable="yes"/>
+    <Meter start="3921|1|0" note-type="4.000000" beats-per-bar="5.000000" movable="yes"/>
+    <Tempo start="4262|1|0" beats-per-minute="150.000000" note-type="4.000000" movable="yes"/>
meter-changes.txt (5,950 bytes)   

tokenrove

2009-01-21 17:53

reporter   ~0005626

Strange, I'm using 2.7.1 from 4297 and it seems to be working fine for me (meter changes every few bars, 10-15 minute long songs), although these days I'm not doing much rearrangement of the meters, usually just adding them sequentially. Can you reduce it to a more trivial case where a single repeatable action demonstrates the fault? I see you have some 7s; it might be worth testing with those, as it was a bar of 5 that broke the original code consistently, and I think I tested the patch mostly on combinations of 3, 4, 5, and 9.

tokenrove

2009-01-21 18:06

reporter   ~0005627

Just an addendum; does this have anything to do with removing meters or tempos in the middle of a project? Just toying with it now, that seems to be the only case for me where things get strange -- I think there's (at least) a display bug where the bars:beats line gets really messed up when you do that.

csuwi

2009-01-21 18:09

reporter   ~0005628

I haven't found an exact reproducible case, but try this:

Create a new session. Put a 7/4 change at bar 5. Put a 4/4 change at bar 7. Zoom out to about 0m00s - 3m00s. Turn off snapping. Drag 7/4 marker backwards and forwards between the two 4/4 markers.

For me eventually something happens to the right hand 4/4 marker.

If I move the 7/4 marker to the right of the 4/4 and continue fiddling eventually I also get the 4/4 to the left of the 7/4 jumping when I move the 7/4 right -- i.e. in a way that can't possibly be expected to influence it.

I think part of the problem I'm getting with making something perfectly reproducible is that the corruption happens in memory and a later redraw shows it. So whatever causes it is an operation or two before the one where I notice it.

Let me know whether you get anything.

csuwi

2009-01-21 18:15

reporter   ~0005629

The stripped down case above is just moving things.

I'd guess that things are more complicated behind the scenes than one would first expect and I'm seeing a combination of unexpected but by-design behaviour and genuine bugs.

What does it mean to move a time signature change a bar later. How should later meter changes react? What if you move the meter change 10 bars to the right. Does the user mean 10 bars, or 20 seconds, or To The Start Of This Region?

And what does it mean to have a meter change halfway through a bar? To me that's nonsensical (what you mean is a meter change in the previous bar and a different one here) but perhaps the code treats it one way and aligned-to-bar meter changes another, for the purposes of realigning any subsequent chances?


But to remphasise -- I've had this happen even when I've got snap-to-bar on so it's possible to blow things up even without intra-bar meter changes.

tokenrove

2009-01-21 18:44

reporter   ~0005630

I see what you mean (I find the 4/4 at bar 7 inevitably gets pushed to bar 8 once I put the 7/4 in the middle of a bar and then move it out), although it's part of the behavior that I decided I wasn't going to try and figure out, since, like you, I don't use (non-trivial) fractional meters, and figured that whoever does can have a long, hard think about it when they come to it. I think one treads down a dangerous path as soon as you're placing a meter change anywhere but the first beat of a bar.

I've never had the snapped meters behave badly, but I know that the snap to bar can behave strangely; it might be worth taking a closer look at how bbt_time_with_metric() interacts with TempoMap::round_to_type() in your cases; I seem to recall I thought there might be a problem there when I was originally patching it, but I decided to keep my changes as minimal as possible (as the current behavior has worked for me so far). Uncommenting the diagnostic messages on stderr in bbt_time_with_metric() is pretty much essential for debugging any of these problems. Also check out timestamp_metrics(), and its diagnostic cerrs, which is where the meters are probably getting "pushed". That's probably what you should do next if you want to figure out exactly what's happening, or you can wait until it bites me (probably in the middle of a long, desperate recording session) and I'm forced to fix it. ;-)

Hope this helps.

csuwi

2009-01-21 19:01

reporter   ~0005631

I'm hoping someone familiar with the code base will look at this. I haven't used C++ in ten years and though I keep meaning to get involved with ardour dev work I also keep finding I don't have the time to work my way through the code and do anything useful.

Maybe this the time to dive in and start working on it. But then again, right now I'm trying to record a great big chunk of stuff against a tight deadline so perhaps it can wait :-)

Thanks for your help so far though.

2009-03-17 17:12

 

move-meter-snap-to-bar.patch (761 bytes)   
Index: libs/ardour/tempo.cc
===================================================================
--- libs/ardour/tempo.cc	(revision 4865)
+++ libs/ardour/tempo.cc	(working copy)
@@ -304,7 +304,21 @@
 void
 TempoMap::move_meter (MeterSection& meter, const BBT_Time& when)
 {
-	if (move_metric_section (meter, when) == 0) {
+	/* a new meter always starts a new bar on the first beat */
+	BBT_Time when_rounded = when;
+	int force_state_changed = 0;
+
+	if (when_rounded.beats != 1) {
+		when_rounded.beats = 1;
+		when_rounded.bars++;
+		force_state_changed = 1;
+	}
+
+	/* new meters *always* start on a beat. */
+		
+	when_rounded.ticks = 0;
+
+	if ((move_metric_section (meter, when_rounded) == 0) || force_state_changed) {
 		StateChanged (Change (0));
 	}
 }

csuwi

2009-03-17 17:16

reporter   ~0005827

Investigating this in libs/ardour/tempo.cc I noticed that add_meter snaps to bar but move_meter doesn't. Discussions of Bartok notwithstanding I've added snap-to-bar behaviour to move_meter and so far on light testing I haven't had any further instances of tempo map corruption. It's fixing a trigger rather than the underlying cause but I've also found that 3.0 seems not to suffer from the bug anyway.

Perhaps someone else can confirm this patch works for them and it can be used as workaround for the 2.0 series?

(See brief discussion on #ardour, 17 March, 2009, 17:15pm GMT)

bigstumpi

2009-03-18 01:39

reporter   ~0005829

Tried your patch. Moving meter markers still moves tempo markers in an unwanted way. But maybe the behavior is better than before...?

csuwi

2009-03-18 10:44

reporter   ~0005830

I tried tokenrove's and heiko's cases and both seemed fine. My testing also seems fine so far, but it hasn't been exhaustive.

I can't reproduce any strange effects on tempo changes from moving time signatures around. **bigstumpi** Can you give me a specific example so I can confirm this?

What I /can/ trigger is the corruption on moving tempo markers around (again, triggered when they land mid-bar). As tempo changes mid-bar are really not an unheard of thing this is still something that needs fixing, ideally. It points to the bug being in the code that adjusts the timeline after moving things around.

I note that mid-bar tempo changes seem to trigger the code to start a new bar anyway, so even if it's a feature we want, it's broken. Checking in 3.0 I see that this behaviour still exists and, unfortunately, I have now managed to trigger the tempo map corruption there too.

I will spend a little time looking into this this morning, but the code is new to me and this stuff is a lot more subtle than you'd first think, particularly the relationship between the parallel but not tightly locked time measures of bars and beats vs seconds. What the user means when they move a marker is not necessarily obvious. We seem (from observation) to treat meters and tempos as anchored to bar/beat time iff they fall exactly on bars/beats.

Pragmatically I'd suggest that even though we want mid-bar changes they are currently broken and ought to be disabled (much like my previous patch) until we can fix the logic.

bigstumpi

2009-03-18 16:55

reporter   ~0005833

I can't give you a specific example, because it seems to appear randomly.
What I did is:
1.) create a blank Project
2.) activate "Snap to"
2.) add a meter marker [3/4 at 65,1,00]
3.) add a tempo marker [150 at 59,1,00]
4.) move the tempo marker somewhere behind the meter change
5.) move it back in front of the meter change
6.) repeat steps 4 and 5 until something strange happens (about 20 times?)

There it is. No definite behavior - sometimes both markers move somewhere else, sometimes only one. An interesting effect can be seen by "undo"ing the last two or three changes - the error has multiple steps. Perhaps this helps to find a possible reason?

csuwi

2009-03-18 18:36

reporter   ~0005834

Ah, moving tempo markers is something my patch didn't address. I've applied virtually the same patch to the move_tempo method and in my version I can't reproduce your bug despite spending about 5 minutes throwing the markers around.

Can you try that?

bigstumpi

2009-03-19 12:38

reporter   ~0005838

Last edited: 2009-03-19 12:39

Works fine for me! Thanks!
Like you said, tempo and meter mid-bar changes are no longer realizable.

At this point I can say that in my case all bugs related with markers are solved - see http://tracker.ardour.org/view.php?id=2496


- Adding a patch which combines the "move-meter" and the new "move-tempo" patch -> "move-all-snap-to-bar.patch"

2009-03-19 12:38

 

move-all-snap-to-bar.patch (1,293 bytes)   
--- libs/ardour/tempo.cc.old	2009-03-19 13:30:15.000000000 +0100
+++ libs/ardour/tempo.cc	2009-03-19 13:31:02.000000000 +0100
@@ -296,7 +296,21 @@
 void
 TempoMap::move_tempo (TempoSection& tempo, const BBT_Time& when)
 {
-	if (move_metric_section (tempo, when) == 0) {
+	/* a new tempo always starts a new bar on the first beat */
+	BBT_Time when_rounded = when;
+	int force_state_changed = 0;
+
+	if (when_rounded.beats != 1) {
+		when_rounded.beats = 1;
+		when_rounded.bars++;
+		force_state_changed = 1;
+	}
+
+	/* new tempo *always* start on a beat. */
+		
+	when_rounded.ticks = 0;
+
+	if ((move_metric_section (tempo, when_rounded) == 0) || force_state_changed) {
 		StateChanged (Change (0));
 	}
 }
@@ -304,7 +318,21 @@
 void
 TempoMap::move_meter (MeterSection& meter, const BBT_Time& when)
 {
-	if (move_metric_section (meter, when) == 0) {
+	/* a new meter always starts a new bar on the first beat */
+	BBT_Time when_rounded = when;
+	int force_state_changed = 0;
+
+	if (when_rounded.beats != 1) {
+		when_rounded.beats = 1;
+		when_rounded.bars++;
+		force_state_changed = 1;
+	}
+
+	/* new meters *always* start on a beat. */
+		
+	when_rounded.ticks = 0;
+
+	if ((move_metric_section (meter, when_rounded) == 0) || force_state_changed) {
 		StateChanged (Change (0));
 	}
 }
move-all-snap-to-bar.patch (1,293 bytes)   

nettings

2009-08-10 13:21

manager   ~0006497

problem persists in current 2.0 (r5445). which of these patches is actually in svn?

moving meter markers f$ยง&s up the time line - after dragging, bar numbers are way off. workaround is to always remove/create instead of dragging.

this bug should be upped to "MAJOR" imho - tempo maps are very hard to use as it is now.

marnen

2010-04-22 03:56

reporter   ~0007515

Yes, this should be marked as "major". It is still present with 2.8.7 (r6630) on Snow Leopard.

What seems to be going on is that the tempo start points are adding to each other. In other words, if I try to put tempo changes at bar x|1|0 and y|1|0, what I will get instead is x|1|0 and (x+y-1)|1|0 or something like that. For example, from a file where I intended to have a tempo marker at the beginning of each measure:

<TempoMap>
    <Tempo start="1|1|0" beats-per-minute="112.500000" note-type="4.000000" movable="no"/>
    <Meter start="1|1|0" note-type="4.000000" beats-per-bar="1.000000" movable="no"/>
    <Tempo start="2|1|0" beats-per-minute="109.956782" note-type="4.000000" movable="yes"/>
    <Tempo start="4|1|0" beats-per-minute="117.453835" note-type="4.000000" movable="yes"/>
    <Tempo start="7|1|0" beats-per-minute="117.453835" note-type="4.000000" movable="yes"/>
    <Tempo start="11|1|0" beats-per-minute="117.453835" note-type="4.000000" movable="yes"/>
    <Tempo start="16|1|0" beats-per-minute="112.347147" note-type="4.000000" movable="yes"/>
    <Tempo start="22|1|0" beats-per-minute="93.963068" note-type="4.000000" movable="yes"/>
    <Tempo start="28|1|0" beats-per-minute="83.354335" note-type="4.000000" movable="yes"/>
    <Tempo start="34|1|0" beats-per-minute="120.190779" note-type="4.000000" movable="yes"/>
    <Tempo start="44|1|0" beats-per-minute="132.898041" note-type="4.000000" movable="yes"/>
    <Tempo start="56|1|0" beats-per-minute="120.185320" note-type="4.000000" movable="yes"/>
....

This makes the tempo feature *completely useless*. The bug should be fixed. Now.

paul

2010-09-24 12:26

administrator   ~0009155

i've just committed the move_all patch to SVN for 2.X, so if marnen and nettings would like to try it out from SVN and confirm that this does indeed fix the issue, that would be helpful.

bartok is overrated, anyway.

bigstumpi

2010-09-27 16:48

reporter   ~0009178

I tested the behaviour with svn-7847 and it seems as the issue is fixed now!
Why did the commit take so much time? The patch was uploaded over a year ago...

I think the two related issues could be closed, too.

paul

2010-09-27 17:16

administrator   ~0009179

the commit took time because i still don't believe that its the correct answer, but i'd rather see the clearly incorrect behaviour go away.

also, you might not have noticed but there are thousands of bug reports here, and most development work has shifted to 3.X, which also makes it more likely that a patch for 2.X will sit around for too long.

paul

2010-09-27 17:16

administrator   ~0009180

see notes.

efti

2010-09-28 13:05

reporter   ~0009184

Hi Paul!

Thanks for committing this. I have discovered this patch not long ago and applied it to 2.8.11 where it seemed to fix the issue (at least I have not encountered it since). I did not try the latest svn yet.

Hope this helps.

system

2020-04-19 20:13

developer   ~0021783

Issue has been closed automatically, by Trigger Close Plugin.
Feel free to re-open with additional information if you think the issue is not resolved.

Issue History

Date Modified Username Field Change
2008-08-27 00:05 heiko New Issue
2008-09-01 09:04 mari8i Note Added: 0005130
2008-09-16 15:40 tokenrove File Added: ardour.patch
2008-09-16 15:42 tokenrove Note Added: 0005138
2008-09-17 19:44 tokenrove File Added: ardour-tempo.patch
2008-09-17 19:45 tokenrove Note Added: 0005140
2008-09-19 05:21 kevinc Note Added: 0005141
2008-09-19 22:53 tokenrove Note Added: 0005142
2008-10-10 23:18 paul Note Added: 0005171
2008-10-11 01:02 tokenrove Note Added: 0005172
2008-10-23 13:10 csuwi Note Added: 0005204
2008-11-24 11:41 seablade Status new => assigned
2008-11-24 11:41 seablade Assigned To => paul
2008-11-24 11:41 seablade Note Added: 0005331
2009-01-15 17:50 csuwi Note Added: 0005610
2009-01-15 20:34 paul Note Added: 0005612
2009-01-16 15:31 csuwi Note Added: 0005616
2009-01-21 16:56 csuwi Note Added: 0005623
2009-01-21 17:03 csuwi File Added: meterweirdness.png
2009-01-21 17:35 csuwi Note Added: 0005624
2009-01-21 17:35 csuwi File Added: meter-changes.txt
2009-01-21 17:53 tokenrove Note Added: 0005626
2009-01-21 18:06 tokenrove Note Added: 0005627
2009-01-21 18:09 csuwi Note Added: 0005628
2009-01-21 18:15 csuwi Note Added: 0005629
2009-01-21 18:44 tokenrove Note Added: 0005630
2009-01-21 19:01 csuwi Note Added: 0005631
2009-01-22 00:08 cth103 Relationship added related to 0002496
2009-03-17 17:12 csuwi File Added: move-meter-snap-to-bar.patch
2009-03-17 17:16 csuwi Note Added: 0005827
2009-03-18 01:39 bigstumpi Note Added: 0005829
2009-03-18 10:44 csuwi Note Added: 0005830
2009-03-18 16:55 bigstumpi Note Added: 0005833
2009-03-18 18:36 csuwi Note Added: 0005834
2009-03-19 12:38 bigstumpi Note Added: 0005838
2009-03-19 12:38 bigstumpi File Added: move-all-snap-to-bar.patch
2009-03-19 12:39 bigstumpi Note Edited: 0005838
2009-08-10 13:21 nettings Note Added: 0006497
2010-04-22 03:56 marnen Note Added: 0007515
2010-04-22 12:38 cth103 Relationship added related to 0002370
2010-04-24 10:28 cth103 Category bugs => bugs2
2010-04-24 10:29 cth103 Category bugs2 => bugs
2010-09-20 21:17 cth103 cost => 0.00
2010-09-20 21:17 cth103 Target Version => 2.8.12
2010-09-24 12:26 paul Note Added: 0009155
2010-09-24 12:26 paul Status assigned => feedback
2010-09-27 16:48 bigstumpi Note Added: 0009178
2010-09-27 17:16 paul Note Added: 0009179
2010-09-27 17:16 paul Note Added: 0009180
2010-09-27 17:16 paul Status feedback => resolved
2010-09-27 17:16 paul Resolution open => fixed
2010-09-28 13:05 efti Note Added: 0009184
2020-04-19 20:13 system Note Added: 0021783
2020-04-19 20:13 system Status resolved => closed