Hi,
I’m reporting a bug I’ve noticed on the OSX version of Audacity, but it likely applies to all versions.
When you zoom in close enough to see individual samples, Audacity renders them with lines connecting them. However, DSP tells us specifically that you CAN’T do this. Audacity should instead plot these with a stem plot like Matlab does, or at least give the option to do so.
Otherwise, this is great software! Thanks for all the hard work.
I’m reporting a bug I’ve noticed on the OSX version of Audacity
It’s not a bug. The software is working as designed. It may be awkward or an error, but not a bug.
Koz
Ah, that’s a good point. Should I have posted this elsewhere?
There’s no perfect fit. I think “Audio Technology > Audio Processing” is the closest we have, so I’ve moved the topic here.
Could you post a screenshot showing how Matlab displays samples.
Probably the most “truthful” way to display samples would be like this:
but that’s pretty useless in terms of “visualising” the data, which is the whole point of having a graphic representation of the audio track.
There’s a related discussion here which you may find interesting: Nyquist-Shannon vs Audacity :-D
I have made an attempt to show what I mean.
diff --git a/src/TrackArtist.cpp b/src/TrackArtist.cpp
index 7d4385d..584b856 100644
--- a/src/TrackArtist.cpp
+++ b/src/TrackArtist.cpp
@@ -1363,12 +1363,13 @@ void TrackArtist::DrawIndividualSamples(wxDC &dc, int leftOffset, const wxRect &
}
// Draw lines
- for (decltype(slen) s = 0; s < slen - 1; s++) {
+ for (decltype(slen) s = 0; s < slen; s++) {
AColor::Line(dc,
rect.x + xpos[s], rect.y + ypos[s],
- rect.x + xpos[s + 1], rect.y + ypos[s + 1]);
+ rect.x + xpos[s], rect.y + (rect.height/2));
}
+
if (showPoints)
{
// Draw points where spacing is enough
http://imgur.com/a/Ul0yC shows it at various zoom levels. The first is before showIndividualPoints engages, and it looks like it’s kind of an awkward transition to this style. Also weird is the “space filling” and aliasing you see at lower zoom levels, though I think it looks ok zoomed far in. Might make an interesting non-default render option
That works nicely at some zoom levels
Yes I agree it could be an interesting non-default option, though I think it would need to be tweaked so as to avoid the aliasing.
Perhaps bring in the vertical lines only when the dots are show?
(around line 1365 of TrackArtist.cpp)
if (showPoints)
{
// Draw points where spacing is enough
const int tickSize = bigPoints ? 4 : 3;// Bigger ellipses when draggable.
wxRect pr;
pr.width = tickSize;
pr.height = tickSize;
//different colour when draggable.
dc.SetBrush( bigPoints ? dragsampleBrush : sampleBrush);
for (decltype(slen) s = 0; s < slen; s++) {
if (ypos[s] >= 0 && ypos[s] < rect.height) {
pr.x = rect.x + xpos[s] - tickSize/2;
pr.y = rect.y + ypos[s] - tickSize/2;
dc.DrawEllipse(pr);
}
}
// Draw vertical lines
for (decltype(slen) s = 0; s < slen; s++) {
AColor::Line(dc,
rect.x + xpos[s], rect.y + ypos[s],
rect.x + xpos[s], rect.y + (rect.height/2));
}
}
else {
// Draw joining lines
// At this zoom level, linear interpolation is generally a reasonable approximation
// of the analog waveform.
for (decltype(slen) s = 0; s < slen - 1; s++) {
AColor::Line(dc,
rect.x + xpos[s], rect.y + ypos[s],
rect.x + xpos[s + 1], rect.y + ypos[s + 1]);
}
}
Just thinking, “Matlab” is a registered trademark, so we may not be able to call such an option “Matlab style samples”. Any ideas what this style of graphic representation could be called?
Hm, I think that’s a good improvement. I do like the stem version at one zoom level up from there, though.
A related question is: where exactly should the “dot” be located? Should it be at the beginning of the sample period (as now), in the middle, somewhere else, why?
I believe these are often called Stem plots.
https://en.wikibooks.org/wiki/Digital_Signal_Processing/Discrete_Data#Stem_Plots
That’s a nice descriptive name.
I think my favourite would be “sinc interpolation”, which has the benefit of giving an indication of the “true peak level”, though I’ve not yet attempted to code it.
(this is a mock up / simulation)
Do you have code for that?
I changed where the zoom engages, fixed a bug with the stems not going to proper zero, and I changed zooming to lock in to a multiple of the project’s sample rate. This gets rid of that aliasing.
diff --git a/src/TrackArtist.cpp b/src/TrackArtist.cpp
index 7d4385d..2c116ad 100644
--- a/src/TrackArtist.cpp
+++ b/src/TrackArtist.cpp
@@ -1308,6 +1308,7 @@ void TrackArtist::DrawMinMaxRMS(wxDC &dc, const wxRect & rect, const double env[
void TrackArtist::DrawIndividualSamples(wxDC &dc, int leftOffset, const wxRect &rect,
float zoomMin, float zoomMax,
+ int zeroLevelYCoordinate,
bool dB, float dBRange,
const WaveClip *clip,
const ZoomInfo &zoomInfo,
@@ -1363,12 +1364,13 @@ void TrackArtist::DrawIndividualSamples(wxDC &dc, int leftOffset, const wxRect &
}
// Draw lines
- for (decltype(slen) s = 0; s < slen - 1; s++) {
+ for (decltype(slen) s = 0; s < slen; s++) {
AColor::Line(dc,
rect.x + xpos[s], rect.y + ypos[s],
- rect.x + xpos[s + 1], rect.y + ypos[s + 1]);
+ rect.x + xpos[s], zeroLevelYCoordinate);
}
+
if (showPoints)
{
// Draw points where spacing is enough
@@ -1808,7 +1810,7 @@ void TrackArtist::DrawClipWaveform(const WaveTrack *track,
const unsigned nPortions = portions.size();
// Require at least 1/2 pixel per sample for drawing individual samples.
- const double threshold1 = 0.5 * rate;
+ const double threshold1 = 1.0 * rate;
// Require at least 3 pixels per sample for drawing the draggable points.
const double threshold2 = 3 * rate;
@@ -1917,6 +1919,7 @@ void TrackArtist::DrawClipWaveform(const WaveTrack *track,
}
else
DrawIndividualSamples(dc, leftOffset, rect, zoomMin, zoomMax,
+ track->ZeroLevelYCoordinate(mid),
dB, dBRange,
clip, zoomInfo,
bigPoints, showPoints, muted);
diff --git a/src/TrackArtist.h b/src/TrackArtist.h
index f5c76ae..aa1acb7 100644
--- a/src/TrackArtist.h
+++ b/src/TrackArtist.h
@@ -157,6 +157,7 @@ class AUDACITY_DLL_API TrackArtist {
);
void DrawIndividualSamples(wxDC & dc, int leftOffset, const wxRect & rect,
float zoomMin, float zoomMax,
+ int zeroLevelYCoordinate,
bool dB, float dBRange,
const WaveClip *clip,
const ZoomInfo &zoomInfo,
diff --git a/src/ViewInfo.cpp b/src/ViewInfo.cpp
index 089554e..6580374 100644
--- a/src/ViewInfo.cpp
+++ b/src/ViewInfo.cpp
@@ -90,6 +90,14 @@ bool ZoomInfo::ZoomOutAvailable() const
void ZoomInfo::SetZoom(double pixelsPerSecond)
{
zoom = std::max(gMinZoom, std::min(gMaxZoom, pixelsPerSecond));
+ AudacityProject * project = GetActiveProject();
+ if ( project ) {
+ double samplesPerSecond = project->GetRate();
+ double pixelsPerSample = pixelsPerSecond/samplesPerSecond;
+ if ( pixelsPerSample > 1 ) {
+ zoom = floor(pixelsPerSample) * samplesPerSecond;
+ }
+ }
#ifdef EXPERIMENTAL_DA
// Disable snapping if user zooms in a long way.
// Helps stop users be trapped in snap-to.
Unfortunately you’ve broken something and created a bug. “Zoom to Fit” (Ctrl+E) no longer works reliably.
Example:
With default preferences, generate 10 second sine wave.
From the selection Toolbar, select from 0 to 437 samples.
Zoom to fit.
Depending on the window size, the selection probably does not fit the window.
This is true, you can’t reliably select based on time with this.
Would it make sense to refer to it as a ‘snap to’ behavior? Although it’s different from Audacity’s typical behavior, it seems like the concept of equally spaced samples is a consistent idea, but one which is inherently inconsistent with perfectly filling a space ‘x’ pixels wide with ‘y’ samples.
Perhaps you’re not seeing what I’m seeing. I don’t just mean that it’s not snapping to the sample boundary. I mean it’s way off.
Here is the result of the described steps with un-patched Audacity 2.1.3 RC2 version. Note that the selection fits almost exactly in the available window area:
And here is the result with the patched version. Note that the selection does not fill the window:
To be clear, what we’re snapping to with this patch is the number of pixels between each sample, not the samples themselves. To do some napkin math, if the track in your image were 800 pixels wide, it’s filled by ~0.012 seconds, which comes out to approximately 70,000 pixels/second. At a sample rate of 44,100 samples/second, (pixels/second)/(samples/second) = 1.51 pixels/sample. Since this only considers integer pixels/sample, it’d either have to reduce to 1 pixel/sample spacing, which would leave a large gap like the one you’ve posted, or it could move up to 2 pixels/sample, which would push the selection past the end of the screen.
You could probably mitigate this by changing the threshold where the “snap” to integer ratios of pixels/sample starts happening from 1 to 4. I’ve put the patch for that here. Looking at the example you’ve given, at a sample rate of 48000Hz, it looks like the snapping begins at [0, 290] samples and becomes most dramatic at something like [0, 232] samples. Still, I’d argue that this could be correct behavior if the user wanted equally spaced samples.
diff --git a/src/TrackArtist.cpp b/src/TrackArtist.cpp
index 7d4385d..8fcaf9a 100644
--- a/src/TrackArtist.cpp
+++ b/src/TrackArtist.cpp
@@ -1308,6 +1308,7 @@ void TrackArtist::DrawMinMaxRMS(wxDC &dc, const wxRect & rect, const double env[
void TrackArtist::DrawIndividualSamples(wxDC &dc, int leftOffset, const wxRect &rect,
float zoomMin, float zoomMax,
+ int zeroLevelYCoordinate,
bool dB, float dBRange,
const WaveClip *clip,
const ZoomInfo &zoomInfo,
@@ -1363,12 +1364,13 @@ void TrackArtist::DrawIndividualSamples(wxDC &dc, int leftOffset, const wxRect &
}
// Draw lines
- for (decltype(slen) s = 0; s < slen - 1; s++) {
+ for (decltype(slen) s = 0; s < slen; s++) {
AColor::Line(dc,
rect.x + xpos[s], rect.y + ypos[s],
- rect.x + xpos[s + 1], rect.y + ypos[s + 1]);
+ rect.x + xpos[s], zeroLevelYCoordinate);
}
+
if (showPoints)
{
// Draw points where spacing is enough
@@ -1807,17 +1809,17 @@ void TrackArtist::DrawClipWaveform(const WaveTrack *track,
FindWavePortions(portions, rect, zoomInfo, params);
const unsigned nPortions = portions.size();
- // Require at least 1/2 pixel per sample for drawing individual samples.
- const double threshold1 = 0.5 * rate;
- // Require at least 3 pixels per sample for drawing the draggable points.
- const double threshold2 = 3 * rate;
+ // Require at least 4 pixels per sample for drawing individual samples.
+ const double threshold1 = 4 * rate;
+ // Require at least 4 pixels per sample for drawing the draggable points.
+ const double threshold2 = 4 * rate;
{
bool showIndividualSamples = false;
for (unsigned ii = 0; !showIndividualSamples && ii < nPortions; ++ii) {
const WavePortion &portion = portions[ii];
showIndividualSamples =
- !portion.inFisheye && portion.averageZoom > threshold1;
+ !portion.inFisheye && portion.averageZoom >= threshold1;
}
if (!showIndividualSamples) {
@@ -1839,8 +1841,8 @@ void TrackArtist::DrawClipWaveform(const WaveTrack *track,
for (unsigned ii = 0; ii < nPortions; ++ii) {
WavePortion &portion = portions[ii];
- const bool showIndividualSamples = portion.averageZoom > threshold1;
- const bool showPoints = portion.averageZoom > threshold2;
+ const bool showIndividualSamples = portion.averageZoom >= threshold1;
+ const bool showPoints = portion.averageZoom >= threshold2;
wxRect& rect = portion.rect;
rect.Intersect(mid);
wxASSERT(rect.width >= 0);
@@ -1917,6 +1919,7 @@ void TrackArtist::DrawClipWaveform(const WaveTrack *track,
}
else
DrawIndividualSamples(dc, leftOffset, rect, zoomMin, zoomMax,
+ track->ZeroLevelYCoordinate(mid),
dB, dBRange,
clip, zoomInfo,
bigPoints, showPoints, muted);
diff --git a/src/TrackArtist.h b/src/TrackArtist.h
index f5c76ae..aa1acb7 100644
--- a/src/TrackArtist.h
+++ b/src/TrackArtist.h
@@ -157,6 +157,7 @@ class AUDACITY_DLL_API TrackArtist {
);
void DrawIndividualSamples(wxDC & dc, int leftOffset, const wxRect & rect,
float zoomMin, float zoomMax,
+ int zeroLevelYCoordinate,
bool dB, float dBRange,
const WaveClip *clip,
const ZoomInfo &zoomInfo,
diff --git a/src/ViewInfo.cpp b/src/ViewInfo.cpp
index 089554e..fdc8485 100644
--- a/src/ViewInfo.cpp
+++ b/src/ViewInfo.cpp
@@ -90,6 +90,15 @@ bool ZoomInfo::ZoomOutAvailable() const
void ZoomInfo::SetZoom(double pixelsPerSecond)
{
zoom = std::max(gMinZoom, std::min(gMaxZoom, pixelsPerSecond));
+ AudacityProject * project = GetActiveProject();
+ if ( project ) {
+ double samplesPerSecond = project->GetRate();
+ double pixelsPerSample = pixelsPerSecond/samplesPerSecond;
+ if ( pixelsPerSample >= 4 ) {
+ zoom = floor(pixelsPerSample) * samplesPerSecond;
+ }
+ }
#ifdef EXPERIMENTAL_DA
// Disable snapping if user zooms in a long way.
// Helps stop users be trapped in snap-to.
“I” could do …? Oh no no no. 
I wouldn’t dream of interfering with your project. I’m just offering feedback. I’ve got plenty of my own work to do.
OK, I can see what you are doing, but I think it highly unlikely that this would be accepted into the main code base with the “apparent” breakage of “Fit Selection”.
On the other hand, I think that the basic idea of “stem plots” as an “option” for displaying wave data at sample level, could be accepted, if implemented in a way that is in keeping with Audacity as a whole. (though I’d add that it’s not my decision ).
I’m not sure I understand this comment since the code I included has this change. Wasn’t it clear I meant it in a general “you”?
Rendering the samples with uneven spacing is inherently misrepresentative of the underlying data. This is true for the existing lines plot but it’s exacerbated by the stem plot. Isn’t this something that could be gated behind a user option?
No offence intended or taken - just checking that we’re both clear.