Improved hinting the ReadAheadmanager. It uses now frames instead of …

…stereo samples

src/engine/cachingreader.cpp

@@ -9,20 +9,13 @@
 #include "util/math.h"
 #include "util/sample.h"
 
+
 namespace {
 
-// NOTE(uklotzde): The following comment has been adopted without
-// modifications and should be rephrased.
-//
-// To prevent every bit of code having to guess how many samples
-// forward it makes sense to keep in memory, the hinter can provide
-// either 0 for a forward hint or -1 for a backward hint. We should
-// be calculating an appropriate number of samples to go backward as
-// some function of the latency, but for now just leave this as a
-// constant. 2048 is a pretty good number of samples because 25ms
-// latency corresponds to 1102.5 mono samples and we need double
-// that for stereo samples.
-const SINT kDefaultHintSamples = 1024 * CachingReaderChunk::kChannels;
+// This is the default hint frameCount that is adopted in case of Hint::kFrameCountForward and
+// Hint::kFrameCountBackward count is provided. It matches 23 ms @ 44.1 kHz
+// TODO() Do we suffer chache misses if we use an audio buffer of above 23 ms?
+const SINT kDefaultHintFrames = 1024;
 
 } // anonymous namespace
 
@@ -372,30 +365,27 @@ void CachingReader::hintAndMaybeWake(const HintVector& hintList) {
     // any are not, then wake.
     bool shouldWake = false;
 
-    for (HintVector::const_iterator it = hintList.constBegin();
-         it != hintList.constEnd(); ++it) {
-        // Copy, don't use reference.
-        Hint hint = *it;
+    for (const auto& hint: hintList) {
+        SINT hintFrame = hint.frame;
+        SINT hintFrameCount = hint.frameCount;
 
         // Handle some special length values
-        if (hint.length == 0) {
-            hint.length = kDefaultHintSamples;
-        } else if (hint.length == -1) {
-            hint.sample -= kDefaultHintSamples;
-            hint.length = kDefaultHintSamples;
-            if (hint.sample < 0) {
-                hint.length += hint.sample;
-                hint.sample = 0;
+        if (hintFrameCount == Hint::kFrameCountForward) {
+        	hintFrameCount = kDefaultHintFrames;
+        } else if (hintFrameCount == Hint::kFrameCountBackward) {
+        	hintFrame -= kDefaultHintFrames;
+        	hintFrameCount = kDefaultHintFrames;
+            if (hintFrame < 0) {
+            	hintFrameCount += hintFrame;
+                hintFrame = 0;
             }
         }
-        if (hint.length < 0) {
-            qDebug() << "ERROR: Negative hint length. Ignoring.";
+
+        VERIFY_OR_DEBUG_ASSERT(hintFrameCount > 0) {
+            qWarning() << "ERROR: Negative hint length. Ignoring.";
             continue;
         }
 
-        const SINT hintFrame = CachingReaderChunk::samples2frames(hint.sample);
-        const SINT hintFrameCount = CachingReaderChunk::samples2frames(hint.length);
-
         SINT minReadableFrameIndex = hintFrame;
         SINT maxReadableFrameIndex = hintFrame + hintFrameCount;
         mixxx::AudioSource::clampFrameInterval(&minReadableFrameIndex, &maxReadableFrameIndex, m_maxReadableFrameIndex);

src/engine/cachingreader.h

@@ -22,17 +22,22 @@
 // SoundSource will be used 'soon' and so it should be brought into memory by
 // the reader work thread.
 typedef struct Hint {
-    // The sample to ensure is present in memory.
-    int sample;
-    // If a range of samples should be present, use length to indicate that the
-    // range (sample, sample+length) should be present in memory.
-    int length;
+    // The frame to ensure is present in memory.
+    SINT frame;
+    // If a range of frames should be present, use frameCount to indicate that the
+    // range (frame, frame + frameCount) should be present in memory.
+    SINT frameCount;
     // Currently unused -- but in the future could be used to prioritize certain
     // hints over others. A priority of 1 is the highest priority and should be
     // used for samples that will be read imminently. Hints for samples that
     // have the potential to be read (i.e. a cue point) should be issued with
     // priority >10.
     int priority;
+
+    // for the default frame count in forward direction
+    static constexpr SINT kFrameCountForward = 0;
+    static constexpr SINT kFrameCountBackward = -1;
+
 } Hint;
 
 // Note that we use a QVarLengthArray here instead of a QVector. Since this list

src/engine/cuecontrol.cpp

@@ -550,8 +550,8 @@ void CueControl::hintReader(HintVector* pHintList) {
     Hint cue_hint;
     double cuePoint = m_pCuePoint->get();
     if (cuePoint >= 0) {
-        cue_hint.sample = SampleUtil::floorPlayPosToFrameStart(m_pCuePoint->get(), 2);
-        cue_hint.length = 0;
+        cue_hint.frame = SampleUtil::floorPlayPosToFrame(m_pCuePoint->get());
+        cue_hint.frameCount = Hint::kFrameCountForward;
         cue_hint.priority = 10;
         pHintList->append(cue_hint);
     }
@@ -562,8 +562,8 @@ void CueControl::hintReader(HintVector* pHintList) {
     for (const auto& pControl: m_hotcueControls) {
         double position = pControl->getPosition();
         if (position != -1) {
-            cue_hint.sample = SampleUtil::floorPlayPosToFrameStart(position, 2);
-            cue_hint.length = 0;
+            cue_hint.frame = SampleUtil::floorPlayPosToFrame(position);
+            cue_hint.frameCount = Hint::kFrameCountForward;
             cue_hint.priority = 10;
             pHintList->append(cue_hint);
         }

src/engine/enginebuffer.cpp

@@ -1244,9 +1244,13 @@ void EngineBuffer::hintReader(const double dRate) {
     //if slipping, hint about virtual position so we're ready for it
     if (m_bSlipEnabledProcessing) {
         Hint hint;
-        hint.length = 2048; //default length please
-        hint.sample = m_dSlipRate >= 0 ? m_dSlipPosition : m_dSlipPosition - 2048;
+        hint.frame = SampleUtil::floorPlayPosToFrame(m_dSlipPosition);
         hint.priority = 1;
+        if (m_dSlipRate >= 0) {
+            hint.frameCount = Hint::kFrameCountForward;
+        } else {
+            hint.frameCount = Hint::kFrameCountBackward;
+        }
         m_hintList.append(hint);
     }
 

src/engine/loopingcontrol.cpp

@@ -390,21 +390,21 @@ void LoopingControl::hintReader(HintVector* pHintList) {
         // aren't that bad to make anyway.
         if (loopSamples.start >= 0) {
             loop_hint.priority = 2;
-            loop_hint.sample = SampleUtil::floorPlayPosToFrameStart(loopSamples.start, 2);
-            loop_hint.length = 0; // Let it issue the default length
+            loop_hint.frame = SampleUtil::floorPlayPosToFrame(loopSamples.start);
+            loop_hint.frameCount = Hint::kFrameCountForward;
             pHintList->append(loop_hint);
         }
         if (loopSamples.end >= 0) {
             loop_hint.priority = 10;
-            loop_hint.sample = SampleUtil::ceilPlayPosToFrameStart(loopSamples.end, 2);
-            loop_hint.length = -1; // Let it issue the default (backwards) length
+            loop_hint.frame = SampleUtil::ceilPlayPosToFrame(loopSamples.end);
+            loop_hint.frameCount = Hint::kFrameCountBackward;
             pHintList->append(loop_hint);
         }
     } else {
         if (loopSamples.start >= 0) {
             loop_hint.priority = 10;
-            loop_hint.sample = SampleUtil::floorPlayPosToFrameStart(loopSamples.start, 2);
-            loop_hint.length = 0; // Let it issue the default length
+            loop_hint.frame = SampleUtil::floorPlayPosToFrame(loopSamples.start);
+            loop_hint.frameCount = Hint::kFrameCountForward;
             pHintList->append(loop_hint);
         }
     }

src/engine/readaheadmanager.cpp

@@ -175,21 +175,23 @@ void ReadAheadManager::hintReader(double dRate, HintVector* pHintList) {
 
     // SoundTouch can read up to 2 chunks ahead. Always keep 2 chunks ahead in
     // cache.
-    SINT length_to_cache = 2 * CachingReaderChunk::kSamples;
+    SINT frameCountToCache = 2 * CachingReaderChunk::kFrames;
 
     // this called after the precious chunk was consumed
-    int sample = SampleUtil::roundPlayPosToFrameStart(
-            m_currentPosition, kNumChannels);
-    current_position.length = length_to_cache;
-    current_position.sample = in_reverse ?
-            sample - length_to_cache :
-            sample;
+    SINT frame = SampleUtil::roundPlayPosToFrameStart(
+            m_currentPosition, kNumChannels) / kNumChannels;
+    current_position.frameCount = frameCountToCache;
+    current_position.frame = in_reverse ?
+            frame - frameCountToCache:
+            frame;
 
     // If we are trying to cache before the start of the track,
     // Then we don't need to cache because it's all zeros!
-    if (current_position.sample < 0 &&
-        current_position.sample + current_position.length < 0)
-        return;
+    if (current_position.frame < 0 &&
+            current_position.frame + current_position.frameCount < 0)
+    {
+    	return;
+    }
 
     // top priority, we need to read this data immediately
     current_position.priority = 1;

src/util/sample.h

@@ -21,6 +21,11 @@ class SampleUtil {
     };
     Q_DECLARE_FLAGS(CLIP_STATUS, CLIP_FLAG);
 
+    // The PlayPosition, Loops and Cue Points used in the Database and
+    // Mixxx CO interface are expressed as a floating point number of stereo samples.
+    // This is some legacy, we cannot easily revert.
+    static constexpr double kPlayPositionChannels = 2.0;
+
     // Allocated a buffer of CSAMPLE's with length size. Ensures that the buffer
     // is 16-byte aligned for SSE enhancement.
     static CSAMPLE* alloc(SINT size);
@@ -108,6 +113,23 @@ class SampleUtil {
         return playPosFrames * numChannels;
     }
 
+    inline static SINT roundPlayPosToFrame(double playPos) {
+        return static_cast<SINT>(round(playPos / kPlayPositionChannels));
+    }
+
+    inline static SINT truncPlayPosToFrame(double playPos) {
+        return static_cast<SINT>(playPos / kPlayPositionChannels);
+    }
+
+    inline static SINT floorPlayPosToFrame(double playPos) {
+        return static_cast<SINT>(floor(playPos / kPlayPositionChannels));
+
+    }
+
+    inline static SINT ceilPlayPosToFrame(double playPos) {
+        return static_cast<SINT>(ceil(playPos / kPlayPositionChannels));
+    }
+
     // Multiply every sample in pBuffer by gain
     static void applyGain(CSAMPLE* pBuffer, CSAMPLE gain,
             SINT numSamples);