EVRCustomPresenter: fix processing samples by the Scheduler

Fix several issues in how the Scheduler processed the sample
queue:

- Potential data race on accessing to m_scheduledSamples member,
because it was not guarded by the mutex in the `while` condition.

- Potential infinite loop in the scheduler thread caused by
incorrect 0 clock time being returned. This causes the sample
to always be from the future, and it therefore remains queued.
So, the scheduler cannot stop.

Fixes: QTBUG-120266
Pick-to: 6.5
Change-Id: Icb5a92edf92ab555c9660feeb2a0fce32a037c34
Reviewed-by: Pavel Dubsky <pavel.dubsky@qt.io>
Reviewed-by: Artem Dyomin <artem.dyomin@qt.io>
Reviewed-by: Jøger Hansegård <joger.hansegard@qt.io>
(cherry picked from commit cc8492f75c285b58b90c16d8712b19ba181db443)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit a2d8025178c3e64059314968e857f8b5ef1f20b5)

2 files changed, 59 insertions, 60 deletions

diff --git a/src/plugins/multimedia/windows/evr/evrcustompresenter.cpp b/src/plugins/multimedia/windows/evr/evrcustompresenter.cpp
index b0f97cb7f..837ea7c6f 100644
--- a/src/plugins/multimedia/windows/evr/evrcustompresenter.cpp
+++ b/src/plugins/multimedia/windows/evr/evrcustompresenter.cpp
@@ -238,22 +238,36 @@ HRESULT Scheduler::processSamplesInQueue(LONG *nextSleep)
     HRESULT hr = S_OK;
     LONG wait = 0;
 
+    QQueue<ComPtr<IMFSample>> scheduledSamples;
+
+    m_mutex.lock();
+    m_scheduledSamples.swap(scheduledSamples);
+    m_mutex.unlock();
+
     // Process samples until the queue is empty or until the wait time > 0.
-    while (!m_scheduledSamples.isEmpty()) {
-        m_mutex.lock();
-        ComPtr<IMFSample> sample = m_scheduledSamples.dequeue();
-        m_mutex.unlock();
+    while (!scheduledSamples.isEmpty()) {
+        ComPtr<IMFSample> sample = scheduledSamples.dequeue();
 
         // Process the next sample in the queue. If the sample is not ready
         // for presentation. the value returned in wait is > 0, which
         // means the scheduler should sleep for that amount of time.
+        if (isSampleReadyToPresent(sample.Get(), &wait)) {
+            m_presenter->presentSample(sample.Get());
+            continue;
+        }
 
-        hr = processSample(sample.Get(), &wait);
-
-        if (FAILED(hr) || wait > 0)
+        if (wait > 0) {
+            // return the sample to scheduler
+            scheduledSamples.prepend(sample);
             break;
+        }
     }
 
+    m_mutex.lock();
+    scheduledSamples.append(std::move(m_scheduledSamples));
+    m_scheduledSamples.swap(scheduledSamples);
+    m_mutex.unlock();
+
     // If the wait time is zero, it means we stopped because the queue is
     // empty (or an error occurred). Set the wait time to infinite; this will
     // make the scheduler thread sleep until it gets another thread message.
@@ -264,65 +278,50 @@ HRESULT Scheduler::processSamplesInQueue(LONG *nextSleep)
     return hr;
 }
 
