View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0002896 | ardour | bugs | public | 2009-11-02 11:03 | 2010-04-24 10:31 |
Reporter | mschwarzenberg | Assigned To | |||
Priority | normal | Severity | major | Reproducibility | always |
Status | feedback | Resolution | open | ||
Product Version | SVN/2.0-ongoing | ||||
Summary | 0002896: split region and undo split region incorretly handles regions fade in/out activation state | ||||
Description | consider the following work sequence, in which a region is split twice and a fade in is modified. Legend | | region i ... deactivated (inactive) Region fade in a ... activated Region fade in, expected A ... activated Region fade in, inexpected / Error Start: we have a single region with deactivated region fade in/out: | i i | Split 1: v ok, so far ... | i i | i i | now: drag region fade in: .>>>. the fade in of the second | i i | a i | region is automatically activated. that's ok. now split the second region, again: v Error: the beginning of | i i | a i|A i | the now 3rd region has an activated fade in! Now let's undo this step by step: undo 2nd split | A A | a A | ERROR: expected is: | i i | a i|A i | all further undo steps now contain wrong fade in/out .<<<. activation state, undo drag| A A | A A | ERROR: expected is: | i i | i i | undo 1st split: | A A | ERROR: expected is: | i i | | ||||
Additional Information | Ardour version 2.8.3+ / r 5997 this bug has been present already in earlier versions | ||||
Tags | No tags attached. | ||||
related to | 0000671 | confirmed | Clicks at splits. |
2009-11-02 11:04
|
ardour-split-bug.txt (1,119 bytes)
use a monospaced font to display! Start: we have a single region with deactivated region fade in/out: | i i | Split 1: v ok, so far ... | i i | i i | now: drag region fade in: .>>>. the fade in of the second | i i | a i | region is automatically activated. that's ok. now split the second region, again: v Error: the beginning of | i i | a i|A i | the now 3rd region has an activated fade in! Now let's undo this step by step: undo 2nd split | A A | a A | ERROR: expected is: | i i | a i|A i | all further undo steps now contain wrong fade in/out .<<<. activation state, undo drag| A A | A A | ERROR: expected is: | i i | i i | undo 1st split: | A A | ERROR: expected is: | i i | |
|
the added file ardour-split-bug.txt contains the description with ascii-art correct spacing. |
|
The attached patch should fix the strange undo behaviour. Can you justify why you think the behaviour in step 3 (the second split) is wrong? To play devil's advocate; in that case, the two regions that have come from the split have been given the same fade activations. |
2009-11-05 00:45
|
2896.patch (1,093 bytes)
diff --git a/libs/ardour/audioregion.cc b/libs/ardour/audioregion.cc index 6163897..80ede2a 100644 --- a/libs/ardour/audioregion.cc +++ b/libs/ardour/audioregion.cc @@ -724,7 +724,7 @@ AudioRegion::state (bool full) child->add_child_nocopy (_fade_in.get_state ()); } - child->add_property (X_("active"), _fade_in_disabled ? X_("no") : X_("yes")); + child->add_property (X_("active"), fade_in_active () ? X_("yes") : X_("no")); child = node.add_child (X_("FadeOut")); @@ -734,7 +734,7 @@ AudioRegion::state (bool full) child->add_child_nocopy (_fade_out.get_state ()); } - child->add_property (X_("active"), _fade_out_disabled ? X_("no") : X_("yes")); + child->add_property (X_("active"), fade_out_active () ? X_("yes") : X_("no")); } child = node.add_child ("Envelope"); @@ -850,7 +850,7 @@ AudioRegion::set_live_state (const XMLNode& node, Change& what_changed, bool sen if (string_is_affirmative (prop->value())) { set_fade_in_active (true); } else { - set_fade_in_active (true); + set_fade_in_active (false); } } |
|
> Can you justify why you think the behaviour in step 3 > (the second split) is wrong? Audibly, splitting should be (at least to my POV) a transparent operation. this is not the case, when the second part in the second split gets the additional fade in. practical example: sometimes i need to split a region in order to move a part of the region into another track at exactly the same position in time. both tracks are activated, and the regions audio must not be interrupted by the second fade in. The rationale behind using the 2nd track is to modify the regions second parts end afterwards, for example to let it overlap with another region without using crossfades. Example, ony expected behavior shown: TR1:_______|a========R1=========a|_|a========R2==========a| TR2:_______(empty)_________________________________________ split Region 1: TR1:_______|a===R1.1==i|i=R1.2==a|_|a========R2==========a| TR2:_______(empty)_________________________________________ move Region 1.1 to Track 2: TR1:_______|a===R1.1==i|___________|a========R2==========a| TR2:___________________|i=R1.2==a|_________________________ overlap R1.2 and R2 TR1:_______|a===R1.1==i|___________|a========R2==========a| TR2:___________________|i=R1.2===========a| Legend: |======R1======| Region 1 (trimmed, contains more audio) |======R2======| another region ________________ silence a active fade in/out i inactive fade in/out |
|
perhaps a good idea would be configurable splitting behaviour: *1* transparent: deactivated fade in/out at splitting point *2* copy: fade in/out setting from original region *3* auto activate fade in/out at splitting point |
|
the 2896.patch indeed fixes the undo issue. thanks! For the remaining transparent splitting issue the file 2896_split_inner_fades_off.patch is attached. Admittedly not very nice, but works as i would expect. |
2009-11-05 22:08
|
2896_split_inner_fades_off.patch (1,275 bytes)
Index: libs/ardour/playlist.cc =================================================================== --- libs/ardour/playlist.cc (Revision 5997) +++ libs/ardour/playlist.cc (Arbeitskopie) @@ -1106,6 +1106,19 @@ _split_region (region, playlist_position); } +static void deactivate_fade(boost::shared_ptr<Region> region, bool fade_in) +{ + boost::shared_ptr<AudioRegion> other; + + if ((other = boost::dynamic_pointer_cast<AudioRegion>(region)) != 0) { + if(fade_in) { + other->set_fade_in_active(false); + } else { + other->set_fade_out_active(false); + } + } +} + void Playlist::_split_region (boost::shared_ptr<Region> region, nframes_t playlist_position) { @@ -1135,9 +1148,11 @@ _session.region_name (before_name, region->name(), false); left = RegionFactory::create (region, 0, before, before_name, region->layer(), Region::Flag (region->flags()|Region::LeftOfSplit)); + deactivate_fade(left, false); _session.region_name (after_name, region->name(), false); right = RegionFactory::create (region, before, after, after_name, region->layer(), Region::Flag (region->flags()|Region::RightOfSplit)); + deactivate_fade(right, true); add_region_internal (left, region->position()); add_region_internal (right, region->position() + before); |
Date Modified | Username | Field | Change |
---|---|---|---|
2009-11-02 11:03 | mschwarzenberg | New Issue | |
2009-11-02 11:04 | mschwarzenberg | File Added: ardour-split-bug.txt | |
2009-11-02 11:06 | mschwarzenberg | Note Added: 0007066 | |
2009-11-05 00:45 | cth103 | Note Added: 0007074 | |
2009-11-05 00:45 | cth103 | Status | new => feedback |
2009-11-05 00:45 | cth103 | File Added: 2896.patch | |
2009-11-05 08:32 | mschwarzenberg | Note Added: 0007079 | |
2009-11-05 12:42 | mschwarzenberg | Note Added: 0007080 | |
2009-11-05 12:49 | cth103 | Relationship added | related to 0000671 |
2009-11-05 22:07 | mschwarzenberg | Note Added: 0007083 | |
2009-11-05 22:08 | mschwarzenberg | File Added: 2896_split_inner_fades_off.patch | |
2010-04-24 10:28 | cth103 | Category | bugs => bugs2 |
2010-04-24 10:31 | cth103 | Category | bugs2 => bugs |