From 9c7e2364422772afffa8c5ac2d7292eb8b385548 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Fri, 26 May 2017 13:50:22 -0400 Subject: [PATCH 1/3] Prefer "sampleDur" to "sampleTime" for some variable names --- src/Envelope.cpp | 18 +++++++++--------- src/Envelope.h | 10 +++++----- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Envelope.cpp b/src/Envelope.cpp index 520f9e525..786c8470d 100644 --- a/src/Envelope.cpp +++ b/src/Envelope.cpp @@ -606,7 +606,7 @@ bool EnvelopeEditor::MouseEvent(const wxMouseEvent & event, wxRect & r, return false; } -void Envelope::CollapseRegion( double t0, double t1, double sampleTime ) +void Envelope::CollapseRegion( double t0, double t1, double sampleDur ) // NOFAIL-GUARANTEE { // This gets called when somebody clears samples. @@ -615,7 +615,7 @@ void Envelope::CollapseRegion( double t0, double t1, double sampleTime ) // For the boundaries of the interval, preserve the left-side limit at the // start and right-side limit at the end. - const auto epsilon = sampleTime / 2; + const auto epsilon = sampleDur / 2; t0 = std::max( 0.0, std::min( mTrackLen, t0 - mOffset ) ); t1 = std::max( 0.0, std::min( mTrackLen, t1 - mOffset ) ); @@ -983,9 +983,9 @@ void Envelope::GetPoints(double *bufferWhen, } } -void Envelope::Cap( double sampleTime ) +void Envelope::Cap( double sampleDur ) { - auto range = EqualRange( mTrackLen, sampleTime ); + auto range = EqualRange( mTrackLen, sampleDur ); if ( range.first == range.second ) InsertOrReplaceRelative( mTrackLen, GetValueRelative( mTrackLen ) ); } @@ -1061,13 +1061,13 @@ int Envelope::InsertOrReplaceRelative(double when, double value) return i; } -std::pair Envelope::EqualRange( double when, double sampleTime ) const +std::pair Envelope::EqualRange( double when, double sampleDur ) const { // Find range of envelope points matching the given time coordinate - // (within an interval of length sampleTime) + // (within an interval of length sampleDur) // by binary search; if empty, it still indicates where to // insert. - const auto tolerance = sampleTime / 2; + const auto tolerance = sampleDur / 2; auto begin = mEnv.begin(); auto end = mEnv.end(); auto first = std::lower_bound( @@ -1090,11 +1090,11 @@ void Envelope::SetOffset(double newOffset) mOffset = newOffset; } -void Envelope::SetTrackLen( double trackLen, double sampleTime ) +void Envelope::SetTrackLen( double trackLen, double sampleDur ) // NOFAIL-GUARANTEE { // Preserve the left-side limit at trackLen. - auto range = EqualRange( trackLen, sampleTime ); + auto range = EqualRange( trackLen, sampleDur ); bool needPoint = ( range.first == range.second && trackLen < mTrackLen ); double value; if ( needPoint ) diff --git a/src/Envelope.h b/src/Envelope.h index 44bc72790..4ccb19a64 100644 --- a/src/Envelope.h +++ b/src/Envelope.h @@ -118,9 +118,9 @@ public: float zoomMin, float zoomMax, bool mirrored) const; // Handling Cut/Copy/Paste events - // sampleTime determines when the endpoint of the collapse is near enough + // sampleDur determines when the endpoint of the collapse is near enough // to an endpoint of the domain, that an extra control point is not needed. - void CollapseRegion(double t0, double t1, double sampleTime); + void CollapseRegion(double t0, double t1, double sampleDur); void Paste(double t0, const Envelope *e); void InsertSpace(double t0, double tlen); @@ -128,7 +128,7 @@ public: // Control void SetOffset(double newOffset); - void SetTrackLen( double trackLen, double sampleTime = 0.0 ); + void SetTrackLen( double trackLen, double sampleDur = 0.0 ); void RescaleValues(double minValue, double maxValue); void RescaleTimes( double newLength ); @@ -148,7 +148,7 @@ public: (double *buffer, int bufferLen, int leftOffset, const ZoomInfo &zoomInfo) const; // Guarantee an envelope point at the end of the domain. - void Cap( double sampleTime ); + void Cap( double sampleDur ); private: double GetValueRelative(double t) const; @@ -198,7 +198,7 @@ private: return mEnv[index]; } - std::pair EqualRange( double when, double sampleTime ) const; + std::pair EqualRange( double when, double sampleDur ) const; public: /** \brief Returns the sets of when and value pairs */ From 537ccfbc4ff886dd510f1321820361bdbc615fc9 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Fri, 26 May 2017 14:08:27 -0400 Subject: [PATCH 2/3] Change evaluation of envelope near discontinuities... ... So that even if the time is "slightly" (less than 1/2 sample interval) left of the discontinuity, the right-hand limit is always used. Thus this compensates for some roundoff errors when pasting one clip with an envelope into another. This overcomes the objections that were in a comment in Envelope::Paste to making control points with exactly equal times. And therefore Paste can be rewritten to do so, but that has not happened yet. Envelope points at exactly equal time coordinates can already be made by dragging points in the envelope editor. --- src/Envelope.cpp | 55 +++++++++++++++++++++++++-------------------- src/Envelope.h | 2 +- src/TrackArtist.cpp | 5 ++++- src/TrackPanel.cpp | 8 ++++--- src/WaveTrack.cpp | 4 +++- src/WaveTrack.h | 4 ++++ 6 files changed, 48 insertions(+), 30 deletions(-) diff --git a/src/Envelope.cpp b/src/Envelope.cpp index 786c8470d..34fee7819 100644 --- a/src/Envelope.cpp +++ b/src/Envelope.cpp @@ -992,19 +992,6 @@ void Envelope::Cap( double sampleDur ) // Private methods -// We no longer tolerate multiple envelope control points at the exact -// same t; the behavior can be well-defined, but it is still incorrect -// in that it vastly complicates paste operations behaving as a user -// reasonably expects. The most common problem occurs pasting an -// envelope into another track; the boundary behavior causes the -// t=insert_point envelope level of the insertee to apply to sample 0 -// of the inserted sample, causing a pop. This most visibly manifests -// itself in undo and mixing when a v=1.0 sample magically shows -// up at boundaries causing a pop. - -// Although this renders the name a slight misnomer, a duplicate -// 'replaces' the current control point. - /** @brief Add a control point to the envelope * * @param when the time in seconds when the envelope point should be created. @@ -1127,12 +1114,12 @@ void Envelope::RescaleTimes( double newLength ) } // Accessors -double Envelope::GetValue(double t) const +double Envelope::GetValue( double t, double sampleDur ) const { // t is absolute time double temp; - GetValues(&temp, 1, t, 1.0); + GetValues( &temp, 1, t, sampleDur ); return temp; } @@ -1140,7 +1127,7 @@ double Envelope::GetValueRelative(double t) const { double temp; - GetValuesRelative(&temp, 1, t, 1.0); + GetValuesRelative(&temp, 1, t, 0.0); return temp; } @@ -1205,8 +1192,8 @@ double Envelope::GetInterpolationStartValueAtPoint( int iPoint ) const return log10(v); } -void Envelope::GetValues(double *buffer, int bufferLen, - double t0, double tstep) const +void Envelope::GetValues( double *buffer, int bufferLen, + double t0, double tstep ) const { // Convert t0 from absolute to clip-relative time t0 -= mOffset; @@ -1219,9 +1206,14 @@ void Envelope::GetValuesRelative(double *buffer, int bufferLen, // JC: If bufferLen ==0 we have probably just allocated a zero sized buffer. // wxASSERT( bufferLen > 0 ); + const auto epsilon = tstep / 2; int len = mEnv.size(); double t = t0; + double increment = 0; + if ( len > 0 && t <= mEnv[0].GetT() && mEnv[0].GetT() == mEnv[1].GetT() ) + increment = epsilon; + double tprev, vprev, tnext = 0, vnext, vstep = 0; for (int b = 0; b < bufferLen; b++) { @@ -1233,20 +1225,24 @@ void Envelope::GetValuesRelative(double *buffer, int bufferLen, t += tstep; continue; } + + auto tplus = t + increment; + // IF before envelope THEN first value - if (t <= mEnv[0].GetT()) { + if ( tplus <= mEnv[0].GetT() ) { buffer[b] = mEnv[0].GetVal(); t += tstep; continue; } // IF after envelope THEN last value - if (t >= mEnv[len - 1].GetT()) { + if ( tplus >= mEnv[len - 1].GetT() ) { buffer[b] = mEnv[len - 1].GetVal(); t += tstep; continue; } - if (b == 0 || t > tnext) { + // Note >= not > , to get the right limit in case epsilon == 0 + if ( b == 0 || tplus >= tnext ) { // We're beyond our tnext, so find the next one. // Don't just increment lo or hi because we might @@ -1254,12 +1250,23 @@ void Envelope::GetValuesRelative(double *buffer, int bufferLen, // points to move over. That's why we binary search. int lo,hi; - BinarySearchForTime( lo, hi, t ); - // mEnv[0] is before t because of eliminations above, therefore lo >= 0 - // mEnv[len - 1] is after t, therefore hi <= len - 1 + BinarySearchForTime( lo, hi, tplus ); + // mEnv[0] is before tplus because of eliminations above, therefore lo >= 0 + // mEnv[len - 1] is after tplus, therefore hi <= len - 1 + tprev = mEnv[lo].GetT(); tnext = mEnv[hi].GetT(); + if ( hi + 1 < len && tnext == mEnv[ hi + 1 ].GetT() ) + // There is a discontinuity after this point-to-point interval. + // Will stop evaluating in this interval when time is slightly + // before tNext, then use the right limit. This is the right intent + // in case small roundoff errors cause a sample time to be a little + // before the envelope point time. + increment = epsilon; + else + increment = 0; + vprev = GetInterpolationStartValueAtPoint( lo ); vnext = GetInterpolationStartValueAtPoint( hi ); diff --git a/src/Envelope.h b/src/Envelope.h index 4ccb19a64..e90476912 100644 --- a/src/Envelope.h +++ b/src/Envelope.h @@ -134,7 +134,7 @@ public: // Accessors /** \brief Get envelope value at time t */ - double GetValue(double t) const; + double GetValue( double t, double sampleDur = 0 ) const; /** \brief Get many envelope points at once. * diff --git a/src/TrackArtist.cpp b/src/TrackArtist.cpp index be9ef3f34..1079b06ee 100644 --- a/src/TrackArtist.cpp +++ b/src/TrackArtist.cpp @@ -1352,7 +1352,10 @@ void TrackArtist::DrawIndividualSamples(wxDC &dc, int leftOffset, const wxRect & (int)(zoomInfo.TimeToPosition(time, -leftOffset)))); xpos[s] = xx; - const double tt = buffer[s] * clip->GetEnvelope()->GetValue(time); + // Calculate sample as it would be rendered, so quantize time + double value = + clip->GetEnvelope()->GetValue( time, 1.0 / clip->GetRate() ); + const double tt = buffer[s] * value; if (clipped && mShowClipping && ((tt <= -MAX_AUDIO) || (tt >= MAX_AUDIO))) clipped[clipcnt++] = xx; diff --git a/src/TrackPanel.cpp b/src/TrackPanel.cpp index 1e663f134..390a36767 100644 --- a/src/TrackPanel.cpp +++ b/src/TrackPanel.cpp @@ -4625,7 +4625,8 @@ float TrackPanel::FindSampleEditingLevel(wxMouseEvent &event, double dBRange, do Envelope *const env = mDrawingTrack->GetEnvelopeAtX(event.m_x); if (env) { - double envValue = env->GetValue(t0); + // Calculate sample as it would be rendered, so quantize time + double envValue = env->GetValue( t0, 1.0 / mDrawingTrack->GetRate() ); if (envValue > 0) newLevel /= envValue; else @@ -6904,7 +6905,7 @@ bool TrackPanel::HitTestEnvelope(Track *track, const wxRect &rect, const wxMouse // Get envelope point, range 0.0 to 1.0 const bool dB = !wavetrack->GetWaveformSettings().isLinear(); - // Convert x to time. + const double envValue = envelope->GetValue(mViewInfo->PositionToTime(event.m_x, rect.x)); float zoomMin, zoomMax; @@ -6992,7 +6993,8 @@ bool TrackPanel::HitTestSamples(Track *track, const wxRect &rect, const wxMouseE double envValue = 1.0; Envelope* env = wavetrack->GetEnvelopeAtX(event.GetX()); if (env) - envValue = env->GetValue(tt); + // Calculate sample as it would be rendered, so quantize time + envValue = env->GetValue( tt, 1.0 / wavetrack->GetRate() ); int yValue = GetWaveYPos( oneSample * envValue, zoomMin, zoomMax, diff --git a/src/WaveTrack.cpp b/src/WaveTrack.cpp index f56e56ef0..45b482f98 100644 --- a/src/WaveTrack.cpp +++ b/src/WaveTrack.cpp @@ -2130,7 +2130,7 @@ void WaveTrack::GetEnvelopeValues(double *buffer, size_t bufferLen, // Since this does not guarantee that the entire buffer is filled with values we need // to initialize the entire buffer to a default value. // - // This does mean that, in the cases where a usuable clip is located, the buffer value will + // This does mean that, in the cases where a usable clip is located, the buffer value will // be set twice. Unfortunately, there is no easy way around this since the clips are not // stored in increasing time order. If they were, we could just track the time as the // buffer is filled. @@ -2180,6 +2180,8 @@ void WaveTrack::GetEnvelopeValues(double *buffer, size_t bufferLen, rlen = limitSampleBufferSize( rlen, nClipLen ); rlen = std::min(rlen, size_t(floor(0.5 + (dClipEndTime - rt0) / tstep))); } + // Samples are obtained for the purpose of rendering a wave track, + // so quantize time clip->GetEnvelope()->GetValues(rbuf, rlen, rt0, tstep); } } diff --git a/src/WaveTrack.h b/src/WaveTrack.h index bbe74f972..273769fe9 100644 --- a/src/WaveTrack.h +++ b/src/WaveTrack.h @@ -259,8 +259,12 @@ class AUDACITY_DLL_API WaveTrack final : public PlayableTrack { fillFormat fill = fillZero, bool mayThrow = true) const; void Set(samplePtr buffer, sampleFormat format, sampleCount start, size_t len); + + // Fetch envelope values corresponding to uniformly separated sample times + // starting at the given time. void GetEnvelopeValues(double *buffer, size_t bufferLen, double t0) const; + std::pair GetMinMax( double t0, double t1, bool mayThrow = true) const; float GetRMS(double t0, double t1, bool mayThrow = true) const; From e428425c7dc31d39b73272553a475c03934a1379 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Fri, 26 May 2017 14:19:29 -0400 Subject: [PATCH 3/3] Change the drawing of wave track envelope at high zoom... ... The function defined by the envelope is evaluated exactly at the times of samples, but not between samples -- instead there is a simple interpolation. Therefore the curve might not go through the control points when they are not at sample times. The exact value of the function defined by the envelope has no influence on rendering of the sound in between samples. So this can make it clear that the middle point has no influence at all in case three points are very close. Drawing of other envelopes (Time track, Equalization curves) is not changed. --- src/Envelope.cpp | 41 ++++++++++++++++++++++++++++++++++++++--- src/Envelope.h | 7 +++++-- src/TimeTrack.cpp | 3 ++- src/TrackArtist.cpp | 6 ++++-- 4 files changed, 49 insertions(+), 8 deletions(-) diff --git a/src/Envelope.cpp b/src/Envelope.cpp index 34fee7819..e4b40afd9 100644 --- a/src/Envelope.cpp +++ b/src/Envelope.cpp @@ -1306,10 +1306,45 @@ void Envelope::GetValuesRelative(double *buffer, int bufferLen, } void Envelope::GetValues - (double *buffer, int bufferLen, int leftOffset, const ZoomInfo &zoomInfo) const + ( double alignedTime, double sampleDur, + double *buffer, int bufferLen, int leftOffset, + const ZoomInfo &zoomInfo ) + const { - for (int xx = 0; xx < bufferLen; ++xx) - buffer[xx] = GetValue(zoomInfo.PositionToTime(xx, -leftOffset)); + // Getting many envelope values, corresponding to pixel columns, which may + // not be uniformly spaced in time when there is a fisheye. + + double prevDiscreteTime, prevSampleVal, nextSampleVal; + for ( int xx = 0; xx < bufferLen; ++xx ) { + auto time = zoomInfo.PositionToTime( xx, -leftOffset ); + if ( sampleDur <= 0 ) + // Sample interval not defined (as for time track) + buffer[xx] = GetValue( time ); + else { + // The level of zoom-in may resolve individual samples. + // If so, then instead of evaluating the envelope directly, + // we draw a piecewise curve with knees at each sample time. + // This actually makes clearer what happens as you drag envelope + // points and make discontinuities. + auto leftDiscreteTime = alignedTime + + sampleDur * floor( ( time - alignedTime ) / sampleDur ); + if ( xx == 0 || leftDiscreteTime != prevDiscreteTime ) { + prevDiscreteTime = leftDiscreteTime; + prevSampleVal = + GetValue( prevDiscreteTime, sampleDur ); + nextSampleVal = + GetValue( prevDiscreteTime + sampleDur, sampleDur ); + } + auto ratio = ( time - leftDiscreteTime ) / sampleDur; + if ( GetExponential() ) + buffer[ xx ] = exp( + ( 1.0 - ratio ) * log( prevSampleVal ) + + ratio * log( nextSampleVal ) ); + else + buffer[ xx ] = + ( 1.0 - ratio ) * prevSampleVal + ratio * nextSampleVal; + } + } } // relative time diff --git a/src/Envelope.h b/src/Envelope.h index e90476912..736032537 100644 --- a/src/Envelope.h +++ b/src/Envelope.h @@ -142,10 +142,13 @@ public: * more than one value in a row. */ void GetValues(double *buffer, int len, double t0, double tstep) const; - /** \brief Get many envelope points at once, but don't assume uniform time step. + /** \brief Get many envelope points for pixel columns at once, + * but don't assume uniform time per pixel. */ void GetValues - (double *buffer, int bufferLen, int leftOffset, const ZoomInfo &zoomInfo) const; + ( double aligned_time, double sampleDur, + double *buffer, int bufferLen, int leftOffset, + const ZoomInfo &zoomInfo) const; // Guarantee an envelope point at the end of the domain. void Cap( double sampleDur ); diff --git a/src/TimeTrack.cpp b/src/TimeTrack.cpp index bc2260e61..0448b7a11 100644 --- a/src/TimeTrack.cpp +++ b/src/TimeTrack.cpp @@ -281,7 +281,8 @@ void TimeTrack::Draw(wxDC & dc, const wxRect & r, const ZoomInfo &zoomInfo) cons mRuler->Draw(dc, this); Doubles envValues{ size_t(mid.width) }; - GetEnvelope()->GetValues(envValues.get(), mid.width, 0, zoomInfo); + GetEnvelope()->GetValues + ( 0, 0, envValues.get(), mid.width, 0, zoomInfo ); dc.SetPen(AColor::envelopePen); diff --git a/src/TrackArtist.cpp b/src/TrackArtist.cpp index 1079b06ee..87f3a6c12 100644 --- a/src/TrackArtist.cpp +++ b/src/TrackArtist.cpp @@ -1781,7 +1781,8 @@ void TrackArtist::DrawClipWaveform(const WaveTrack *track, std::vector vEnv(mid.width); double *const env = &vEnv[0]; - clip->GetEnvelope()->GetValues(env, mid.width, leftOffset, zoomInfo); + clip->GetEnvelope()->GetValues + ( tOffset, 1.0 / rate, env, mid.width, leftOffset, zoomInfo ); // Draw the background of the track, outlining the shape of // the envelope and using a colored pen for the selected @@ -1912,7 +1913,8 @@ void TrackArtist::DrawClipWaveform(const WaveTrack *track, if (!showIndividualSamples) { std::vector vEnv2(rect.width); double *const env2 = &vEnv2[0]; - clip->GetEnvelope()->GetValues(env2, rect.width, leftOffset, zoomInfo); + clip->GetEnvelope()->GetValues + ( tOffset, 1.0 / rate, env2, rect.width, leftOffset, zoomInfo ); DrawMinMaxRMS(dc, rect, env2, zoomMin, zoomMax, dB, dBRange,