sys/winks/gstksvideodevice.c (GST_DEBUG_IS_ENABLED, last_timestamp, gst_ks_video_device_prepare_buffers, gst_ks_video...

Original commit message from CVS:
* sys/winks/gstksvideodevice.c (GST_DEBUG_IS_ENABLED, last_timestamp,
gst_ks_video_device_prepare_buffers, gst_ks_video_device_create_pin,
gst_ks_video_device_set_state, gst_ks_video_device_request_frame,
gst_ks_video_device_read_frame):
Guard against capturing old frames by keeping track of the last
timestamp and also zero-fill the buffers before each capture.
Only assign a master clock if the pin hasn't already got one.
Actually free buffers on the way down to avoid a huge memory leak,
as this was previously done when changing state to ACQUIRE downwards
and we now skip that state on the way down.
Add some debug.
* sys/winks/gstksvideosrc.c (DEFAULT_DEVICE_PATH, DEFAULT_DEVICE_NAME,
DEFAULT_DEVICE_INDEX, KS_WORKER_LOCK, KS_WORKER_UNLOCK,
KS_WORKER_WAIT, KS_WORKER_NOTIFY, KS_WORKER_WAIT_FOR_RESULT,
KS_WORKER_NOTIFY_RESULT, KS_WORKER_STATE_STARTING,
KS_WORKER_STATE_READY, KS_WORKER_STATE_STOPPING,
KS_WORKER_STATE_ERROR, KsWorkerState, device_path, device_name,
device_index, running, worker_thread, worker_lock,
worker_notify_cond, worker_result_cond, worker_state,
worker_pending_caps, worker_setcaps_result, worker_pending_run,
worker_run_result, gst_ks_video_src_reset,
gst_ks_video_src_apply_driver_quirks, gst_ks_video_src_open_device,
gst_ks_video_src_close_device, gst_ks_video_src_worker_func,
gst_ks_video_src_start_worker, gst_ks_video_src_stop_worker,
gst_ks_video_src_change_state, gst_ks_video_src_set_clock,
gst_ks_video_src_set_caps, gst_ks_video_src_timestamp_buffer,
gst_ks_video_src_create):
Remove ENABLE_CLOCK_DEBUG define, it's GST_LEVEL_DEBUG after all.
Get rid of PROP_ENSLAVE_KSCLOCK and always slave the ks clock to the
GStreamer clock, it doesn't seem to hurt and matches DirectShow's
behavior. As an added bonus we usually get PresentationTime set for
each frame, so we can expand on this later for smarter latency
reporting (by looking at the diff between the timestamp from the
driver and the time according to the GStreamer clock).
Use an internal worker thread for opening the device, setting caps,
changing its state and closing it. This way we're a lot more
compatible with drivers that rely on hacks to do video-effects
between the low-level NT API and the application. Ick.
Start the ks clock and set the pin to KSSTATE_RUN on the first
create() so that we'll hopefully get hold of the GStreamer clock
from the very beginning. This way there's no chance that the
timestamps will make a sudden jump in the beginning of the stream
when we're running with a clock.
* sys/winks/kshelpers.c (CHECK_OPTIONS_FLAG,
ks_options_flags_to_string):
Reorder the flags to match the headerfile order, and make the string
a bit more compact.
* sys/winks/ksvideohelpers.c (ks_video_probe_filter_for_caps):
Avoid leaking KSPROPERTY_PIN_DATARANGES.

5 files changed, 375 insertions, 111 deletions

diff --git a/ChangeLog b/ChangeLog
index d6180cb4..6c2d5391 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,55 @@
+2008-09-10  Ole André Vadla Ravnås  <ole.andre.ravnas@tandberg.com>
+
+	* sys/winks/gstksvideodevice.c (GST_DEBUG_IS_ENABLED, last_timestamp,
+	  gst_ks_video_device_prepare_buffers, gst_ks_video_device_create_pin,
+	  gst_ks_video_device_set_state, gst_ks_video_device_request_frame,
+	  gst_ks_video_device_read_frame):
+	  Guard against capturing old frames by keeping track of the last
+	  timestamp and also zero-fill the buffers before each capture.
+	  Only assign a master clock if the pin hasn't already got one.
+	  Actually free buffers on the way down to avoid a huge memory leak,
+	  as this was previously done when changing state to ACQUIRE downwards
+	  and we now skip that state on the way down.
+	  Add some debug.
+	* sys/winks/gstksvideosrc.c (DEFAULT_DEVICE_PATH, DEFAULT_DEVICE_NAME,
+	  DEFAULT_DEVICE_INDEX, KS_WORKER_LOCK, KS_WORKER_UNLOCK,
+	  KS_WORKER_WAIT, KS_WORKER_NOTIFY, KS_WORKER_WAIT_FOR_RESULT,
+	  KS_WORKER_NOTIFY_RESULT, KS_WORKER_STATE_STARTING,
+	  KS_WORKER_STATE_READY, KS_WORKER_STATE_STOPPING,
+	  KS_WORKER_STATE_ERROR, KsWorkerState, device_path, device_name,
+	  device_index, running, worker_thread, worker_lock,
+	  worker_notify_cond, worker_result_cond, worker_state,
+	  worker_pending_caps, worker_setcaps_result, worker_pending_run,
+	  worker_run_result, gst_ks_video_src_reset,
+	  gst_ks_video_src_apply_driver_quirks, gst_ks_video_src_open_device,
+	  gst_ks_video_src_close_device, gst_ks_video_src_worker_func,
+	  gst_ks_video_src_start_worker, gst_ks_video_src_stop_worker,
+	  gst_ks_video_src_change_state, gst_ks_video_src_set_clock,
+	  gst_ks_video_src_set_caps, gst_ks_video_src_timestamp_buffer,
+	  gst_ks_video_src_create):
+	  Remove ENABLE_CLOCK_DEBUG define, it's GST_LEVEL_DEBUG after all.
+	  Get rid of PROP_ENSLAVE_KSCLOCK and always slave the ks clock to the
+	  GStreamer clock, it doesn't seem to hurt and matches DirectShow's
+	  behavior. As an added bonus we usually get PresentationTime set for
+	  each frame, so we can expand on this later for smarter latency
+	  reporting (by looking at the diff between the timestamp from the
+	  driver and the time according to the GStreamer clock).
+	  Use an internal worker thread for opening the device, setting caps,
+	  changing its state and closing it. This way we're a lot more
+	  compatible with drivers that rely on hacks to do video-effects
+	  between the low-level NT API and the application. Ick.
+	  Start the ks clock and set the pin to KSSTATE_RUN on the first
+	  create() so that we'll hopefully get hold of the GStreamer clock
+	  from the very beginning. This way there's no chance that the
+	  timestamps will make a sudden jump in the beginning of the stream
+	  when we're running with a clock.
+	* sys/winks/kshelpers.c (CHECK_OPTIONS_FLAG,
+	  ks_options_flags_to_string):
+	  Reorder the flags to match the headerfile order, and make the string
+	  a bit more compact.
+	* sys/winks/ksvideohelpers.c (ks_video_probe_filter_for_caps):
+	  Avoid leaking KSPROPERTY_PIN_DATARANGES.
+
 2008-09-09  Mark Nauwelaerts  <mark.nauwelaerts@collabora.co.uk>
 
 	* configure.ac:
diff --git a/sys/winks/gstksvideodevice.c b/sys/winks/gstksvideodevice.c
index cb094831..db04d521 100644
--- a/sys/winks/gstksvideodevice.c
+++ b/sys/winks/gstksvideodevice.c
@@ -33,6 +33,9 @@
 GST_DEBUG_CATEGORY_EXTERN (gst_ks_debug);
 #define GST_CAT_DEFAULT gst_ks_debug
 
+#define GST_DEBUG_IS_ENABLED() \
+    (gst_debug_category_get_threshold (GST_CAT_DEFAULT) >= GST_LEVEL_DEBUG)
+
 enum
 {
   PROP_0,
@@ -81,6 +84,7 @@ typedef struct
   gulong num_requests;
   GArray *requests;
   GArray *request_events;
+  GstClockTime last_timestamp;
 } GstKsVideoDevicePrivate;
 
 #define GST_KS_VIDEO_DEVICE_GET_PRIVATE(o) \
@@ -301,6 +305,13 @@ gst_ks_video_device_prepare_buffers (GstKsVideoDevice * self)
   }
 
   g_array_append_val (priv->request_events, priv->cancel_event);
