Clean up the videoOutput() API

The current API was rather messy, with multiple overloads.
Change this and make the videoOutput a QObject * based property.
To account for QVideoSink, add a videoSink()/setVideoSink()
overload.

This also helps identifying the correct sink in all cases.

Add some code to protect against deletion of the videoOutput
or videoSink from somewhere else.

Adjust autotests and fix the qquickvideooutput autotests.

With this change we are now passing all existing autotests
again on Linux/gstreamer.

Change-Id: I77b9863e88a7863abf27ab465fadd9c24f486502
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

9 files changed, 112 insertions, 108 deletions

diff --git a/src/multimedia/playback/qmediaplayer.cpp b/src/multimedia/playback/qmediaplayer.cpp
index db236093e..02805469b 100644
--- a/src/multimedia/playback/qmediaplayer.cpp
+++ b/src/multimedia/playback/qmediaplayer.cpp
@@ -680,27 +680,7 @@ void QMediaPlayer::setActiveSubtitleTrack(int index)
         d->control->setActiveTrack(QPlatformMediaPlayer::SubtitleStream, index);
 }
 
-/*!
-    Attach a video \a output to the media player.
-
-    If the media player has already video output attached,
-    it will be replaced with a new one.
-*/
-void QMediaPlayer::setVideoOutput(const QVariant &output)
-{
-    QVideoSink *s = output.value<QVideoSink *>();
-    if (s) {
-        setVideoOutput(s);
-        return;
-    }
-    QObject *o = output.value<QObject *>();
-    if (o) {
-        setVideoOutput(o);
-        return;
-    }
-}
-
-QVariant QMediaPlayer::videoOutput() const
+QObject *QMediaPlayer::videoOutput() const
 {
     Q_D(const QMediaPlayer);
     return d->videoOutput;
@@ -717,35 +697,37 @@ void QMediaPlayer::setVideoOutput(QObject *output)
     Q_D(QMediaPlayer);
     if (!d->control)
         return;
+    if (d->videoOutput == output)
+        return;
 
-    QVideoSink *sink = nullptr;
-    if (output) {
+    QVideoSink *sink = qobject_cast<QVideoSink *>(output);
+    if (!sink && output) {
         auto *mo = output->metaObject();
         if (output)
             mo->invokeMethod(output, "videoSink", Q_RETURN_ARG(QVideoSink *, sink));
     }
-    QVariant out = QVariant::fromValue(output);
-    if (d->videoOutput == out)
-        return;
-    d->videoOutput = out;
-    d->control->setVideoSink(sink);
-    emit videoOutputChanged();
+    d->videoOutput = output;
+    d->setVideoSink(sink);
 }
 
-void QMediaPlayer::setVideoOutput(QVideoSink *sink)
+void QMediaPlayer::setVideoSink(QVideoSink *sink)
 {
     Q_D(QMediaPlayer);
     if (!d->control)
         return;
 
-    QVariant out = QVariant::fromValue(sink);
-    if (d->videoOutput == out)
-        return;
-    d->videoOutput = out;
-    d->control->setVideoSink(sink);
-    emit videoOutputChanged();
+    d->videoOutput = nullptr;
+    d->setVideoSink(sink);
 }
 
+QVideoSink *QMediaPlayer::videoSink() const
+{
+    Q_D(const QMediaPlayer);
+    return d->videoSink;
+}
+
+
+#if 0
 /*!
     \since 5.15
     Sets multiple video sinks as the video output of a media player.
@@ -760,6 +742,7 @@ void QMediaPlayer::setVideoOutput(const QList<QVideoSink *> &sinks)
     Q_UNUSED(sinks);
 //    setVideoOutput(!surfaces.empty() ? new QVideoSurfaces(surfaces, this) : nullptr);
 }
+#endif
 
 /*!
     Returns true if the media player is supported on this platform.
diff --git a/src/multimedia/playback/qmediaplayer.h b/src/multimedia/playback/qmediaplayer.h
index 746725ebb..4ee762fed 100644
--- a/src/multimedia/playback/qmediaplayer.h
+++ b/src/multimedia/playback/qmediaplayer.h
@@ -73,7 +73,7 @@ class Q_MULTIMEDIA_EXPORT QMediaPlayer : public QObject
     Q_PROPERTY(QMediaMetaData metaData READ metaData NOTIFY metaDataChanged)
     Q_PROPERTY(Error error READ error NOTIFY errorChanged)
     Q_PROPERTY(QString errorString READ errorString NOTIFY errorChanged)
-    Q_PROPERTY(QVariant videoOutput READ videoOutput WRITE setVideoOutput NOTIFY videoOutputChanged)
+    Q_PROPERTY(QObject *videoOutput READ videoOutput WRITE setVideoOutput NOTIFY videoOutputChanged)
     Q_ENUMS(PlaybackState)
     Q_ENUMS(MediaStatus)
     Q_ENUMS(Error)
@@ -129,12 +129,14 @@ public:
     void setActiveVideoTrack(int index);
     void setActiveSubtitleTrack(int index);
 
-    void setVideoOutput(const QVariant &output);
-    QVariant videoOutput() const;
-
     void setVideoOutput(QObject *);
+    QObject *videoOutput() const;
+#if 0
     void setVideoOutput(const QList<QVideoSink *> &sinks);
-    void setVideoOutput(QVideoSink *sink);
+#endif
+
+    void setVideoSink(QVideoSink *sink);
+    QVideoSink *videoSink() const;
 
     QUrl source() const;
     const QIODevice *sourceStream() const;
diff --git a/src/multimedia/playback/qmediaplayer_p.h b/src/multimedia/playback/qmediaplayer_p.h
index 9c609de28..ee519fb86 100644
--- a/src/multimedia/playback/qmediaplayer_p.h
+++ b/src/multimedia/playback/qmediaplayer_p.h
@@ -53,6 +53,7 @@
 
 #include "qmediaplayer.h"
 #include "qmediametadata.h"
+#include "qvideosink.h"
 #include <private/qplatformmediaplayer_p.h>
 
 #include "private/qobject_p.h"
@@ -76,7 +77,8 @@ public:
     QPlatformMediaPlayer* control = nullptr;
     QString errorString;
 
-    QVariant videoOutput;
+    QVideoSink *videoSink = nullptr;
+    QPointer<QObject> videoOutput;
     QUrl qrcMedia;
     QScopedPointer<QFile> qrcFile;
     QUrl source;
@@ -95,6 +97,20 @@ public:
     void setState(QMediaPlayer::PlaybackState state);
     void setStatus(QMediaPlayer::MediaStatus status);
     void setError(int error, const QString &errorString);
+
+    void setVideoSink(QVideoSink *sink)
+    {
+        Q_Q(QMediaPlayer);
+        if (sink == videoSink)
+            return;
+        if (videoSink)
+            videoSink->setSource(nullptr);
+        videoSink = sink;
+        if (sink)
+            sink->setSource(q);
+        control->setVideoSink(sink);
+        emit q->videoOutputChanged();
+    }
 };
 
 QT_END_NAMESPACE
diff --git a/src/multimedia/recording/qmediacapturesession.cpp b/src/multimedia/recording/qmediacapturesession.cpp
index 727f5bac6..6e9df9c8d 100644
--- a/src/multimedia/recording/qmediacapturesession.cpp
+++ b/src/multimedia/recording/qmediacapturesession.cpp
@@ -44,6 +44,8 @@
 #include "qcameraimagecapture.h"
 #include "qvideosink.h"
 
+#include <qpointer.h>
+
 #include "qplatformmediaintegration_p.h"
 #include "qplatformmediacapture_p.h"
 
@@ -58,28 +60,18 @@ public:
     QCamera *camera = nullptr;
     QCameraImageCapture *imageCapture = nullptr;
     QMediaEncoder *encoder = nullptr;
-    QVariant videoOutput;
     QVideoSink *videoSink = nullptr;
-
-    void _q_sinkDestroyed(QObject *sink)
-    {
-        if (sink == videoSink) {
-            captureSession->setVideoPreview(nullptr);
-            videoOutput = {};
-            videoSink = nullptr;
-        }
-    }
+    QPointer<QObject> videoOutput;
 
     void setVideoSink(QVideoSink *sink)
     {
+        if (sink == videoSink)
+            return;
         if (videoSink)
-            QObject::disconnect(videoSink, SIGNAL(destroyed(QObject *)), q, SLOT(_q_sinkDestroyed(QObject *)));
+            videoSink->setSource(nullptr);
         videoSink = sink;
-        if (videoSink)
-            QObject::connect(videoSink, SIGNAL(destroyed(QObject *)), q, SLOT(_q_sinkDestroyed(QObject *)));
-        else
-            videoOutput = {};
-
+        if (sink)
+            sink->setSource(q);
         captureSession->setVideoPreview(sink);
         emit q->videoOutputChanged();
     }
@@ -286,27 +278,7 @@ void QMediaCaptureSession::setEncoder(QMediaEncoder *encoder)
     emit encoderChanged();
 }
 
-/*!
-    Attach a video \a output to the media player.
-
-    If the media player has already video output attached,
-    it will be replaced with a new one.
-*/
-void QMediaCaptureSession::setVideoOutput(const QVariant &output)
-{
-    QVideoSink *s = output.value<QVideoSink *>();
-    if (s) {
-        setVideoOutput(s);
-        return;
-    }
-    QObject *o = output.value<QObject *>();
-    if (o) {
-        setVideoOutput(o);
-        return;
-    }
-}
-
-QVariant QMediaCaptureSession::videoOutput() const
+QObject *QMediaCaptureSession::videoOutput() const
 {
     Q_D(const QMediaCaptureSession);
     return d->videoOutput;
@@ -323,28 +295,30 @@ QVariant QMediaCaptureSession::videoOutput() const
 void QMediaCaptureSession::setVideoOutput(QObject *output)
 {
     Q_D(QMediaCaptureSession);
-    QVariant out = QVariant::fromValue(output);
-    if (d->videoOutput == out)
+    if (d->videoOutput == output)
         return;
-    d->videoOutput = out;
-    QVideoSink *sink = nullptr;
-    if (output) {
+    QVideoSink *sink = qobject_cast<QVideoSink *>(output);
+    if (!sink && output) {
         auto *mo = output->metaObject();
         mo->invokeMethod(output, "videoSink", Q_RETURN_ARG(QVideoSink *, sink));
     }
+    d->videoOutput = output;
     d->setVideoSink(sink);
 }
 
-void QMediaCaptureSession::setVideoOutput(QVideoSink *sink)
+void QMediaCaptureSession::setVideoSink(QVideoSink *sink)
 {
     Q_D(QMediaCaptureSession);
-    QVariant out = QVariant::fromValue(sink);
-    if (d->videoOutput == out)
-        return;
-    d->videoOutput = out;
+    d->videoOutput = nullptr;
     d->setVideoSink(sink);
 }
 
+QVideoSink *QMediaCaptureSession::videoSink() const
+{
+    Q_D(const QMediaCaptureSession);
+    return d->videoSink;
+}
+
 /*!
     \internal
 */
diff --git a/src/multimedia/recording/qmediacapturesession.h b/src/multimedia/recording/qmediacapturesession.h
index 9b6178579..23701a938 100644
--- a/src/multimedia/recording/qmediacapturesession.h
+++ b/src/multimedia/recording/qmediacapturesession.h
@@ -63,7 +63,7 @@ class Q_MULTIMEDIA_EXPORT QMediaCaptureSession : public QObject
     Q_PROPERTY(QCamera *camera READ camera WRITE setCamera NOTIFY cameraChanged)
     Q_PROPERTY(QCameraImageCapture *imageCapture READ imageCapture WRITE setImageCapture NOTIFY imageCaptureChanged)
     Q_PROPERTY(QMediaEncoder *encoder READ encoder WRITE setEncoder NOTIFY encoderChanged)
-    Q_PROPERTY(QVariant videoOutput READ videoOutput WRITE setVideoOutput NOTIFY videoOutputChanged)
+    Q_PROPERTY(QObject *videoOutput READ videoOutput WRITE setVideoOutput NOTIFY videoOutputChanged)
 public:
     explicit QMediaCaptureSession(QObject *parent = nullptr);
     ~QMediaCaptureSession();
@@ -87,11 +87,11 @@ public:
     QMediaEncoder *encoder();
     void setEncoder(QMediaEncoder *recorder);
 
-    void setVideoOutput(const QVariant &output);
-    QVariant videoOutput() const;
-
     void setVideoOutput(QObject *preview);
-    void setVideoOutput(QVideoSink *preview);
+    QObject *videoOutput() const;
+
+    void setVideoSink(QVideoSink *preview);
+    QVideoSink *videoSink() const;
 
     QPlatformMediaCaptureSession *platformSession() const;
 
@@ -108,7 +108,6 @@ private:
     QMediaCaptureSessionPrivate *d_ptr;
     Q_DISABLE_COPY(QMediaCaptureSession)
     Q_DECLARE_PRIVATE(QMediaCaptureSession)
-    Q_PRIVATE_SLOT(d_func(), void _q_sinkDestroyed(QObject *))
 };
 
 QT_END_NAMESPACE
diff --git a/src/multimedia/recording/qmediarecorder.cpp b/src/multimedia/recording/qmediarecorder.cpp
index 3a2fe5315..72e9b199b 100644
--- a/src/multimedia/recording/qmediarecorder.cpp
+++ b/src/multimedia/recording/qmediarecorder.cpp
@@ -460,12 +460,7 @@ QMediaCaptureSession *QMediaRecorder::captureSession() const
     return d->captureSession;
 }
 
-void QMediaRecorder::setVideoOutput(const QVariant &output)
-{
-    d_ptr->captureSession->setVideoOutput(output);
-}
-
-QVariant QMediaRecorder::videoOutput() const
+QObject *QMediaRecorder::videoOutput() const
 {
     return d_ptr->captureSession->videoOutput();
 }
@@ -475,11 +470,17 @@ void QMediaRecorder::setVideoOutput(QObject *output)
     d_ptr->captureSession->setVideoOutput(output);
 }
 
