From 8baad13658b1df29d967a4999a58f0cac17387c4 Mon Sep 17 00:00:00 2001 From: Tim Blechmann Date: Tue, 27 Feb 2024 11:03:14 +0800 Subject: 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 (cherry picked from commit bea05730e0bb40c86fe1ddc5ddefa70bdfdf9397) Reviewed-by: Qt Cherry-pick Bot --- .../multimedia/gstreamer/common/qgstpipeline_p.h | 13 ++- .../gstreamer/common/qgstreamermediaplayer.cpp | 38 ++++---- .../gstreamer/common/qgstreamervideooutput.cpp | 38 ++++---- .../gstreamer/common/qgstreamervideosink.cpp | 19 ++-- .../mediacapture/qgstreamermediacapture.cpp | 108 +++++++++++---------- .../mediacapture/qgstreamermediaencoder.cpp | 14 +-- 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 + 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(output); - if (gstAudioOutput) { - gstAudioOutput->setPipeline(playerPipeline); - connectOutput(ts); - } - playerPipeline.endConfig(); + playerPipeline.modifyPipelineWhileNotRunning([&] { + if (gstAudioOutput) { + removeOutput(ts); + gstAudioOutput->setPipeline({}); + } + gstAudioOutput = static_cast(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(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(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(output); - if (gstAudioOutput && gstAudioInput) { - gstPipeline.add(gstAudioOutput->gstElement()); - gstAudioOutput->gstElement().setState(GST_STATE_PLAYING); - linkTeeToPad(gstAudioTee, gstAudioOutput->gstElement().staticPad("sink")); - } + gstAudioOutput = static_cast(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"); -- cgit v1.2.3