View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0005953 | ardour | bugs | public | 2014-08-11 14:52 | 2020-04-19 20:16 |
| Reporter | tartina | Assigned To | tartina | ||
| Priority | normal | Severity | major | Reproducibility | sometimes |
| Status | closed | Resolution | fixed | ||
| Product Version | 3.0 | ||||
| Summary | 0005953: Click noise on crossfade boundaries, also when exporting | ||||
| Description | I'm having click noise on some crossfade boundaries, version is 3.5.380 from fedora repo, but also with ardour compiled from source (version 3.5-385-g1ed92db). I get only one bad sample which causes the click noise, and the position in the file is always at a binary number which ends with 10 or more ones (i.e. 1310719 base 10 which is 100111111111111111111 binary). The other odd thing is that if I move the start locator and export the session again I don't get the noise! If I play the session from start I almost always ear the noise, but if I play from other points I rarely get it, or I get it a little bit later! | ||||
| Additional Information | I've got a session I'm using to test this problem, because it's a slow ballad without drums and you can ear the noise easily, but it's about 300 MBytes. I also have the exported file (32 bit float) which is about 64MB, but I can truncate after the noise. | ||||
| Tags | No tags attached. | ||||
| Users sponsoring this issue | ||
| Sponsors List |
Total Sponsorship = US$ 20 2014-08-11 15:13: tartina (US$ 20) |
|
|---|---|---|
|
|
I did some other investigation with only one track. The sample that causes the click has a value of 0 (zero), seen with audacity. It's at the end of a small region which is above a longer one; it's a small correction of a bad played bar. If I export the session I always have the click. If I play starting in the middle of the small region I don't ear the noise, but if I start playing before the small region I can ear it, sometimes a little bit later than the exported file. |
|
|
Now I can reproduce the click noise and I think I have a patch for it. It happens when the end of the overlapping region is at the end of the playback buffer, which is usually 65536 bytes. The last sample isn't copied to the buffer, so the last sample of the buffer remains at value 0. It's rather difficult to ear the click if the real value is near 0. When I was searching for this problem, I also got another issue, which happens when the overlapping region end is one frame over the next playback buffer, i.e. is one frame shifted to right of the previous issue. I got a strange gain value from the Curve::_get_vector method, it was 63. The method reported a wrong value when veclen == 1. Also I think some Overlap check are not correct, because if a region has the same start point of another region but ends before the other's end, it's not correctly reported as Internal (I need some confirmation that it's correct). There were also IMHO some off-by-one problems in Range. I attach the patch that seems to solve these issues, but it needs deep testing. P.S. English is not my native language, so please sorry if I didn't explain my findings clearly... |
|
2014-08-16 12:00
|
ardour-click.patch (3,692 bytes)
diff --git a/libs/evoral/evoral/Range.hpp b/libs/evoral/evoral/Range.hpp
index 02d9210..abd119d 100644
--- a/libs/evoral/evoral/Range.hpp
+++ b/libs/evoral/evoral/Range.hpp
@@ -40,7 +40,7 @@ OverlapType coverage (T sa, T ea, T sb, T eb) {
of A and B for each OverlapType.
Notes:
- Internal: the start points cannot coincide
+ Internal: the start and end points cannot coincide
External: the start and end points can coincide
Start: end points can coincide
End: start points can coincide
@@ -53,13 +53,14 @@ OverlapType coverage (T sa, T ea, T sb, T eb) {
|--------------------| A
|------| B
|-----------------| B
+ |----------------| B
"B is internal to A"
*/
- if ((sb > sa) && (eb <= ea)) {
+ if (((sb > sa) && (eb <= ea)) || ((sb == sa) && (eb < ea))) {
return OverlapInternal;
}
@@ -73,7 +74,7 @@ OverlapType coverage (T sa, T ea, T sb, T eb) {
*/
- if ((eb >= sa) && (eb <= ea)) {
+ if ((eb >= sa) && (eb <= ea) && (sb < sa)) {
return OverlapStart;
}
/*
@@ -85,11 +86,11 @@ OverlapType coverage (T sa, T ea, T sb, T eb) {
"B overlaps the end of A"
*/
- if ((sb > sa) && (sb <= ea)) {
+ if ((sb > sa) && (sb <= ea) && (eb > ea)) {
return OverlapEnd;
}
/*
- |--------------------| A
+ |--------------------| A
-------------------------- B
|----------------------- B
----------------------| B
@@ -198,14 +199,13 @@ RangeList<T> subtract (Range<T> range, RangeList<T> sub)
/* The basic idea here is to keep a list of the result ranges, and subtract
the bits of `sub' from them one by one.
*/
+
+ /* Here's where we'll put the new current result after subtracting *i from it */
+ RangeList<T> new_result;
+ typename RangeList<T>::List r = result.get ();
for (typename RangeList<T>::List::const_iterator i = s.begin(); i != s.end(); ++i) {
- /* Here's where we'll put the new current result after subtracting *i from it */
- RangeList<T> new_result;
-
- typename RangeList<T>::List r = result.get ();
-
/* Work on all parts of the current result using this range *i */
for (typename RangeList<T>::List::const_iterator j = r.begin(); j != r.end(); ++j) {
@@ -220,16 +220,16 @@ RangeList<T> subtract (Range<T> range, RangeList<T> sub)
/* Internal overlap of the thing we're subtracting from this bit of the result,
so we might end up with two bits left over.
*/
- if (j->from < (i->from - 1)) {
+ if (j->from < i->from) {
new_result.add (Range<T> (j->from, i->from - 1));
}
- if (j->to != i->to) {
- new_result.add (Range<T> (i->to, j->to));
+ if (j->to > i->to) {
+ new_result.add (Range<T> (i->to + 1, j->to));
}
break;
case OverlapStart:
/* The bit we're subtracting overlaps the start of the bit of the result */
- new_result.add (Range<T> (i->to, j->to - 1));
+ new_result.add (Range<T> (i->to, j->to));
break;
case OverlapEnd:
/* The bit we're subtracting overlaps the end of the bit of the result */
diff --git a/libs/evoral/src/Curve.cpp b/libs/evoral/src/Curve.cpp
index 6f3532f..7491bab 100644
--- a/libs/evoral/src/Curve.cpp
+++ b/libs/evoral/src/Curve.cpp
@@ -301,14 +301,11 @@ Curve::_get_vector (double x0, double x1, float *vec, int32_t veclen)
if (veclen > 1) {
dx_num = hx - lx;
dx_den = veclen - 1;
- }
-
- if (veclen > 1) {
for (int i = 0; i < veclen; ++i) {
vec[i] = (lx * (m_num / m_den) + m_num * i * dx_num / (m_den * dx_den)) + c;
}
} else {
- vec[0] = lx;
+ vec[0] = lx * (m_num / m_den) + c;
}
return;
|
|
2014-08-19 14:15
|
|
|
|
I uploaded the session used for debugging, it's only a sine wave (turn down the volume). If you start playback from the start locator you will get the click at the end of the upper region, if you start from another point you probably won't get the clip, because you have to start in multiple of 65536 frame position - 1 or - 2 from the end of the region. |
|
2014-08-25 13:29
|
ardour-click_2.patch (3,295 bytes)
diff --git a/libs/evoral/evoral/Range.hpp b/libs/evoral/evoral/Range.hpp
index 02d9210..044f350 100644
--- a/libs/evoral/evoral/Range.hpp
+++ b/libs/evoral/evoral/Range.hpp
@@ -40,7 +40,7 @@ OverlapType coverage (T sa, T ea, T sb, T eb) {
of A and B for each OverlapType.
Notes:
- Internal: the start points cannot coincide
+ Internal: the start and end points cannot both coincide
External: the start and end points can coincide
Start: end points can coincide
End: start points can coincide
@@ -53,13 +53,14 @@ OverlapType coverage (T sa, T ea, T sb, T eb) {
|--------------------| A
|------| B
|-----------------| B
+ |----------------| B
"B is internal to A"
*/
- if ((sb > sa) && (eb <= ea)) {
+ if (((sb > sa) && (eb <= ea)) || ((sb == sa) && (eb < ea))) {
return OverlapInternal;
}
@@ -73,7 +74,7 @@ OverlapType coverage (T sa, T ea, T sb, T eb) {
*/
- if ((eb >= sa) && (eb <= ea)) {
+ if ((eb >= sa) && (eb <= ea) && (sb < sa)) {
return OverlapStart;
}
/*
@@ -85,11 +86,11 @@ OverlapType coverage (T sa, T ea, T sb, T eb) {
"B overlaps the end of A"
*/
- if ((sb > sa) && (sb <= ea)) {
+ if ((sb > sa) && (sb <= ea) && (eb > ea)) {
return OverlapEnd;
}
/*
- |--------------------| A
+ |--------------------| A
-------------------------- B
|----------------------- B
----------------------| B
@@ -198,7 +199,7 @@ RangeList<T> subtract (Range<T> range, RangeList<T> sub)
/* The basic idea here is to keep a list of the result ranges, and subtract
the bits of `sub' from them one by one.
*/
-
+
for (typename RangeList<T>::List::const_iterator i = s.begin(); i != s.end(); ++i) {
/* Here's where we'll put the new current result after subtracting *i from it */
@@ -220,16 +221,16 @@ RangeList<T> subtract (Range<T> range, RangeList<T> sub)
/* Internal overlap of the thing we're subtracting from this bit of the result,
so we might end up with two bits left over.
*/
- if (j->from < (i->from - 1)) {
+ if (j->from < i->from) {
new_result.add (Range<T> (j->from, i->from - 1));
}
- if (j->to != i->to) {
- new_result.add (Range<T> (i->to, j->to));
+ if (j->to > i->to) {
+ new_result.add (Range<T> (i->to + 1, j->to));
}
break;
case OverlapStart:
/* The bit we're subtracting overlaps the start of the bit of the result */
- new_result.add (Range<T> (i->to, j->to - 1));
+ new_result.add (Range<T> (i->to, j->to));
break;
case OverlapEnd:
/* The bit we're subtracting overlaps the end of the bit of the result */
diff --git a/libs/evoral/src/Curve.cpp b/libs/evoral/src/Curve.cpp
index 6f3532f..7491bab 100644
--- a/libs/evoral/src/Curve.cpp
+++ b/libs/evoral/src/Curve.cpp
@@ -301,14 +301,11 @@ Curve::_get_vector (double x0, double x1, float *vec, int32_t veclen)
if (veclen > 1) {
dx_num = hx - lx;
dx_den = veclen - 1;
- }
-
- if (veclen > 1) {
for (int i = 0; i < veclen; ++i) {
vec[i] = (lx * (m_num / m_den) + m_num * i * dx_num / (m_den * dx_den)) + c;
}
} else {
- vec[0] = lx;
+ vec[0] = lx * (m_num / m_den) + c;
}
return;
|
|
|
I reworked the patch and I uploaded a second version. The first patch wrongly moved new_result out of the loop. Please test ardour-click_2.patch instead. It seems I cannot delete attachments... |
|
|
impressive detective work. some of the changes in coverage() were already made in the cairocanvas branch which is where most development has happened for several months. the rest of the patch looks quite believable. we'll try to get some testing done. Thanks! |
|
|
I had a session experiencing this problem, applied the ardour-click2.patch and confirmed that there was no click on export. Without the patch there is a click at the same spot on every export from position zero. Thanks a lot. |
|
2014-12-05 00:43
|
0001-Curve-_get_vector-fix-return-value-when-veclen-1.patch (1,044 bytes)
From cc48334f73498b246907a512d3702de89ffcfc5b Mon Sep 17 00:00:00 2001
From: Guido Aulisi <guido.aulisi@gmail.com>
Date: Sat, 16 Aug 2014 12:26:33 +0200
Subject: [PATCH] Curve::_get_vector: fix return value when veclen == 1
When the crossfade length is only 1 frame, I got strange
gain coefficients from get_vector (63 in my case).
The function wrongly returned the x axis value.
---
libs/evoral/src/Curve.cpp | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/libs/evoral/src/Curve.cpp b/libs/evoral/src/Curve.cpp
index 6f3532f..7491bab 100644
--- a/libs/evoral/src/Curve.cpp
+++ b/libs/evoral/src/Curve.cpp
@@ -301,14 +301,11 @@ Curve::_get_vector (double x0, double x1, float *vec, int32_t veclen)
if (veclen > 1) {
dx_num = hx - lx;
dx_den = veclen - 1;
- }
-
- if (veclen > 1) {
for (int i = 0; i < veclen; ++i) {
vec[i] = (lx * (m_num / m_den) + m_num * i * dx_num / (m_den * dx_den)) + c;
}
} else {
- vec[0] = lx;
+ vec[0] = lx * (m_num / m_den) + c;
}
return;
--
1.9.3
|
|
|
I added a new patch (0001-Curve-_get_vector-fix-return-value-when-veclen-1.patch) for the current master branch (currently 3.5-3798-ge1e1679), because after the recent changes to coverage the previous patch can't be applied anymore. IMHO I think this patch solves the second issue I noticed. |
|
2015-01-08 14:20
|
debug_prints_for_get_vector.patch (1,151 bytes)
diff --git a/libs/ardour/audioregion.cc b/libs/ardour/audioregion.cc
index e9b0b1f..6dc7d87 100644
--- a/libs/ardour/audioregion.cc
+++ b/libs/ardour/audioregion.cc
@@ -567,6 +567,7 @@ AudioRegion::read_at (Sample *buf, Sample *mixdown_buffer, float *gain_buffer,
if (read_from_sources (_sources, _length, mixdown_buffer, position, to_read, chan_n) != to_read) {
return 0;
}
+ DEBUG_TRACE (DEBUG::AudioPlayback, string_compose("\tMixdown buffer start value %1\n", mixdown_buffer[0]));
/* APPLY REGULAR GAIN CURVES AND SCALING TO mixdown_buffer */
@@ -670,6 +671,8 @@ AudioRegion::read_at (Sample *buf, Sample *mixdown_buffer, float *gain_buffer,
_fade_out->curve().get_vector (curve_offset, curve_offset + fade_out_limit, gain_buffer, fade_out_limit);
}
+ DEBUG_TRACE (DEBUG::AudioPlayback, string_compose("\tGain buffer start value %1, buffer start value %2\n", gain_buffer[0], buf[0]));
+
/* Mix our newly-read data with whatever was already there,
with the fade out applied to our data.
*/
|
|
|
thanks, applied in 3.5-4331-gc6e71a6 There's are also simple unit-tests now: see libs/evoral/test/CurveTest.cpp (..more tests to be added) ./waf configure --with-backends=jack,dummy --test ./waf && ./gtk2_ardour/artest evoral |
|
|
please test |
|
|
I did some quick tests and both issues seem solved now. Waiting for someone else to confirm it's all ok, then I think this bug can be closed. |
|
|
I can confirm that the test session now plays without clicks for me, too. Also, I took a good look at Evoral::Curve, and the fix looks completely reasonable to me, so I think we can finally mark this one resolved! |
|
|
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. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2014-08-11 14:52 | tartina | New Issue | |
| 2014-08-11 15:13 | tartina | Sponsorship Added | tartina: US$ 20 |
| 2014-08-11 15:13 | tartina | Sponsorship Total | 0 => 20 |
| 2014-08-12 13:42 | tartina | Note Added: 0015860 | |
| 2014-08-16 11:59 | tartina | Note Added: 0015864 | |
| 2014-08-16 12:00 | tartina | File Added: ardour-click.patch | |
| 2014-08-19 14:15 | tartina | File Added: ardour_click_test_session.tar.bz2 | |
| 2014-08-19 14:18 | tartina | Note Added: 0015867 | |
| 2014-08-25 13:29 | tartina | File Added: ardour-click_2.patch | |
| 2014-08-25 13:32 | tartina | Note Added: 0015874 | |
| 2014-08-28 17:35 | paul | Note Added: 0015876 | |
| 2014-10-27 20:25 | nick_m | Note Added: 0015945 | |
| 2014-12-05 00:43 | tartina | File Added: 0001-Curve-_get_vector-fix-return-value-when-veclen-1.patch | |
| 2014-12-05 00:46 | tartina | Note Added: 0016013 | |
| 2015-01-08 14:20 | tartina | File Added: debug_prints_for_get_vector.patch | |
| 2015-01-19 23:00 | x42 | Note Added: 0016261 | |
| 2015-01-19 23:01 | x42 | Note Added: 0016262 | |
| 2015-01-19 23:01 | x42 | Status | new => feedback |
| 2015-01-19 23:03 | x42 | Note Edited: 0016261 | |
| 2015-01-20 11:35 | tartina | Note Added: 0016263 | |
| 2015-01-20 11:51 | colinf | Note Added: 0016264 | |
| 2015-01-20 18:53 | tartina | Status | feedback => resolved |
| 2015-01-20 18:53 | tartina | Resolution | open => fixed |
| 2015-01-20 18:53 | tartina | Assigned To | => tartina |
| 2020-04-19 20:16 | system | Note Added: 0023318 | |
| 2020-04-19 20:16 | system | Status | resolved => closed |