-HRESULT Scheduler::processSample(const ComPtr<IMFSample> &sample, LONG *pNextSleep)
+bool Scheduler::isSampleReadyToPresent(IMFSample *sample, LONG *pNextSleep) const
 {
-    HRESULT hr = S_OK;
-
-    LONGLONG hnsPresentationTime = 0;
-    LONGLONG hnsTimeNow = 0;
-    MFTIME   hnsSystemTime = 0;
-
-    bool presentNow = true;
-    LONG nextSleep = 0;
-
-    if (m_clock) {
-        // Get the sample's time stamp. It is valid for a sample to
-        // have no time stamp.
-        hr = sample->GetSampleTime(&hnsPresentationTime);
-
-        // Get the clock time. (But if the sample does not have a time stamp,
-        // we don't need the clock time.)
-        if (SUCCEEDED(hr))
-            hr = m_clock->GetCorrelatedTime(0, &hnsTimeNow, &hnsSystemTime);
-
-        // Calculate the time until the sample's presentation time.
-        // A negative value means the sample is late.
-        LONGLONG hnsDelta = hnsPresentationTime - hnsTimeNow;
-        if (m_playbackRate < 0) {
-            // For reverse playback, the clock runs backward. Therefore, the
-            // delta is reversed.
-            hnsDelta = - hnsDelta;
-        }
+    *pNextSleep = 0;
+    if (!m_clock)
+        return true;
 
-        if (hnsDelta < - m_perFrame_1_4th) {
-            // This sample is late.
-            presentNow = true;
-        } else if (hnsDelta > (3 * m_perFrame_1_4th)) {
-            // This sample is still too early. Go to sleep.
-            nextSleep = MFTimeToMsec(hnsDelta - (3 * m_perFrame_1_4th));
+    MFTIME hnsPresentationTime = 0;
+    MFTIME hnsTimeNow = 0;
+    MFTIME hnsSystemTime = 0;
 
-            // Adjust the sleep time for the clock rate. (The presentation clock runs
-            // at m_fRate, but sleeping uses the system clock.)
-            if (m_playbackRate != 0)
-                nextSleep = (LONG)(nextSleep / qFabs(m_playbackRate));
+    // Get the sample's time stamp. It is valid for a sample to
+    // have no time stamp.
+    HRESULT hr = sample->GetSampleTime(&hnsPresentationTime);
 
-            // Don't present yet.
-            presentNow = false;
-        }
+    // Get the clock time. (But if the sample does not have a time stamp,
+    // we don't need the clock time.)
+    if (SUCCEEDED(hr))
+        hr = m_clock->GetCorrelatedTime(0, &hnsTimeNow, &hnsSystemTime);
+
+    // Calculate the time until the sample's presentation time.
+    // A negative value means the sample is late.
+    MFTIME hnsDelta = hnsPresentationTime - hnsTimeNow;
+    if (m_playbackRate < 0) {
+        // For reverse playback, the clock runs backward. Therefore, the
+        // delta is reversed.
+        hnsDelta = - hnsDelta;
     }
 
-    if (presentNow) {
-        m_presenter->presentSample(sample);
+    if (hnsDelta < - m_perFrame_1_4th) {
+        // This sample is late - skip.
+        return false;
+    } else if (hnsDelta > (3 * m_perFrame_1_4th)) {
+        // This sample came too early - reschedule
+        *pNextSleep = MFTimeToMsec(hnsDelta - (3 * m_perFrame_1_4th));
+
+        // Adjust the sleep time for the clock rate. (The presentation clock runs
+        // at m_fRate, but sleeping uses the system clock.)
+        if (m_playbackRate != 0)
+            *pNextSleep = (LONG)(*pNextSleep / qFabs(m_playbackRate));
+        return *pNextSleep == 0;
     } else {
-        // The sample is not ready yet. Return it to the queue.
-        m_mutex.lock();
-        m_scheduledSamples.prepend(sample);
-        m_mutex.unlock();
+        // This sample can be presented right now
+        return true;
     }
-
-    *pNextSleep = nextSleep;
-
-    return hr;
 }
 
 DWORD WINAPI Scheduler::schedulerThreadProc(LPVOID parameter)
diff --git a/src/plugins/multimedia/windows/evr/evrcustompresenter_p.h b/src/plugins/multimedia/windows/evr/evrcustompresenter_p.h
index f3dc25bb5..28f1cbc68 100644
--- a/src/plugins/multimedia/windows/evr/evrcustompresenter_p.h
+++ b/src/plugins/multimedia/windows/evr/evrcustompresenter_p.h
@@ -115,7 +115,6 @@ public:
 
     HRESULT scheduleSample(const ComPtr<IMFSample> &sample, bool presentNow);
     HRESULT processSamplesInQueue(LONG *nextSleep);
-    HRESULT processSample(const ComPtr<IMFSample> &sample, LONG *nextSleep);
     HRESULT flush();
 
     bool areSamplesScheduled();
@@ -125,6 +124,7 @@ public:
 
 private:
     DWORD schedulerThreadProcPrivate();
+    bool isSampleReadyToPresent(IMFSample *sample, LONG *pNextSleep) const;
 
     EVRCustomPresenter *m_presenter;
 
@@ -138,7 +138,7 @@ private:
     EventHandle m_flushEvent;
 
     float m_playbackRate;
-    LONGLONG m_perFrame_1_4th; // 1/4th of the frame duration.
+    MFTIME m_perFrame_1_4th; // 1/4th of the frame duration.
 
     QMutex m_mutex;
 };