Refactor the engine to process samples in the range of [-1.0, 1.0]. Fixes Bug #1204039.

src/analyserqueue.cpp

@@ -177,8 +177,10 @@ bool AnalyserQueue::doAnalysis(TrackPointer tio, SoundSourceProxy* pSoundSource)
             dieflag = true;
         }
 
+        // Normalize the samples from [SHRT_MIN, SHRT_MAX] to [-1.0, 1.0].
+        // TODO(rryan): Change the SoundSource API to do this for us.
         for (int i = 0; i < read; ++i) {
-            samples[i] = ((float)data16[i])/32767.0f;
+            samples[i] = static_cast<CSAMPLE>(data16[i]) / SHRT_MAX;
         }
 
         QListIterator<Analyser*> it(m_aq);

src/cachingreaderworker.cpp

@@ -1,5 +1,6 @@
 
 #include <math.h>
+#include <limits.h>
 
 #include <QtDebug>
 #include <QFileInfo>
@@ -85,6 +86,13 @@ void CachingReaderWorker::processChunkReadRequest(ChunkReadRequest* request,
     CSAMPLE* buffer = request->chunk->data;
     //qDebug() << "Reading into " << buffer;
     SampleUtil::convert(buffer, m_pSample, samples_read);
+
+    // Normalize the samples from [SHRT_MIN, SHRT_MAX] to [-1.0, 1.0].
+    // TODO(rryan): Change the SoundSource API to do this for us.
+    for (int i = 0; i < samples_read; ++i) {
+        buffer[i] /= SHRT_MAX;
+    }
+
     update->status = CHUNK_READ_SUCCESS;
     update->chunk->length = samples_read;
 }

src/encoder/encoderffmpegcore.cpp

