Refactor the gstreamer video sink

Simplify the code avoiding async operations. Rather pause the
pipeline, do the changes and resume as we do in all other places.

Prepare the sink, so that we can handle an additional preprocessing
element where required (e.g. on iMX8)

Change-Id: I5cb019c294bdd104102db325ebbeb3f4139ac749
Reviewed-by: Bartlomiej Moskal <bartlomiej.moskal@qt.io>
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

4 files changed, 58 insertions, 77 deletions

diff --git a/src/multimedia/platform/gstreamer/common/qgstreamervideooutput.cpp b/src/multimedia/platform/gstreamer/common/qgstreamervideooutput.cpp
index c4b88cfae..626e8af53 100644
--- a/src/multimedia/platform/gstreamer/common/qgstreamervideooutput.cpp
+++ b/src/multimedia/platform/gstreamer/common/qgstreamervideooutput.cpp
@@ -75,14 +75,13 @@ void QGstreamerVideoOutput::setVideoSink(QVideoSink *sink)
     if (videoSink == m_videoWindow)
         return;
 
-    if (m_videoWindow) {
-        disconnect(m_videoWindow, SIGNAL(sinkChanged()), this, SLOT(sinkChanged()));
-    }
+    if (m_videoWindow)
+        m_videoWindow->setPipeline({});
 
     m_videoWindow = videoSink;
-    if (m_videoWindow) {
-        connect(m_videoWindow, SIGNAL(sinkChanged()), this, SLOT(sinkChanged()));
-    }
+    if (m_videoWindow)
+        m_videoWindow->setPipeline(gstPipeline);
+
     auto state = gstPipeline.state();
     if (state == GST_STATE_PLAYING)
         gstPipeline.setStateSync(GST_STATE_PAUSED);
@@ -94,7 +93,8 @@ void QGstreamerVideoOutput::setVideoSink(QVideoSink *sink)
 void QGstreamerVideoOutput::setPipeline(const QGstPipeline &pipeline)
 {
     gstPipeline = pipeline;
-    stoppedStateHandler = gstPipeline.inStoppedState()->onValueChanged(std::function([this]() {updatePrerollFrame();}));
+    if (m_videoWindow)
+        m_videoWindow->setPipeline(gstPipeline);
 }
 
 void QGstreamerVideoOutput::linkSubtitleStream(QGstElement src)
@@ -133,37 +133,6 @@ void QGstreamerVideoOutput::setIsPreview()
     videoQueue.set("max-size-time", 0);
 }
 
-void QGstreamerVideoOutput::updateVideoSink(const QGstElement &sink)
-{
-    if (videoSink == sink)
-        return;
-
-    newVideoSink = sink;
-    gstVideoOutput.add(newVideoSink);
-
-    qCDebug(qLcMediaVideoOutput) << "setVideoSurface: Reconfiguring video output" << QThread::currentThreadId();
-
-    auto state = gstPipeline.state();
-
-    if (state != GST_STATE_PLAYING) {
-        changeVideoOutput();
-        return;
-    }
-
-    // This doesn't quite work, as we're be getting the callback in another thread where state changes aren't allowed.
-    auto pad = videoConvert.staticPad("src");
-    pad.addProbe<&QGstreamerVideoOutput::prepareVideoOutputChange>(this, GstPadProbeType(GST_PAD_PROBE_TYPE_BUFFER | GST_PAD_PROBE_TYPE_BLOCKING));
-}
-
-void QGstreamerVideoOutput::updatePrerollFrame()
-{
-    bool stopped = true;
-    if (!gstPipeline.isNull() && !*gstPipeline.inStoppedState())
-        stopped = false;
-    if (!isFakeSink)
-        videoSink.set("show-preroll-frame", !stopped);
-}
-
 void QGstreamerVideoOutput::sinkChanged()
 {
     QGstElement gstSink;
@@ -174,43 +143,29 @@ void QGstreamerVideoOutput::sinkChanged()
         gstSink = QGstElement("fakesink", "fakevideosink");
         isFakeSink = true;
     }
