GStreamer: clean up sync APIs

replace `beginConfig`/`endConfig` with a scoped functor API
`modifyPipelineWhileNotRunning` to make code easier to reason about.

Pick-to: 6.6 6.5
Change-Id: Ifd47aaca9f8b34173f28c7f573ff7c8d02310e21
Reviewed-by: Artem Dyomin <artem.dyomin@qt.io>
(cherry picked from commit bea05730e0bb40c86fe1ddc5ddefa70bdfdf9397)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

6 files changed, 123 insertions, 107 deletions

diff --git a/src/plugins/multimedia/gstreamer/common/qgstpipeline_p.h b/src/plugins/multimedia/gstreamer/common/qgstpipeline_p.h
index 8997159a1..1163b11d5 100644
--- a/src/plugins/multimedia/gstreamer/common/qgstpipeline_p.h
+++ b/src/plugins/multimedia/gstreamer/common/qgstpipeline_p.h
@@ -85,8 +85,13 @@ public:
 #endif
     }
 
-    void beginConfig();
-    void endConfig();
+    template <typename Functor>
+    void modifyPipelineWhileNotRunning(Functor &&fn)
+    {
+        beginConfig();
+        fn();
+        endConfig();
+    }
 
     void flush();
 
@@ -98,6 +103,10 @@ public:
     qint64 position() const;
 
     qint64 duration() const;
+
+private:
+    void beginConfig();
+    void endConfig();
 };
 
 QT_END_NAMESPACE
diff --git a/src/plugins/multimedia/gstreamer/common/qgstreamermediaplayer.cpp b/src/plugins/multimedia/gstreamer/common/qgstreamermediaplayer.cpp
index bdd6ef921..92ead9eb7 100644
--- a/src/plugins/multimedia/gstreamer/common/qgstreamermediaplayer.cpp
+++ b/src/plugins/multimedia/gstreamer/common/qgstreamermediaplayer.cpp
@@ -768,17 +768,17 @@ void QGstreamerMediaPlayer::setAudioOutput(QPlatformAudioOutput *output)
 
     auto &ts = trackSelector(AudioStream);
 
-    playerPipeline.beginConfig();
-    if (gstAudioOutput) {
-        removeOutput(ts);
-        gstAudioOutput->setPipeline({});
-    }
-    gstAudioOutput = static_cast<QGstreamerAudioOutput *>(output);
-    if (gstAudioOutput) {
-        gstAudioOutput->setPipeline(playerPipeline);
-        connectOutput(ts);
-    }
-    playerPipeline.endConfig();
+    playerPipeline.modifyPipelineWhileNotRunning([&] {
+        if (gstAudioOutput) {
+            removeOutput(ts);
+            gstAudioOutput->setPipeline({});
+        }
+        gstAudioOutput = static_cast<QGstreamerAudioOutput *>(output);
+        if (gstAudioOutput) {
+            gstAudioOutput->setPipeline(playerPipeline);
+            connectOutput(ts);
+        }
+    });
 }
 
 QMediaMetaData QGstreamerMediaPlayer::metaData() const
@@ -916,14 +916,14 @@ void QGstreamerMediaPlayer::setActiveTrack(TrackType type, int index)
     if (type == QPlatformMediaPlayer::SubtitleStream)
         gstVideoOutput->flushSubtitles();
 
-    playerPipeline.beginConfig();
-    if (track.isNull()) {
-        removeOutput(ts);
-    } else {
-        ts.setActiveInputPad(track);
-        connectOutput(ts);
-    }
-    playerPipeline.endConfig();
+    playerPipeline.modifyPipelineWhileNotRunning([&] {
+        if (track.isNull()) {
+            removeOutput(ts);
+        } else {
+            ts.setActiveInputPad(track);
+            connectOutput(ts);
+        }
+    });
 
     // seek to force an immediate change of the stream
     if (playerPipeline.state() == GST_STATE_PLAYING)
diff --git a/src/plugins/multimedia/gstreamer/common/qgstreamervideooutput.cpp b/src/plugins/multimedia/gstreamer/common/qgstreamervideooutput.cpp
index 3825e7f20..304819df3 100644
--- a/src/plugins/multimedia/gstreamer/common/qgstreamervideooutput.cpp
+++ b/src/plugins/multimedia/gstreamer/common/qgstreamervideooutput.cpp
@@ -73,21 +73,20 @@ void QGstreamerVideoOutput::setVideoSink(QVideoSink *sink)
     if (videoSink == gstSink)
         return;
 
