Remove confuzzling offset_relative stuff from region construction (pre-properties...
authorDavid Robillard <d@drobilla.net>
Tue, 21 Dec 2010 17:03:16 +0000 (17:03 +0000)
committerDavid Robillard <d@drobilla.net>
Tue, 21 Dec 2010 17:03:16 +0000 (17:03 +0000)
This commit (in theory) only reorganizes code, not change actual functionality.
RegionFactory now uses a distinct Region constructor for each case, which is a bit easier to wrap around.
Note comment at region.cc:276, this case seems pretty weird to me (more hangover?).

git-svn-id: svn://localhost/ardour2/branches/3.0@8320 d708f5d6-7413-0410-9779-e7cbd77b26cf

libs/ardour/ardour/audioregion.h
libs/ardour/ardour/midi_region.h
libs/ardour/ardour/region.h
libs/ardour/ardour/region_factory.h
libs/ardour/audioregion.cc
libs/ardour/crossfade.cc
libs/ardour/midi_region.cc
libs/ardour/region.cc
libs/ardour/region_factory.cc

index 07766698747d3b9a7aa1ce3c1ea0ef29cd28c902..8d929827c5b00a3c40ed57af0888c3df8d367f97 100644 (file)
@@ -183,7 +183,8 @@ class AudioRegion : public Region
 
        AudioRegion (boost::shared_ptr<AudioSource>);
        AudioRegion (const SourceList &);
-       AudioRegion (boost::shared_ptr<const AudioRegion>, frameoffset_t offset = 0, bool offset_relative = true);
+       AudioRegion (boost::shared_ptr<const AudioRegion>);
+       AudioRegion (boost::shared_ptr<const AudioRegion>, frameoffset_t offset);
        AudioRegion (boost::shared_ptr<const AudioRegion>, const SourceList&);
        AudioRegion (SourceList &);
 
index edd9ef31e40cb1990c9ff6cab8a4a3aea8cd17a2..016536b8b47a54444aa846a55aff42bad2d5e559 100644 (file)
@@ -121,7 +121,8 @@ class MidiRegion : public Region
        PBD::Property<Evoral::MusicalTime> _length_beats;
 
        MidiRegion (const SourceList&);
-       MidiRegion (boost::shared_ptr<const MidiRegion>, frameoffset_t offset = 0, bool offset_relative = true);
+       MidiRegion (boost::shared_ptr<const MidiRegion>);
+       MidiRegion (boost::shared_ptr<const MidiRegion>, frameoffset_t offset);
 
        framecnt_t _read_at (const SourceList&, Evoral::EventSink<framepos_t>& dst,
                             framepos_t position,
index 63ac879eeb276fba6e5b78fa6aa31bd103ebee43..3bb5d0aac8746d995820dbfa32763b003a6d49b3 100644 (file)
@@ -307,8 +307,12 @@ class Region
        /** Construct a region from multiple sources*/
        Region (const SourceList& srcs);
 
+       /** Construct a region from another region */
+       Region (boost::shared_ptr<const Region>);
+
        /** Construct a region from another region, at an offset within that region */
-       Region (boost::shared_ptr<const Region>, frameoffset_t start_offset = 0, bool start_relative = true);
+       Region (boost::shared_ptr<const Region>, frameoffset_t start_offset);
+       
        /** Construct a region as a copy of another region, but with different sources */
        Region (boost::shared_ptr<const Region>, const SourceList&);
 
index af6c2759cf7b25e0efe3d3924b24b546336da9a5..b47ba71cc9bfedae156244c3ae706603ed377e93 100644 (file)
@@ -92,10 +92,6 @@ public:
   
   private:
 
-       static boost::shared_ptr<Region> create (boost::shared_ptr<Region>, frameoffset_t offset,
-                                                bool offset_relative,
-                                                const PBD::PropertyList&, bool announce = true);
-
        static void region_changed (PBD::PropertyChange const &, boost::weak_ptr<Region>);
        
        static Glib::StaticMutex region_map_lock;
index f3a5be9da64ef5229731329cd517f38fa208a266..cb3e00cea991bf141ebebbb92f9b5bcd42aea458 100644 (file)
@@ -158,8 +158,32 @@ AudioRegion::AudioRegion (const SourceList& srcs)
        assert (_sources.size() == _master_sources.size());
 }
 
-AudioRegion::AudioRegion (boost::shared_ptr<const AudioRegion> other, framecnt_t offset, bool offset_relative)
-       : Region (other, offset, offset_relative)
+AudioRegion::AudioRegion (boost::shared_ptr<const AudioRegion> other)
+       : Region (other)
+       , AUDIOREGION_COPY_STATE (other)
+       , _automatable (other->session())
+       , _fade_in (new AutomationList (*other->_fade_in))
+       , _fade_out (new AutomationList (*other->_fade_out))
+         /* As far as I can see, the _envelope's times are relative to region position, and have nothing
+            to do with sources (and hence _start).  So when we copy the envelope, we just use the supplied offset.
+         */
+       , _envelope (new AutomationList (*other->_envelope, 0, other->_length))
+       , _fade_in_suspended (0)
+       , _fade_out_suspended (0)
+{
+       /* don't use init here, because we got fade in/out from the other region
+       */
+       register_properties ();
+       listen_to_my_curves ();
+       connect_to_analysis_changed ();
+       connect_to_header_position_offset_changed ();
+
+       assert(_type == DataType::AUDIO);
+       assert (_sources.size() == _master_sources.size());
+}
+
+AudioRegion::AudioRegion (boost::shared_ptr<const AudioRegion> other, framecnt_t offset)
+       : Region (other, offset)
        , AUDIOREGION_COPY_STATE (other)
        , _automatable (other->session())
        , _fade_in (new AutomationList (*other->_fade_in))
index b22dd15c9cf4e31ed2d6eb972ae3a4d1c118f1d4..a988585b906e24c3d66bc46b1fe620ed3ade8895 100644 (file)
@@ -197,7 +197,7 @@ Crossfade::Crossfade (const Playlist& playlist, XMLNode const & node)
 }
 
 Crossfade::Crossfade (boost::shared_ptr<Crossfade> orig, boost::shared_ptr<AudioRegion> newin, boost::shared_ptr<AudioRegion> newout)
