From 1bbe08af1a44f46c13873b762decc7629274929d Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sun, 11 Dec 2011 14:50:36 +0000 Subject: [PATCH] Keep track of MIDI region's start positions in beats, to match the source, so that starts are not corrupted when tempos change (#4494). git-svn-id: svn://localhost/ardour2/branches/3.0@10976 d708f5d6-7413-0410-9779-e7cbd77b26cf --- libs/ardour/ardour/midi_region.h | 5 + libs/ardour/ardour/region.h | 2 +- libs/ardour/ardour/tempo.h | 1 + libs/ardour/midi_region.cc | 32 ++++- libs/ardour/playlist.cc | 2 +- libs/ardour/region.cc | 2 +- libs/ardour/tempo.cc | 135 ++++++++++++++++-- libs/ardour/test/framepos_minus_beats_test.cc | 134 +++++++++++++++++ libs/ardour/test/framepos_minus_beats_test.h | 21 +++ libs/ardour/test/framepos_plus_beats_test.cc | 4 + libs/ardour/test/framewalk_to_beats_test.cc | 10 +- libs/ardour/wscript | 1 + 12 files changed, 332 insertions(+), 17 deletions(-) create mode 100644 libs/ardour/test/framepos_minus_beats_test.cc create mode 100644 libs/ardour/test/framepos_minus_beats_test.h diff --git a/libs/ardour/ardour/midi_region.h b/libs/ardour/ardour/midi_region.h index 9e8c7441fc..a5c9e23a0f 100644 --- a/libs/ardour/ardour/midi_region.h +++ b/libs/ardour/ardour/midi_region.h @@ -35,6 +35,7 @@ namespace ARDOUR { MidiModel used by the MidiRegion */ extern PBD::PropertyDescriptor midi_data; + extern PBD::PropertyDescriptor start_beats; extern PBD::PropertyDescriptor length_beats; } } @@ -111,6 +112,7 @@ class MidiRegion : public Region private: friend class RegionFactory; + PBD::Property _start_beats; PBD::Property _length_beats; MidiRegion (const SourceList&); @@ -138,6 +140,9 @@ class MidiRegion : public Region void model_automation_state_changed (Evoral::Parameter const &); void model_contents_changed (); + void set_start_beats_from_start_frames (); + void update_after_tempo_map_change (); + std::set _filtered_parameters; ///< parameters that we ask our source not to return when reading PBD::ScopedConnection _model_connection; PBD::ScopedConnection _source_connection; diff --git a/libs/ardour/ardour/region.h b/libs/ardour/ardour/region.h index a2cc3f4cf7..8be4709fbf 100644 --- a/libs/ardour/ardour/region.h +++ b/libs/ardour/ardour/region.h @@ -189,7 +189,7 @@ class Region void set_position (framepos_t); void set_position_on_top (framepos_t); void special_set_position (framepos_t); - void update_position_after_tempo_map_change (); + virtual void update_after_tempo_map_change (); void nudge_position (frameoffset_t); bool at_natural_position () const; diff --git a/libs/ardour/ardour/tempo.h b/libs/ardour/ardour/tempo.h index a6f7a36541..05d65602cf 100644 --- a/libs/ardour/ardour/tempo.h +++ b/libs/ardour/ardour/tempo.h @@ -249,6 +249,7 @@ class TempoMap : public PBD::StatefulDestructible framepos_t framepos_plus_bbt (framepos_t pos, Timecode::BBT_Time b) const; framepos_t framepos_plus_beats (framepos_t, Evoral::MusicalTime) const; + framepos_t framepos_minus_beats (framepos_t, Evoral::MusicalTime) const; Evoral::MusicalTime framewalk_to_beats (framepos_t pos, framecnt_t distance) const; void change_existing_tempo_at (framepos_t, double bpm, double note_type); diff --git a/libs/ardour/midi_region.cc b/libs/ardour/midi_region.cc index f501390c24..3dd0e1e203 100644 --- a/libs/ardour/midi_region.cc +++ b/libs/ardour/midi_region.cc @@ -40,6 +40,7 @@ #include "ardour/playlist.h" #include "ardour/region_factory.h" #include "ardour/session.h" +#include "ardour/tempo.h" #include "ardour/types.h" #include "i18n.h" @@ -52,6 +53,7 @@ using namespace PBD; namespace ARDOUR { namespace Properties { PBD::PropertyDescriptor midi_data; + PBD::PropertyDescriptor start_beats; PBD::PropertyDescriptor length_beats; } } @@ -61,6 +63,8 @@ MidiRegion::make_property_quarks () { Properties::midi_data.property_id = g_quark_from_static_string (X_("midi-data")); DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for midi-data = %1\n", Properties::midi_data.property_id)); + Properties::start_beats.property_id = g_quark_from_static_string (X_("start-beats")); + DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for start-beats = %1\n", Properties::start_beats.property_id)); Properties::length_beats.property_id = g_quark_from_static_string (X_("length-beats")); DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for length-beats = %1\n", Properties::length_beats.property_id)); } @@ -68,12 +72,14 @@ MidiRegion::make_property_quarks () void MidiRegion::register_properties () { + add_property (_start_beats); add_property (_length_beats); } /* Basic MidiRegion constructor (many channels) */ MidiRegion::MidiRegion (const SourceList& srcs) : Region (srcs) + , _start_beats (Properties::start_beats, 0) , _length_beats (Properties::length_beats, midi_source(0)->length_beats()) { register_properties (); @@ -86,6 +92,7 @@ MidiRegion::MidiRegion (const SourceList& srcs) MidiRegion::MidiRegion (boost::shared_ptr other) : Region (other) + , _start_beats (Properties::start_beats, other->_start_beats) , _length_beats (Properties::length_beats, (Evoral::MusicalTime) 0) { update_length_beats (); @@ -96,14 +103,16 @@ MidiRegion::MidiRegion (boost::shared_ptr other) model_changed (); } -/** Create a new MidiRegion, that is part of an existing one */ +/** Create a new MidiRegion that is part of an existing one */ MidiRegion::MidiRegion (boost::shared_ptr other, frameoffset_t offset) : Region (other, offset) + , _start_beats (Properties::start_beats, (Evoral::MusicalTime) 0) , _length_beats (Properties::length_beats, (Evoral::MusicalTime) 0) { BeatsFramesConverter bfc (_session.tempo_map(), _position); Evoral::MusicalTime const offset_beats = bfc.from (offset); + _start_beats = other->_start_beats + offset_beats; _length_beats = other->_length_beats - offset_beats; register_properties (); @@ -133,6 +142,7 @@ MidiRegion::clone () const plist.add (Properties::name, ms->name()); plist.add (Properties::whole_file, true); plist.add (Properties::start, _start); + plist.add (Properties::start_beats, _start_beats); plist.add (Properties::length, _length); plist.add (Properties::length_beats, _length_beats); plist.add (Properties::layer, 0); @@ -145,9 +155,18 @@ MidiRegion::post_set (const PropertyChange& pc) { if (pc.contains (Properties::length) && !pc.contains (Properties::length_beats)) { update_length_beats (); + } else if (pc.contains (Properties::start) && !pc.contains (Properties::start_beats)) { + set_start_beats_from_start_frames (); } } +void +MidiRegion::set_start_beats_from_start_frames () +{ + BeatsFramesConverter c (_session.tempo_map(), _position - _start); + _start_beats = c.from (_start); +} + void MidiRegion::set_length_internal (framecnt_t len) { @@ -155,6 +174,17 @@ MidiRegion::set_length_internal (framecnt_t len) update_length_beats (); } +void +MidiRegion::update_after_tempo_map_change () +{ + Region::update_after_tempo_map_change (); + + /* _position has now been updated for the new tempo map */ + _start = _position - _session.tempo_map().framepos_minus_beats (_position, _start_beats); + + send_change (Properties::start); +} + void MidiRegion::update_length_beats () { diff --git a/libs/ardour/playlist.cc b/libs/ardour/playlist.cc index 4aae95ea76..2586a49fc4 100644 --- a/libs/ardour/playlist.cc +++ b/libs/ardour/playlist.cc @@ -2979,7 +2979,7 @@ Playlist::update_after_tempo_map_change () freeze (); for (RegionList::iterator i = copy.begin(); i != copy.end(); ++i) { - (*i)->update_position_after_tempo_map_change (); + (*i)->update_after_tempo_map_change (); } thaw (); diff --git a/libs/ardour/region.cc b/libs/ardour/region.cc index 71aca0cce6..43a4270501 100644 --- a/libs/ardour/region.cc +++ b/libs/ardour/region.cc @@ -559,7 +559,7 @@ Region::set_position_lock_style (PositionLockStyle ps) } void -Region::update_position_after_tempo_map_change () +Region::update_after_tempo_map_change () { boost::shared_ptr pl (playlist()); diff --git a/libs/ardour/tempo.cc b/libs/ardour/tempo.cc index f0880fed91..351b5979e6 100644 --- a/libs/ardour/tempo.cc +++ b/libs/ardour/tempo.cc @@ -1935,19 +1935,30 @@ TempoMap::bbt_subtract (const BBT_Time& start, const BBT_Time& decrement) const return result; } -/** Add some (fractional) beats to a frame position, and return the result in frames */ +/** Add some (fractional) beats to a session frame position, and return the result in frames. + * pos can be -ve, if required. + */ framepos_t TempoMap::framepos_plus_beats (framepos_t pos, Evoral::MusicalTime beats) const { Metrics::const_iterator i; const TempoSection* tempo; const MeterSection* meter; - + /* Find the starting metrics for tempo & meter */ for (i = metrics->begin(); i != metrics->end(); ++i) { - if ((*i)->frame() > pos) { + /* This is a bit of a hack, but pos could be -ve, and if it is, + we consider the initial metric changes (at time 0) to actually + be in effect at pos. + */ + framepos_t f = (*i)->frame (); + if (pos < 0 && f == 0) { + f = pos; + } + + if (f > pos) { break; } @@ -1970,11 +1981,11 @@ TempoMap::framepos_plus_beats (framepos_t pos, Evoral::MusicalTime beats) const while (beats) { - /* End of this section */ - framepos_t end = i == metrics->end() ? max_framepos : (*i)->frame (); + /* Distance to the end of this section in frames */ + framecnt_t distance_frames = i == metrics->end() ? max_framepos : ((*i)->frame() - pos); /* Distance to the end in beats */ - Evoral::MusicalTime distance_beats = (end - pos) / tempo->frames_per_beat (_frame_rate, *meter); + Evoral::MusicalTime distance_beats = distance_frames / tempo->frames_per_beat (_frame_rate, *meter); /* Amount to subtract this time */ double const sub = min (distance_beats, beats); @@ -2001,6 +2012,112 @@ TempoMap::framepos_plus_beats (framepos_t pos, Evoral::MusicalTime beats) const return pos; } +/** Subtract some (fractional) beats to a frame position, and return the result in frames */ +framepos_t +TempoMap::framepos_minus_beats (framepos_t pos, Evoral::MusicalTime beats) const +{ + Metrics::const_iterator i; + const TempoSection* tempo = 0; + const MeterSection* meter = 0; + + /* Find the starting metrics for tempo & meter */ + + for (i = metrics->begin(); i != metrics->end(); ++i) { + + if ((*i)->frame() > pos) { + break; + } + + const TempoSection* t; + const MeterSection* m; + + if ((t = dynamic_cast(*i)) != 0) { + tempo = t; + } else if ((m = dynamic_cast(*i)) != 0) { + meter = m; + } + } + + /* Move i back to the metric before "pos" */ + if (i != metrics->begin ()) { + --i; + } + + /* We now have: + + meter -> the Meter for "pos" + tempo -> the Tempo for "pos" + i -> the first metric before "pos", possibly metrics->end() + */ + + while (beats) { + + /* End of this section (looking backwards) */ + framepos_t end = i == metrics->end() ? max_framepos : (*i)->frame (); + + /* Distance to the end in beats */ + Evoral::MusicalTime distance_beats = (pos - end) / tempo->frames_per_beat (_frame_rate, *meter); + + /* Amount to subtract this time */ + double const sub = min (distance_beats, beats); + + /* Update */ + beats -= sub; + pos -= sub * tempo->frames_per_beat (_frame_rate, *meter); + + /* Move i, tempo and meter back, if there's anything to move to. + This is more complicated than the forward case, as we have to + a) move back to the previous change in tempo or metric + then b) scan back further to the last change in the opposite thing + so that tempo/meter are both set up correctly. + + e.g. if we have (where M is a meter change and T a tempo change): + M1 T1 T2 T3 M2 + + and we move i back to M2, we must also move tempo back to T3 so + that tempo/meter continue to reflect the current state. + + Moving further back we'd move i to T3, and meter to M1, then + i to T2 and meter (still) to M1, etc. + + XXX: this is slightly farcical. + */ + + if (i != metrics->begin ()) { + + --i; + + bool found_tempo = false; + bool found_meter = false; + + const TempoSection* t; + const MeterSection* m; + + if ((t = dynamic_cast(*i)) != 0) { + tempo = t; + found_tempo = true; + } else if ((m = dynamic_cast(*i)) != 0) { + meter = m; + found_meter = true; + } + + Metrics::const_iterator j = i; + while (j != metrics->begin ()) { + --j; + if (found_tempo && ((m = dynamic_cast (*j)) != 0)) { + meter = m; + break; + } else if (found_meter && ((t = dynamic_cast (*j)) != 0)) { + tempo = t; + break; + } + } + } + } + + return pos; +} + /** Add the BBT interval op to pos and return the result */ framepos_t TempoMap::framepos_plus_bbt (framepos_t pos, BBT_Time op) const @@ -2135,7 +2252,9 @@ TempoMap::framepos_plus_bbt (framepos_t pos, BBT_Time op) const return pos; } -/** Count the number of beats that are equivalent to distance when starting at pos */ +/** Count the number of beats that are equivalent to distance when going forward, + starting at pos. +*/ Evoral::MusicalTime TempoMap::framewalk_to_beats (framepos_t pos, framecnt_t distance) const { @@ -2165,7 +2284,7 @@ TempoMap::framewalk_to_beats (framepos_t pos, framecnt_t distance) const meter -> the Meter for "pos" tempo -> the Tempo for "pos" - i -> for first new metric after "pos", possibly metrics->end() + i -> the first metric after "pos", possibly metrics->end() */ Evoral::MusicalTime beats = 0; diff --git a/libs/ardour/test/framepos_minus_beats_test.cc b/libs/ardour/test/framepos_minus_beats_test.cc new file mode 100644 index 0000000000..f80f57d969 --- /dev/null +++ b/libs/ardour/test/framepos_minus_beats_test.cc @@ -0,0 +1,134 @@ +#include "framepos_minus_beats_test.h" +#include "ardour/tempo.h" +#include "timecode/bbt_time.h" + +CPPUNIT_TEST_SUITE_REGISTRATION (FrameposMinusBeatsTest); + +using namespace std; +using namespace ARDOUR; +using namespace Timecode; + +/* Basic tests with no tempo / meter changes */ +void +FrameposMinusBeatsTest::singleTempoTest () +{ + int const sampling_rate = 48000; + int const bpm = 120; + + double const frames_per_beat = (60 / double (bpm)) * double (sampling_rate); + + TempoMap map (sampling_rate); + Tempo tempo (bpm); + Meter meter (4, 4); + + map.add_meter (meter, BBT_Time (1, 1, 0)); + map.add_tempo (tempo, BBT_Time (1, 1, 0)); + + /* Subtract 1 beat from beat 3 of the first bar */ + framepos_t r = map.framepos_minus_beats (frames_per_beat * 2, 1); + CPPUNIT_ASSERT_EQUAL (r, framepos_t (frames_per_beat * 1)); +} + +/* Test adding things that overlap a tempo change */ +void +FrameposMinusBeatsTest::doubleTempoTest () +{ + int const sampling_rate = 48000; + + TempoMap map (sampling_rate); + Meter meter (4, 4); + map.add_meter (meter, BBT_Time (1, 1, 0)); + + /* + 120bpm at bar 1, 240bpm at bar 4 + + 120bpm = 24e3 samples per beat + 240bpm = 12e3 samples per beat + */ + + + /* + + 120bpm 240bpm + 0 beats 12 beats + 0 frames 288e3 frames + | | | | | + | 1.1 1.2 1.3 1.4 | 2.1 2.2 2.3.2.4 | 3.1 3.2 3.3 3.4 | 4.1 4.2 4.3 4.4 | + + */ + + Tempo tempoA (120); + map.add_tempo (tempoA, BBT_Time (1, 1, 0)); + Tempo tempoB (240); + map.add_tempo (tempoB, BBT_Time (4, 1, 0)); + + /* Now some tests */ + + /* Subtract 1 beat from 1|2 */ + framepos_t r = map.framepos_minus_beats (24e3, 1); + CPPUNIT_ASSERT_EQUAL (r, framepos_t (0)); + + /* Subtract 2 beats from 4|2 (over the tempo change) */ + r = map.framepos_minus_beats (288e3 + 12e3, 2); + CPPUNIT_ASSERT_EQUAL (r, framepos_t (288e3 - 24e3)); + + /* Subtract 2.5 beats from 4|2 (over the tempo change) */ + r = map.framepos_minus_beats (288e3 + 12e3, 2.5); + CPPUNIT_ASSERT_EQUAL (r, framepos_t (288e3 - 24e3 - 12e3)); +} + +/* Same as doubleTempoTest () except put a meter change at the same time as the + tempo change (which shouldn't affect anything, since we are just dealing with + beats) +*/ + +void +FrameposMinusBeatsTest::doubleTempoWithMeterTest () +{ + int const sampling_rate = 48000; + + TempoMap map (sampling_rate); + Meter meterA (4, 4); + map.add_meter (meterA, BBT_Time (1, 1, 0)); + + /* + 120bpm at bar 1, 240bpm at bar 4 + + 120bpm = 24e3 samples per beat + 240bpm = 12e3 samples per beat + */ + + + /* + + 120bpm 240bpm + 0 beats 12 beats + 0 frames 288e3 frames + | | | | | + | 1.1 1.2 1.3 1.4 | 2.1 2.2 2.3.2.4 | 3.1 3.2 3.3 3.4 | 4.1 4.2 4.3 | + + */ + + Tempo tempoA (120); + map.add_tempo (tempoA, BBT_Time (1, 1, 0)); + Tempo tempoB (240); + map.add_tempo (tempoB, BBT_Time (4, 1, 0)); + Meter meterB (3, 4); + map.add_meter (meterB, BBT_Time (4, 1, 0)); + + /* Now some tests */ + + /* Subtract 1 beat from 1|2 */ + framepos_t r = map.framepos_minus_beats (24e3, 1); + CPPUNIT_ASSERT_EQUAL (r, framepos_t (0)); + + /* Subtract 2 beats from 4|2 (over the tempo change) */ + r = map.framepos_minus_beats (288e3 + 12e3, 2); + CPPUNIT_ASSERT_EQUAL (r, framepos_t (288e3 - 24e3)); + + /* Subtract 2.5 beats from 4|2 (over the tempo change) */ + r = map.framepos_minus_beats (288e3 + 12e3, 2.5); + CPPUNIT_ASSERT_EQUAL (r, framepos_t (288e3 - 24e3 - 12e3)); +} + + diff --git a/libs/ardour/test/framepos_minus_beats_test.h b/libs/ardour/test/framepos_minus_beats_test.h new file mode 100644 index 0000000000..648ef71918 --- /dev/null +++ b/libs/ardour/test/framepos_minus_beats_test.h @@ -0,0 +1,21 @@ +#include +#include +#include + +class FrameposMinusBeatsTest : public CppUnit::TestFixture +{ + CPPUNIT_TEST_SUITE (FrameposMinusBeatsTest); + CPPUNIT_TEST (singleTempoTest); + CPPUNIT_TEST (doubleTempoTest); + CPPUNIT_TEST (doubleTempoWithMeterTest); + CPPUNIT_TEST_SUITE_END (); + +public: + void setUp () {} + void tearDown () {} + + void singleTempoTest (); + void doubleTempoTest (); + void doubleTempoWithMeterTest (); +}; + diff --git a/libs/ardour/test/framepos_plus_beats_test.cc b/libs/ardour/test/framepos_plus_beats_test.cc index 19aa7f29c6..882b6b5721 100644 --- a/libs/ardour/test/framepos_plus_beats_test.cc +++ b/libs/ardour/test/framepos_plus_beats_test.cc @@ -27,6 +27,10 @@ FrameposPlusBeatsTest::singleTempoTest () /* Add 1 beat to beat 3 of the first bar */ framepos_t r = map.framepos_plus_beats (frames_per_beat * 2, 1); CPPUNIT_ASSERT_EQUAL (r, framepos_t (frames_per_beat * 3)); + + /* Add 4 beats to a -ve frame of 1 beat before zero */ + r = map.framepos_plus_beats (-frames_per_beat * 1, 4); + CPPUNIT_ASSERT_EQUAL (r, framepos_t (frames_per_beat * 3)); } /* Test adding things that overlap a tempo change */ diff --git a/libs/ardour/test/framewalk_to_beats_test.cc b/libs/ardour/test/framewalk_to_beats_test.cc index 7e50aedcb5..2b2a5a782c 100644 --- a/libs/ardour/test/framewalk_to_beats_test.cc +++ b/libs/ardour/test/framewalk_to_beats_test.cc @@ -74,23 +74,23 @@ FramewalkToBeatsTest::doubleTempoTest () /* Now some tests */ - /* Walk 1 beat from 1.2 */ + /* Walk 1 beat from 1|2 */ double r = map.framewalk_to_beats (24e3, 24e3); CPPUNIT_ASSERT_EQUAL (r, 1.0); - /* Walk 2 beats from 3.3 to 4.1 (over the tempo change) */ + /* Walk 2 beats from 3|3 to 4|1 (over the tempo change) */ r = map.framewalk_to_beats (264e3, (24e3 + 12e3)); CPPUNIT_ASSERT_EQUAL (r, 2.0); - /* Walk 2.5 beats from 3.3-and-a-half to 4.2 (over the tempo change) */ + /* Walk 2.5 beats from 3|3.5 to 4.2 (over the tempo change) */ r = map.framewalk_to_beats (264e3 - 12e3, (24e3 + 12e3 + 12e3)); CPPUNIT_ASSERT_EQUAL (r, 2.5); - /* Walk 3 beats from 3.4-and-a-half to 4.3-and-a-half (over the tempo change) */ + /* Walk 3 beats from 3|4.5 to 4|3.5 (over the tempo change) */ r = map.framewalk_to_beats (264e3 - 12e3, (24e3 + 12e3 + 12e3 + 6e3)); CPPUNIT_ASSERT_EQUAL (r, 3.0); - /* Walk 3.5 beats from 3.4-and-a-half to 4.4 (over the tempo change) */ + /* Walk 3.5 beats from 3|4.5 to 4.4 (over the tempo change) */ r = map.framewalk_to_beats (264e3 - 12e3, (24e3 + 12e3 + 12e3 + 12e3)); CPPUNIT_ASSERT_EQUAL (r, 3.5); } diff --git a/libs/ardour/wscript b/libs/ardour/wscript index 0eb4f0ceb6..1c26a16fd0 100644 --- a/libs/ardour/wscript +++ b/libs/ardour/wscript @@ -429,6 +429,7 @@ def build(bld): test/resampled_source_test.cc test/framewalk_to_beats_test.cc test/framepos_plus_beats_test.cc + test/framepos_minus_beats_test.cc test/testrunner.cc '''.split()