-    gstPipeline.beginConfig();
-    if (!videoSink.isNull())
-        gstVideoOutput.stopAndRemoveElements(videoSink);
+    gstPipeline.modifyPipelineWhileNotRunning([&] {
+        if (!videoSink.isNull())
+            gstVideoOutput.stopAndRemoveElements(videoSink);
 
-    videoSink = gstSink;
-    gstVideoOutput.add(videoSink);
+        videoSink = gstSink;
+        gstVideoOutput.add(videoSink);
 
-    qLinkGstElements(videoConvert, videoSink);
-    GstEvent *event = gst_event_new_reconfigure();
-    gst_element_send_event(videoSink.element(), event);
-    videoSink.syncStateWithParent();
+        qLinkGstElements(videoConvert, videoSink);
+        GstEvent *event = gst_event_new_reconfigure();
+        gst_element_send_event(videoSink.element(), event);
+        videoSink.syncStateWithParent();
 
-    doLinkSubtitleStream();
-
-    gstPipeline.endConfig();
+        doLinkSubtitleStream();
+    });
 
     qCDebug(qLcMediaVideoOutput) << "sinkChanged" << gstSink.name();
 
@@ -111,10 +110,10 @@ void QGstreamerVideoOutput::linkSubtitleStream(QGstElement src)
     if (src == subtitleSrc)
         return;
 
-    gstPipeline.beginConfig();
-    subtitleSrc = src;
-    doLinkSubtitleStream();
-    gstPipeline.endConfig();
+    gstPipeline.modifyPipelineWhileNotRunning([&] {
+        subtitleSrc = src;
+        doLinkSubtitleStream();
+    });
 }
 
 void QGstreamerVideoOutput::unlinkSubtitleStream()
@@ -124,9 +123,10 @@ void QGstreamerVideoOutput::unlinkSubtitleStream()
     qCDebug(qLcMediaVideoOutput) << "unlink subtitle stream";
     subtitleSrc = {};
     if (!subtitleSink.isNull()) {
-        gstPipeline.beginConfig();
-        gstPipeline.stopAndRemoveElements(subtitleSink);
-        gstPipeline.endConfig();
+        gstPipeline.modifyPipelineWhileNotRunning([&] {
+            gstPipeline.stopAndRemoveElements(subtitleSink);
+            return;
+        });
         subtitleSink = {};
     }
     if (m_videoSink)
diff --git a/src/plugins/multimedia/gstreamer/common/qgstreamervideosink.cpp b/src/plugins/multimedia/gstreamer/common/qgstreamervideosink.cpp
index 4af96b37d..3272854dd 100644
--- a/src/plugins/multimedia/gstreamer/common/qgstreamervideosink.cpp
+++ b/src/plugins/multimedia/gstreamer/common/qgstreamervideosink.cpp
@@ -154,18 +154,17 @@ void QGstreamerVideoSink::updateSinkElement()
     if (newSink == gstVideoSink)
         return;
 
-    gstPipeline.beginConfig();
+    gstPipeline.modifyPipelineWhileNotRunning([&] {
+        if (!gstVideoSink.isNull())
+            sinkBin.stopAndRemoveElements(gstVideoSink);
 
-    if (!gstVideoSink.isNull())
-        sinkBin.stopAndRemoveElements(gstVideoSink);
+        gstVideoSink = newSink;
+        sinkBin.add(gstVideoSink);
+        if (!qLinkGstElements(gstCapsFilter, gstVideoSink))
+            qCDebug(qLcMediaVideoSink) << "couldn't link caps filter and sink";
+        gstVideoSink.setState(GST_STATE_PAUSED);
+    });
 
-    gstVideoSink = newSink;
-    sinkBin.add(gstVideoSink);
-    if (!qLinkGstElements(gstCapsFilter, gstVideoSink))
-        qCDebug(qLcMediaVideoSink) << "couldn't link caps filter and sink";
-    gstVideoSink.setState(GST_STATE_PAUSED);
-
-    gstPipeline.endConfig();
     gstPipeline.dumpGraph("updateVideoSink");
 }
 