-       : AudioRegion (boost::dynamic_pointer_cast<const AudioRegion> (orig), 0, true)
+       : AudioRegion (boost::dynamic_pointer_cast<const AudioRegion> (orig), 0)
        , CROSSFADE_DEFAULT_PROPERTIES
        , _fade_in (orig->_fade_in)
        , _fade_out (orig->_fade_out)
index 4b65e44e220cdf8ce2ef674b4ddb0ce46ff5ab58..afae8eadc8b345c9f9a8cb1336f6f0171986134c 100644 (file)
@@ -83,8 +83,21 @@ MidiRegion::MidiRegion (const SourceList& srcs)
 }
 
 /** Create a new MidiRegion, that is part of an existing one */
-MidiRegion::MidiRegion (boost::shared_ptr<const MidiRegion> other, frameoffset_t offset, bool offset_relative)
-       : Region (other, offset, offset_relative)
+MidiRegion::MidiRegion (boost::shared_ptr<const MidiRegion> other)
+       : Region (other)
+       , _length_beats (Properties::length_beats, (Evoral::MusicalTime) 0)
+{
+       update_length_beats ();
+       register_properties ();
+
+       assert(_name.val().find("/") == string::npos);
+       midi_source(0)->ModelChanged.connect_same_thread (_source_connection, boost::bind (&MidiRegion::model_changed, this));
+       model_changed ();
+}
+
+/** Create a new MidiRegion, that is part of an existing one */
+MidiRegion::MidiRegion (boost::shared_ptr<const MidiRegion> other, frameoffset_t offset)
+       : Region (other, offset)
        , _length_beats (Properties::length_beats, (Evoral::MusicalTime) 0)
 {
        update_length_beats ();
index 6a2a6f1e305190b10f19a0bba5eb55379e722674..5a8970749553a56777071cf8833f82768fe3ea95 100644 (file)
@@ -245,14 +245,8 @@ Region::Region (const SourceList& srcs)
        assert (_type == srcs.front()->type());
 }
 
-/** Create a new Region from part of an existing one, starting at one of two places:
-
-    if \a offset_relative is true, then the start within \a other is given by \a offset
-    (i.e. relative to the start of \a other's sources, the start is \a offset + \a other.start()
-
-    if @param offset_relative is false, then the start within the source is given \a offset.
-*/
-Region::Region (boost::shared_ptr<const Region> other, frameoffset_t offset, bool offset_relative)
+/** Create a new Region from an existing one */
+Region::Region (boost::shared_ptr<const Region> other)
        : SessionObject(other->session(), other->name())
        , _type (other->data_type())
        , REGION_COPY_STATE (other)
