GStreamer: ensure stopping elements before removing

When stopping elements after removing, we've seen deadlocks when setting
the state back to null.
We now ensure that we always stop elements before removing them from
their bin.

Fixes: QTBUG-122638
Pick-to: 6.5
Change-Id: I9b2ac6329082a6a08f8c9807106b75c7ea680bd3
Reviewed-by: Artem Dyomin <artem.dyomin@qt.io>
Reviewed-by: Jøger Hansegård <joger.hansegard@qt.io>
(cherry picked from commit cf84c3164fd5bfa85d2e9c89fd24d8f050cee934)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 6027a24f23c5059f23b4791e8b16d430e5ddafa3)

8 files changed, 28 insertions, 43 deletions

diff --git a/src/plugins/multimedia/gstreamer/common/qgst_p.h b/src/plugins/multimedia/gstreamer/common/qgst_p.h
index 529f98655..f9444853c 100644
--- a/src/plugins/multimedia/gstreamer/common/qgst_p.h
+++ b/src/plugins/multimedia/gstreamer/common/qgst_p.h
@@ -670,6 +670,15 @@ public:
             gst_bin_remove_many(bin(), ts.element()..., nullptr);
     }
 
+    template <typename... Ts>
+    std::enable_if_t<(std::is_base_of_v<QGstElement, Ts> && ...), void>
+    stopAndRemoveElements(Ts... ts)
+    {
+        bool stateChangeSuccessful = (ts.setStateSync(GST_STATE_NULL) && ...);
+        Q_ASSERT(stateChangeSuccessful);
+        remove(ts...);
+    }
+
     GstBin *bin() const { return GST_BIN_CAST(get()); }
 
     void addGhostPad(const QGstElement &child, const char *name)
diff --git a/src/plugins/multimedia/gstreamer/common/qgstreameraudioinput.cpp b/src/plugins/multimedia/gstreamer/common/qgstreameraudioinput.cpp
index 615f73ef1..d6d077cbe 100644
--- a/src/plugins/multimedia/gstreamer/common/qgstreameraudioinput.cpp
+++ b/src/plugins/multimedia/gstreamer/common/qgstreameraudioinput.cpp
@@ -114,8 +114,7 @@ void QGstreamerAudioInput::setAudioDevice(const QAudioDevice &device)
 
     // FIXME: most probably source can be disconnected outside of idle probe
     audioSrc.staticPad("src").doInIdleProbe([&]() { qUnlinkGstElements(audioSrc, audioVolume); });
-    audioSrc.setStateSync(GST_STATE_NULL);
-    gstAudioInput.remove(audioSrc);
+    gstAudioInput.stopAndRemoveElements(audioSrc);
     audioSrc = newSrc;
     gstAudioInput.add(audioSrc);
     qLinkGstElements(audioSrc, audioVolume);
diff --git a/src/plugins/multimedia/gstreamer/common/qgstreamermediaplayer.cpp b/src/plugins/multimedia/gstreamer/common/qgstreamermediaplayer.cpp
index 99617da9b..873418ddf 100644
--- a/src/plugins/multimedia/gstreamer/common/qgstreamermediaplayer.cpp
+++ b/src/plugins/multimedia/gstreamer/common/qgstreamermediaplayer.cpp
@@ -613,8 +613,7 @@ void QGstreamerMediaPlayer::removeOutput(TrackSelector &ts)
 
     if (!e.isNull()) {
         qCDebug(qLcMediaPlayer) << "removing output for track type" << ts.type;
-        playerPipeline.remove(e);
-        e.setStateSync(GST_STATE_NULL);
+        playerPipeline.stopAndRemoveElements(e);
     }
 
     ts.isConnected = false;
diff --git a/src/plugins/multimedia/gstreamer/common/qgstreamervideooutput.cpp b/src/plugins/multimedia/gstreamer/common/qgstreamervideooutput.cpp
index 4186ddcaa..f5318d79a 100644
--- a/src/plugins/multimedia/gstreamer/common/qgstreamervideooutput.cpp
+++ b/src/plugins/multimedia/gstreamer/common/qgstreamervideooutput.cpp
@@ -74,10 +74,9 @@ void QGstreamerVideoOutput::setVideoSink(QVideoSink *sink)
         return;
 
     gstPipeline.beginConfig();
-    if (!videoSink.isNull()) {
-        gstVideoOutput.remove(videoSink);
-        videoSink.setStateSync(GST_STATE_NULL);
-    }
+    if (!videoSink.isNull())
+        gstVideoOutput.stopAndRemoveElements(videoSink);
+
     videoSink = gstSink;
     gstVideoOutput.add(videoSink);
 
