Reworked synchronous video recording stop

The issue with QtCamGStreamerMessageListener::waitForMessage() is that
gst_bus_timed_pop_filtered() will drop the messages not matching our type. This
means our handlers might not process such messages.

What we do now is pass a custom DoneHandler subclass in QtCamVideoMode which will
wait on a QWaitCondition for video-done to be received.

Since video-done is sent from the streaming thread and DoneHandler is installed
as a sync handler, we are sure to block the UI thread until video gets saved.

diff --git a/lib/qtcamgstreamermessagelistener.cpp b/lib/qtcamgstreamermessagelistener.cpp
index 8f6181f..a0fe714 100644 (file)
--- a/lib/qtcamgstreamermessagelistener.cpp
+++ b/lib/qtcamgstreamermessagelistener.cpp
@@ -179,8 +179,10 @@ GstBusSyncReply sync_handler(GstBus *bus, GstMessage *message, gpointer data) {
     static_cast<QtCamGStreamerMessageListenerPrivate *>(data);
 
   if (d_ptr->handleSyncMessage(message)) {
-    gst_message_unref(message);
-    return GST_BUS_DROP;
+    // We need to pass the message.
+    // Issue is we have 2 video-done handlers, a sync and an async.
+    // If we drop the message then the async handler will never see it :|
+    return GST_BUS_PASS;
   }
 
   return GST_BUS_PASS;
@@ -243,27 +245,3 @@ void QtCamGStreamerMessageListener::flushMessages() {
     gst_message_unref(message);
   }
 }
-
-GstMessage *QtCamGStreamerMessageListener::waitForMessage(const QLatin1String& name) {
-  GstMessage *message = 0;
-
-  while (!message) {
-    message = gst_bus_timed_pop_filtered(d_ptr->bus, GST_CLOCK_TIME_NONE, GST_MESSAGE_ELEMENT);
-    if (!message) {
-      // Hmmm, what to do here??
-      continue;
-    }
-
-    d_ptr->handleMessage(message);
-    const GstStructure *s = gst_message_get_structure(message);
-    const gchar *n = s ? gst_structure_get_name(s) : 0;
-    if (s && n && n == name) {
-      return message;
-    }
-
-    gst_message_unref(message);
-    message = 0;
-  }
-
-  return 0;
-}

diff --git a/lib/qtcamgstreamermessagelistener.h b/lib/qtcamgstreamermessagelistener.h
index 1e938a0..82aeb4a 100644 (file)
--- a/lib/qtcamgstreamermessagelistener.h
+++ b/lib/qtcamgstreamermessagelistener.h
@@ -44,8 +44,6 @@ public:
 
   void flushMessages();
 
-  GstMessage *waitForMessage(const QLatin1String& name);
-
 signals:
   void error(const QString& message, int code, const QString& debug);
   void started();

diff --git a/lib/qtcamimagemode.cpp b/lib/qtcamimagemode.cpp
index 7ff9aca..1b27dc7 100644 (file)
--- a/lib/qtcamimagemode.cpp
+++ b/lib/qtcamimagemode.cpp
@@ -42,7 +42,9 @@ public:
 };
 
 QtCamImageMode::QtCamImageMode(QtCamDevicePrivate *dev, QObject *parent) :