@@ -276,70 +270,96 @@ Region::Region (boost::shared_ptr<const Region> other, frameoffset_t offset, boo
 
        use_sources (other->_sources);
 
-       if (!offset_relative) {
-
-               /* not sure why we do this, but its a hangover from ardour before
-                  property lists. this would be nice to remove.
-               */
-
-               _position_lock_style = other->_position_lock_style;
-               _first_edit = other->_first_edit;
-
-               if (offset == 0) {
+       _position_lock_style = other->_position_lock_style;
+       _first_edit = other->_first_edit;
 
-                       _start = 0;
+       _start = 0; // It seems strange _start is not inherited here?
 
-                       /* sync pos is relative to start of file. our start-in-file is now zero,
-                          so set our sync position to whatever the the difference between
-                          _start and _sync_pos was in the other region.
+       /* sync pos is relative to start of file. our start-in-file is now zero,
+          so set our sync position to whatever the the difference between
+          _start and _sync_pos was in the other region.
                           
-                          result is that our new sync pos points to the same point in our source(s)
-                          as the sync in the other region did in its source(s).
+          result is that our new sync pos points to the same point in our source(s)
+          as the sync in the other region did in its source(s).
                           
-                          since we start at zero in our source(s), it is not possible to use a sync point that
-                          is before the start. reset it to _start if that was true in the other region.
-                       */
+          since we start at zero in our source(s), it is not possible to use a sync point that
+          is before the start. reset it to _start if that was true in the other region.
+       */
                        
-                       if (other->sync_marked()) {
-                               if (other->_start < other->_sync_position) {
-                                       /* sync pos was after the start point of the other region */
-                                       _sync_position = other->_sync_position - other->_start;
-                               } else {
-                                       /* sync pos was before the start point of the other region. not possible here. */
-                                       _sync_marked = false;
-                                       _sync_position = _start;
-                               }
-                       } else {
-                               _sync_marked = false;
-                               _sync_position = _start;
-                       }
+       if (other->sync_marked()) {
+               if (other->_start < other->_sync_position) {
+                       /* sync pos was after the start point of the other region */
+                       _sync_position = other->_sync_position - other->_start;
                } else {
-                       /* XXX do something else ! */
-                       fatal << string_compose (_("programming error: %1"), X_("Region+offset constructor used with illegal combination of offset+relative"))
-                             << endmsg;
-                       /*NOTREACHED*/
+                       /* sync pos was before the start point of the other region. not possible here. */
+                       _sync_marked = false;
+                       _sync_position = _start;
                }
-
        } else {
+               _sync_marked = false;
+               _sync_position = _start;
+       }
 
-               _start = other->_start + offset;
-               
-               /* if the other region had a distinct sync point
-                  set, then continue to use it as best we can.
-                  otherwise, reset sync point back to start.
+       if (Profile->get_sae()) {
+               /* reset sync point to start if its ended up
+                  outside region bounds.
                */
+
+               if (_sync_position < _start || _sync_position >= _start + _length) {
+                       _sync_marked = false;
+                       _sync_position = _start;
+               }
+       }
+
+       assert (_type == other->data_type());
+}
+
+/** Create a new Region from part of an existing one.
+
+    the start within \a other is given by \a offset
+    (i.e. relative to the start of \a other's sources, the start is \a offset + \a other.start()
+*/
+Region::Region (boost::shared_ptr<const Region> other, frameoffset_t offset)
+       : SessionObject(other->session(), other->name())
+       , _type (other->data_type())
+       , REGION_COPY_STATE (other)
+       , _last_length (other->_last_length)
+       , _last_position(other->_last_position) \
+       , _first_edit (EditChangesNothing)
+       , _read_data_count(0)
+       , _last_layer_op (0)
+       , _pending_explicit_relayer (false)
+
+{
+       register_properties ();
+
+       /* override state that may have been incorrectly inherited from the other region
+        */
+
+       _position = 0;
+       _locked = false;
+       _whole_file = false;
+       _hidden = false;
+
+       use_sources (other->_sources);
+
+       _start = other->_start + offset;
                
-               if (other->sync_marked()) {
-                       if (other->_sync_position < _start) {
-                               _sync_marked = false;
-                               _sync_position = _start;
-                       } else {
-                               _sync_position = other->_sync_position;
-                       }
-               } else {
+       /* if the other region had a distinct sync point
+          set, then continue to use it as best we can.
+          otherwise, reset sync point back to start.
+       */
+               
+       if (other->sync_marked()) {
+               if (other->_sync_position < _start) {
                        _sync_marked = false;
                        _sync_position = _start;
+               } else {
+                       _sync_position = other->_sync_position;
                }
+       } else {
+               _sync_marked = false;
+               _sync_position = _start;
        }
 
        if (Profile->get_sae()) {
index 459f993676843d8d8cb4217c3a60bb0e6c91e458..4179ec95db4d631ed831a93cb3794ee541bfd4e0 100644 (file)
@@ -53,7 +53,7 @@ RegionFactory::create (boost::shared_ptr<const Region> region, bool announce)
 
        if ((ar = boost::dynamic_pointer_cast<const AudioRegion>(region)) != 0) {
 
-               AudioRegion* arn = new AudioRegion (ar, 0, true);
+               AudioRegion* arn = new AudioRegion (ar, 0);
                boost_debug_shared_ptr_mark_interesting (arn, "Region");
 
                boost::shared_ptr<AudioRegion> arp (arn);
@@ -61,7 +61,7 @@ RegionFactory::create (boost::shared_ptr<const Region> region, bool announce)
 
        } else if ((mr = boost::dynamic_pointer_cast<const MidiRegion>(region)) != 0) {
 
-               MidiRegion* mrn = new MidiRegion (mr, 0, true);
+               MidiRegion* mrn = new MidiRegion (mr, 0);
                boost::shared_ptr<MidiRegion> mrp (mrn);
                ret = boost::static_pointer_cast<Region> (mrp);
 
@@ -86,20 +86,46 @@ RegionFactory::create (boost::shared_ptr<const Region> region, bool announce)
        return ret;
 }
 
-boost::shared_ptr<Region>
-RegionFactory::create (boost::shared_ptr<Region> region, frameoffset_t offset, const PropertyList& plist, bool announce)
-{
-       return create (region, offset, true, plist, announce);
-}
-
 boost::shared_ptr<Region>
 RegionFactory::create (boost::shared_ptr<Region> region, const PropertyList& plist, bool announce)
 {
-       return create (region, 0, false, plist, announce);
+       boost::shared_ptr<Region> ret;
+       boost::shared_ptr<const AudioRegion> other_a;
+       boost::shared_ptr<const MidiRegion> other_m;
+
+       if ((other_a = boost::dynamic_pointer_cast<AudioRegion>(region)) != 0) {
+
+               AudioRegion* ar = new AudioRegion (other_a);
+               boost::shared_ptr<AudioRegion> arp (ar);
+               ret = boost::static_pointer_cast<Region> (arp);
+
+       } else if ((other_m = boost::dynamic_pointer_cast<MidiRegion>(region)) != 0) {
+
+               MidiRegion* mr = new MidiRegion (other_m);
+               boost::shared_ptr<MidiRegion> mrp (mr);
+               ret = boost::static_pointer_cast<Region> (mrp);
+
+       } else {
+               fatal << _("programming error: RegionFactory::create() called with unknown Region type")
+                     << endmsg;
+               /*NOTREACHED*/
+               return boost::shared_ptr<Region>();
+       }
+
+       if (ret) {
+               ret->apply_changes (plist);
+               map_add (ret);
+
+               if (announce) {
+                       CheckNewRegion (ret);
+               }
+       }
+
+       return ret;
 }
 
 boost::shared_ptr<Region>
-RegionFactory::create (boost::shared_ptr<Region> region, frameoffset_t offset, bool offset_relative, const PropertyList& plist, bool announce)
+RegionFactory::create (boost::shared_ptr<Region> region, frameoffset_t offset, const PropertyList& plist, bool announce)
 {
        boost::shared_ptr<Region> ret;
        boost::shared_ptr<const AudioRegion> other_a;
@@ -107,7 +133,7 @@ RegionFactory::create (boost::shared_ptr<Region> region, frameoffset_t offset, b
 
        if ((other_a = boost::dynamic_pointer_cast<AudioRegion>(region)) != 0) {
 
-               AudioRegion* ar = new AudioRegion (other_a, offset, offset_relative);
+               AudioRegion* ar = new AudioRegion (other_a, offset);
                boost_debug_shared_ptr_mark_interesting (ar, "Region");
 
                boost::shared_ptr<AudioRegion> arp (ar);
@@ -115,7 +141,7 @@ RegionFactory::create (boost::shared_ptr<Region> region, frameoffset_t offset, b
 
        } else if ((other_m = boost::dynamic_pointer_cast<MidiRegion>(region)) != 0) {
 
-               MidiRegion* mr = new MidiRegion (other_m, offset, offset_relative);
+               MidiRegion* mr = new MidiRegion (other_m, offset);
                boost::shared_ptr<MidiRegion> mrp (mr);
                ret = boost::static_pointer_cast<Region> (mrp);