Fix a crash in the gstreamer audio output

Closing the audio output would delete the pipeline, and would
lead to deleting the GstAppSrc from within QGstAppSrc::pushData().

Speed up the qsoundeffect autotest (this uncovered the issue).

Change-Id: I3c5843854f5a74d563eb271df553c8a1a5854f63
Reviewed-by: André de la Rocha <andre.rocha@qt.io>
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

3 files changed, 58 insertions, 49 deletions

diff --git a/src/multimedia/platform/gstreamer/audio/qaudiooutput_gstreamer.cpp b/src/multimedia/platform/gstreamer/audio/qaudiooutput_gstreamer.cpp
index eb74c2ee6..1b0fec46a 100644
--- a/src/multimedia/platform/gstreamer/audio/qaudiooutput_gstreamer.cpp
+++ b/src/multimedia/platform/gstreamer/audio/qaudiooutput_gstreamer.cpp
@@ -61,14 +61,35 @@ QGStreamerAudioOutput::QGStreamerAudioOutput(const QAudioDeviceInfo &device)
 {
     gstPipeline.installMessageFilter(this);
 
+    m_appSrc = new QGstAppSrc;
+    connect(m_appSrc, &QGstAppSrc::bytesProcessed, this, &QGStreamerAudioOutput::bytesProcessedByAppSrc);
+    connect(m_appSrc, &QGstAppSrc::noMoreData, this, &QGStreamerAudioOutput::needData);
+    gstAppSrc = m_appSrc->element();
+
+    //    gstDecodeBin = gst_element_factory_make ("decodebin", "dec");
+    QGstElement conv("audioconvert", "conv");
+    gstVolume = QGstElement("volume", "volume");
+    if (m_volume != 1.)
+        gstVolume.set("volume", m_volume);
+
+    // link decodeBin to audioconvert in a callback once we get a pad from the decoder
+    //    g_signal_connect (gstDecodeBin, "pad-added", (GCallback) padAdded, conv);
+
     const auto *audioInfo = static_cast<const QGStreamerAudioDeviceInfo *>(device.handle());
     gstOutput = gst_device_create_element(audioInfo->gstDevice, nullptr);
+
+    gstPipeline.add(gstAppSrc, /*gstDecodeBin, */ conv, gstVolume, gstOutput);
+    gstAppSrc.link(conv, gstVolume, gstOutput);
 }
 
 QGStreamerAudioOutput::~QGStreamerAudioOutput()
 {
     close();
-    QCoreApplication::processEvents();
+    gstPipeline = {};
+    gstVolume = {};
+    gstAppSrc = {};
+    delete m_appSrc;
+    m_appSrc = nullptr;
 }
 
 void QGStreamerAudioOutput::setError(QAudio::Error error)
@@ -203,26 +224,9 @@ bool QGStreamerAudioOutput::open()
     }
 
 //    qDebug() << "GST caps:" << gst_caps_to_string(caps);
-    m_appSrc = new QGstAppSrc;
     m_appSrc->setup(m_audioSource, m_audioSource ? m_audioSource->pos() : 0);
     m_appSrc->setAudioFormat(m_format);
 
-    connect(m_appSrc, &QGstAppSrc::bytesProcessed, this, &QGStreamerAudioOutput::bytesProcessedByAppSrc);
-    connect(m_appSrc, &QGstAppSrc::noMoreData, this, &QGStreamerAudioOutput::needData);
-    gstAppSrc = m_appSrc->element();
-
-//    gstDecodeBin = gst_element_factory_make ("decodebin", "dec");
-    QGstElement conv("audioconvert", "conv");
-    gstVolume = QGstElement("volume", "volume");
-    if (m_volume != 1.)
-        gstVolume.set("volume", m_volume);
-
-    gstPipeline.add(gstAppSrc, /*gstDecodeBin, */ conv, gstVolume, gstOutput);
-    gstAppSrc.link(conv, gstVolume, gstOutput);
-
-    // link decodeBin to audioconvert in a callback once we get a pad from the decoder
-//    g_signal_connect (gstDecodeBin, "pad-added", (GCallback) padAdded, conv);
-
     /* run */
     gstPipeline.setState(GST_STATE_PLAYING);
 
@@ -239,10 +243,8 @@ void QGStreamerAudioOutput::close()
     if (!m_opened)
         return;
 
-    gstPipeline.setStateSync(GST_STATE_NULL);
-    gstPipeline = {};
-    gstVolume = {};
-    gstAppSrc = {};
+    if (!gstPipeline.setStateSync(GST_STATE_NULL))
+        qWarning() << "failed to close the audio output stream";
 
     if (!m_pullMode && m_audioSource)
         delete m_audioSource;
diff --git a/src/multimedia/platform/gstreamer/common/qgstappsrc.cpp b/src/multimedia/platform/gstreamer/common/qgstappsrc.cpp
index 7ff9b177f..2cf33fbce 100644
--- a/src/multimedia/platform/gstreamer/common/qgstappsrc.cpp
+++ b/src/multimedia/platform/gstreamer/common/qgstappsrc.cpp
@@ -56,6 +56,9 @@ QGstAppSrc::QGstAppSrc(QObject *parent)
 
 QGstAppSrc::~QGstAppSrc()
 {
+    m_appSrc.setStateSync(GST_STATE_NULL);
+    streamDestroyed();
+    qCDebug(qLcAppSrc) << "~QGstAppSrc";
 }
 
 bool QGstAppSrc::setup(QIODevice *stream, qint64 offset)