-    qCDebug(qLcMediaVideoOutput) << "sinkChanged" << gstSink.name();
-    updateVideoSink(gstSink);
-}
 
-void QGstreamerVideoOutput::changeVideoOutput()
-{
-    qCDebug(qLcMediaVideoOutput) << "Changing video output" << QThread::currentThreadId();
+    if (videoSink == gstSink)
+        return;
 
-    gstPipeline.setState(GST_STATE_PAUSED);
+    if (!videoSink.isNull()) {
+        videoSink.setStateSync(GST_STATE_NULL);
+        gstVideoOutput.remove(videoSink);
+    }
+    videoSink = gstSink;
+    gstVideoOutput.add(videoSink);
 
-    auto state = videoSink.state();
-    videoSink.setState(GST_STATE_NULL);
-    gstVideoOutput.remove(videoSink);
-    videoSink = newVideoSink;
-    updatePrerollFrame();
     videoConvert.link(videoSink);
     GstEvent *event = gst_event_new_reconfigure();
     gst_element_send_event(videoSink.element(), event);
-    videoSink.setState(state);
-    newVideoSink = {};
+    videoSink.setState(GST_STATE_PAUSED);
 
     gstPipeline.setState(GST_STATE_PLAYING);
+    qCDebug(qLcMediaVideoOutput) << "sinkChanged" << gstSink.name();
 
     GST_DEBUG_BIN_TO_DOT_FILE(gstPipeline.bin(),
-                              GstDebugGraphDetails(/*GST_DEBUG_GRAPH_SHOW_ALL |*/ GST_DEBUG_GRAPH_SHOW_MEDIA_TYPE | GST_DEBUG_GRAPH_SHOW_NON_DEFAULT_PARAMS | GST_DEBUG_GRAPH_SHOW_STATES),
+                              GstDebugGraphDetails(/*GST_DEBUG_GRAPH_SHOW_ALL |*/ GST_DEBUG_GRAPH_SHOW_MEDIA_TYPE |
+                                                   GST_DEBUG_GRAPH_SHOW_NON_DEFAULT_PARAMS | GST_DEBUG_GRAPH_SHOW_STATES),
                               videoSink.name());
-
-}
-
-void QGstreamerVideoOutput::prepareVideoOutputChange(const QGstPad &/*pad*/)
-{
-    qCDebug(qLcMediaVideoOutput) << "Reconfiguring video output" << QThread::currentThreadId();
-
-    if (QThread::currentThread() == this->thread())
-        changeVideoOutput();
-    else
-        QMetaObject::invokeMethod(this, "changeVideoOutput", Qt::BlockingQueuedConnection);
 }
 
 
diff --git a/src/multimedia/platform/gstreamer/common/qgstreamervideooutput_p.h b/src/multimedia/platform/gstreamer/common/qgstreamervideooutput_p.h
index 8058cbc93..26fde3c4f 100644
--- a/src/multimedia/platform/gstreamer/common/qgstreamervideooutput_p.h
+++ b/src/multimedia/platform/gstreamer/common/qgstreamervideooutput_p.h
@@ -81,18 +81,13 @@ public:
 
     void setIsPreview();
 
-    void updateVideoSink(const QGstElement &sink);
-    void updatePrerollFrame();
-public slots:
+private:
     void sinkChanged();
-    void changeVideoOutput();
 
 private:
-    void prepareVideoOutputChange(const QGstPad &pad);
 
     QVideoSink *m_videoSink = nullptr;
     QPointer<QGstreamerVideoSink> m_videoWindow;
-    std::optional<QPropertyChangeHandler<std::function<void()>>> stoppedStateHandler;
     bool isFakeSink = true;
 
     // Gst elements
@@ -104,7 +99,6 @@ private:
     QGstElement videoConvert;
     QGstElement subtitleOverlay;
     QGstElement videoSink;
-    QGstElement newVideoSink;
 
     QGstElement subtitleSrc;
 };