@@ -126,9 +125,8 @@ void QGstreamerVideoOutput::unlinkSubtitleStream()
     subtitleSrc = {};
     if (!subtitleSink.isNull()) {
         gstPipeline.beginConfig();
-        gstPipeline.remove(subtitleSink);
+        gstPipeline.stopAndRemoveElements(subtitleSink);
         gstPipeline.endConfig();
-        subtitleSink.setStateSync(GST_STATE_NULL);
         subtitleSink = {};
     }
     if (m_videoSink)
@@ -138,8 +136,7 @@ void QGstreamerVideoOutput::unlinkSubtitleStream()
 void QGstreamerVideoOutput::doLinkSubtitleStream()
 {
     if (!subtitleSink.isNull()) {
-        gstPipeline.remove(subtitleSink);
-        subtitleSink.setStateSync(GST_STATE_NULL);
+        gstPipeline.stopAndRemoveElements(subtitleSink);
         subtitleSink = {};
     }
     if (!m_videoSink || subtitleSrc.isNull())
diff --git a/src/plugins/multimedia/gstreamer/common/qgstreamervideosink.cpp b/src/plugins/multimedia/gstreamer/common/qgstreamervideosink.cpp
index 466502ee6..73276bae5 100644
--- a/src/plugins/multimedia/gstreamer/common/qgstreamervideosink.cpp
+++ b/src/plugins/multimedia/gstreamer/common/qgstreamervideosink.cpp
@@ -136,10 +136,8 @@ void QGstreamerVideoSink::updateSinkElement()
 
     gstPipeline.beginConfig();
 
-    if (!gstVideoSink.isNull()) {
-        gstVideoSink.setStateSync(GST_STATE_NULL);
-        sinkBin.remove(gstVideoSink);
-    }
+    if (!gstVideoSink.isNull())
+        sinkBin.stopAndRemoveElements(gstVideoSink);
 
     gstVideoSink = newSink;
     sinkBin.add(gstVideoSink);
diff --git a/src/plugins/multimedia/gstreamer/mediacapture/qgstreamercamera.cpp b/src/plugins/multimedia/gstreamer/mediacapture/qgstreamercamera.cpp
index b604746fd..1e27b29e3 100644
--- a/src/plugins/multimedia/gstreamer/mediacapture/qgstreamercamera.cpp
+++ b/src/plugins/multimedia/gstreamer/mediacapture/qgstreamercamera.cpp
@@ -104,10 +104,7 @@ void QGstreamerCamera::setCamera(const QCameraDevice &camera)
             f.pixelFormat() == QVideoFrameFormat::Format_Jpeg ? "jpegdec" : "identity");
 
     qUnlinkGstElements(gstCamera, gstCapsFilter, gstDecode, gstVideoConvert);
-    gstCameraBin.remove(gstCamera, gstDecode);
-
-    gstCamera.setStateSync(GST_STATE_NULL);
-    gstDecode.setStateSync(GST_STATE_NULL);
+    gstCameraBin.stopAndRemoveElements(gstCamera, gstDecode);
 
     gstCapsFilter.set("caps", caps);
 
@@ -153,8 +150,7 @@ bool QGstreamerCamera::setCameraFormat(const QCameraFormat &format)
             qWarning() << "linking filtered camera to decoder failed" << gstCamera.name() << caps;
     });
 
-    gstCameraBin.remove(gstDecode);
-    gstDecode.setStateSync(GST_STATE_NULL);
+    gstCameraBin.stopAndRemoveElements(gstDecode);
 
     gstDecode = newGstDecode;
 
diff --git a/src/plugins/multimedia/gstreamer/mediacapture/qgstreamermediacapture.cpp b/src/plugins/multimedia/gstreamer/mediacapture/qgstreamermediacapture.cpp
index 982ca626c..2c1b944cc 100644
--- a/src/plugins/multimedia/gstreamer/mediacapture/qgstreamermediacapture.cpp
+++ b/src/plugins/multimedia/gstreamer/mediacapture/qgstreamermediacapture.cpp
@@ -95,11 +95,7 @@ void QGstreamerMediaCapture::setCamera(QPlatformCamera *camera)
 
         auto camera = gstCamera->gstElement();
 
-        gstPipeline.remove(camera, gstVideoTee, gstVideoOutput->gstElement());
-
-        camera.setStateSync(GST_STATE_NULL);
-        gstVideoTee.setStateSync(GST_STATE_NULL);
-        gstVideoOutput->gstElement().setStateSync(GST_STATE_NULL);
+        gstPipeline.stopAndRemoveElements(camera, gstVideoTee, gstVideoOutput->gstElement());
 
         gstVideoTee = {};
         gstCamera->setCaptureSession(nullptr);
