Do not leak GstMessage and avoid accidental decrease in reference cnt

getMessage() call creates an object that should be unreferenced after
using it. This patch encapsulate the returned object in
QGstreamerMesssage with proper referance handling.

Calling QGstObject(GST_MESSAGE_SRC(gm)).name() will effectively decrease
the reference counter on gm, as the object is initialized without
increasing reference on it. The patch relaces it with .source().name().

Avoid implicit conversion from raw pointers with reference counting
to c++ objects. This may cause accidental errors.

Pick-to: 6.2
Change-Id: Ica994bb5c15ddb355af4ed24cbc26fd3a9a288bd
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

4 files changed, 24 insertions, 29 deletions

diff --git a/src/multimedia/platform/gstreamer/common/qgst_p.h b/src/multimedia/platform/gstreamer/common/qgst_p.h
index 0130ca00f..7e902c352 100644
--- a/src/multimedia/platform/gstreamer/common/qgst_p.h
+++ b/src/multimedia/platform/gstreamer/common/qgst_p.h
@@ -179,13 +179,6 @@ public:
 
     QGValue operator[](const char *name) const { return gst_structure_get_value(structure, name); }
 
-    GstMessage *getMessage() const
-    {
-        GstMessage *msg;
-        gst_structure_get(structure, "message", GST_TYPE_MESSAGE, &msg, nullptr);
-        return msg;
-    }
-
     Q_MULTIMEDIA_EXPORT QSize resolution() const;
     Q_MULTIMEDIA_EXPORT QVideoFrameFormat::PixelFormat pixelFormat() const;
     Q_MULTIMEDIA_EXPORT QGRange<float> frameRateRange() const;
@@ -290,7 +283,7 @@ public:
     enum RefMode { HasRef, NeedsRef };
 
     QGstObject() = default;
-    QGstObject(GstObject *o, RefMode mode = HasRef)
+    explicit QGstObject(GstObject *o, RefMode mode = HasRef)
         : m_object(o)
     {
         if (o && mode == NeedsRef)
@@ -345,7 +338,7 @@ public:
     qint64 getInt64(const char *property) const { gint64 i = 0; g_object_get(m_object, property, &i, nullptr); return i; }
     float getFloat(const char *property) const { gfloat d = 0; g_object_get(m_object, property, &d, nullptr); return d; }
     double getDouble(const char *property) const { gdouble d = 0; g_object_get(m_object, property, &d, nullptr); return d; }
-    QGstObject getObject(const char *property) const { GstObject *o = nullptr; g_object_get(m_object, property, &o, nullptr); return o; }
+    QGstObject getObject(const char *property) const { GstObject *o = nullptr; g_object_get(m_object, property, &o, nullptr); return QGstObject(o, HasRef); }
 
     void connect(const char *name, GCallback callback, gpointer userData) { g_signal_connect(m_object, name, callback, userData); }
 
diff --git a/src/multimedia/platform/gstreamer/common/qgstreamermessage.cpp b/src/multimedia/platform/gstreamer/common/qgstreamermessage.cpp
index 950cc46cb..02a5dd3bc 100644
--- a/src/multimedia/platform/gstreamer/common/qgstreamermessage.cpp
+++ b/src/multimedia/platform/gstreamer/common/qgstreamermessage.cpp
@@ -60,6 +60,10 @@ QGstreamerMessage::QGstreamerMessage(QGstreamerMessage const& m):
     gst_message_ref(m_message);
 }
 