@@ -68,6 +71,7 @@ bool QGstAppSrc::setup(QIODevice *stream, qint64 offset)
 
     auto *appSrc = GST_APP_SRC(m_appSrc.element());
     GstAppSrcCallbacks m_callbacks;
+    memset(&m_callbacks, 0, sizeof(GstAppSrcCallbacks));
     m_callbacks.need_data   = &QGstAppSrc::on_need_data;
     m_callbacks.enough_data = &QGstAppSrc::on_enough_data;
     m_callbacks.seek_data   = &QGstAppSrc::on_seek_data;
@@ -141,20 +145,20 @@ void QGstAppSrc::write(const char *data, qsizetype size)
     Q_ASSERT(!m_stream);
     m_buffer.append(data, size);
     m_noMoreData = false;
-    if (m_dataRequestSize)
-        pushData();
+    pushData();
 }
 
 void QGstAppSrc::onDataReady()
 {
     qCDebug(qLcAppSrc) << "onDataReady" << m_stream->bytesAvailable() << m_stream->size();
-    if (m_dataRequestSize)
-        pushData();
+    pushData();
 }
 
 void QGstAppSrc::streamDestroyed()
 {
+    qCDebug(qLcAppSrc) << "stream destroyed";
     m_stream = nullptr;
+    m_dataRequestSize = 0;
     sendEOS();
 }
 
@@ -166,6 +170,7 @@ void QGstAppSrc::pushData()
     qCDebug(qLcAppSrc) << "pushData" << m_stream << m_stream->atEnd() << m_buffer.size();
     if ((m_stream && m_stream->atEnd())) {
         eosOrIdle();
+        qCDebug(qLcAppSrc) << "end pushData" << m_stream << m_stream->atEnd() << m_buffer.size();
         return;
     }
 
@@ -213,6 +218,7 @@ void QGstAppSrc::pushData()
     if (bytesRead == 0) {
         gst_buffer_unref(buffer);
         eosOrIdle();
+        qCDebug(qLcAppSrc) << "end pushData" << m_stream << m_stream->atEnd() << m_buffer.size();
         return;
     }
     m_noMoreData = false;
@@ -224,6 +230,8 @@ void QGstAppSrc::pushData()
     } else if (ret == GST_FLOW_FLUSHING) {
         qWarning() << "QGstAppSrc: push buffer wrong state";
     }
+    qCDebug(qLcAppSrc) << "end pushData" << m_stream << m_stream->atEnd() << m_buffer.size();
+
 }
 
 bool QGstAppSrc::doSeek(qint64 value)
@@ -264,6 +272,7 @@ void QGstAppSrc::on_need_data(GstAppSrc *, guint arg0, gpointer userdata)
     Q_ASSERT(self);
     self->m_dataRequestSize = arg0;
     QMetaObject::invokeMethod(self, "pushData", Qt::AutoConnection);
+    qCDebug(qLcAppSrc) << "done on_need_data";
 }
 
 void QGstAppSrc::sendEOS()
diff --git a/tests/auto/integration/qsoundeffect/tst_qsoundeffect.cpp b/tests/auto/integration/qsoundeffect/tst_qsoundeffect.cpp
index 020daeea6..4367810fc 100644
--- a/tests/auto/integration/qsoundeffect/tst_qsoundeffect.cpp
+++ b/tests/auto/integration/qsoundeffect/tst_qsoundeffect.cpp
@@ -127,8 +127,8 @@ void tst_QSoundEffect::testSource()
 
     QTestEventLoop::instance().enterLoop(1);
     sound->play();
-
-    QTest::qWait(3000);
+    QTRY_COMPARE(sound->isPlaying(), false);
+    QCOMPARE(sound->loopsRemaining(), 0);
 }
 
 void tst_QSoundEffect::testLooping()
@@ -139,9 +139,9 @@ void tst_QSoundEffect::testLooping()
     QSignalSpy readSignal_Count(sound, SIGNAL(loopCountChanged()));
     QSignalSpy readSignal_Remaining(sound, SIGNAL(loopsRemainingChanged()));
 
-    sound->setLoopCount(5);
+    sound->setLoopCount(3);
     sound->setVolume(0.1f);
-    QCOMPARE(sound->loopCount(), 5);
+    QCOMPARE(sound->loopCount(), 3);
     QCOMPARE(readSignal_Count.count(), 1);
     QCOMPARE(sound->loopsRemaining(), 0);
     QCOMPARE(readSignal_Remaining.count(), 0);
@@ -149,11 +149,11 @@ void tst_QSoundEffect::testLooping()
     sound->play();
     QVERIFY(readSignal_Remaining.count() > 0);
 