diff --git a/src/multimedia/platform/gstreamer/common/qgstreamervideosink.cpp b/src/multimedia/platform/gstreamer/common/qgstreamervideosink.cpp
index 09f6c8ad9..d4b8f4616 100644
--- a/src/multimedia/platform/gstreamer/common/qgstreamervideosink.cpp
+++ b/src/multimedia/platform/gstreamer/common/qgstreamervideosink.cpp
@@ -51,6 +51,10 @@ Q_LOGGING_CATEGORY(qLcMediaVideoSink, "qt.multimedia.videosink")
 QGstreamerVideoSink::QGstreamerVideoSink(QVideoSink *parent)
     : QPlatformVideoSink(parent)
 {
+    sinkBin = QGstBin("videoSinkBin");
+    gstPreprocess = QGstElement("identity");
+    sinkBin.add(gstPreprocess);
+    sinkBin.addGhostPad(gstPreprocess, "sink");
     createOverlay();
     createRenderer();
 }
@@ -63,9 +67,8 @@ QGstreamerVideoSink::~QGstreamerVideoSink()
 
 QGstElement QGstreamerVideoSink::gstSink()
 {
-    if (m_fullScreen || m_windowId)
-        return m_videoOverlay->videoSink();
-    return m_videoRenderer->gstVideoSink();
+    updateSinkElement();
+    return sinkBin;
 }
 
 void QGstreamerVideoSink::setWinId(WId id)
@@ -123,7 +126,7 @@ void QGstreamerVideoSink::setFullScreen(bool fullScreen)
         return;
     m_fullScreen = fullScreen;
     if (!m_windowId)
-        emit sinkChanged();
+        updateSinkElement();
 }
 
 QSize QGstreamerVideoSink::nativeSize() const
@@ -144,3 +147,26 @@ void QGstreamerVideoSink::createRenderer()
 {
     m_videoRenderer = new QGstreamerVideoRenderer(sink);
 }
+
+void QGstreamerVideoSink::updateSinkElement()
+{
+    auto state = gstPipeline.isNull() ? GST_STATE_NULL : gstPipeline.state();
+    if (state == GST_STATE_PLAYING)
+        gstPipeline.setStateSync(GST_STATE_PAUSED);
+
+    if (!gstVideoSink.isNull()) {
+        gstVideoSink.setStateSync(GST_STATE_NULL);
+        sinkBin.remove(gstVideoSink);
+    }
+    if (m_fullScreen || m_windowId)
+        gstVideoSink = m_videoOverlay->videoSink();
+    else
+        gstVideoSink = m_videoRenderer->gstVideoSink();
+
+    sinkBin.add(gstVideoSink);
+    gstPreprocess.link(gstVideoSink);
+    gstVideoSink.setState(GST_STATE_PAUSED);
+
+    if (state == GST_STATE_PLAYING)
+        gstPipeline.setState(GST_STATE_PLAYING);
+}
diff --git a/src/multimedia/platform/gstreamer/common/qgstreamervideosink_p.h b/src/multimedia/platform/gstreamer/common/qgstreamervideosink_p.h
index d98dfb385..8bffe3f7c 100644
--- a/src/multimedia/platform/gstreamer/common/qgstreamervideosink_p.h
+++ b/src/multimedia/platform/gstreamer/common/qgstreamervideosink_p.h
@@ -91,12 +91,18 @@ public:
 
     bool isReady() const { return m_windowId != 0; }
 
-signals:
-    void sinkChanged();
+    void setPipeline(QGstPipeline pipeline) { gstPipeline = pipeline; }
 
 private:
     void createOverlay();
     void createRenderer();
+    void updateSinkElement();
+
+    QGstPipeline gstPipeline;
+    QGstBin sinkBin;
+    QGstElement gstPreprocess;
+    QGstElement gstVideoSink;
+
     QGstreamerVideoOverlay *m_videoOverlay = nullptr;
     QGstreamerVideoRenderer *m_videoRenderer = nullptr;
     WId m_windowId = 0;