View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0001681 | ardour | bugs | public | 2007-05-16 20:07 | 2007-08-28 22:12 |
| Reporter | SaBer | Assigned To | paul | ||
| Priority | normal | Severity | tweak | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| Product Version | 2.0 | ||||
| Summary | 0001681: Transparent regions shouldn't have crossfades | ||||
| Description | When you move a transparent (non-opaque) region on top of another, you get a crossfade. However, having a crossfade on a transparent region simply doesn't make sense. It also can cause nasty sounds if the underlying region has sound on it. I suggest disabling crossfades on transparent regions all together. | ||||
| Tags | No tags attached. | ||||
|
2007-08-23 07:16
|
xfades_transparency.patch (1,885 bytes)
Index: libs/ardour/crossfade.cc
===================================================================
--- libs/ardour/crossfade.cc (revision 2337)
+++ libs/ardour/crossfade.cc (working copy)
@@ -268,6 +268,13 @@
offset = start - _position;
+ /* Prevent data from piling up inthe crossfade buffers when reading a transparent region */
+ if (!(_out->opaque())) {
+ memset (crossfade_buffer_out, 0, sizeof (Sample) * to_write);
+ } else if (!(_in->opaque())) {
+ memset (crossfade_buffer_in, 0, sizeof (Sample) * to_write);
+ }
+
_out->read_at (crossfade_buffer_out, mixdown_buffer, gain_buffer, start, to_write, chan_n, read_frames, skip_frames);
_in->read_at (crossfade_buffer_in, mixdown_buffer, gain_buffer, start, to_write, chan_n, read_frames, skip_frames);
@@ -332,6 +339,13 @@
return false;
}
+ /* Top layer shouldn't be transparent */
+
+ if (!((layer_relation > 0 ? _in : _out)->opaque())) {
+ Invalidated (shared_from_this());
+ return false;
+ }
+
/* layer ordering cannot change */
int32_t new_layer_relation = (int32_t) (_in->layer() - _out->layer());
Index: libs/ardour/playlist.cc
===================================================================
--- libs/ardour/playlist.cc (revision 2337)
+++ libs/ardour/playlist.cc (working copy)
@@ -1170,7 +1170,7 @@
save = !(_splicing || _nudging);
}
- if ((what_changed & Region::MuteChanged) &&
+ if ((what_changed & our_interests) &&
!(what_changed & Change (ARDOUR::PositionChanged|ARDOUR::LengthChanged))) {
check_dependents (region, false);
}
Index: libs/ardour/audio_playlist.cc
===================================================================
--- libs/ardour/audio_playlist.cc (revision 2337)
+++ libs/ardour/audio_playlist.cc (working copy)
@@ -380,6 +380,10 @@
top = other;
bottom = region;
}
+
+ if (!(top->opaque())) {
+ continue;
+ }
|
|
|
Uploaded a patch for this. Since "one region is muted" already causes ardour to disable the crossfade, I added similar handling for transparency to all the relevant files, disabling the crossfade if the top region is transparent. While playing around with transparent regions I stumbled upon another bug with crossfades and transparency: when going from transparent to opaque, the crossfade buffers aren't emptied, which leads to parts of the crossfade being added to the buffer again and again. That is also fixed in the patch. |
|
|
shouldn't the xfade be disabled if *either* region is opaque, not just the top layer? |
|
|
"shouldn't the xfade be disabled if *either* region is opaque, not just the top layer?" No, if the top layer is opaque (you meant transparent not opaque, right?) the crossfade cant "see" whats underneath, so it doesn't really matter whether or not the layer below is opaque. It also came to my mind that it might be better to call the option transparent (off by default) instead of opaque (on by default). (I uploaded a cleaner version of the patch...) |
|
2007-08-26 14:07
|
xfades_transparency2.patch (1,881 bytes)
Index: libs/ardour/crossfade.cc
===================================================================
--- libs/ardour/crossfade.cc (revision 2337)
+++ libs/ardour/crossfade.cc (working copy)
@@ -268,6 +268,14 @@
offset = start - _position;
+ /* Prevent data from piling up in the crossfade buffers when reading a transparent region */
+
+ if (!_out->opaque()) {
+ memset (crossfade_buffer_out, 0, sizeof (Sample) * to_write);
+ } else if (!_in->opaque()) {
+ memset (crossfade_buffer_in, 0, sizeof (Sample) * to_write);
+ }
+
_out->read_at (crossfade_buffer_out, mixdown_buffer, gain_buffer, start, to_write, chan_n, read_frames, skip_frames);
_in->read_at (crossfade_buffer_in, mixdown_buffer, gain_buffer, start, to_write, chan_n, read_frames, skip_frames);
@@ -332,6 +340,13 @@
return false;
}
+ /* Top layer shouldn't be transparent */
+
+ if (!(layer_relation > 0 ? _in : _out)->opaque()) {
+ Invalidated (shared_from_this());
+ return false;
+ }
+
/* layer ordering cannot change */
int32_t new_layer_relation = (int32_t) (_in->layer() - _out->layer());
Index: libs/ardour/playlist.cc
===================================================================
--- libs/ardour/playlist.cc (revision 2337)
+++ libs/ardour/playlist.cc (working copy)
@@ -1170,7 +1170,7 @@
save = !(_splicing || _nudging);
}
- if ((what_changed & Region::MuteChanged) &&
+ if ((what_changed & our_interests) &&
!(what_changed & Change (ARDOUR::PositionChanged|ARDOUR::LengthChanged))) {
check_dependents (region, false);
}
Index: libs/ardour/audio_playlist.cc
===================================================================
--- libs/ardour/audio_playlist.cc (revision 2337)
+++ libs/ardour/audio_playlist.cc (working copy)
@@ -380,6 +380,10 @@
top = other;
bottom = region;
}
+
+ if (!top->opaque()) {
+ continue;
+ }
|
|
|
patch applied to branches/2.0-ongoing, thanks! |
|
|
this patch will need a small amount of reworking for trunk. if you feel interested in doing it, that would be great. |
|
|
Fixed in trunk and branches/2.0-ongoing. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2007-05-16 20:07 | SaBer | New Issue | |
| 2007-08-23 07:16 | SaBer | File Added: xfades_transparency.patch | |
| 2007-08-23 07:24 | SaBer | Note Added: 0004284 | |
| 2007-08-24 12:33 | paul | Note Added: 0004287 | |
| 2007-08-24 13:09 | SaBer | Note Added: 0004290 | |
| 2007-08-26 14:07 | SaBer | File Added: xfades_transparency2.patch | |
| 2007-08-26 14:07 | SaBer | Note Edited: 0004290 | |
| 2007-08-27 13:03 | paul | Status | new => resolved |
| 2007-08-27 13:03 | paul | Resolution | open => fixed |
| 2007-08-27 13:03 | paul | Assigned To | => paul |
| 2007-08-27 13:03 | paul | Note Added: 0004304 | |
| 2007-08-27 13:04 | paul | Note Added: 0004305 | |
| 2007-08-28 22:12 | SaBer | Status | resolved => closed |
| 2007-08-28 22:12 | SaBer | Note Added: 0004313 |
