qtvideosink: rework the way the buffer format is propagated from set_caps down to the painter

In 0.10 the format was part of the buffer, so every buffer was associated by
its format and we could pass it down to the painter with 1-1 match, meaning
that there was no possibility of interpreting an old buffer according to
the new buffer format.

In 1.0 the format is not part of the buffer, because it's not needed actually
- it rarely changes - but there is still chance of it changing while we are
playing, so we need to make sure that we don't accidentally interpret an
old buffer with a new format. So, instead of setting it in a member of
BaseDelegate from the streaming thread using a mutex, we send it to BaseDelegate
using an event, which will be enqueued in the event loop and therefore it
will be delivered in the same order as the streaming thread calls
show_frame() with a buffer that has the old format, set_caps() and
show_frame() later with a buffer that has the new format. In the previous
implementation, the mutex could not guarantee the order of the calls.

8 files changed, 39 insertions, 51 deletions

diff --git a/elements/gstqtvideosink/delegates/basedelegate.cpp b/elements/gstqtvideosink/delegates/basedelegate.cpp
index 897f4cb..82b465e 100644
--- a/elements/gstqtvideosink/delegates/basedelegate.cpp
+++ b/elements/gstqtvideosink/delegates/basedelegate.cpp
@@ -147,23 +147,6 @@ void BaseDelegate::setForceAspectRatio(bool force)
 
 //-------------------------------------
 
