I can check off on the EngineBufferScaleLinear changes -- they look fine to me. Also the dialog stuff looks fine too. One note about the rateramp sensitivity -- is there a more usable way we can represent the sensitivity to the user? Given the ramping equations, can you characterize the sensitivity in some form of units? i.e. samples / second^2 ? It might be good to throw that in the label so you understand more what those numbers go into. Here are my specific comments about ratecontrol.cpp -- in general it all looks great! I did find one bug -- when you turn the rate down load (-30% or so) and then pitch bend down, the track can go backwards. The rest of my comments are inline: === modified file 'mixxx/src/engine/ratecontrol.cpp' --- mixxx/src/engine/ratecontrol.cpp 2009-09-28 02:12:07 +0000 +++ mixxx/src/engine/ratecontrol.cpp 2009-11-19 02:43:44 +0000 In this chunk -- can you add a safety check that the result of calculateRate is not NaN? (if it is NaN, just return 1*baserate or something) We need to make sure that NaN's don't end up in the engine, because they screw things up really fast -- they'll trigger an assert in our Butterworth filters. @@ -318,15 +358,17 @@ } else { // The buffer is playing, so calculate the buffer rate. - // There are three rate effects we apply: wheel, scratch, and jog. + // There are four rate effects we apply: wheel, scratch, jog and temp. // Wheel: a linear additive effect // Scratch: a rate multiplier - // Jog: a linear additive effect whose value is filtered - + // Jog: a linear additive effect whose value is filtered + // Temp: pitch bend + rate = (1. + getRawRate()) * baserate; rate += wheelFactor/10.; rate *= scratchFactor; rate += jogFactor; + rate += (getTempRate()) * baserate; // If we are reversing, flip the rate. if (m_pReverseButton->get()) { @@ -340,6 +382,135 @@ double RateControl::process(const double rate, const double currentSample, const double totalSamples, - const int iBufferSize) { - return 0; + const int bufferSamples) +{ + /* + * Code to handle temporary rate change buttons. + * + * We support two behaviours, the standard ramped pitch bending + * and pitch shift stepping, which is the old behaviour. + */ + + /* + * Initialize certain values necessary for pitchbending. Most of this + * code should be handled inside a slot, but we'd need to connect to + * the troublesome Latency ControlObject... Either the Master or Soundcard + * one. + */ + + double latrate = ((double)bufferSamples / (double)m_pSampleRate->get()); + + + if ((m_ePbPressed) && (!m_bTempStarted)) + { + m_bTempStarted = true; + m_dOldRate = m_pRateSlider->get(); + + + if ( m_eRateRampMode == RATERAMP_STEP ) + { Here, could you consolidate the multiple get() calls on the same CO's? Remember, each get() call corresponds to a lock of the static CO lock, and this is in the PortAudio callback thread. Also, could you add checks on all of your divisions that you are not dividing by zero? A MIDI script for example, could set rateRange to 0. Dividing by zero could cause a NaN to cascade through here and be output from calculateRate(). + // old temporary pitch shift behaviour + double change = m_pRateDir->get() * m_dTemp / + (100. * m_pRateRange->get()); + double csmall = m_pRateDir->get() * m_dTempSmall / + (100. * m_pRateRange->get()); + + + if (buttonRateTempUp->get()) + addRateTemp(change); + else if (buttonRateTempDown->get()) + subRateTemp(change); + else if (buttonRateTempUpSmall->get()) + addRateTemp(csmall); + else if (buttonRateTempDownSmall->get()) + subRateTemp(csmall); + } + else + { + m_dTempRateChange = ((double)latrate / ((double)m_iRateRampSensitivity / 100.)); + + if (m_eRampBackMode == RATERAMP_RAMPBACK_PERIOD) + m_dRateTempRampbackChange = 0.0; + } + + } + + if ((m_ePbCurrent) && (m_eRateRampMode == RATERAMP_LINEAR)) + { + // apply ramped pitchbending + if ( m_ePbCurrent == RateControl::RATERAMP_UP ) + addRateTemp(m_dTempRateChange); + else if ( m_ePbCurrent == RateControl::RATERAMP_DOWN ) + subRateTemp(m_dTempRateChange); + } + else if ((!m_ePbCurrent) && (m_eRateRampMode == RATERAMP_STEP)) + { + resetRateTemp(); + } + else if ((m_bTempStarted) || ((m_eRampBackMode != RATERAMP_RAMPBACK_NONE) && (m_dRateTemp != 0.0))) + { + // No buttons pressed, so time to deinitialize + m_bTempStarted = false; + + + if ( m_eRateRampMode == RATERAMP_STEP ) + m_pRateSlider->set(m_dOldRate); + else { + + if ((m_eRampBackMode == RATERAMP_RAMPBACK_PERIOD) && (m_dRateTempRampbackChange == 0.0 )) + { + int period = 2; + if (period) + m_dRateTempRampbackChange = fabs(m_dRateTemp / (double)period); + else { + resetRateTemp(); + return 1; + } + + } + + if ((m_eRampBackMode != RATERAMP_RAMPBACK_NONE)) + { + + if ( fabs(m_dRateTemp) < m_dRateTempRampbackChange) + resetRateTemp(); + else if ( m_dRateTemp > 0 ) + subRateTemp(m_dRateTempRampbackChange); + else + addRateTemp(m_dRateTempRampbackChange); + } + else + resetRateTemp(); + } + } + + return 1; +} + +double RateControl::getTempRate() { + return (m_pRateDir->get() * (m_dRateTemp * m_pRateRange->get())); +} + Since all the add/sub methods use this one, could you add NaN checking to this method as well? +void RateControl::setRateTemp(double v) +{ + m_dRateTemp = v; + if ( m_dRateTemp < -1.0 ) + m_dRateTemp = -1.0; + if ( m_dRateTemp > 1.0 ) + m_dRateTemp = 1.0; +} + +void RateControl::addRateTemp(double v) +{ + setRateTemp(m_dRateTemp + v); +} + +void RateControl::subRateTemp(double v) +{ + setRateTemp(m_dRateTemp - v); +} + +void RateControl::resetRateTemp(void) +{ + setRateTemp(0.0); } Thanks Phil!