Merge pull request #1646 from uklotzde/lp1766834_windows_m4a_decoding_errors

lp1766834: Detect M4A decoding errors on Windows

2 files changed, 46 insertions, 20 deletions

diff --git a/plugins/soundsourcemediafoundation/soundsourcemediafoundation.cpp b/plugins/soundsourcemediafoundation/soundsourcemediafoundation.cpp
index 3ed0d3b5e8..8eac78220d 100755
--- a/plugins/soundsourcemediafoundation/soundsourcemediafoundation.cpp
+++ b/plugins/soundsourcemediafoundation/soundsourcemediafoundation.cpp
@@ -351,10 +351,31 @@ ReadableSampleFrames SoundSourceMediaFoundation::readSampleFramesClamped(
         }
         DEBUG_ASSERT(pSample != nullptr);
         SINT readerFrameIndex = m_streamUnitConverter.toFrameIndex(streamPos);
-        DEBUG_ASSERT(
-                (m_currentFrameIndex == kUnknownFrameIndex) || // unknown position after seeking
-                (m_currentFrameIndex == readerFrameIndex));
-        m_currentFrameIndex = readerFrameIndex;
+        if (m_currentFrameIndex == kUnknownFrameIndex) {
+            // Unknown position after seeking
+            m_currentFrameIndex = readerFrameIndex;
+            /*
+            kLogger.debug()
+                    << "Stream position (in sample frames) after seeking:"
+                    << "target =" << writableSampleFrames.frameIndexRange().end()
+                    << "current =" << readerFrameIndex;
+            */
+        } else {
+            // Both positions should match, otherwise readerFrameIndex
+            // is inaccurate due to rounding errors after conversion from
+            // stream units to frames! But if this ever happens we better
+            // trust m_currentFrameIndex that is continuously updated while
+            // reading in forward direction.
+            VERIFY_OR_DEBUG_ASSERT(m_currentFrameIndex == readerFrameIndex) {
+                kLogger.debug()
+                        << "streamPos [100 ns] =" << streamPos
+                        << ", sampleRate [Hz] =" << sampleRate();
+                kLogger.warning()
+                        << "Stream position (in sample frames) while reading is inaccurate:"
+                        << "expected =" << m_currentFrameIndex
+                        << "actual =" << readerFrameIndex;
+            }
+        }
 
         DWORD dwSampleBufferCount = 0;
         HRESULT hrGetBufferCount =
diff --git a/plugins/soundsourcemediafoundation/soundsourcemediafoundation.h b/plugins/soundsourcemediafoundation/soundsourcemediafoundation.h
index 7e1abf4da5..e9a8d608f1 100755
--- a/plugins/soundsourcemediafoundation/soundsourcemediafoundation.h
+++ b/plugins/soundsourcemediafoundation/soundsourcemediafoundation.h
@@ -17,37 +17,42 @@ class StreamUnitConverter final {
   public:
     StreamUnitConverter()
         : m_pAudioSource(nullptr),
-          m_streamUnitsPerFrame(0.0),
-          m_toFrameIndexBias(0) {
+          m_fromSampleFramesToStreamUnits(0),
+          m_fromStreamUnitsToSampleFrames(0) {
     }
     explicit StreamUnitConverter(const AudioSource* pAudioSource)
         : m_pAudioSource(pAudioSource),
-          m_streamUnitsPerFrame(double(kStreamUnitsPerSecond) / double(pAudioSource->sampleRate())),
-          m_toFrameIndexBias(kStreamUnitsPerSecond / pAudioSource->sampleRate() / 2) {
-        // The stream units should actually be much shorter
-        // than the frames to minimize jitter. Even a frame
-        // at 192 kHz has a length of about 5000 ns >> 100 ns.
-        DEBUG_ASSERT(m_streamUnitsPerFrame >= 50);
-        DEBUG_ASSERT(m_toFrameIndexBias > 0);
+          m_fromSampleFramesToStreamUnits(double(kStreamUnitsPerSecond) / double(pAudioSource->sampleRate())),
+          m_fromStreamUnitsToSampleFrames(double(pAudioSource->sampleRate()) / double(kStreamUnitsPerSecond)){
+        // The stream units should actually be much shorter than
+        // sample frames to minimize jitter and rounding. Even a
+        // frame at 192 kHz has a length of about 5000 ns >> 100 ns.
+        DEBUG_ASSERT(m_fromStreamUnitsToSampleFrames >= 50);
     }
 
     LONGLONG fromFrameIndex(SINT frameIndex) const {
+        DEBUG_ASSERT(m_fromSampleFramesToStreamUnits > 0);
         // Used for seeking, so we need to round down to hit the
         // corresponding stream unit where the given stream unit
-        // starts
-        return floor((frameIndex - m_pAudioSource->frameIndexMin()) * m_streamUnitsPerFrame);
+        // starts. The reader will skip samples until it reaches
+        // the actual target position for reading.
+        const SINT frameIndexOffset = frameIndex - m_pAudioSource->frameIndexMin();
+        return static_cast<LONGLONG>(floor(frameIndexOffset * m_fromSampleFramesToStreamUnits));
     }
 
     SINT toFrameIndex(LONGLONG streamPos) const {
-        // NOTE(uklotzde): Add m_toFrameIndexBias to account for rounding errors
-        return m_pAudioSource->frameIndexMin() +
-                static_cast<SINT>(floor((streamPos + m_toFrameIndexBias) / m_streamUnitsPerFrame));
+        DEBUG_ASSERT(m_fromStreamUnitsToSampleFrames > 0);
+        // The stream reports positions in units of 100ns. We have
+        // to round(!) this value to obtain the actual position in
+        // sample frames.
+        const SINT frameIndexOffset = static_cast<SINT>(round(streamPos * m_fromStreamUnitsToSampleFrames));
+        return m_pAudioSource->frameIndexMin() + frameIndexOffset;
     }
 
   private:
     const AudioSource* m_pAudioSource;
-    double m_streamUnitsPerFrame;
-    SINT m_toFrameIndexBias;
+    double m_fromSampleFramesToStreamUnits;
+    double m_fromStreamUnitsToSampleFrames;
 };
 
 class SoundSourceMediaFoundation: public mixxx::SoundSourcePlugin {