-BufferFormat BaseDelegate::bufferFormat() const
-{
-    QReadLocker l(&m_bufferFormatLock);
-    return m_bufferFormat;
-}
-
-void BaseDelegate::setBufferFormat(const BufferFormat &format)
-{
-    QWriteLocker l(&m_bufferFormatLock);
-    if (m_bufferFormat != format) {
-        m_bufferFormat = format;
-        m_formatDirty = true;
-    }
-}
-
-//-------------------------------------
-
 bool BaseDelegate::event(QEvent *event)
 {
     switch((int) event->type()) {
@@ -176,14 +159,24 @@ bool BaseDelegate::event(QEvent *event)
 
         if (isActive()) {
             gst_buffer_replace (&m_buffer, bufEvent->buffer);
-            if (bufEvent->formatDirty) {
-                m_formatDirty = true;
-            }
             update();
         }
 
         return true;
     }
+    case BufferFormatEventType:
+    {
+        BufferFormatEvent *bufFmtEvent = dynamic_cast<BufferFormatEvent*>(event);
+        Q_ASSERT(bufFmtEvent);
+
+        GST_TRACE_OBJECT (m_sink, "Received buffer format event. New format: %s",
+                          gst_video_format_to_string(bufFmtEvent->format.videoFormat()));
+
+        m_formatDirty = true;
+        m_bufferFormat = bufFmtEvent->format;
+
+        return true;
+    }
     case DeactivateEventType:
     {
         GST_LOG_OBJECT(m_sink, "Received deactivate event");
diff --git a/elements/gstqtvideosink/delegates/basedelegate.h b/elements/gstqtvideosink/delegates/basedelegate.h
index 3b7b6f9..5e177d2 100644
--- a/elements/gstqtvideosink/delegates/basedelegate.h
+++ b/elements/gstqtvideosink/delegates/basedelegate.h
@@ -34,6 +34,7 @@ class BaseDelegate : public QObject
 public:
     enum EventType {
         BufferEventType = QEvent::User,
+        BufferFormatEventType,
         DeactivateEventType
     };
 
@@ -42,10 +43,9 @@ public:
     class BufferEvent : public QEvent
     {
     public:
-        inline BufferEvent(GstBuffer *buf, bool formatDirty)
+        inline BufferEvent(GstBuffer *buf)
             : QEvent(static_cast<QEvent::Type>(BufferEventType)),
-              buffer(gst_buffer_ref(buf)),
-              formatDirty(formatDirty)
+              buffer(gst_buffer_ref(buf))
         {}
 
         virtual ~BufferEvent() {
@@ -53,7 +53,17 @@ public:
         }
 
         GstBuffer *buffer;
-        bool formatDirty;
+    };
+
+    class BufferFormatEvent : public QEvent
+    {
+    public:
+        inline BufferFormatEvent(const BufferFormat &format)
+            : QEvent(static_cast<QEvent::Type>(BufferFormatEventType)),
+            format(format)
+        {}
+
+        BufferFormat format;
     };
 
     class DeactivateEvent : public QEvent
@@ -95,9 +105,6 @@ public:
     bool forceAspectRatio() const;
     void setForceAspectRatio(bool force);
 
-    BufferFormat bufferFormat() const;
-    void setBufferFormat(const BufferFormat &format);
-
 protected:
     // internal event handling
     virtual bool event(QEvent *event);
@@ -125,7 +132,6 @@ protected:
 
     // format caching
     bool m_formatDirty;
-    mutable QReadWriteLock m_bufferFormatLock;
     BufferFormat m_bufferFormat;
     PaintAreas m_areas;
 
diff --git a/elements/gstqtvideosink/delegates/qtquick2videosinkdelegate.cpp b/elements/gstqtvideosink/delegates/qtquick2videosinkdelegate.cpp
index 4b4ae83..8cb261f 100644
--- a/elements/gstqtvideosink/delegates/qtquick2videosinkdelegate.cpp
+++ b/elements/gstqtvideosink/delegates/qtquick2videosinkdelegate.cpp
@@ -44,8 +44,6 @@ QSGNode* QtQuick2VideoSinkDelegate::updateNode(QSGNode *node, const QRectF & tar
             vnode->updateGeometry(m_areas);
         }
     } else {
-        QReadLocker bufferFormatLocker(&m_bufferFormatLock);
-
         //change format before geometry, so that we change QSGGeometry as well
         if (m_formatDirty) {
             vnode->changeFormat(m_bufferFormat);
diff --git a/elements/gstqtvideosink/delegates/qtvideosinkdelegate.cpp b/elements/gstqtvideosink/delegates/qtvideosinkdelegate.cpp
index 302eb0f..1fac7fe 100644
--- a/elements/gstqtvideosink/delegates/qtvideosinkdelegate.cpp
+++ b/elements/gstqtvideosink/delegates/qtvideosinkdelegate.cpp
@@ -54,8 +54,6 @@ void QtVideoSinkDelegate::paint(QPainter *painter, const QRectF & targetArea)
     if (!m_buffer) {
         painter->fillRect(targetArea, Qt::black);
     } else {
-        QReadLocker bufferFormatLocker(&m_bufferFormatLock);
-
         //recalculate the video area if needed
         QReadLocker forceAspectRatioLocker(&m_forceAspectRatioLock);
         if (targetArea != m_areas.targetArea || m_formatDirty
diff --git a/elements/gstqtvideosink/gstqtglvideosinkbase.cpp b/elements/gstqtvideosink/gstqtglvideosinkbase.cpp
index b0c8a1d..6849b13 100644
--- a/elements/gstqtvideosink/gstqtglvideosinkbase.cpp
+++ b/elements/gstqtvideosink/gstqtglvideosinkbase.cpp
@@ -18,6 +18,7 @@
 #include "gstqtglvideosinkbase.h"
 #include "painters/openglsurfacepainter.h"
 #include "delegates/qtvideosinkdelegate.h"
+#include <QCoreApplication>
 
 #define CAPS_FORMATS "{ BGRA, BGRx, ARGB, xRGB, RGB, RGB16, BGR, v308, AYUV, YV12, I420 }"
 
@@ -252,7 +253,8 @@ gboolean GstQtGLVideoSinkBase::set_caps(GstBaseSink *base, GstCaps *caps)
     GST_LOG_OBJECT(sink, "new caps %" GST_PTR_FORMAT, caps);
     BufferFormat format = BufferFormat::fromCaps(caps);
     if (OpenGLSurfacePainter::supportedPixelFormats().contains(format.videoFormat())) {
-        sink->delegate->setBufferFormat(format);
+        QCoreApplication::postEvent(sink->delegate,
+                                    new BaseDelegate::BufferFormatEvent(format));
         return TRUE;
     } else {
         return FALSE;
diff --git a/elements/gstqtvideosink/gstqtquick2videosink.cpp b/elements/gstqtvideosink/gstqtquick2videosink.cpp
index e5e7037..7889fbf 100644
--- a/elements/gstqtvideosink/gstqtquick2videosink.cpp
+++ b/elements/gstqtvideosink/gstqtquick2videosink.cpp
@@ -34,7 +34,6 @@ struct _GstQtQuick2VideoSinkPrivate
 {
     QtQuick2VideoSinkDelegate *delegate;
     GList *channels_list;
-    bool formatDirty;
 };
 
 static void gst_qt_quick2_video_sink_colorbalance_init (GstColorBalanceInterface * iface, gpointer data);
@@ -83,7 +82,6 @@ gst_qt_quick2_video_sink_init (GstQtQuick2VideoSink *self)
 
     // delegate
     self->priv->delegate = new QtQuick2VideoSinkDelegate(GST_ELEMENT(self));
-    self->priv->formatDirty = true;
 
     // colorbalance
     GstColorBalanceChannel *channel;
@@ -237,7 +235,8 @@ gst_qt_quick2_video_sink_set_caps(GstBaseSink *sink, GstCaps *caps)
     //it should conform to the template caps formats, unless gstreamer
     //core has a bug.
     if (format.videoFormat() != GST_VIDEO_FORMAT_UNKNOWN) {
-        self->priv->delegate->setBufferFormat(format);
+        QCoreApplication::postEvent(self->priv->delegate,
+                                    new BaseDelegate::BufferFormatEvent(format));
         return TRUE;
     } else {
         return FALSE;
@@ -249,13 +248,10 @@ gst_qt_quick2_video_sink_show_frame(GstVideoSink *sink, GstBuffer *buffer)
 {
     GstQtQuick2VideoSink *self = GST_QT_QUICK2_VIDEO_SINK (sink);
 
-    GST_TRACE_OBJECT(self, "Posting new buffer (%"GST_PTR_FORMAT") for rendering. "
-                           "Format dirty: %d", buffer, (int)self->priv->formatDirty);
+    GST_TRACE_OBJECT(self, "Posting new buffer (%"GST_PTR_FORMAT") for rendering.", buffer);
 
-    QCoreApplication::postEvent(self->priv->delegate,
-            new BaseDelegate::BufferEvent(buffer, self->priv->formatDirty));
+    QCoreApplication::postEvent(self->priv->delegate, new BaseDelegate::BufferEvent(buffer));
 
-    self->priv->formatDirty = false;
     return GST_FLOW_OK;
 }
 
diff --git a/elements/gstqtvideosink/gstqtvideosinkbase.cpp b/elements/gstqtvideosink/gstqtvideosinkbase.cpp
index 955c973..118d0a2 100644
--- a/elements/gstqtvideosink/gstqtvideosinkbase.cpp
+++ b/elements/gstqtvideosink/gstqtvideosinkbase.cpp
@@ -92,10 +92,9 @@ void GstQtVideoSinkBase::class_init(gpointer g_class, gpointer class_data)
 
 void GstQtVideoSinkBase::init(GTypeInstance *instance, gpointer g_class)
 {
-    GstQtVideoSinkBase *sink = GST_QT_VIDEO_SINK_BASE(instance);
+    Q_UNUSED(instance);
     Q_UNUSED(g_class);
 
-    sink->formatDirty = true;
     /* sink->delegate is initialized in the subclasses */
 }
 
@@ -194,7 +193,8 @@ gboolean GstQtVideoSinkBase::set_caps(GstBaseSink *base, GstCaps *caps)
     GST_LOG_OBJECT(sink, "new caps %" GST_PTR_FORMAT, caps);
     BufferFormat format = BufferFormat::fromCaps(caps);
     if (GenericSurfacePainter::supportedPixelFormats().contains(format.videoFormat())) {
-        sink->delegate->setBufferFormat(format);
+        QCoreApplication::postEvent(sink->delegate,
+                                    new BaseDelegate::BufferFormatEvent(format));
         return TRUE;
     } else {
         return FALSE;
@@ -207,12 +207,9 @@ GstFlowReturn GstQtVideoSinkBase::show_frame(GstVideoSink *video_sink, GstBuffer
 {
     GstQtVideoSinkBase *sink = GST_QT_VIDEO_SINK_BASE(video_sink);
 
-    GST_TRACE_OBJECT(sink, "Posting new buffer (%"GST_PTR_FORMAT") for rendering. "
-                           "Format dirty: %d", buffer, (int)sink->formatDirty);
+    GST_TRACE_OBJECT(sink, "Posting new buffer (%"GST_PTR_FORMAT") for rendering.", buffer);
 
-    QCoreApplication::postEvent(sink->delegate,
-            new QtVideoSinkDelegate::BufferEvent(buffer, sink->formatDirty));
+    QCoreApplication::postEvent(sink->delegate, new BaseDelegate::BufferEvent(buffer));
 
-    sink->formatDirty = false;
     return GST_FLOW_OK;
 }
diff --git a/elements/gstqtvideosink/gstqtvideosinkbase.h b/elements/gstqtvideosink/gstqtvideosinkbase.h
index 75c5d40..c5cdfdb 100644
--- a/elements/gstqtvideosink/gstqtvideosinkbase.h
+++ b/elements/gstqtvideosink/gstqtvideosinkbase.h
@@ -66,8 +66,6 @@ public:
     QtVideoSinkDelegate *delegate;
 
 private:
-    bool formatDirty;
-
     static GstVideoSinkClass *s_parent_class;
 };