+QGstreamerMessage::QGstreamerMessage(const QGstStructure &structure)
+{
+    gst_structure_get(structure.structure, "message", GST_TYPE_MESSAGE, &m_message, nullptr);
+}
 
 QGstreamerMessage::~QGstreamerMessage()
 {
diff --git a/src/multimedia/platform/gstreamer/common/qgstreamermessage_p.h b/src/multimedia/platform/gstreamer/common/qgstreamermessage_p.h
index 8d5ff440e..3ba2447d0 100644
--- a/src/multimedia/platform/gstreamer/common/qgstreamermessage_p.h
+++ b/src/multimedia/platform/gstreamer/common/qgstreamermessage_p.h
@@ -63,13 +63,16 @@ class Q_MULTIMEDIA_EXPORT QGstreamerMessage
 {
 public:
     QGstreamerMessage() = default;
-    QGstreamerMessage(GstMessage* message);
     QGstreamerMessage(QGstreamerMessage const& m);
+    explicit QGstreamerMessage(GstMessage* message);
+    explicit QGstreamerMessage(const QGstStructure &structure);
+
     ~QGstreamerMessage();
 
     bool isNull() const { return !m_message; }
     GstMessageType type() const { return GST_MESSAGE_TYPE(m_message); }
     QGstObject source() const { return QGstObject(GST_MESSAGE_SRC(m_message), QGstObject::NeedsRef); }
+    QGstStructure structure() const { return QGstStructure(gst_message_get_structure(m_message)); }
 
     GstMessage* rawMessage() const;
 
diff --git a/src/multimedia/platform/gstreamer/mediacapture/qgstreamermediaencoder.cpp b/src/multimedia/platform/gstreamer/mediacapture/qgstreamermediaencoder.cpp
index c8f2cb2a7..1c82f5128 100644
--- a/src/multimedia/platform/gstreamer/mediacapture/qgstreamermediaencoder.cpp
+++ b/src/multimedia/platform/gstreamer/mediacapture/qgstreamermediaencoder.cpp
@@ -89,9 +89,9 @@ void QGstreamerMediaEncoder::handleSessionError(QMediaRecorder::Error code, cons
 
 bool QGstreamerMediaEncoder::processBusMessage(const QGstreamerMessage &message)
 {
-    GstMessage *gm = message.rawMessage();
-    if (!gm)
+    if (message.isNull())
         return false;
+    auto msg = message;
 
 //    qCDebug(qLcMediaEncoder) << "received event from" << message.source().name() << Qt::hex << message.type();
 //    if (message.type() == GST_MESSAGE_STATE_CHANGED) {
@@ -101,45 +101,40 @@ bool QGstreamerMediaEncoder::processBusMessage(const QGstreamerMessage &message)
 //        gst_message_parse_state_changed(gm, &oldState, &newState, &pending);
 //        qCDebug(qLcMediaEncoder) << "received state change from" << message.source().name() << oldState << newState << pending;
 //    }
-    if (message.type() == GST_MESSAGE_ELEMENT) {
-        QGstStructure s = gst_message_get_structure(gm);
-        qCDebug(qLcMediaEncoder) << "received element message from" << message.source().name() << s.name();
+    if (msg.type() == GST_MESSAGE_ELEMENT) {
+        QGstStructure s = msg.structure();
+        qCDebug(qLcMediaEncoder) << "received element message from" << msg.source().name() << s.name();
         if (s.name() == "GstBinForwarded")
-            gm = s.getMessage();
-        if (!gm)
+            msg = QGstreamerMessage(s);
+        if (msg.isNull())
             return false;
     }
 
-    if (GST_MESSAGE_TYPE(gm) == GST_MESSAGE_EOS) {
-        qCDebug(qLcMediaEncoder) << "received EOS from" << QGstObject(GST_MESSAGE_SRC(gm)).name();
+    if (msg.type() == GST_MESSAGE_EOS) {
+        qCDebug(qLcMediaEncoder) << "received EOS from" << msg.source().name();
         finalize();
         return false;
     }
 
-    if (GST_MESSAGE_TYPE(gm) == GST_MESSAGE_ERROR) {
+    if (msg.type() == GST_MESSAGE_ERROR) {
         GError *err;
         gchar *debug;
-        gst_message_parse_error(gm, &err, &debug);
+        gst_message_parse_error(msg.rawMessage(), &err, &debug);
         error(QMediaRecorder::ResourceError, QString::fromUtf8(err->message));
         g_error_free(err);
         g_free(debug);
         finalize();
     }
 
-    if (GST_MESSAGE_SRC(gm) == gstEncoder.object()) {
-        switch (GST_MESSAGE_TYPE(gm))  {
-        case GST_MESSAGE_STATE_CHANGED: {
+    if (msg.source() == gstEncoder) {
+        if (msg.type() == GST_MESSAGE_STATE_CHANGED) {
             GstState    oldState;
             GstState    newState;
             GstState    pending;
-            gst_message_parse_state_changed(gm, &oldState, &newState, &pending);
+            gst_message_parse_state_changed(msg.rawMessage(), &oldState, &newState, &pending);
 
             if (newState == GST_STATE_PAUSED && !m_metaData.isEmpty())
                 setMetaData(m_metaData);
-            break;
-        }
-        default:
-            break;
         }
     }
     return false;