diff --git a/src/plugins/multimedia/gstreamer/mediacapture/qgstreamermediacapture.cpp b/src/plugins/multimedia/gstreamer/mediacapture/qgstreamermediacapture.cpp
index 2c1b944cc..d6c680bd3 100644
--- a/src/plugins/multimedia/gstreamer/mediacapture/qgstreamermediacapture.cpp
+++ b/src/plugins/multimedia/gstreamer/mediacapture/qgstreamermediacapture.cpp
@@ -136,21 +136,23 @@ void QGstreamerMediaCapture::setImageCapture(QPlatformImageCapture *imageCapture
     if (m_imageCapture == control)
         return;
 
-    if (m_imageCapture) {
-        unlinkTeeFromPad(gstVideoTee, imageCaptureSink);
-        gstPipeline.stopAndRemoveElements(m_imageCapture->gstElement());
-        imageCaptureSink = {};
-        m_imageCapture->setCaptureSession(nullptr);
-    }
+    gstPipeline.modifyPipelineWhileNotRunning([&] {
+        if (m_imageCapture) {
+            unlinkTeeFromPad(gstVideoTee, imageCaptureSink);
+            gstPipeline.stopAndRemoveElements(m_imageCapture->gstElement());
+            imageCaptureSink = {};
+            m_imageCapture->setCaptureSession(nullptr);
+        }
 
-    m_imageCapture = control;
-    if (m_imageCapture) {
-        imageCaptureSink = m_imageCapture->gstElement().staticPad("sink");
-        gstPipeline.add(m_imageCapture->gstElement());
-        m_imageCapture->gstElement().setState(GST_STATE_PLAYING);
-        linkTeeToPad(gstVideoTee, imageCaptureSink);
-        m_imageCapture->setCaptureSession(this);
-    }
+        m_imageCapture = control;
+        if (m_imageCapture) {
+            imageCaptureSink = m_imageCapture->gstElement().staticPad("sink");
+            gstPipeline.add(m_imageCapture->gstElement());
+            m_imageCapture->gstElement().setState(GST_STATE_PLAYING);
+            linkTeeToPad(gstVideoTee, imageCaptureSink);
+            m_imageCapture->setCaptureSession(this);
+        }
+    });
 
     gstPipeline.dumpGraph("imageCapture");
 
@@ -238,38 +240,40 @@ void QGstreamerMediaCapture::setAudioInput(QPlatformAudioInput *input)
     if (gstAudioInput == input)
         return;
 
-    if (gstAudioInput) {
-        unlinkTeeFromPad(gstAudioTee, encoderAudioSink);
+    gstPipeline.modifyPipelineWhileNotRunning([&] {
+        if (gstAudioInput) {
+            unlinkTeeFromPad(gstAudioTee, encoderAudioSink);
 
-        if (gstAudioOutput) {
-            unlinkTeeFromPad(gstAudioTee, gstAudioOutput->gstElement().staticPad("sink"));
-            gstPipeline.remove(gstAudioOutput->gstElement());
-            gstAudioOutput->gstElement().setStateSync(GST_STATE_NULL);
-        }
+            if (gstAudioOutput) {
+                unlinkTeeFromPad(gstAudioTee, gstAudioOutput->gstElement().staticPad("sink"));
+                gstPipeline.remove(gstAudioOutput->gstElement());
+                gstAudioOutput->gstElement().setStateSync(GST_STATE_NULL);
+            }
 
-        gstPipeline.stopAndRemoveElements(gstAudioInput->gstElement(), gstAudioTee);
-        gstAudioTee = {};
-    }
+            gstPipeline.stopAndRemoveElements(gstAudioInput->gstElement(), gstAudioTee);
+            gstAudioTee = {};
+        }
 
-    gstAudioInput = static_cast<QGstreamerAudioInput *>(input);
-    if (gstAudioInput) {
-        Q_ASSERT(gstAudioTee.isNull());
-        gstAudioTee = QGstElement::createFromFactory("tee", "audiotee");
-        gstAudioTee.set("allow-not-linked", true);
-        gstPipeline.add(gstAudioInput->gstElement(), gstAudioTee);
-        qLinkGstElements(gstAudioInput->gstElement(), gstAudioTee);
+        gstAudioInput = static_cast<QGstreamerAudioInput *>(input);
+        if (gstAudioInput) {
+            Q_ASSERT(gstAudioTee.isNull());
+            gstAudioTee = QGstElement::createFromFactory("tee", "audiotee");
+            gstAudioTee.set("allow-not-linked", true);
+            gstPipeline.add(gstAudioInput->gstElement(), gstAudioTee);
+            qLinkGstElements(gstAudioInput->gstElement(), gstAudioTee);
 
-        if (gstAudioOutput) {
-            gstPipeline.add(gstAudioOutput->gstElement());
-            gstAudioOutput->gstElement().setState(GST_STATE_PLAYING);
-            linkTeeToPad(gstAudioTee, gstAudioOutput->gstElement().staticPad("sink"));
-        }
+            if (gstAudioOutput) {
+                gstPipeline.add(gstAudioOutput->gstElement());
+                gstAudioOutput->gstElement().setState(GST_STATE_PLAYING);
+                linkTeeToPad(gstAudioTee, gstAudioOutput->gstElement().staticPad("sink"));
+            }
 
-        gstAudioTee.setState(GST_STATE_PLAYING);
-        gstAudioInput->gstElement().setStateSync(GST_STATE_PLAYING);
+            gstAudioTee.setState(GST_STATE_PLAYING);
+            gstAudioInput->gstElement().setStateSync(GST_STATE_PLAYING);
 
-        linkTeeToPad(gstAudioTee, encoderAudioSink);
-    }
+            linkTeeToPad(gstAudioTee, encoderAudioSink);
+        }
+    });
 }
 
 void QGstreamerMediaCapture::setVideoPreview(QVideoSink *sink)