-void QMediaRecorder::setVideoOutput(QVideoSink *output)
+void QMediaRecorder::setVideoSink(QVideoSink *output)
 {
-    d_ptr->captureSession->setVideoOutput(output);
+    d_ptr->captureSession->setVideoSink(output);
 }
 
+QVideoSink *QMediaRecorder::videoSink() const
+{
+    return d_ptr->captureSession->videoSink();
+}
+
+
 /*!
     \fn QMediaRecorder::videoOutputChanged()
 
diff --git a/src/multimedia/recording/qmediarecorder.h b/src/multimedia/recording/qmediarecorder.h
index b6ea41c36..e887f58c5 100644
--- a/src/multimedia/recording/qmediarecorder.h
+++ b/src/multimedia/recording/qmediarecorder.h
@@ -58,7 +58,7 @@ class Q_MULTIMEDIA_EXPORT QMediaRecorder : public QMediaEncoderBase
     Q_PROPERTY(qreal volume READ volume WRITE setVolume NOTIFY volumeChanged)
     Q_PROPERTY(QMediaMetaData metaData READ metaData WRITE setMetaData NOTIFY metaDataChanged)
     Q_PROPERTY(CaptureMode captureMode READ captureMode WRITE setCaptureMode NOTIFY captureModeChanged)
-    Q_PROPERTY(QVariant videoOutput READ videoOutput WRITE setVideoOutput NOTIFY videoOutputChanged)
+    Q_PROPERTY(QObject *videoOutput READ videoOutput WRITE setVideoOutput NOTIFY videoOutputChanged)
     Q_PROPERTY(QCamera camera READ camera)
 public:
 
@@ -101,11 +101,11 @@ public:
 
     QMediaCaptureSession *captureSession() const;
 
-    void setVideoOutput(const QVariant &output);
-    QVariant videoOutput() const;
-
     void setVideoOutput(QObject *output);
-    void setVideoOutput(QVideoSink *output);
+    QObject *videoOutput() const;
+
+    void setVideoSink(QVideoSink *output);
+    QVideoSink *videoSink() const;
 
 public Q_SLOTS:
     void record();
diff --git a/src/multimedia/video/qvideosink.cpp b/src/multimedia/video/qvideosink.cpp
index 22692ddd7..3776b603b 100644
--- a/src/multimedia/video/qvideosink.cpp
+++ b/src/multimedia/video/qvideosink.cpp
@@ -41,6 +41,8 @@
 
 #include "qvideoframeformat.h"
 #include "qvideoframe.h"
+#include "qmediaplayer.h"
+#include "qmediacapturesession.h"
 
 #include <qvariant.h>
 #include <qpainter.h>
@@ -62,8 +64,21 @@ public:
     {
         delete videoSink;
     }
+    void unregisterSource()
+    {
+        if (!source)
+            return;
+        auto *old = source;
+        source = nullptr;
+        if (auto *player = qobject_cast<QMediaPlayer *>(old))
+            player->setVideoSink(nullptr);
+        else if (auto *capture = qobject_cast<QMediaCaptureSession *>(old))
+            capture->setVideoSink(nullptr);
+    }
+
     QVideoSink *q_ptr = nullptr;
     QPlatformVideoSink *videoSink = nullptr;
+    QObject *source = nullptr;
     bool fullScreen = false;
     WId window = 0;
     QRhi *rhi = nullptr;
@@ -117,6 +132,7 @@ QVideoSink::QVideoSink(QObject *parent)
  */
 QVideoSink::~QVideoSink()
 {
+    d->unregisterSource();
     delete d;
 }
 
@@ -386,6 +402,15 @@ QSize QVideoSink::videoSize() const
     return d->videoSink->nativeSize();
 }
 
+void QVideoSink::setSource(QObject *source)
+{
+    if (d->source == source)
+        return;
+    if (source)
+        d->unregisterSource();
+    d->source = source;
+}
+
 QT_END_NAMESPACE
 
 #include "moc_qvideosink.cpp"
diff --git a/src/multimedia/video/qvideosink.h b/src/multimedia/video/qvideosink.h
index 7802c32ed..93506db37 100644
--- a/src/multimedia/video/qvideosink.h
+++ b/src/multimedia/video/qvideosink.h
@@ -109,6 +109,10 @@ Q_SIGNALS:
     void aspectRatioModeChanged(Qt::AspectRatioMode mode);
 
 private:
+    friend class QMediaPlayerPrivate;
+    friend class QMediaCaptureSessionPrivate;
+    void setSource(QObject *source);
+
     QVideoSinkPrivate *d = nullptr;
 };