@@ -142,8 +138,7 @@ void QGstreamerMediaCapture::setImageCapture(QPlatformImageCapture *imageCapture
 
     if (m_imageCapture) {
         unlinkTeeFromPad(gstVideoTee, imageCaptureSink);
-        gstPipeline.remove(m_imageCapture->gstElement());
-        m_imageCapture->gstElement().setStateSync(GST_STATE_NULL);
+        gstPipeline.stopAndRemoveElements(m_imageCapture->gstElement());
         imageCaptureSink = {};
         m_imageCapture->setCaptureSession(nullptr);
     }
@@ -151,8 +146,8 @@ void QGstreamerMediaCapture::setImageCapture(QPlatformImageCapture *imageCapture
     m_imageCapture = control;
     if (m_imageCapture) {
         imageCaptureSink = m_imageCapture->gstElement().staticPad("sink");
-        m_imageCapture->gstElement().setState(GST_STATE_PLAYING);
         gstPipeline.add(m_imageCapture->gstElement());
+        m_imageCapture->gstElement().setState(GST_STATE_PLAYING);
         linkTeeToPad(gstVideoTee, imageCaptureSink);
         m_imageCapture->setCaptureSession(this);
     }
@@ -223,16 +218,14 @@ void QGstreamerMediaCapture::unlinkEncoder()
     if (!encoderVideoCapsFilter.isNull()) {
         encoderVideoCapsFilter.src().unlinkPeer();
         unlinkTeeFromPad(gstVideoTee, encoderVideoCapsFilter.sink());
-        gstPipeline.remove(encoderVideoCapsFilter);
-        encoderVideoCapsFilter.setStateSync(GST_STATE_NULL);
+        gstPipeline.stopAndRemoveElements(encoderVideoCapsFilter);
         encoderVideoCapsFilter = {};
     }
 
     if (!encoderAudioCapsFilter.isNull()) {
         encoderAudioCapsFilter.src().unlinkPeer();
         unlinkTeeFromPad(gstAudioTee, encoderAudioCapsFilter.sink());
-        gstPipeline.remove(encoderAudioCapsFilter);
-        encoderAudioCapsFilter.setStateSync(GST_STATE_NULL);
+        gstPipeline.stopAndRemoveElements(encoderAudioCapsFilter);
         encoderAudioCapsFilter = {};
     }
 
@@ -254,9 +247,7 @@ void QGstreamerMediaCapture::setAudioInput(QPlatformAudioInput *input)
             gstAudioOutput->gstElement().setStateSync(GST_STATE_NULL);
         }
 
-        gstPipeline.remove(gstAudioInput->gstElement(), gstAudioTee);
-        gstAudioInput->gstElement().setStateSync(GST_STATE_NULL);
-        gstAudioTee.setStateSync(GST_STATE_NULL);
+        gstPipeline.stopAndRemoveElements(gstAudioInput->gstElement(), gstAudioTee);
         gstAudioTee = {};
     }
 
@@ -294,8 +285,7 @@ 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"));
-        gstPipeline.remove(gstAudioOutput->gstElement());
-        gstAudioOutput->gstElement().setStateSync(GST_STATE_NULL);
+        gstPipeline.stopAndRemoveElements(gstAudioOutput->gstElement());
     }
 
     gstAudioOutput = static_cast<QGstreamerAudioOutput *>(output);
diff --git a/src/plugins/multimedia/gstreamer/mediacapture/qgstreamermediaencoder.cpp b/src/plugins/multimedia/gstreamer/mediacapture/qgstreamermediaencoder.cpp
index aac86468c..65d47b932 100644
--- a/src/plugins/multimedia/gstreamer/mediacapture/qgstreamermediaencoder.cpp
+++ b/src/plugins/multimedia/gstreamer/mediacapture/qgstreamermediaencoder.cpp
@@ -364,10 +364,7 @@ void QGstreamerMediaEncoder::finalize()
 
     qCDebug(qLcMediaEncoderGst) << "finalize";
 
-    gstPipeline.remove(gstEncoder);
-    gstPipeline.remove(gstFileSink);
-    gstEncoder.setStateSync(GST_STATE_NULL);
-    gstFileSink.setStateSync(GST_STATE_NULL);
+    gstPipeline.stopAndRemoveElements(gstEncoder, gstFileSink);
     gstFileSink = {};
     gstEncoder = {};
     m_finalizing = false;