-    // test.wav is about 200ms, wait until it has finished playing 5 times
+    // test.wav is about 200ms, wait until it has finished playing 3 times
     QTestEventLoop::instance().enterLoop(3);
 
     QTRY_COMPARE(sound->loopsRemaining(), 0);
-    QVERIFY(readSignal_Remaining.count() >= 6);
+    QVERIFY(readSignal_Remaining.count() == 4);
     QTRY_VERIFY(!sound->isPlaying());
 
     // QTBUG-36643 (setting the loop count while playing should work)
@@ -161,8 +161,8 @@ void tst_QSoundEffect::testLooping()
         readSignal_Count.clear();
         readSignal_Remaining.clear();
 
-        sound->setLoopCount(30);
-        QCOMPARE(sound->loopCount(), 30);
+        sound->setLoopCount(10);
+        QCOMPARE(sound->loopCount(), 10);
         QCOMPARE(readSignal_Count.count(), 1);
         QCOMPARE(sound->loopsRemaining(), 0);
         QCOMPARE(readSignal_Remaining.count(), 0);
@@ -171,21 +171,21 @@ void tst_QSoundEffect::testLooping()
         QVERIFY(readSignal_Remaining.count() > 0);
 
         // wait for the sound to be played several times
-        QTRY_VERIFY(sound->loopsRemaining() <= 20);
-        QVERIFY(readSignal_Remaining.count() >= 10);
+        QTRY_VERIFY(sound->loopsRemaining() <= 7);
+        QVERIFY(readSignal_Remaining.count() >= 3);
         readSignal_Count.clear();
         readSignal_Remaining.clear();
 
         // change the loop count while playing
-        sound->setLoopCount(5);
-        QCOMPARE(sound->loopCount(), 5);
+        sound->setLoopCount(3);
+        QCOMPARE(sound->loopCount(), 3);
         QCOMPARE(readSignal_Count.count(), 1);
-        QCOMPARE(sound->loopsRemaining(), 5);
+        QCOMPARE(sound->loopsRemaining(), 3);
         QCOMPARE(readSignal_Remaining.count(), 1);
 
         // wait for all the loops to be completed
         QTRY_COMPARE(sound->loopsRemaining(), 0);
-        QTRY_VERIFY(readSignal_Remaining.count() >= 6);
+        QTRY_VERIFY(readSignal_Remaining.count() == 4);
         QTRY_VERIFY(!sound->isPlaying());
     }
 
@@ -203,7 +203,7 @@ void tst_QSoundEffect::testLooping()
         QTRY_COMPARE(sound->loopsRemaining(), int(QSoundEffect::Infinite));
         QCOMPARE(readSignal_Remaining.count(), 1);
 
-        QTest::qWait(1500);
+        QTest::qWait(500);
         QVERIFY(sound->isPlaying());
         readSignal_Count.clear();
         readSignal_Remaining.clear();
@@ -228,8 +228,7 @@ void tst_QSoundEffect::testVolume()
     sound->setVolume(0.5);
     QCOMPARE(sound->volume(),0.5);
 
-    QTest::qWait(20);
-    QCOMPARE(readSignal.count(),1);
+    QTRY_COMPARE(readSignal.count(),1);
 }
 
 void tst_QSoundEffect::testMuting()
@@ -239,8 +238,7 @@ void tst_QSoundEffect::testMuting()
     sound->setMuted(true);
     QCOMPARE(sound->isMuted(),true);
 
-    QTest::qWait(20);
-    QCOMPARE(readSignal.count(),1);
+    QTRY_COMPARE(readSignal.count(),1);
 }
 
 void tst_QSoundEffect::testPlaying()
@@ -303,7 +301,7 @@ void tst_QSoundEffect::testDestroyWhilePlaying()
     instance->setVolume(0.1f);
     QTestEventLoop::instance().enterLoop(1);
     instance->play();
-    QTest::qWait(500);
+    QTest::qWait(100);
     delete instance;
     QTestEventLoop::instance().enterLoop(1);
 }
@@ -315,7 +313,7 @@ void tst_QSoundEffect::testDestroyWhileRestartPlaying()
     instance->setVolume(0.1f);
     QTestEventLoop::instance().enterLoop(1);
     instance->play();
-    QTest::qWait(1000);
+    QTRY_COMPARE(instance->isPlaying(), false);
     //restart playing
     instance->play();
     delete instance;
@@ -324,7 +322,7 @@ void tst_QSoundEffect::testDestroyWhileRestartPlaying()
 
 void tst_QSoundEffect::testSetSourceWhileLoading()
 {
-    for (int i = 0; i < 10; i++) {
+    for (int i = 0; i < 2; i++) {
         sound->setSource(url);
         QVERIFY(sound->status() == QSoundEffect::Loading || sound->status() == QSoundEffect::Ready);
         sound->setSource(url); // set same source again
@@ -352,7 +350,7 @@ void tst_QSoundEffect::testSetSourceWhileLoading()
 
 void tst_QSoundEffect::testSetSourceWhilePlaying()
 {
-    for (int i = 0; i < 10; i++) {
+    for (int i = 0; i < 2; i++) {
         sound->setSource(url);
         QTRY_COMPARE(sound->status(), QSoundEffect::Ready);
         sound->play();