@@ -137,12 +137,13 @@ void EncoderFfmpegCore::encodeBuffer(const CSAMPLE *samples, const int size) {
     unsigned int l_iBufPos = 0;
     unsigned int l_iPos = 0;
 
+    // TODO(XXX): Get rid of repeated malloc here!
     float *l_fNormalizedSamples = (float *)malloc(size * sizeof(float));
 
-    // Because of normalization to SHORT_MAX = 0x7FFF we have to make this not to clip!
-    // Comments also: https://bugs.launchpad.net/mixxx/+bug/1204039
+    // We use normalized floats in the engine [-1.0, 1.0] and FFMPEG expects
+    // samples in the range [-1.0, 1.0] so no conversion is required.
     for (j = 0; j < size; j++) {
-        l_fNormalizedSamples[j] = samples[j] / 0x7FFF;
+        l_fNormalizedSamples[j] = samples[j];
     }
 
     // In MP3 this writes Header same In ogg

src/encoder/encodermp3.cpp

@@ -17,6 +17,7 @@
 
 #include <QtDebug>
 #include <QObject>
+#include <limits.h>
 
 #include "encoder/encodermp3.h"
 #include "encoder/encodercallback.h"
@@ -282,18 +283,17 @@ void EncoderMp3::encodeBuffer(const CSAMPLE *samples, const int size) {
         return;
     int outsize = 0;
     int rc = 0;
-    int i = 0;
 
     outsize = (int)((1.25 * size + 7200) + 1);
     bufferOutGrow(outsize);
 
     bufferInGrow(size);
 
-    // Deinterleave samples
-    for (i = 0; i < size/2; ++i)
-    {
-        m_bufferIn[0][i] = samples[i*2];
-        m_bufferIn[1][i] = samples[i*2+1];
+    // Deinterleave samples. We use normalized floats in the engine [-1.0, 1.0]
+    // but LAME expects samples in the range [SHRT_MIN, SHRT_MAX].
+    for (int i = 0; i < size/2; ++i) {
+        m_bufferIn[0][i] = samples[i*2] * SHRT_MAX;
+        m_bufferIn[1][i] = samples[i*2+1] * SHRT_MAX;
     }
 
     rc = lame_encode_buffer_float(m_lameFlags, m_bufferIn[0], m_bufferIn[1],
@@ -362,4 +362,3 @@ void EncoderMp3::updateMetaData(char* artist, char* title, char* album){
     m_metaDataArtist = artist;
     m_metaDataAlbum = album;
 }
-

src/encoder/encodervorbis.cpp

@@ -122,11 +122,12 @@ void EncoderVorbis::writePage() {
 void EncoderVorbis::encodeBuffer(const CSAMPLE *samples, const int size) {
     float **buffer = vorbis_analysis_buffer(&m_vdsp, size);
 
-    // Deinterleave samples
-    for (int i = 0; i < size/2; ++i)
-    {
-        buffer[0][i] = samples[i*2]/32768.f;
-        buffer[1][i] = samples[i*2+1]/32768.f;
+    // Deinterleave samples. We use normalized floats in the engine [-1.0, 1.0]
+    // and libvorbis expects samples in the range [-1.0, 1.0] so no conversion
+    // is required.
+    for (int i = 0; i < size/2; ++i) {
+        buffer[0][i] = samples[i*2];
+        buffer[1][i] = samples[i*2+1];
     }
     /** encodes audio **/
     vorbis_analysis_wrote(&m_vdsp, size/2);

src/engine/enginebuffer.cpp

@@ -102,9 +102,12 @@ EngineBuffer::EngineBuffer(const char* _group, ConfigObject<ConfigValue>* _confi
     m_iCrossFadeSamples(0),
     m_iLastBufferSize(0) {
 
-    // Generate dither values
+    // Generate dither values. When engine samples used to be within [SHRT_MIN,
+    // SHRT_MAX] dithering values were in the range [-0.5, 0.5]. Now that we
+    // normalize engine samples to the range [-1.0, 1.0] we divide by SHRT_MAX
+    // to preserve the previous behavior.
     for (int i = 0; i < MAX_BUFFER_LEN; ++i) {
-        m_pDitherBuffer[i] = static_cast<float>(rand() % 32768) / 32768.0 - 0.5;
+        m_pDitherBuffer[i] = (static_cast<CSAMPLE>(rand() % RAND_MAX) / RAND_MAX - 0.5) / SHRT_MAX;
     }
 
     // zero out crossfade buffer

src/engine/engineclipping.cpp

@@ -31,10 +31,8 @@ EngineClipping::~EngineClipping()
     delete m_ctrlClipping;
 }
 
-void EngineClipping::process(const CSAMPLE* pIn, CSAMPLE* pOutput, const int iBufferSize)
-{
-    static const FLOAT_TYPE kfMaxAmp = 32767.;
-    // static const FLOAT_TYPE kfClip = 0.8*kfMaxAmp;
+void EngineClipping::process(const CSAMPLE* pIn, CSAMPLE* pOutput, const int iBufferSize) {
+    const CSAMPLE kfMaxAmp = 1.0;
 
     // SampleUtil clamps the buffer and if pIn and pOut are aliased will not copy.
     clipped = SampleUtil::copyClampBuffer(kfMaxAmp, -kfMaxAmp,

src/engine/enginedeck.cpp

@@ -129,7 +129,7 @@ bool EngineDeck::isActive() {
     return (m_pBuffer->isTrackLoaded() || isPassthroughActive());
 }
 
-void EngineDeck::receiveBuffer(AudioInput input, const short* pBuffer, unsigned int nFrames) {
+void EngineDeck::receiveBuffer(AudioInput input, const CSAMPLE* pBuffer, unsigned int nFrames) {
     // Skip receiving audio input if passthrough is not active
     if (!m_bPassthroughIsActive) {
         return;
@@ -150,29 +150,36 @@ void EngineDeck::receiveBuffer(AudioInput input, const short* pBuffer, unsigned
         nFrames = MAX_BUFFER_LEN / iChannels;
     }
 
+    const CSAMPLE* pWriteBuffer = NULL;
+    unsigned int samplesToWrite = 0;
+
     if (iChannels == 1) {
-        // Do mono -> stereo conversion. There isn't a suitable SampleUtil
-        // method that can do mono->stereo and short->float in one pass.
+        // Do mono -> stereo conversion.
         for (unsigned int i = 0; i < nFrames; ++i) {
             m_pConversionBuffer[i*2 + 0] = pBuffer[i];
             m_pConversionBuffer[i*2 + 1] = pBuffer[i];
         }
+        pWriteBuffer = m_pConversionBuffer;
+        samplesToWrite = nFrames * 2;
     } else if (iChannels == 2) {
-        // Use the conversion buffer to both convert from short and double.
-        SampleUtil::convert(m_pConversionBuffer, pBuffer, nFrames*iChannels);
+        // Already in stereo. Use pBuffer as-is.
+        pWriteBuffer = pBuffer;
+        samplesToWrite = nFrames * iChannels;
     } else {
         qWarning() << "EnginePassthrough got greater than stereo input. Not currently handled.";
     }
 
-    const unsigned int samplesToWrite = nFrames * iChannels;
-
-    // TODO(rryan) do we need to verify the input is the one we asked for? Oh well.
-    unsigned int samplesWritten = m_sampleBuffer.write(m_pConversionBuffer, samplesToWrite);
-    if (samplesWritten < samplesToWrite) {
-        // Buffer overflow. We aren't processing samples fast enough. This
-        // shouldn't happen since the deck spits out samples just as fast as they
-        // come in, right?
-        qWarning() << "ERROR: Buffer overflow in EngineMicrophone. Dropping samples on the floor.";
+    if (pWriteBuffer != NULL) {
+        // TODO(rryan) do we need to verify the input is the one we asked for?
+        // Oh well.
+        unsigned int samplesWritten = m_sampleBuffer.write(pWriteBuffer,
+                                                           samplesToWrite);
+        if (samplesWritten < samplesToWrite) {
+            // Buffer overflow. We aren't processing samples fast enough. This
+            // shouldn't happen since the deck spits out samples just as fast as
+            // they come in, right?
+            qWarning() << "ERROR: Buffer overflow in EngineMicrophone. Dropping samples on the floor.";
+        }
     }
 }
 

src/engine/enginedeck.h

@@ -54,7 +54,8 @@ class EngineDeck : public EngineChannel, public AudioDestination {
 
     // This is called by SoundManager whenever there are new samples from the
     // deck to be processed.
-    virtual void receiveBuffer(AudioInput input, const short *pBuffer, unsigned int nFrames);
+    virtual void receiveBuffer(AudioInput input, const CSAMPLE* pBuffer,
+                               unsigned int nFrames);
 
     // Called by SoundManager whenever the passthrough input is connected to a
     // soundcard input.

src/engine/enginemicrophone.cpp

@@ -64,43 +64,53 @@ void EngineMicrophone::onInputDisconnected(AudioInput input) {
     m_pEnabled->set(0.0f);
 }
 
-void EngineMicrophone::receiveBuffer(AudioInput input, const short* pBuffer, unsigned int nFrames) {
-
-    if (input.getType() != AudioPath::MICROPHONE ||
-        AudioInput::channelsNeededForType(input.getType()) != 1) {
+void EngineMicrophone::receiveBuffer(AudioInput input, const CSAMPLE* pBuffer,
+                                     unsigned int nFrames) {
+    if (input.getType() != AudioPath::MICROPHONE) {
         // This is an error!
         qWarning() << "EngineMicrophone receieved an AudioInput for a non-Microphone type or a non-mono buffer!";
         return;
     }
 
-    // Use the conversion buffer to both convert from short and double into
-    // stereo.
+    const unsigned int iChannels = AudioInput::channelsNeededForType(input.getType());
 
     // Check that the number of mono frames doesn't exceed MAX_BUFFER_LEN/2
     // because thats our conversion buffer size.
-    if (nFrames > MAX_BUFFER_LEN / 2) {
+    if (nFrames > MAX_BUFFER_LEN / iChannels) {
         qWarning() << "Dropping microphone samples because the input buffer is too large.";
-        nFrames = MAX_BUFFER_LEN / 2;
+        nFrames = MAX_BUFFER_LEN / iChannels;
     }
 
-    // There isn't a suitable SampleUtil method that can do mono->stereo and
-    // short->float in one pass.
-    // SampleUtil::convert(m_pConversionBuffer, pBuffer, iNumSamples);
-    for (unsigned int i = 0; i < nFrames; ++i) {
-        m_pConversionBuffer[i*2 + 0] = pBuffer[i];
-        m_pConversionBuffer[i*2 + 1] = pBuffer[i];
-    }
+    const CSAMPLE* pWriteBuffer = NULL;
+    unsigned int samplesToWrite = 0;
 
-    // m_pConversionBuffer is now stereo, so double the number of samples
-    const unsigned int iNumSamples = nFrames * 2;
+    if (iChannels == 1) {
+        // Do mono -> stereo conversion.
+        for (unsigned int i = 0; i < nFrames; ++i) {
+            m_pConversionBuffer[i*2 + 0] = pBuffer[i];
+            m_pConversionBuffer[i*2 + 1] = pBuffer[i];
+        }
+        pWriteBuffer = m_pConversionBuffer;
+        samplesToWrite = nFrames * 2;
+    } else if (iChannels == 2) {
+        // Already in stereo. Use pBuffer as-is.
+        pWriteBuffer = pBuffer;
+        samplesToWrite = nFrames * iChannels;
+    } else {
+        qWarning() << "EngineMicrophone got greater than stereo input. Not currently handled.";
+    }
 
-    // TODO(rryan) do we need to verify the input is the one we asked for? Oh well.
-    unsigned int samplesWritten = m_sampleBuffer.write(m_pConversionBuffer, iNumSamples);
-    if (samplesWritten < iNumSamples) {
-        // Buffer overflow. We aren't processing samples fast enough. This
-        // shouldn't happen since the mic spits out samples just as fast as they
-        // come in, right?
-        qWarning() << "ERROR: Buffer overflow in EngineMicrophone. Dropping samples on the floor.";
+    if (pWriteBuffer != NULL) {
+        // TODO(rryan) do we need to verify the input is the one we asked for?
+        // Oh well.
+        unsigned int samplesWritten = m_sampleBuffer.write(pWriteBuffer,
+                                                           samplesToWrite);
+        if (samplesWritten < samplesToWrite) {
+            // Buffer overflow. We aren't processing samples fast enough. This
+            // shouldn't happen since the mic spits out samples just as fast as they
+            // come in, right?
+            qWarning() << "ERROR: Buffer overflow in EngineMicrophone. Dropping samples on the floor.";
+        }
     }
 }
 

src/engine/enginemicrophone.h

@@ -28,7 +28,8 @@ class EngineMicrophone : public EngineChannel, public AudioDestination {
 
     // This is called by SoundManager whenever there are new samples from the
     // microphone to be processed
-    virtual void receiveBuffer(AudioInput input, const short* pBuffer, unsigned int iNumSamples);
+    virtual void receiveBuffer(AudioInput input, const CSAMPLE* pBuffer,
+                               unsigned int iNumSamples);
 
     // Called by SoundManager whenever the microphone input is connected to a
     // soundcard input.

src/engine/enginepassthrough.cpp

@@ -62,34 +62,54 @@ void EnginePassthrough::onInputDisconnected(AudioInput input) {
     m_pEnabled->set(0.0f);
 }
 
-void EnginePassthrough::receiveBuffer(AudioInput input, const short* pBuffer, unsigned int nFrames) {
-
+void EnginePassthrough::receiveBuffer(AudioInput input, const CSAMPLE* pBuffer,
+                                      unsigned int nFrames) {
     if (input.getType() != AudioPath::EXTPASSTHROUGH) {
         // This is an error!
         qDebug() << "WARNING: EnginePassthrough receieved an AudioInput for a non-passthrough type!";
         return;
     }
 
-    // Use the conversion buffer to both convert from short and double into
-    // stereo.
+    const unsigned int iChannels = AudioInput::channelsNeededForType(input.getType());
 
     // Check that the number of mono frames doesn't exceed MAX_BUFFER_LEN/2
     // because thats our conversion buffer size.
-    if (nFrames > MAX_BUFFER_LEN / 2) {
+    if (nFrames > MAX_BUFFER_LEN / iChannels) {
         qDebug() << "WARNING: Dropping passthrough samples because the input buffer is too large.";
-        nFrames = MAX_BUFFER_LEN / 2;
+        nFrames = MAX_BUFFER_LEN / iChannels;
     }
 
-    const unsigned int iNumSamples = nFrames * 2;
-    SampleUtil::convert(m_pConversionBuffer, pBuffer, iNumSamples);
+    const CSAMPLE* pWriteBuffer = NULL;
+    unsigned int samplesToWrite = 0;
 
-    // TODO(rryan) (or bkgood?) do we need to verify the input is the one we asked for? Oh well.
-    unsigned int samplesWritten = m_sampleBuffer.write(m_pConversionBuffer, iNumSamples);
-    if (samplesWritten < iNumSamples) {
-        // Buffer overflow. We aren't processing samples fast enough. This
-        // shouldn't happen since the deck spits out samples just as fast as they
-        // come in, right?
-        qWarning() << "ERROR: Buffer overflow in EnginePassthrough. Dropping samples on the floor.";
+    if (iChannels == 1) {
+        // Do mono -> stereo conversion.
+        for (unsigned int i = 0; i < nFrames; ++i) {
+            m_pConversionBuffer[i*2 + 0] = pBuffer[i];
+            m_pConversionBuffer[i*2 + 1] = pBuffer[i];
+        }
+        pWriteBuffer = m_pConversionBuffer;
+        samplesToWrite = nFrames * 2;
+    } else if (iChannels == 2) {
+        // Already in stereo. Use pBuffer as-is.
+        pWriteBuffer = pBuffer;
+        samplesToWrite = nFrames * iChannels;
+    } else {
+        qWarning() << "EnginePassthrough got greater than stereo input. Not currently handled.";
+    }
+
+
+    if (pWriteBuffer != NULL) {
+        // TODO(rryan) do we need to verify the input is the one we asked for?
+        // Oh well.
+        unsigned int samplesWritten = m_sampleBuffer.write(m_pConversionBuffer,
+                                                           samplesToWrite);
+        if (samplesWritten < samplesToWrite) {
+            // Buffer overflow. We aren't processing samples fast enough. This
+            // shouldn't happen since the deck spits out samples just as fast as they
+            // come in, right?
+            qWarning() << "ERROR: Buffer overflow in EnginePassthrough. Dropping samples on the floor.";
+        }
     }
 }
 

src/engine/enginepassthrough.h

@@ -29,7 +29,8 @@ class EnginePassthrough : public EngineChannel, public AudioDestination {
 
     // This is called by SoundManager whenever there are new samples from the
     // deck to be processed
-    virtual void receiveBuffer(AudioInput input, const short *pBuffer, unsigned int nFrames);
+    virtual void receiveBuffer(AudioInput input, const CSAMPLE* pBuffer,
+                               unsigned int nFrames);
 
     // Called by SoundManager whenever the passthrough input is connected to a
     // soundcard input.