GStreamer: remove an element from a pipeline before setState(NULL)

If an element is a part of a pipeline bin, the bin tries to synchronize
its state with its own. Thus, after setting the element state to NULL,
the state can be set back to READY, PAUSE, or PLAYING before it is
removed from the pipeline. This patch first removes an element from
the pipeline and then sets its state to NULL to avoid this race.

Pick-to: 6.2
Change-Id: Ied9cd037fabab19682a53c5ab3d0c7335c58d9c9
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

5 files changed, 45 insertions, 15 deletions

diff --git a/src/multimedia/platform/gstreamer/common/qgstreamervideooutput.cpp b/src/multimedia/platform/gstreamer/common/qgstreamervideooutput.cpp
index 36e0823d8..56cf0a9a4 100644
--- a/src/multimedia/platform/gstreamer/common/qgstreamervideooutput.cpp
+++ b/src/multimedia/platform/gstreamer/common/qgstreamervideooutput.cpp
@@ -95,8 +95,8 @@ void QGstreamerVideoOutput::setVideoSink(QVideoSink *sink)
 
     gstPipeline.beginConfig();
     if (!videoSink.isNull()) {
-        videoSink.setStateSync(GST_STATE_NULL);
         gstVideoOutput.remove(videoSink);
+        videoSink.setStateSync(GST_STATE_NULL);
     }
     videoSink = gstSink;
     gstVideoOutput.add(videoSink);
@@ -104,7 +104,7 @@ void QGstreamerVideoOutput::setVideoSink(QVideoSink *sink)
     videoConvert.link(videoSink);
     GstEvent *event = gst_event_new_reconfigure();
     gst_element_send_event(videoSink.element(), event);
-    videoSink.setState(GST_STATE_PAUSED);
+    videoSink.syncStateWithParent();
 
     doLinkSubtitleStream();
 
@@ -141,8 +141,8 @@ void QGstreamerVideoOutput::linkSubtitleStream(QGstElement src)
 void QGstreamerVideoOutput::doLinkSubtitleStream()
 {
     if (!subtitleSink.isNull()) {
-        subtitleSink.setStateSync(GST_STATE_NULL);
         gstPipeline.remove(subtitleSink);
+        subtitleSink.setStateSync(GST_STATE_NULL);
         subtitleSink = {};
     }
     if (!m_videoSink || subtitleSrc.isNull())
diff --git a/src/multimedia/platform/gstreamer/mediacapture/qgstreamermediacapture.cpp b/src/multimedia/platform/gstreamer/mediacapture/qgstreamermediacapture.cpp
index 6a0f103a4..3c1793b15 100644
--- a/src/multimedia/platform/gstreamer/mediacapture/qgstreamermediacapture.cpp
+++ b/src/multimedia/platform/gstreamer/mediacapture/qgstreamermediacapture.cpp
@@ -119,14 +119,15 @@ void QGstreamerMediaCapture::setCamera(QPlatformCamera *camera)
         unlinkTeeFromPad(gstVideoTee, imageCaptureSink);
 
         auto camera = gstCamera->gstElement();
-        camera.setStateSync(GST_STATE_NULL);
-        gstVideoTee.setStateSync(GST_STATE_NULL);
-        gstVideoOutput->gstElement().setStateSync(GST_STATE_NULL);
 
         gstPipeline.remove(camera);
         gstPipeline.remove(gstVideoTee);
         gstPipeline.remove(gstVideoOutput->gstElement());
 
+        camera.setStateSync(GST_STATE_NULL);
+        gstVideoTee.setStateSync(GST_STATE_NULL);
+        gstVideoOutput->gstElement().setStateSync(GST_STATE_NULL);
+
         gstVideoTee = {};
         gstCamera->setCaptureSession(nullptr);
     }
@@ -168,8 +169,8 @@ void QGstreamerMediaCapture::setImageCapture(QPlatformImageCapture *imageCapture
 
     if (m_imageCapture) {
         unlinkTeeFromPad(gstVideoTee, imageCaptureSink);
-        m_imageCapture->gstElement().setStateSync(GST_STATE_NULL);
         gstPipeline.remove(m_imageCapture->gstElement());
+        m_imageCapture->gstElement().setStateSync(GST_STATE_NULL);
         imageCaptureSink = {};
         m_imageCapture->setCaptureSession(nullptr);
     }
@@ -245,16 +246,16 @@ void QGstreamerMediaCapture::unlinkEncoder()
     if (!encoderVideoCapsFilter.isNull()) {
         encoderVideoCapsFilter.src().unlinkPeer();
         unlinkTeeFromPad(gstVideoTee, encoderVideoCapsFilter.sink());
-        encoderVideoCapsFilter.setStateSync(GST_STATE_NULL);
         gstPipeline.remove(encoderVideoCapsFilter);
+        encoderVideoCapsFilter.setStateSync(GST_STATE_NULL);
         encoderVideoCapsFilter = {};
     }
 
     if (!encoderAudioCapsFilter.isNull()) {
         encoderAudioCapsFilter.src().unlinkPeer();
         unlinkTeeFromPad(gstAudioTee, encoderAudioCapsFilter.sink());
-        encoderAudioCapsFilter.setStateSync(GST_STATE_NULL);
         gstPipeline.remove(encoderAudioCapsFilter);
+        encoderAudioCapsFilter.setStateSync(GST_STATE_NULL);
         encoderAudioCapsFilter = {};
     }
 
@@ -272,14 +273,14 @@ void QGstreamerMediaCapture::setAudioInput(QPlatformAudioInput *input)
 
         if (gstAudioOutput) {
             unlinkTeeFromPad(gstAudioTee, gstAudioOutput->gstElement().staticPad("sink"));
-            gstAudioOutput->gstElement().setStateSync(GST_STATE_NULL);
             gstPipeline.remove(gstAudioOutput->gstElement());
+            gstAudioOutput->gstElement().setStateSync(GST_STATE_NULL);
         }
 
-        gstAudioInput->gstElement().setStateSync(GST_STATE_NULL);
         gstPipeline.remove(gstAudioInput->gstElement());
-        gstAudioTee.setStateSync(GST_STATE_NULL);
         gstPipeline.remove(gstAudioTee);
+        gstAudioInput->gstElement().setStateSync(GST_STATE_NULL);
+        gstAudioTee.setStateSync(GST_STATE_NULL);
         gstAudioTee = {};
     }
 