-  QtCamMode(new QtCamImageModePrivate(dev), "mode-image", "image-done", parent) {
+  QtCamMode(new QtCamImageModePrivate(dev), "mode-image", parent) {
+
+  d_ptr->init(new DoneHandler(d_ptr, "image-done", this));
 
   d = (QtCamImageModePrivate *)d_ptr;
 

diff --git a/lib/qtcammode.cpp b/lib/qtcammode.cpp
index d49e5dc..d843eb5 100644 (file)
--- a/lib/qtcammode.cpp
+++ b/lib/qtcammode.cpp
@@ -23,7 +23,6 @@
 #include "qtcamdevice_p.h"
 #include "qtcamdevice.h"
 #include <QDebug>
-#include "qtcamgstreamermessagehandler.h"
 #include "qtcamgstreamermessagelistener.h"
 #include <gst/video/video.h>
 #include <QImage>
@@ -83,47 +82,13 @@ public:
   QtCamMode *mode;
 };
 
-class DoneHandler : public QtCamGStreamerMessageHandler {
-public:
-  DoneHandler(QtCamModePrivate *m, const char *done, QObject *parent = 0) :
-    QtCamGStreamerMessageHandler(done, parent) {
-    mode = m;
-  }
-
-  virtual void handleMessage(GstMessage *message) {
-    // If we have a temp file then we rename it:
-    if (!mode->tempFileName.isEmpty() && !mode->fileName.isEmpty()) {
-      if (!QFile::rename(mode->tempFileName, mode->fileName)) {
-       qCritical() << "Failed to rename" << mode->tempFileName << "to" << mode->fileName;
-      }
-    }
-
-    QString fileName;
-    const GstStructure *s = gst_message_get_structure(message);
-    if (gst_structure_has_field(s, "filename")) {
-      const char *str = gst_structure_get_string(s, "filename");
-      if (str) {
-       fileName = QString::fromUtf8(str);
-      }
-    }
-
-    if (fileName.isEmpty()) {
-      fileName = mode->fileName;
-    }
-
-    QMetaObject::invokeMethod(mode->q_ptr, "saved", Q_ARG(QString, fileName));
-  }
-
-  QtCamModePrivate *mode;
-};
-
-QtCamMode::QtCamMode(QtCamModePrivate *d, const char *mode, const char *done, QObject *parent) :
+QtCamMode::QtCamMode(QtCamModePrivate *d, const char *mode, QObject *parent) :
   QObject(parent), d_ptr(d) {
 
   d_ptr->q_ptr = this;
   d_ptr->id = d_ptr->modeId(mode);
   d_ptr->previewImageHandler = new PreviewImageHandler(this, this);
-  d_ptr->doneHandler = new DoneHandler(d_ptr, done, this);
+  d_ptr->doneHandler = 0;
 }
 
 QtCamMode::~QtCamMode() {
@@ -149,7 +114,10 @@ void QtCamMode::activate() {
   g_object_set(d_ptr->dev->cameraBin, "mode", d_ptr->id, NULL);
 
   d_ptr->dev->listener->addHandler(d_ptr->previewImageHandler);
-  d_ptr->dev->listener->addHandler(d_ptr->doneHandler);
+
+  // This has to be sync. VideoDoneHandler will lock a mutex that is already
+  // locked from the main thread.
+  d_ptr->dev->listener->addSyncHandler(d_ptr->doneHandler);
 
   start();
 
@@ -166,7 +134,7 @@ void QtCamMode::deactivate() {
   }
 
   d_ptr->dev->listener->removeHandler(d_ptr->previewImageHandler);
-  d_ptr->dev->listener->removeHandler(d_ptr->doneHandler);
+  d_ptr->dev->listener->removeSyncHandler(d_ptr->doneHandler);
 
   d_ptr->previewImageHandler->setParent(this);
   d_ptr->doneHandler->setParent(this);

diff --git a/lib/qtcammode.h b/lib/qtcammode.h
index ea4a238..3dfd016 100644 (file)
--- a/lib/qtcammode.h
+++ b/lib/qtcammode.h
@@ -39,7 +39,7 @@ class QtCamMode : public QObject {
   Q_PROPERTY(bool active READ isActive NOTIFY activeChanged);
 
 public:
-  QtCamMode(QtCamModePrivate *d, const char *mode, const char *done, QObject *parent = 0);
+  QtCamMode(QtCamModePrivate *d, const char *mode, QObject *parent = 0);
   virtual ~QtCamMode();
 
   void deactivate();

diff --git a/lib/qtcammode_p.h b/lib/qtcammode_p.h
index 9894de8..0a37ebf 100644 (file)
--- a/lib/qtcammode_p.h
+++ b/lib/qtcammode_p.h
@@ -30,6 +30,7 @@
 #include "qtcamanalysisbin.h"
 #include <gst/pbutils/encoding-profile.h>
 #include <gst/pbutils/encoding-target.h>
+#include "qtcamgstreamermessagehandler.h"
 
 #ifndef GST_USE_UNSTABLE_API
 #define GST_USE_UNSTABLE_API
@@ -47,6 +48,10 @@ public:
   QtCamModePrivate(QtCamDevicePrivate *d) : id(-1), dev(d) {}
   virtual ~QtCamModePrivate() {}
 
+  void init(DoneHandler *handler) {
+    doneHandler = handler;
+  }
+
   int modeId(const char *mode) {
     if (!dev->cameraBin) {
       return -1;
@@ -217,4 +222,41 @@ public:
   QString tempFileName;
 };
 
+class DoneHandler : public QtCamGStreamerMessageHandler {
+public:
+  DoneHandler(QtCamModePrivate *m, const char *done, QObject *parent = 0) :
+    QtCamGStreamerMessageHandler(done, parent) {
+    mode = m;
+  }
+
+  virtual ~DoneHandler() { }
+
+  virtual void handleMessage(GstMessage *message) {
+    // If we have a temp file then we rename it:
+    if (!mode->tempFileName.isEmpty() && !mode->fileName.isEmpty()) {
+      if (!QFile::rename(mode->tempFileName, mode->fileName)) {
+       qCritical() << "Failed to rename" << mode->tempFileName << "to" << mode->fileName;
+      }
+    }
+
+    QString fileName;
+    const GstStructure *s = gst_message_get_structure(message);
+    if (gst_structure_has_field(s, "filename")) {
+      const char *str = gst_structure_get_string(s, "filename");
+      if (str) {
+       fileName = QString::fromUtf8(str);
+      }
+    }
+
+    if (fileName.isEmpty()) {
+      fileName = mode->fileName;
+    }
+
+    QMetaObject::invokeMethod(mode->q_ptr, "saved", Qt::QueuedConnection,
+                             Q_ARG(QString, fileName));
+  }
+
+  QtCamModePrivate *mode;
+};
+
 #endif /* QT_CAM_MODE_P_H */

diff --git a/lib/qtcamvideomode.cpp b/lib/qtcamvideomode.cpp
index bf5d0d6..21feb5a 100644 (file)
--- a/lib/qtcamvideomode.cpp
+++ b/lib/qtcamvideomode.cpp
@@ -25,7 +25,8 @@
 #include "qtcamdevice.h"
 #include "qtcamvideosettings.h"
 #include "qtcamnotifications.h"
-#include "qtcamgstreamermessagelistener.h"
+#include <QMutex>
+#include <QWaitCondition>
 
 class QtCamVideoModePrivate : public QtCamModePrivate {
 public:
@@ -51,8 +52,44 @@ public:
   QtCamVideoResolution resolution;
 };
 
+class VideoDoneHandler : public DoneHandler {
+public:
+  VideoDoneHandler(QtCamModePrivate *d, QObject *parent = 0) :
+    DoneHandler(d, "video-done", parent) {}
+
+  virtual void handleMessage(GstMessage *message) {
+    DoneHandler::handleMessage(message);
+
+    wake();
+  }
+
+  void wake() {
+    lock();
+    m_cond.wakeOne();
+    unlock();
+  }
+
+  void lock() {
+    m_mutex.lock();
+  }
+
+  void unlock() {
+    m_mutex.unlock();
+  }
+
+  void wait() {
+    m_cond.wait(&m_mutex);
+  }
+
+private:
+  QMutex m_mutex;
+  QWaitCondition m_cond;
+};
+
 QtCamVideoMode::QtCamVideoMode(QtCamDevicePrivate *dev, QObject *parent) :
-  QtCamMode(new QtCamVideoModePrivate(dev), "mode-video", "video-done", parent) {
+  QtCamMode(new QtCamVideoModePrivate(dev), "mode-video", parent) {
+
+  d_ptr->init(new VideoDoneHandler(d_ptr, this));
 
   d = (QtCamVideoModePrivate *)QtCamMode::d_ptr;
 
@@ -135,13 +172,16 @@ bool QtCamVideoMode::startRecording(const QString& fileName, const QString& tmpF
 
 void QtCamVideoMode::stopRecording(bool sync) {
   if (isRecording()) {
+    VideoDoneHandler *handler = dynamic_cast<VideoDoneHandler *>(d_ptr->doneHandler);
+    if (sync) {
+      handler->lock();
+    }
+
     g_signal_emit_by_name(d_ptr->dev->cameraBin, "stop-capture", NULL);
 
     if (sync) {
-      GstMessage *msg = d_ptr->dev->listener->waitForMessage(QLatin1String("video-done"));
-      if (msg) {
-       gst_message_unref(msg);
-      }
+      handler->wait();
+      handler->unlock();
     }
   }
 }