Fix bugs in QSoundEffect

Those lead to sound effects not being played back. Also, the
audiodecoder example would not close the wav decoder, leading
to invalid sizes defined inside the wav file.

There's still a bug left that causes the playback to be chopped
into chunks, but that seems to be a separate problem inside the
gstreamer code base.

Change-Id: Id432e7ded128bd2cc0253e12bba06a1595900a3d
Reviewed-by: Doris Verria <doris.verria@qt.io>
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

4 files changed, 49 insertions, 70 deletions

diff --git a/examples/multimedia/audiodecoder/audiodecoder.cpp b/examples/multimedia/audiodecoder/audiodecoder.cpp
index 36bbb2919..4f9232b2c 100644
--- a/examples/multimedia/audiodecoder/audiodecoder.cpp
+++ b/examples/multimedia/audiodecoder/audiodecoder.cpp
@@ -53,7 +53,8 @@
 #include <stdio.h>
 
 AudioDecoder::AudioDecoder(bool isPlayback, bool isDelete, const QString &targetFileName)
-    : m_cout(stdout, QIODevice::WriteOnly)
+    : m_cout(stdout, QIODevice::WriteOnly),
+    m_targetFilename(targetFileName)
 {
     m_isPlayback = isPlayback;
     m_isDelete = isDelete;
@@ -168,6 +169,7 @@ void AudioDecoder::stateChanged(QAudioDecoder::State newState)
 
 void AudioDecoder::finished()
 {
+    m_waveDecoder->close();
     m_cout << "Decoding finished\n";
 
     if (m_isPlayback) {
diff --git a/src/multimedia/audio/qsamplecache_p.cpp b/src/multimedia/audio/qsamplecache_p.cpp
index e18fac9f3..3c5df717d 100644
--- a/src/multimedia/audio/qsamplecache_p.cpp
+++ b/src/multimedia/audio/qsamplecache_p.cpp
@@ -45,7 +45,9 @@
 #include <QtNetwork/QNetworkRequest>
 
 #include <QtCore/QDebug>
-//#define QT_SAMPLECACHE_DEBUG
+#include <QtCore/qloggingcategory.h>
+
+Q_LOGGING_CATEGORY(qLcSampleCache, "qt.multimedia.samplecache")
 
 #include <mutex>
 
@@ -170,9 +172,7 @@ QSample* QSampleCache::requestSample(const QUrl& url)
     if (!m_loadingThread.isRunning())
         m_loadingThread.start();
 
-#ifdef QT_SAMPLECACHE_DEBUG
-    qDebug() << "QSampleCache: request sample [" << url << "]";
-#endif
+    qCDebug(qLcSampleCache) << "QSampleCache: request sample [" << url << "]";
     std::unique_lock<QRecursiveMutex> locker(m_mutex);
     QMap<QUrl, QSample*>::iterator it = m_samples.find(url);
     QSample* sample;
@@ -196,9 +196,7 @@ void QSampleCache::setCapacity(qint64 capacity)
     const std::lock_guard<QRecursiveMutex> locker(m_mutex);
     if (m_capacity == capacity)
         return;
-#ifdef QT_SAMPLECACHE_DEBUG
-    qDebug() << "QSampleCache: capacity changes from " << m_capacity << "to " << capacity;
-#endif
+    qCDebug(qLcSampleCache) << "QSampleCache: capacity changes from " << m_capacity << "to " << capacity;
     if (m_capacity > 0 && capacity <= 0) { //memory management strategy changed
         for (QMap<QUrl, QSample*>::iterator it = m_samples.begin(); it != m_samples.end();) {
             QSample* sample = *it;
@@ -231,9 +229,7 @@ void QSampleCache::refresh(qint64 usageChange)
     if (m_capacity <= 0 || m_usage <= m_capacity)
         return;
 
-#ifdef QT_SAMPLECACHE_DEBUG
     qint64 recoveredSize = 0;
-#endif
 
     //free unused samples to keep usage under capacity limit.
     for (QMap<QUrl, QSample*>::iterator it = m_samples.begin(); it != m_samples.end();) {
@@ -242,20 +238,16 @@ void QSampleCache::refresh(qint64 usageChange)
             ++it;
             continue;
         }
-#ifdef QT_SAMPLECACHE_DEBUG
         recoveredSize += sample->m_soundData.size();
-#endif
         unloadSample(sample);
         it = m_samples.erase(it);
         if (m_usage <= m_capacity)
             return;
     }
 
-#ifdef QT_SAMPLECACHE_DEBUG
-    qDebug() << "QSampleCache: refresh(" << usageChange
+    qCDebug(qLcSampleCache) << "QSampleCache: refresh(" << usageChange
              << ") recovered size =" << recoveredSize
              << "new usage =" << m_usage;
-#endif
 
     if (m_usage > m_capacity)
         qWarning() << "QSampleCache: usage[" << m_usage << " out of limit[" << m_capacity << "]";
@@ -276,9 +268,7 @@ QSample::~QSample()
     m_parent->removeUnreferencedSample(this);
 
     QMutexLocker locker(&m_mutex);
-#ifdef QT_SAMPLECACHE_DEBUG
-    qDebug() << "~QSample" << this << ": deleted [" << m_url << "]" << QThread::currentThread();
-#endif
+    qCDebug(qLcSampleCache) << "~QSample" << this << ": deleted [" << m_url << "]" << QThread::currentThread();
     cleanup();
 }
 
@@ -313,9 +303,7 @@ bool QSampleCache::notifyUnreferencedSample(QSample* sample)
 void QSample::release()
 {
     QMutexLocker locker(&m_mutex);
-#ifdef QT_SAMPLECACHE_DEBUG
-    qDebug() << "Sample:: release" << this << QThread::currentThread() << m_ref;
-#endif
+    qCDebug(qLcSampleCache) << "Sample:: release" << this << QThread::currentThread() << m_ref;
     if (--m_ref == 0) {
         locker.unlock();
         m_parent->notifyUnreferencedSample(this);
@@ -326,6 +314,7 @@ void QSample::release()
 // must be called locked.
 void QSample::cleanup()
 {
+    qCDebug(qLcSampleCache) << "QSample: cleanup";
     if (m_waveDecoder)
         m_waveDecoder->deleteLater();
     if (m_stream)
@@ -346,12 +335,10 @@ void QSample::readSample()
 {
     Q_ASSERT(QThread::currentThread()->objectName() == QLatin1String("QSampleCache::LoadingThread"));
     QMutexLocker m(&m_mutex);
-#ifdef  QT_SAMPLECACHE_DEBUG
-    qDebug() << "QSample: readSample";
-#endif
     qint64 read = m_waveDecoder->read(m_soundData.data() + m_sampleReadLength,
                       qMin(m_waveDecoder->bytesAvailable(),
                            qint64(m_waveDecoder->size() - m_sampleReadLength)));
+    qCDebug(qLcSampleCache) << "QSample: readSample" << read;
     if (read > 0)
         m_sampleReadLength += read;
     if (m_sampleReadLength < m_waveDecoder->size())
@@ -365,14 +352,13 @@ void QSample::decoderReady()
 {
     Q_ASSERT(QThread::currentThread()->objectName() == QLatin1String("QSampleCache::LoadingThread"));
     QMutexLocker m(&m_mutex);
-#ifdef QT_SAMPLECACHE_DEBUG
-    qDebug() << "QSample: decoder ready";
-#endif
+    qCDebug(qLcSampleCache) << "QSample: decoder ready";
     m_parent->refresh(m_waveDecoder->size());
 
     m_soundData.resize(m_waveDecoder->size());
     m_sampleReadLength = 0;
     qint64 read = m_waveDecoder->read(m_soundData.data(), m_waveDecoder->size());
+    qCDebug(qLcSampleCache) << "    bytes read" << read;
     if (read > 0)
         m_sampleReadLength += read;
     if (m_sampleReadLength >= m_waveDecoder->size())
@@ -391,9 +377,7 @@ QSample::State QSample::state() const
 void QSample::load()
 {
     Q_ASSERT(QThread::currentThread()->objectName() == QLatin1String("QSampleCache::LoadingThread"));
-#ifdef QT_SAMPLECACHE_DEBUG
-    qDebug() << "QSample: load [" << m_url << "]";
-#endif
+    qCDebug(qLcSampleCache) << "QSample: load [" << m_url << "]";
     m_stream = m_parent->networkAccessManager().get(QNetworkRequest(m_url));
     connect(m_stream, SIGNAL(errorOccurred(QNetworkReply::NetworkError)), SLOT(decoderError()));
     m_waveDecoder = new QWaveDecoder(m_stream);
@@ -409,9 +393,7 @@ void QSample::decoderError()
 {
     Q_ASSERT(QThread::currentThread()->objectName() == QLatin1String("QSampleCache::LoadingThread"));
     QMutexLocker m(&m_mutex);
-#ifdef QT_SAMPLECACHE_DEBUG
-    qDebug() << "QSample: decoder error";
-#endif
+    qCDebug(qLcSampleCache) << "QSample: decoder error";
     cleanup();
     m_state = QSample::Error;
     qobject_cast<QSampleCache*>(m_parent)->loadingRelease();
@@ -422,10 +404,8 @@ void QSample::decoderError()
 void QSample::onReady()
 {
     Q_ASSERT(QThread::currentThread()->objectName() == QLatin1String("QSampleCache::LoadingThread"));
-#ifdef QT_SAMPLECACHE_DEBUG
-    qDebug() << "QSample: load ready";
-#endif
     m_audioFormat = m_waveDecoder->audioFormat();
+    qCDebug(qLcSampleCache) << "QSample: load ready format:" << m_audioFormat;
     cleanup();
     m_state = QSample::Ready;
     qobject_cast<QSampleCache*>(m_parent)->loadingRelease();
diff --git a/src/multimedia/audio/qsoundeffect.cpp b/src/multimedia/audio/qsoundeffect.cpp
index 974df3139..6de24b964 100644
--- a/src/multimedia/audio/qsoundeffect.cpp
+++ b/src/multimedia/audio/qsoundeffect.cpp
@@ -43,6 +43,9 @@
 #include "qaudiodeviceinfo.h"
 #include "qaudiooutput.h"
 #include "qmediadevicemanager.h"
+#include <QtCore/qloggingcategory.h>
+
+Q_LOGGING_CATEGORY(qLcSoundEffect, "qt.multimedia.soundeffect")
 
 QT_BEGIN_NAMESPACE
 
@@ -59,9 +62,15 @@ public:
     qint64 size() const override {
         return m_loopCount == QSoundEffect::Infinite ? 0 : m_loopCount * m_sample->data().size();
     }
+    qint64 bytesAvailable() const override {
+        return m_loopCount == QSoundEffect::Infinite ? 4*4096 : m_runningCount * m_sample->data().size() - m_offset;
+    }
     bool isSequential() const override {
         return m_loopCount == QSoundEffect::Infinite;
     }
+    bool atEnd() const override {
+        return m_runningCount == 0;
+    }
 
     void setLoopsRemaining(int loopsRemaining);
     void setStatus(QSoundEffect::Status status);
@@ -103,16 +112,11 @@ void QSoundEffectPrivate::sampleReady()
     if (m_status == QSoundEffect::Error)
         return;
 
-#ifdef QT_QAUDIO_DEBUG
-    qDebug() << this << "sampleReady "<<m_playing;
-#endif
+    qCDebug(qLcSoundEffect) << this << "sampleReady: sample size:" << m_sample->data().size();
     disconnect(m_sample, &QSample::error, this, &QSoundEffectPrivate::decoderError);
     disconnect(m_sample, &QSample::ready, this, &QSoundEffectPrivate::sampleReady);
     if (!m_audioOutput) {
-        if (m_audioDevice.isNull())
-            m_audioOutput = new QAudioOutput(m_sample->format());
-        else
-            m_audioOutput = new QAudioOutput(m_audioDevice, m_sample->format());
+        m_audioOutput = new QAudioOutput(m_audioDevice, m_sample->format());
         connect(m_audioOutput, &QAudioOutput::stateChanged, this, &QSoundEffectPrivate::stateChanged);
         if (!m_muted)
             m_audioOutput->setVolume(m_volume);
@@ -122,8 +126,10 @@ void QSoundEffectPrivate::sampleReady()
     m_sampleReady = true;
     setStatus(QSoundEffect::Ready);
 
-    if (m_playing && m_audioOutput->state() == QAudio::StoppedState)
+    if (m_playing && m_audioOutput->state() == QAudio::StoppedState) {
+        qCDebug(qLcSoundEffect) << this << "starting playback on audiooutput";
         m_audioOutput->start(this);
+    }
 }
 
 void QSoundEffectPrivate::decoderError()
@@ -137,15 +143,14 @@ void QSoundEffectPrivate::decoderError()
 
 void QSoundEffectPrivate::stateChanged(QAudio::State state)
 {
-#ifdef QT_QAUDIO_DEBUG
-    qDebug() << this << "stateChanged " << state;
-#endif
+    qCDebug(qLcSoundEffect) << this << "stateChanged " << state;
     if ((state == QAudio::IdleState && m_runningCount == 0) || state == QAudio::StoppedState)
         emit q_ptr->stop();
 }
 
 qint64 QSoundEffectPrivate::readData(char *data, qint64 len)
 {
+    qCDebug(qLcSoundEffect) << this << "readData" << len << m_runningCount;
     if (m_sample->state() != QSample::Ready)
         return 0;
     if (m_runningCount == 0 || !m_playing)
@@ -184,18 +189,14 @@ void QSoundEffectPrivate::setLoopsRemaining(int loopsRemaining)
 {
     if (m_runningCount == loopsRemaining)
         return;
-#ifdef QT_QAUDIO_DEBUG
-    qDebug() << this << "setLoopsRemaining " << loopsRemaining;
-#endif
+    qCDebug(qLcSoundEffect) << this << "setLoopsRemaining " << loopsRemaining;
     m_runningCount = loopsRemaining;
     emit q_ptr->loopsRemainingChanged();
 }
 
 void QSoundEffectPrivate::setStatus(QSoundEffect::Status status)
 {
-#ifdef QT_QAUDIO_DEBUG
-    qDebug() << this << "setStatus" << status;
-#endif
+    qCDebug(qLcSoundEffect) << this << "setStatus" << status;
     if (m_status == status)
         return;
     bool oldLoaded = q_ptr->isLoaded();
@@ -207,9 +208,7 @@ void QSoundEffectPrivate::setStatus(QSoundEffect::Status status)
 
 void QSoundEffectPrivate::setPlaying(bool playing)
 {
-#ifdef QT_QAUDIO_DEBUG
-    qDebug() << this << "setPlaying(" << playing << ")";
-#endif
+    qCDebug(qLcSoundEffect) << this << "setPlaying(" << playing << ")";
     if (m_playing == playing)
         return;
     m_playing = playing;
@@ -353,12 +352,10 @@ QUrl QSoundEffect::source() const
 /*! Set the current URL to play to \a url. */
 void QSoundEffect::setSource(const QUrl &url)
 {
+    qCDebug(qLcSoundEffect) << this << "setSource current=" << d->m_url << ", to=" << url;
     if (d->m_url == url)
         return;
 
-#ifdef QT_QAUDIO_DEBUG
-    qDebug() << this << "setSource current=" << d->m_url << ", to=" << url;
-#endif
     Q_ASSERT(d->m_url != url);
 
     stop();
@@ -624,9 +621,7 @@ void QSoundEffect::play()
 {
     d->m_offset = 0;
     d->setLoopsRemaining(d->m_loopCount);
-#ifdef QT_QAUDIO_DEBUG
-    qDebug() << this << "play";
-#endif
+    qCDebug(qLcSoundEffect) << this << "play";
     if (d->m_status == QSoundEffect::Null || d->m_status == QSoundEffect::Error) {
         d->setStatus(QSoundEffect::Null);
         return;
@@ -775,9 +770,7 @@ void QSoundEffect::stop()
 {
     if (!d->m_playing)
         return;
-#ifdef QT_QAUDIO_DEBUG
-    qDebug() << "stop()";
-#endif
+    qCDebug(qLcSoundEffect) << "stop()";
     d->m_offset = 0;
 
     d->setPlaying(false);
diff --git a/src/multimedia/audio/qwavedecoder.cpp b/src/multimedia/audio/qwavedecoder.cpp
index ed5fc900e..d318b551f 100644
--- a/src/multimedia/audio/qwavedecoder.cpp
+++ b/src/multimedia/audio/qwavedecoder.cpp
@@ -120,10 +120,8 @@ void QWaveDecoder::close()
 {
     if (isOpen() && (openMode() & QIODevice::WriteOnly)) {
         Q_ASSERT(dataSize < INT_MAX);
-        if (device->isOpen())
-            Q_ASSERT(writeDataLength());
-        else
-            qWarning() << "Failed to finalize output because output device was closed";
+        if (!device->isOpen() || !writeDataLength())
+            qWarning() << "Failed to finalize wav file";
     }
     QIODevice::close();
 }
@@ -265,13 +263,16 @@ bool QWaveDecoder::writeDataLength()
     // only implemented for LITTLE ENDIAN
     return false;
 #endif
+    qDebug() << "writeDataLength" << dataSize << device->isSequential();
 
     if (isSequential())
         return false;
 
     // seek to RIFF header size, see header.riff.descriptor.size above
-    if (!device->seek(4))
+    if (!device->seek(4)) {
+        qDebug() << "can't seek";
         return false;
+    }
 
     quint32 length = dataSize + HeaderLength - 8;
     if (device->write(reinterpret_cast<const char *>(&length), 4) != 4)
@@ -410,6 +411,9 @@ void QWaveDecoder::handleData()
                 descriptor.size = qFromLittleEndian<quint32>(descriptor.size);
 
             dataSize = descriptor.size; //means the data size from the data header, not the actual file size
+            if (!dataSize)
+                dataSize = device->size() - headerLength();
+            qDebug() << "dataSize" << dataSize << device->size() << headerLength() << device->isSequential();
 
             haveFormat = true;
             connect(device, SIGNAL(readyRead()), SIGNAL(readyRead()));