@@ -282,18 +286,20 @@ void QGstreamerMediaCapture::setAudioOutput(QPlatformAudioOutput *output)
     if (gstAudioOutput == output)
         return;
 
-    if (gstAudioOutput && gstAudioInput) {
-        // If audio input is set, the output is in the pipeline
-        unlinkTeeFromPad(gstAudioTee, gstAudioOutput->gstElement().staticPad("sink"));
-        gstPipeline.stopAndRemoveElements(gstAudioOutput->gstElement());
-    }
+    gstPipeline.modifyPipelineWhileNotRunning([&] {
+        if (gstAudioOutput && gstAudioInput) {
+            // If audio input is set, the output is in the pipeline
+            unlinkTeeFromPad(gstAudioTee, gstAudioOutput->gstElement().staticPad("sink"));
+            gstPipeline.stopAndRemoveElements(gstAudioOutput->gstElement());
+        }
 
-    gstAudioOutput = static_cast<QGstreamerAudioOutput *>(output);
-    if (gstAudioOutput && gstAudioInput) {
-        gstPipeline.add(gstAudioOutput->gstElement());
-        gstAudioOutput->gstElement().setState(GST_STATE_PLAYING);
-        linkTeeToPad(gstAudioTee, gstAudioOutput->gstElement().staticPad("sink"));
-    }
+        gstAudioOutput = static_cast<QGstreamerAudioOutput *>(output);
+        if (gstAudioOutput && gstAudioInput) {
+            gstPipeline.add(gstAudioOutput->gstElement());
+            gstAudioOutput->gstElement().setState(GST_STATE_PLAYING);
+            linkTeeToPad(gstAudioTee, gstAudioOutput->gstElement().staticPad("sink"));
+        }
+    });
 }
 
 QGstreamerVideoSink *QGstreamerMediaCapture::gstreamerVideoSink() const
diff --git a/src/plugins/multimedia/gstreamer/mediacapture/qgstreamermediaencoder.cpp b/src/plugins/multimedia/gstreamer/mediacapture/qgstreamermediaencoder.cpp
index 65d47b932..9f3d7dee6 100644
--- a/src/plugins/multimedia/gstreamer/mediacapture/qgstreamermediaencoder.cpp
+++ b/src/plugins/multimedia/gstreamer/mediacapture/qgstreamermediaencoder.cpp
@@ -309,14 +309,16 @@ void QGstreamerMediaEncoder::record(QMediaEncoderSettings &settings)
             videoPauseControl.installOn(videoSink);
     }
 
-    gstPipeline.add(gstEncoder, gstFileSink);
-    qLinkGstElements(gstEncoder, gstFileSink);
-    m_metaData.setMetaData(gstEncoder.bin());
+    gstPipeline.modifyPipelineWhileNotRunning([&] {
+        gstPipeline.add(gstEncoder, gstFileSink);
+        qLinkGstElements(gstEncoder, gstFileSink);
+        m_metaData.setMetaData(gstEncoder.bin());
 
-    m_session->linkEncoder(audioSink, videoSink);
+        m_session->linkEncoder(audioSink, videoSink);
 
-    gstEncoder.syncStateWithParent();
-    gstFileSink.syncStateWithParent();
+        gstEncoder.syncStateWithParent();
+        gstFileSink.syncStateWithParent();
+    });
 
     signalDurationChangedTimer.start();
     gstPipeline.dumpGraph("recording");