@@ -317,8 +318,8 @@ void QGstreamerMediaCapture::setAudioOutput(QPlatformAudioOutput *output)
     if (gstAudioOutput && gstAudioInput) {
         // If audio input is set, the output is in the pipeline
         unlinkTeeFromPad(gstAudioTee, gstAudioOutput->gstElement().staticPad("sink"));
-        gstAudioOutput->gstElement().setStateSync(GST_STATE_NULL);
         gstPipeline.remove(gstAudioOutput->gstElement());
+        gstAudioOutput->gstElement().setStateSync(GST_STATE_NULL);
     }
 
     gstAudioOutput = static_cast<QGstreamerAudioOutput *>(output);
diff --git a/src/multimedia/platform/gstreamer/mediacapture/qgstreamermediaencoder.cpp b/src/multimedia/platform/gstreamer/mediacapture/qgstreamermediaencoder.cpp
index 618544d0d..0461b9576 100644
--- a/src/multimedia/platform/gstreamer/mediacapture/qgstreamermediaencoder.cpp
+++ b/src/multimedia/platform/gstreamer/mediacapture/qgstreamermediaencoder.cpp
@@ -390,10 +390,10 @@ void QGstreamerMediaEncoder::finalize()
 
     qCDebug(qLcMediaEncoder) << "finalize";
 
-    gstEncoder.setStateSync(GST_STATE_NULL);
-    gstFileSink.setStateSync(GST_STATE_NULL);
     gstPipeline.remove(gstEncoder);
     gstPipeline.remove(gstFileSink);
+    gstEncoder.setStateSync(GST_STATE_NULL);
+    gstFileSink.setStateSync(GST_STATE_NULL);
     gstFileSink = {};
     gstEncoder = {};
     m_finalizing = false;
diff --git a/tests/auto/integration/qmediacapturesession/CMakeLists.txt b/tests/auto/integration/qmediacapturesession/CMakeLists.txt
index 60fcace14..63ac2e99f 100644
--- a/tests/auto/integration/qmediacapturesession/CMakeLists.txt
+++ b/tests/auto/integration/qmediacapturesession/CMakeLists.txt
@@ -12,3 +12,9 @@ qt_internal_add_test(tst_qmediacapturesession
         Qt::MultimediaPrivate
         Qt::MultimediaWidgets
 )
+
+if(QT_FEATURE_gstreamer)
+    set_tests_properties(tst_qmediacapturesession
+        PROPERTIES ENVIRONMENT "G_DEBUG=fatal_criticals"
+    )
+endif()
diff --git a/tests/auto/integration/qmediacapturesession/tst_qmediacapturesession.cpp b/tests/auto/integration/qmediacapturesession/tst_qmediacapturesession.cpp
index 97e977288..76a69b205 100644
--- a/tests/auto/integration/qmediacapturesession/tst_qmediacapturesession.cpp
+++ b/tests/auto/integration/qmediacapturesession/tst_qmediacapturesession.cpp
@@ -63,6 +63,8 @@ class tst_QMediaCaptureSession: public QObject
     Q_OBJECT
 
 private slots:
+    void stress_test_setup_and_teardown();
+
     void can_add_and_remove_AudioInput_with_and_without_AudioOutput_attached();
     void can_change_AudioDevices_on_attached_AudioInput();
     void can_change_AudioInput_during_recording();
@@ -132,6 +134,27 @@ void tst_QMediaCaptureSession::recordFail(QMediaCaptureSession &session)
     QTRY_VERIFY_WITH_TIMEOUT(recorder.recorderState() == QMediaRecorder::StoppedState, 1000);
 }
 
+void tst_QMediaCaptureSession::stress_test_setup_and_teardown()
+{
+    for (int i = 0; i < 50; i++) {
+        QMediaCaptureSession session;
+        QMediaRecorder recorder;
+        QCamera camera;
+        QAudioInput input;
+        QAudioOutput output;
+        QVideoWidget video;
+
+        session.setAudioInput(&input);
+        session.setAudioOutput(&output);
+        session.setRecorder(&recorder);
+        session.setCamera(&camera);
+        session.setVideoOutput(&video);
+
+        QRandomGenerator rng;
+        QTest::qWait(rng.bounded(200));
+    }
+}
+
 void tst_QMediaCaptureSession::can_add_and_remove_AudioInput_with_and_without_AudioOutput_attached()
 {
     QAudioInput input;