From 537ccfbc4ff886dd510f1321820361bdbc615fc9 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Fri, 26 May 2017 14:08:27 -0400 Subject: [PATCH] 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;