+
+  /*
+   * REVISIT: Could probably remove this later, for now it's here to help
+   *          track down the case where we capture old frames. This has been
+   *          observed with UVC cameras, presumably with some system load.
+   */
+  priv->last_timestamp = GST_CLOCK_TIME_NONE;
 }
 
 static void
@@ -558,17 +569,28 @@ gst_ks_video_device_create_pin (GstKsVideoDevice * self,
   }
 
   /*
-   * Override the clock if we have one.
+   * Override the clock if we have one and the pin doesn't have any either.
    */
   if (priv->clock != NULL) {
-    HANDLE clock_handle = gst_ks_clock_get_handle (priv->clock);
-
-    if (ks_object_set_property (pin_handle, KSPROPSETID_Stream,
-            KSPROPERTY_STREAM_MASTERCLOCK, &clock_handle,
-            sizeof (clock_handle))) {
-      gst_ks_clock_prepare (priv->clock);
+    HANDLE *cur_clock_handle = NULL;
+    gulong cur_clock_handle_size = sizeof (HANDLE);
+
+    if (ks_object_get_property (pin_handle, KSPROPSETID_Stream,
+            KSPROPERTY_STREAM_MASTERCLOCK, (gpointer *) & cur_clock_handle,
+            &cur_clock_handle_size)) {
+      GST_DEBUG ("current master clock handle: 0x%08x", *cur_clock_handle);
+      CloseHandle (*cur_clock_handle);
+      g_free (cur_clock_handle);
     } else {
-      GST_WARNING ("failed to set pin's master clock");
+      HANDLE new_clock_handle = gst_ks_clock_get_handle (priv->clock);
+
+      if (ks_object_set_property (pin_handle, KSPROPSETID_Stream,
+              KSPROPERTY_STREAM_MASTERCLOCK, &new_clock_handle,
+              sizeof (new_clock_handle))) {
+        gst_ks_clock_prepare (priv->clock);
+      } else {
+        GST_WARNING ("failed to set pin's master clock");
+      }
     }
   }
 
@@ -779,7 +801,7 @@ gst_ks_video_device_set_state (GstKsVideoDevice * self, KSSTATE state)
 
       if (priv->state == KSSTATE_PAUSE && addend > 0)
         gst_ks_video_device_prepare_buffers (self);
-      else if (priv->state == KSSTATE_ACQUIRE && addend < 0)
+      else if (priv->state == KSSTATE_STOP && addend < 0)
         gst_ks_video_device_clear_buffers (self);
     } else {
       GST_WARNING ("Failed to change pin state to %s",
@@ -856,6 +878,15 @@ gst_ks_video_device_request_frame (GstKsVideoDevice * self, ReadRequest * req,
   params->header.Data = req->buf;
   params->frame_info.ExtendedHeaderSize = sizeof (KS_FRAME_INFO);
 
+  /*
+   * Clear the buffer like DirectShow does
+   *
+   * REVISIT: Could probably remove this later, for now it's here to help
+   *          track down the case where we capture old frames. This has been
+   *          observed with UVC cameras, presumably with some system load.
+   */
+  memset (params->header.Data, 0, params->header.FrameExtent);
+
   success = DeviceIoControl (priv->pin_handle, IOCTL_KS_READ_STREAM, NULL, 0,
       params, params->header.Size, &bytes_returned, &req->overlapped);
   if (!success && GetLastError () != ERROR_IO_PENDING)
@@ -933,46 +964,89 @@ gst_ks_video_device_read_frame (GstKsVideoDevice * self, guint8 * buf,
       ResetEvent (req->overlapped.hEvent);
 
       if (success) {
-        /* Grab the frame data */
-        g_assert (buf_size >= req->params.header.DataUsed);
-        memcpy (buf, req->buf, req->params.header.DataUsed);
-        *bytes_read = req->params.header.DataUsed;
-        if (req->params.header.PresentationTime.Time != 0)
-          *presentation_time = req->params.header.PresentationTime.Time * 100;
-        else
-          *presentation_time = GST_CLOCK_TIME_NONE;
-
-        if (priv->is_mjpeg) {
-          /*
-           * Workaround for cameras/drivers that intermittently provide us with
-           * incomplete or corrupted MJPEG frames.
-           *
-           * Happens with for instance Microsoft LifeCam VX-7000.
-           */
-
-          gboolean valid = FALSE;
-          guint padding = 0;
-
-          /* JFIF SOI marker */
-          if (*bytes_read > MJPEG_MAX_PADDING
-              && buf[0] == 0xff && buf[1] == 0xd8) {
-            guint8 *p = buf + *bytes_read - 2;
-
-            /* JFIF EOI marker (but skip any padding) */
-            while (padding < MJPEG_MAX_PADDING - 1 - 2 && !valid) {
-              if (p[0] == 0xff && p[1] == 0xd9) {
-                valid = TRUE;
-              } else {
-                padding++;
-                p--;
+        KSSTREAM_HEADER *hdr = &req->params.header;
+        KS_FRAME_INFO *frame_info = &req->params.frame_info;
+        GstClockTime timestamp = GST_CLOCK_TIME_NONE;
+        GstClockTime duration = GST_CLOCK_TIME_NONE;
+
+        if (hdr->OptionsFlags & KSSTREAM_HEADER_OPTIONSF_TIMEVALID)
+          timestamp = hdr->PresentationTime.Time * 100;
+
+        if (hdr->OptionsFlags & KSSTREAM_HEADER_OPTIONSF_DURATIONVALID)
+          duration = hdr->Duration * 100;
+
+        /* Assume it's a good frame */
+        *bytes_read = hdr->DataUsed;
+
+        if (G_LIKELY (presentation_time != NULL))
+          *presentation_time = timestamp;
+
+        if (G_UNLIKELY (GST_DEBUG_IS_ENABLED ())) {
+          gchar *options_flags_str =
+              ks_options_flags_to_string (hdr->OptionsFlags);
+
+          GST_DEBUG ("PictureNumber=%" G_GUINT64_FORMAT ", DropCount=%"
+              G_GUINT64_FORMAT ", PresentationTime=%" GST_TIME_FORMAT
+              ", Duration=%" GST_TIME_FORMAT ", OptionsFlags=%s: %d bytes",
+              frame_info->PictureNumber, frame_info->DropCount,
+              GST_TIME_ARGS (timestamp), GST_TIME_ARGS (duration),
+              options_flags_str, hdr->DataUsed);
+
+          g_free (options_flags_str);
+        }
+
+        /* Protect against old frames. This should never happen, see previous
+         * comment on last_timestamp. */
+        if (G_LIKELY (GST_CLOCK_TIME_IS_VALID (timestamp))) {
+          if (G_UNLIKELY (GST_CLOCK_TIME_IS_VALID (priv->last_timestamp) &&
+                  timestamp < priv->last_timestamp)) {
+            GST_WARNING ("got an old frame (last_timestamp=%" GST_TIME_FORMAT
+                ", timestamp=%" GST_TIME_FORMAT ")",
+                GST_TIME_ARGS (priv->last_timestamp),
+                GST_TIME_ARGS (timestamp));
+            *bytes_read = 0;
+          } else {
+            priv->last_timestamp = timestamp;
+          }
+        }
+
+        if (*bytes_read > 0) {
+          /* Grab the frame data */
+          g_assert (buf_size >= hdr->DataUsed);
+          memcpy (buf, req->buf, hdr->DataUsed);
+
+          if (priv->is_mjpeg) {
+            /*
+             * Workaround for cameras/drivers that intermittently provide us
+             * with incomplete or corrupted MJPEG frames.
+             *
+             * Happens with for instance Microsoft LifeCam VX-7000.
+             */
+
+            gboolean valid = FALSE;
+            guint padding = 0;
+
+            /* JFIF SOI marker */
+            if (*bytes_read > MJPEG_MAX_PADDING
+                && buf[0] == 0xff && buf[1] == 0xd8) {
+              guint8 *p = buf + *bytes_read - 2;
+
+              /* JFIF EOI marker (but skip any padding) */
+              while (padding < MJPEG_MAX_PADDING - 1 - 2 && !valid) {
+                if (p[0] == 0xff && p[1] == 0xd9) {
+                  valid = TRUE;
+                } else {
+                  padding++;
+                  p--;
+                }
               }
             }
-          }
 
-          if (valid)
-            *bytes_read -= padding;
-          else
-            *bytes_read = 0;
+            if (valid)
+              *bytes_read -= padding;
+            else
+              *bytes_read = 0;
+          }
         }
       } else if (GetLastError () != ERROR_OPERATION_ABORTED)
         goto error_get_result;
diff --git a/sys/winks/gstksvideosrc.c b/sys/winks/gstksvideosrc.c
index ef1b06fe..9e8560ce 100644
--- a/sys/winks/gstksvideosrc.c
+++ b/sys/winks/gstksvideosrc.c
@@ -45,12 +45,9 @@
 #include "kshelpers.h"
 #include "ksvideohelpers.h"
 
-#define ENABLE_CLOCK_DEBUG 0
-
 #define DEFAULT_DEVICE_PATH     NULL
 #define DEFAULT_DEVICE_NAME     NULL
 #define DEFAULT_DEVICE_INDEX    -1
-#define DEFAULT_ENSLAVE_KSCLOCK FALSE
 #define DEFAULT_DO_STATS        FALSE
 #define DEFAULT_ENABLE_QUIRKS   TRUE
 
@@ -60,7 +57,6 @@ enum
   PROP_DEVICE_PATH,
   PROP_DEVICE_NAME,
   PROP_DEVICE_INDEX,
-  PROP_ENSLAVE_KSCLOCK,
   PROP_DO_STATS,
   PROP_FPS,
   PROP_ENABLE_QUIRKS,
@@ -69,13 +65,30 @@ enum
 GST_DEBUG_CATEGORY (gst_ks_debug);
 #define GST_CAT_DEFAULT gst_ks_debug
 
+#define KS_WORKER_LOCK(priv) g_mutex_lock (priv->worker_lock)
+#define KS_WORKER_UNLOCK(priv) g_mutex_unlock (priv->worker_lock)
+#define KS_WORKER_WAIT(priv) \
+    g_cond_wait (priv->worker_notify_cond, priv->worker_lock)
+#define KS_WORKER_NOTIFY(priv) g_cond_signal (priv->worker_notify_cond)
+#define KS_WORKER_WAIT_FOR_RESULT(priv) \
+    g_cond_wait (priv->worker_result_cond, priv->worker_lock)
+#define KS_WORKER_NOTIFY_RESULT(priv) \
+    g_cond_signal (priv->worker_result_cond)
+
+typedef enum
+{
+  KS_WORKER_STATE_STARTING,
+  KS_WORKER_STATE_READY,
+  KS_WORKER_STATE_STOPPING,
+  KS_WORKER_STATE_ERROR,
+} KsWorkerState;
+
 typedef struct
 {
   /* Properties */
   gchar *device_path;
   gchar *device_name;
   gint device_index;
-  gboolean enslave_ksclock;
   gboolean do_stats;
   gboolean enable_quirks;
 
@@ -85,6 +98,20 @@ typedef struct
 
   guint64 offset;
   GstClockTime prev_ts;
+  gboolean running;
+
+  /* Worker thread */
+  GThread *worker_thread;
+  GMutex *worker_lock;
+  GCond *worker_notify_cond;
+  GCond *worker_result_cond;
+  KsWorkerState worker_state;
+
+  GstCaps *worker_pending_caps;
+  gboolean worker_setcaps_result;
+
+  gboolean worker_pending_run;
+  gboolean worker_run_result;
 
   /* Statistics */
   GstClockTime last_sampling;
@@ -182,10 +209,6 @@ gst_ks_video_src_class_init (GstKsVideoSrcClass * klass)
       g_param_spec_int ("device-index", "Device Index",
           "The zero-based device index", -1, G_MAXINT, DEFAULT_DEVICE_INDEX,
           G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS));
-  g_object_class_install_property (gobject_class, PROP_ENSLAVE_KSCLOCK,
-      g_param_spec_boolean ("enslave-ksclock", "Enslave the clock used by KS",
-          "Enslave the clocked used by Kernel Streaming",
-          DEFAULT_ENSLAVE_KSCLOCK, G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS));
   g_object_class_install_property (gobject_class, PROP_DO_STATS,
       g_param_spec_boolean ("do-stats", "Enable statistics",
           "Enable logging of statistics", DEFAULT_DO_STATS,
@@ -217,7 +240,6 @@ gst_ks_video_src_init (GstKsVideoSrc * self, GstKsVideoSrcClass * gclass)
   priv->device_path = DEFAULT_DEVICE_PATH;
   priv->device_name = DEFAULT_DEVICE_NAME;
   priv->device_index = DEFAULT_DEVICE_INDEX;
-  priv->enslave_ksclock = DEFAULT_ENSLAVE_KSCLOCK;
   priv->do_stats = DEFAULT_DO_STATS;
   priv->enable_quirks = DEFAULT_ENABLE_QUIRKS;
 }
@@ -251,9 +273,6 @@ gst_ks_video_src_get_property (GObject * object, guint prop_id,
     case PROP_DEVICE_INDEX:
       g_value_set_int (value, priv->device_index);
       break;
-    case PROP_ENSLAVE_KSCLOCK:
-      g_value_set_boolean (value, priv->enslave_ksclock);
-      break;
     case PROP_DO_STATS:
       GST_OBJECT_LOCK (object);
       g_value_set_boolean (value, priv->do_stats);
@@ -292,14 +311,6 @@ gst_ks_video_src_set_property (GObject * object, guint prop_id,
     case PROP_DEVICE_INDEX:
       priv->device_index = g_value_get_int (value);
       break;
-    case PROP_ENSLAVE_KSCLOCK:
-      GST_OBJECT_LOCK (object);
-      if (priv->device == NULL)
-        priv->enslave_ksclock = g_value_get_boolean (value);
-      else
-        g_warning ("enslave-ksclock may only be changed while in NULL state");
-      GST_OBJECT_UNLOCK (object);
-      break;
     case PROP_DO_STATS:
       GST_OBJECT_LOCK (object);
       priv->do_stats = g_value_get_boolean (value);
@@ -327,6 +338,8 @@ gst_ks_video_src_reset (GstKsVideoSrc * self)
   /* Reset timestamping state */
   priv->offset = 0;
   priv->prev_ts = GST_CLOCK_TIME_NONE;
+
+  priv->running = FALSE;
 }
 
 static void
@@ -337,21 +350,17 @@ gst_ks_video_src_apply_driver_quirks (GstKsVideoSrc * self)
 
   /*
    * Logitech's driver software injects the following DLL into all processes
-   * spawned. This DLL does lots of nasty tricks, sitting in between the
+   * spawned. This DLL does some nasty tricks, sitting in between the
    * application and the low-level ntdll API (NtCreateFile, NtClose,
    * NtDeviceIoControlFile, NtDuplicateObject, etc.), making all sorts
-   * of assumptions on which application threads do what.
-   *
-   * We could later work around this by having a worker-thread open the
-   * device, take care of doing set_caps() when asked to, closing the device
-   * when shutting down, and so forth.
+   * of assumptions.
    *
    * The only regression that this quirk causes is that the video effects
    * feature doesn't work.
    */
   mod = GetModuleHandle ("LVPrcInj.dll");
   if (mod != NULL) {
-    GST_DEBUG_OBJECT (self, "hostile Logitech DLL detected, neutralizing it");
+    GST_DEBUG_OBJECT (self, "Logitech DLL detected, neutralizing it");
 
     /*
      * We know that no-one's actually keeping this handle around to decrement
@@ -365,7 +374,7 @@ gst_ks_video_src_apply_driver_quirks (GstKsVideoSrc * self)
     /* Paranoia: verify that it's no longer there */
     mod = GetModuleHandle ("LVPrcInj.dll");
     if (mod != NULL)
-      GST_WARNING_OBJECT (self, "failed to neutralize hostile Logitech DLL");
+      GST_WARNING_OBJECT (self, "failed to neutralize Logitech DLL");
   }
 }
 
@@ -404,19 +413,15 @@ gst_ks_video_src_open_device (GstKsVideoSrc * self)
     }
 
     if (match) {
-      priv->ksclock = NULL;
-
-      if (priv->enslave_ksclock) {
-        priv->ksclock = g_object_new (GST_TYPE_KS_CLOCK, NULL);
-        if (priv->ksclock != NULL && !gst_ks_clock_open (priv->ksclock)) {
-          g_object_unref (priv->ksclock);
-          priv->ksclock = NULL;
-        }
-
-        if (priv->ksclock == NULL)
-          GST_WARNING_OBJECT (self, "Failed to create/open KsClock");
+      priv->ksclock = g_object_new (GST_TYPE_KS_CLOCK, NULL);
+      if (priv->ksclock != NULL && !gst_ks_clock_open (priv->ksclock)) {
+        g_object_unref (priv->ksclock);
+        priv->ksclock = NULL;
       }
 
+      if (priv->ksclock == NULL)
+        GST_WARNING_OBJECT (self, "Failed to create/open KsClock");
+
       device = g_object_new (GST_TYPE_KS_VIDEO_DEVICE,
           "clock", priv->ksclock, "device-path", entry->path, NULL);
     }
@@ -489,6 +494,114 @@ gst_ks_video_src_close_device (GstKsVideoSrc * self)
   gst_ks_video_src_reset (self);
 }
 
+/*
+ * Worker thread that takes care of starting, configuring and stopping things.
+ *
+ * This is needed because Logitech's driver software injects a DLL that
+ * intercepts API functions like NtCreateFile, NtClose, NtDeviceIoControlFile
+ * and NtDuplicateObject so that they can provide in-place video effects to
+ * existing applications. Their assumption is that at least one thread tainted
+ * by their code stays around for the lifetime of the capture.
+ */
+static gpointer
+gst_ks_video_src_worker_func (gpointer data)
+{
+  GstKsVideoSrc *self = data;
+  GstKsVideoSrcPrivate *priv = GST_KS_VIDEO_SRC_GET_PRIVATE (self);
+
+  if (!gst_ks_video_src_open_device (self))
+    goto open_failed;
+
+  KS_WORKER_LOCK (priv);
+  priv->worker_state = KS_WORKER_STATE_READY;
+  KS_WORKER_NOTIFY_RESULT (priv);
+
+  while (priv->worker_state != KS_WORKER_STATE_STOPPING) {
+    KS_WORKER_WAIT (priv);
+
+    if (priv->worker_pending_caps != NULL) {
+      priv->worker_setcaps_result =
+          gst_ks_video_device_set_caps (priv->device,
+          priv->worker_pending_caps);
+
+      priv->worker_pending_caps = NULL;
+      KS_WORKER_NOTIFY_RESULT (priv);
+    } else if (priv->worker_pending_run) {
+      if (priv->ksclock != NULL)
+        gst_ks_clock_start (priv->ksclock);
+      priv->worker_run_result =
+          gst_ks_video_device_set_state (priv->device, KSSTATE_RUN);
+
+      priv->worker_pending_run = FALSE;
+      KS_WORKER_NOTIFY_RESULT (priv);
+    }
+  }
+
+  KS_WORKER_UNLOCK (priv);
+
+  gst_ks_video_src_close_device (self);
+
+  return NULL;
+
+  /* ERRORS */
+open_failed:
+  {
+    KS_WORKER_LOCK (priv);
+    priv->worker_state = KS_WORKER_STATE_ERROR;
+    KS_WORKER_NOTIFY_RESULT (priv);
+    KS_WORKER_UNLOCK (priv);
+
+    return NULL;
+  }
+}
+
+static gboolean
+gst_ks_video_src_start_worker (GstKsVideoSrc * self)
+{
+  GstKsVideoSrcPrivate *priv = GST_KS_VIDEO_SRC_GET_PRIVATE (self);
+  gboolean result;
+
+  priv->worker_lock = g_mutex_new ();
+  priv->worker_notify_cond = g_cond_new ();
+  priv->worker_result_cond = g_cond_new ();
+
+  priv->worker_pending_caps = NULL;
+  priv->worker_pending_run = FALSE;
+
+  priv->worker_state = KS_WORKER_STATE_STARTING;
+  priv->worker_thread =
+      g_thread_create (gst_ks_video_src_worker_func, self, TRUE, NULL);
+
+  KS_WORKER_LOCK (priv);
+  while (priv->worker_state < KS_WORKER_STATE_READY)
+    KS_WORKER_WAIT_FOR_RESULT (priv);
+  result = priv->worker_state == KS_WORKER_STATE_READY;
+  KS_WORKER_UNLOCK (priv);
+
+  return result;
+}
+
+static void
+gst_ks_video_src_stop_worker (GstKsVideoSrc * self)
+{
+  GstKsVideoSrcPrivate *priv = GST_KS_VIDEO_SRC_GET_PRIVATE (self);
+
+  KS_WORKER_LOCK (priv);
+  priv->worker_state = KS_WORKER_STATE_STOPPING;
+  KS_WORKER_NOTIFY (priv);
+  KS_WORKER_UNLOCK (priv);
+
+  g_thread_join (priv->worker_thread);
+  priv->worker_thread = NULL;
+
+  g_cond_free (priv->worker_result_cond);
+  priv->worker_result_cond = NULL;
+  g_cond_free (priv->worker_notify_cond);
+  priv->worker_notify_cond = NULL;
+  g_mutex_free (priv->worker_lock);
+  priv->worker_lock = NULL;
+}
+
 static GstStateChangeReturn
 gst_ks_video_src_change_state (GstElement * element, GstStateChange transition)
 {
@@ -500,7 +613,7 @@ gst_ks_video_src_change_state (GstElement * element, GstStateChange transition)
     case GST_STATE_CHANGE_NULL_TO_READY:
       if (priv->enable_quirks)
         gst_ks_video_src_apply_driver_quirks (self);
-      if (!gst_ks_video_src_open_device (self))
+      if (!gst_ks_video_src_start_worker (self))
         goto open_failed;
       break;
   }
@@ -509,7 +622,7 @@ gst_ks_video_src_change_state (GstElement * element, GstStateChange transition)
 
   switch (transition) {
     case GST_STATE_CHANGE_READY_TO_NULL:
-      gst_ks_video_src_close_device (self);
+      gst_ks_video_src_stop_worker (self);
       break;
   }
 
@@ -518,6 +631,7 @@ gst_ks_video_src_change_state (GstElement * element, GstStateChange transition)
   /* ERRORS */
 open_failed:
   {
+    gst_ks_video_src_stop_worker (self);
     return GST_STATE_CHANGE_FAILURE;
   }
 }
@@ -529,7 +643,7 @@ gst_ks_video_src_set_clock (GstElement * element, GstClock * clock)
   GstKsVideoSrcPrivate *priv = GST_KS_VIDEO_SRC_GET_PRIVATE (self);
 
   GST_OBJECT_LOCK (element);
-  if (priv->ksclock != NULL)
+  if (clock != NULL && priv->ksclock != NULL)
     gst_ks_clock_provide_master_clock (priv->ksclock, clock);
   GST_OBJECT_UNLOCK (element);
 
@@ -557,13 +671,14 @@ gst_ks_video_src_set_caps (GstBaseSrc * basesrc, GstCaps * caps)
   if (priv->device == NULL)
     return FALSE;
 
-  if (!gst_ks_video_device_set_caps (priv->device, caps))
-    return FALSE;
-
-  if (!gst_ks_video_device_set_state (priv->device, KSSTATE_RUN))
-    return FALSE;
+  KS_WORKER_LOCK (priv);
+  priv->worker_pending_caps = caps;
+  KS_WORKER_NOTIFY (priv);
+  while (priv->worker_pending_caps == caps)
+    KS_WORKER_WAIT_FOR_RESULT (priv);
+  KS_WORKER_UNLOCK (priv);
 
-  return TRUE;
+  return priv->worker_setcaps_result;
 }
 
 static void
@@ -674,8 +789,12 @@ gst_ks_video_src_timestamp_buffer (GstKsVideoSrc * self, GstBuffer * buf,
       timestamp = 0;
 
     if (GST_CLOCK_TIME_IS_VALID (presentation_time)) {
-      GstClockTimeDiff diff = GST_CLOCK_DIFF (timestamp, presentation_time);
-      GST_DEBUG_OBJECT (self, "Diff between our and the driver's timestamp: %"
+      /*
+       * We don't use this for anything yet, need to ponder how to deal
+       * with pins that use an internal clock and timestamp from 0.
+       */
+      GstClockTimeDiff diff = GST_CLOCK_DIFF (presentation_time, timestamp);
+      GST_DEBUG_OBJECT (self, "diff between gst and driver timestamp: %"
           G_GINT64_FORMAT, diff);
     }
 
@@ -712,15 +831,11 @@ gst_ks_video_src_timestamp_buffer (GstKsVideoSrc * self, GstBuffer * buf,
         GST_BUFFER_FLAG_UNSET (buf, GST_BUFFER_FLAG_DISCONT);
       else if (delta_offset > 1) {
         guint lost = delta_offset - 1;
-#if ENABLE_CLOCK_DEBUG
         GST_INFO_OBJECT (self, "lost %d frame%s, setting discont flag",
             lost, (lost > 1) ? "s" : "");
-#endif
         GST_BUFFER_FLAG_SET (buf, GST_BUFFER_FLAG_DISCONT);
       } else if (delta_offset == 0) {   /* overproduction, skip this frame */
-#if ENABLE_CLOCK_DEBUG
         GST_INFO_OBJECT (self, "skipping frame");
-#endif
         return FALSE;
       }
 
@@ -803,6 +918,19 @@ gst_ks_video_src_create (GstPushSrc * pushsrc, GstBuffer ** buffer)
   if (G_UNLIKELY (result != GST_FLOW_OK))
     goto error_alloc_buffer;
 
+  if (G_UNLIKELY (!priv->running)) {
+    KS_WORKER_LOCK (priv);
+    priv->worker_pending_run = TRUE;
+    KS_WORKER_NOTIFY (priv);
+    while (priv->worker_pending_run)
+      KS_WORKER_WAIT_FOR_RESULT (priv);
+    priv->running = priv->worker_run_result;
+    KS_WORKER_UNLOCK (priv);
+
+    if (!priv->running)
+      goto error_start_capture;
+  }
+
   do {
     gulong bytes_read;
 
@@ -833,6 +961,14 @@ error_no_caps:
 
     return GST_FLOW_ERROR;
   }
+error_start_capture:
+  {
+    GST_ELEMENT_ERROR (self, RESOURCE, OPEN_READ,
+        ("could not start capture"),
+        ("failed to change pin state to KSSTATE_RUN"));
+
+    return GST_FLOW_ERROR;
+  }
 error_alloc_buffer:
   {
     GST_ELEMENT_ERROR (self, CORE, PAD, ("alloc_buffer failed"), (NULL));
diff --git a/sys/winks/kshelpers.c b/sys/winks/kshelpers.c
index d1aa274e..6f614084 100644
--- a/sys/winks/kshelpers.c
+++ b/sys/winks/kshelpers.c
@@ -323,7 +323,7 @@ ks_state_to_string (KSSTATE state)
   if (flags & KSSTREAM_HEADER_OPTIONSF_##flag)\
   {\
     if (str->len > 0)\
-      g_string_append (str, " | ");\
+      g_string_append (str, "|");\
     g_string_append (str, G_STRINGIFY (flag));\
     flags &= ~KSSTREAM_HEADER_OPTIONSF_##flag;\
   }
@@ -336,21 +336,21 @@ ks_options_flags_to_string (gulong flags)
 
   str = g_string_sized_new (128);
 
+  CHECK_OPTIONS_FLAG (SPLICEPOINT);
+  CHECK_OPTIONS_FLAG (PREROLL);
   CHECK_OPTIONS_FLAG (DATADISCONTINUITY);
+  CHECK_OPTIONS_FLAG (TYPECHANGED);
+  CHECK_OPTIONS_FLAG (TIMEVALID);
+  CHECK_OPTIONS_FLAG (TIMEDISCONTINUITY);
+  CHECK_OPTIONS_FLAG (FLUSHONPAUSE);
   CHECK_OPTIONS_FLAG (DURATIONVALID);
   CHECK_OPTIONS_FLAG (ENDOFSTREAM);
-  CHECK_OPTIONS_FLAG (FLUSHONPAUSE);
-  CHECK_OPTIONS_FLAG (LOOPEDDATA);
-  CHECK_OPTIONS_FLAG (PREROLL);
-  CHECK_OPTIONS_FLAG (SPLICEPOINT);
-  CHECK_OPTIONS_FLAG (TIMEDISCONTINUITY);
-  CHECK_OPTIONS_FLAG (TIMEVALID);
-  CHECK_OPTIONS_FLAG (TYPECHANGED);
-  CHECK_OPTIONS_FLAG (VRAM_DATA_TRANSFER);
   CHECK_OPTIONS_FLAG (BUFFEREDTRANSFER);
+  CHECK_OPTIONS_FLAG (VRAM_DATA_TRANSFER);
+  CHECK_OPTIONS_FLAG (LOOPEDDATA);
 
   if (flags != 0)
-    g_string_append_printf (str, " | 0x%08x", flags);
+    g_string_append_printf (str, "|0x%08x", flags);
 
   ret = str->str;
   g_string_free (str, FALSE);
diff --git a/sys/winks/ksvideohelpers.c b/sys/winks/ksvideohelpers.c
index 7ffe6b09..e85b05fd 100644
--- a/sys/winks/ksvideohelpers.c
+++ b/sys/winks/ksvideohelpers.c
@@ -422,6 +422,8 @@ ks_video_probe_filter_for_caps (HANDLE filter_handle)
           /* REVISIT: Each KSDATARANGE should start on a 64-bit boundary */
           range = (KSDATARANGE *) (((guchar *) range) + range->FormatSize);
         }
+
+        g_free (items);
       }
     }
   }