camerabin2: Update v4l2camerasrc to use start/stop signals

Removes the old logic for v4l2camerasrc that used the mode
property switching to start/stop captures to make it identical
to camerabin2 behavior and to allow the future addition of
pausing a video recording.

This also removes the MODE_PREVIEW as it became useless.

gst/camerabin2/gstcamerabin-enum.c

@@ -66,7 +66,7 @@ gst_camerabin_mode_get_type (void)
 
   if (gtype == 0) {
     static const GEnumValue values[] = {
-      {MODE_PREVIEW, "Preview mode (should be default?)", "mode-preview"},
+      /* {MODE_PREVIEW, "Preview mode (should be default?)", "mode-preview"}, */
       {MODE_IMAGE, "Still image capture (default)", "mode-image"},
       {MODE_VIDEO, "Video recording", "mode-video"},
       {0, NULL, NULL}

gst/camerabin2/gstcamerabin-enum.h

@@ -113,7 +113,6 @@ GType gst_camerabin_flags_get_type (void);
 
 /**
  * GstCameraBinMode:
- * @MODE_PREVIEW: preview only (no capture) mode
  * @MODE_IMAGE: image capture
  * @MODE_VIDEO: video capture
  *
@@ -121,12 +120,7 @@ GType gst_camerabin_flags_get_type (void);
  */
 typedef enum
 {
-  /* note:  changed to align with 'capture-mode' property (even though
+  /* MODE_PREVIEW = 0, No use for this */
-   * I have no idea where this property comes from..)  But it somehow
-   * seems more logical for preview to be mode==0 even if it is an ABI
-   * break..
-   */
-  MODE_PREVIEW = 0,
   MODE_IMAGE = 1,
   MODE_VIDEO = 2,
 } GstCameraBinMode;

gst/camerabin2/gstcamerabin2.c

@@ -108,35 +108,25 @@ static void
 gst_camera_bin_start_capture (GstCameraBin * camerabin)
 {
   GST_DEBUG_OBJECT (camerabin, "Received start-capture");
-  g_mutex_lock (camerabin->capture_mutex);
+  g_signal_emit_by_name (camerabin->src, "start-capture", NULL);
-  if (!camerabin->capturing) {
-    GST_INFO_OBJECT (camerabin, "Starting capture, mode: %d", camerabin->mode);
-    g_object_set (camerabin->src, "mode", camerabin->mode, NULL);
-    camerabin->capturing = TRUE;
-  } else {
-    GST_WARNING_OBJECT (camerabin, "Capture already ongoing");
-  }
-  g_mutex_unlock (camerabin->capture_mutex);
 }
 
 static void
 gst_camera_bin_stop_capture (GstCameraBin * camerabin)
 {
-  /* TODO do we need a lock here? it is just an if */
+  g_signal_emit_by_name (camerabin->src, "stop-capture", NULL);
-  if (camerabin->capturing) {
-    /* Capturing is cleaned on the notify function because it works
-     * both for images and videos */
-    g_object_set (camerabin->src, "mode", MODE_PREVIEW, NULL);
-  }
 }
 
 static void
 gst_camera_bin_change_mode (GstCameraBin * camerabin, gint mode)
 {
+  if (mode == camerabin->mode)
+    return;
+
   /* stop any ongoing capture */
   gst_camera_bin_stop_capture (camerabin);
-
   camerabin->mode = mode;
+  g_object_set (camerabin->src, "mode", mode, NULL);
 }
 
 static void
@@ -144,12 +134,9 @@ gst_camera_bin_dispose (GObject * object)
 {
   GstCameraBin *camerabin = GST_CAMERA_BIN_CAST (object);
 
-  if (camerabin->src_mode_notify_id)
-    g_signal_handler_disconnect (camerabin->src, camerabin->src_mode_notify_id);
   if (camerabin->src)
     gst_object_unref (camerabin->src);
 
-  g_mutex_free (camerabin->capture_mutex);
   G_OBJECT_CLASS (parent_class)->dispose (object);
 }
 
@@ -228,28 +215,9 @@ gst_camera_bin_class_init (GstCameraBinClass * klass)
 static void
 gst_camera_bin_init (GstCameraBin * camerabin)
 {
-  camerabin->capturing = FALSE;
-  camerabin->capture_mutex = g_mutex_new ();
   camerabin->mode = MODE_IMAGE;
 }
 
-static void
-gst_camera_bin_src_notify_mode (GObject * src, GParamSpec * pspec,
-    gpointer data)
-{
-  GstCameraBin *camera = GST_CAMERA_BIN_CAST (data);
-  gint mode;
-
-  g_object_get (src, "mode", &mode, NULL);
-
-  if (mode == MODE_PREVIEW) {
-    g_mutex_lock (camera->capture_mutex);
-    g_assert (camera->capturing);
-    camera->capturing = FALSE;
-    g_mutex_unlock (camera->capture_mutex);
-  }
-}
-
 /**
  * gst_camera_bin_create_elements:
  * @param camera: the #GstCameraBin
@@ -285,8 +253,6 @@ gst_camera_bin_create_elements (GstCameraBin * camera)
   vf = gst_element_factory_make ("viewfinderbin", "vf-bin");
 
   camera->src = gst_object_ref (src);
-  camera->src_mode_notify_id = g_signal_connect (src, "notify::mode",
-      (GCallback) gst_camera_bin_src_notify_mode, camera);
 
   vid_queue = gst_element_factory_make ("queue", "video-queue");
   img_queue = gst_element_factory_make ("queue", "image-queue");

gst/camerabin2/gstcamerabin2.h

@@ -38,11 +38,6 @@ struct _GstCameraBin
   GstPipeline pipeline;
 
   GstElement *src;
-  gulong src_mode_notify_id;
-
-  /* concurrency control */
-  GMutex *capture_mutex;
-  gboolean capturing;
 
   /* properties */
   gint mode;

gst/camerabin2/gstv4l2camerasrc.c

@@ -35,6 +35,15 @@
 #include "camerabingeneral.h"
 #include "gstcamerabin-enum.h"
 
+enum
+{
+  /* action signals */
+  START_CAPTURE_SIGNAL,
+  STOP_CAPTURE_SIGNAL,
+  /* emit signals */
+  IMG_DONE_SIGNAL,
+  LAST_SIGNAL
+};
 
 #define CAMERABIN_DEFAULT_VF_CAPS "video/x-raw-yuv,format=(fourcc)I420"
 
@@ -44,6 +53,8 @@
 GST_DEBUG_CATEGORY (v4l2_camera_src_debug);
 #define GST_CAT_DEFAULT v4l2_camera_src_debug
 
+static guint v4l2camerasrc_signals[LAST_SIGNAL];
+
 GST_BOILERPLATE (GstV4l2CameraSrc, gst_v4l2_camera_src, GstBaseCameraSrc,
     GST_TYPE_BASE_CAMERA_SRC);
 
@@ -53,6 +64,9 @@ static void set_capsfilter_caps (GstV4l2CameraSrc * self, GstCaps * new_caps);
 static void
 gst_v4l2_camera_src_dispose (GObject * object)
 {
+  GstV4l2CameraSrc *src = GST_V4L2_CAMERA_SRC (object);
+
+  g_mutex_free (src->capturing_mutex);
   G_OBJECT_CLASS (parent_class)->dispose (object);
 }
 
@@ -147,14 +161,17 @@ gst_v4l2_camera_src_imgsrc_probe (GstPad * pad, GstBuffer * buffer,
     gpointer data)
 {
   GstV4l2CameraSrc *self = GST_V4L2_CAMERA_SRC (data);
-  gboolean ret;
+  gboolean ret = FALSE;
-  GST_DEBUG_OBJECT (self, "pass buffer: %d", self->mode == MODE_IMAGE);
 
-  ret = self->mode == MODE_IMAGE;
+  GST_DEBUG_OBJECT (self, "pass buffer: %d", self->mode == MODE_IMAGE);
-  if (ret) {
+  g_mutex_lock (self->capturing_mutex);
-    self->mode = MODE_PREVIEW;
+  if (self->image_capture_count > 0) {
-    g_object_notify (G_OBJECT (self), "mode");
+    ret = TRUE;
+    self->image_capture_count--;
+    if (self->image_capture_count == 0)
+      self->capturing = FALSE;
   }
+  g_mutex_unlock (self->capturing_mutex);
   return ret;
 }
 
@@ -170,33 +187,29 @@ gst_v4l2_camera_src_vidsrc_probe (GstPad * pad, GstBuffer * buffer,
   GstV4l2CameraSrc *self = GST_V4L2_CAMERA_SRC (data);
   gboolean ret = FALSE;
 
-  GST_DEBUG_OBJECT (self, "pass buffer: %d", self->mode == MODE_VIDEO);
-
   /* TODO do we want to lock for every buffer? */
   /*
    * Note that we can use gst_pad_push_event here because we are a buffer
    * probe.
    */
-  if (self->mode == MODE_VIDEO) {
+  g_mutex_lock (self->capturing_mutex);
-    GST_OBJECT_LOCK (self);
+  if (self->video_rec_status == GST_VIDEO_RECORDING_STATUS_DONE) {
-    if (self->video_rec_status == GST_VIDEO_RECORDING_STATUS_STARTING) {
+    /* NOP */
-      /* send the newseg */
+  } else if (self->video_rec_status == GST_VIDEO_RECORDING_STATUS_STARTING) {
-      gst_pad_push_event (pad, gst_event_new_new_segment (FALSE, 1.0,
+    /* send the newseg */
-              GST_FORMAT_TIME, GST_BUFFER_TIMESTAMP (buffer), -1, 0));
+    gst_pad_push_event (pad, gst_event_new_new_segment (FALSE, 1.0,
-      self->video_rec_status = GST_VIDEO_RECORDING_STATUS_RUNNING;
+            GST_FORMAT_TIME, GST_BUFFER_TIMESTAMP (buffer), -1, 0));
-      ret = TRUE;
+    self->video_rec_status = GST_VIDEO_RECORDING_STATUS_RUNNING;
-    } else if (self->video_rec_status == GST_VIDEO_RECORDING_STATUS_FINISHING) {
+    ret = TRUE;
-      /* send eos */
+  } else if (self->video_rec_status == GST_VIDEO_RECORDING_STATUS_FINISHING) {
-      gst_pad_push_event (pad, gst_event_new_eos ());
+    /* send eos */
-      self->video_rec_status = GST_VIDEO_RECORDING_STATUS_DONE;
+    gst_pad_push_event (pad, gst_event_new_eos ());
-      self->mode = MODE_PREVIEW;
+    self->video_rec_status = GST_VIDEO_RECORDING_STATUS_DONE;
-      g_object_notify (G_OBJECT (self), "mode");
+    self->capturing = FALSE;
-    } else {
+  } else {
-      ret = TRUE;
+    ret = TRUE;
-    }
-    GST_OBJECT_UNLOCK (self);
   }
-
+  g_mutex_unlock (self->capturing_mutex);
   return ret;
 }
 
@@ -706,7 +719,6 @@ gst_v4l2_camera_src_set_mode (GstBaseCameraSrc * bcamsrc, GstCameraBinMode mode)
 {
   GstV4l2CameraSrc *self = GST_V4L2_CAMERA_SRC (bcamsrc);
   GstPhotography *photography = gst_base_camera_src_get_photography (bcamsrc);
-  gint ret = TRUE;
 
   if (photography) {
     if (g_object_class_find_property (G_OBJECT_GET_CLASS (photography),
@@ -715,34 +727,8 @@ gst_v4l2_camera_src_set_mode (GstBaseCameraSrc * bcamsrc, GstCameraBinMode mode)
     }
   }
 
-  switch (mode) {
+  self->mode = mode;
-    case MODE_PREVIEW:
+  return TRUE;
-      if (self->mode == MODE_VIDEO) {
-        GST_OBJECT_LOCK (self);
-        if (self->video_rec_status == GST_VIDEO_RECORDING_STATUS_STARTING)
-          self->video_rec_status = GST_VIDEO_RECORDING_STATUS_DONE;
-        else if (self->video_rec_status == GST_VIDEO_RECORDING_STATUS_RUNNING)
-          self->video_rec_status = GST_VIDEO_RECORDING_STATUS_FINISHING;
-        GST_OBJECT_UNLOCK (self);
-      }
-      break;
-    case MODE_IMAGE:
-      ret = start_image_capture (GST_V4L2_CAMERA_SRC (bcamsrc));
-      break;
-    case MODE_VIDEO:
-      GST_OBJECT_LOCK (self);
-      if (self->video_rec_status == GST_VIDEO_RECORDING_STATUS_DONE)
-        self->video_rec_status = GST_VIDEO_RECORDING_STATUS_STARTING;
-      GST_OBJECT_UNLOCK (self);
-      break;
-    default:
-      g_assert_not_reached ();
-      ret = FALSE;
-  }
-
-  if (ret)
-    self->mode = mode;
-  return ret;
 }
 
 static gboolean
@@ -1051,6 +1037,56 @@ gst_v4l2_camera_src_finish_image_capture (GstBaseCameraSrc * bcamsrc)
   }
 }
 
+static void
+gst_v4l2_camera_src_start_capture (GstV4l2CameraSrc * src)
+{
+  g_mutex_lock (src->capturing_mutex);
+  if (src->capturing) {
+    GST_WARNING_OBJECT (src, "Capturing already ongoing");
+    g_mutex_unlock (src->capturing_mutex);
+    return;
+  }
+
+  src->capturing = TRUE;
+  if (src->mode == MODE_IMAGE) {
+    src->image_capture_count = 1;
+    start_image_capture (src);
+  } else if (src->mode == MODE_VIDEO) {
+    if (src->video_rec_status == GST_VIDEO_RECORDING_STATUS_DONE) {
+      src->video_rec_status = GST_VIDEO_RECORDING_STATUS_STARTING;
+    }
+  } else {
+    g_assert_not_reached ();
+    src->capturing = FALSE;
+  }
+  g_mutex_unlock (src->capturing_mutex);
+}
+
+static void
+gst_v4l2_camera_src_stop_capture (GstV4l2CameraSrc * src)
+{
+  g_mutex_lock (src->capturing_mutex);
+  if (!src->capturing) {
+    GST_DEBUG_OBJECT (src, "No ongoing capture");
+    g_mutex_unlock (src->capturing_mutex);
+    return;
+  }
+  if (src->mode == MODE_VIDEO) {
+    if (src->video_rec_status == GST_VIDEO_RECORDING_STATUS_STARTING) {
+      GST_DEBUG_OBJECT (src, "Aborting not started recording");
+      src->video_rec_status = GST_VIDEO_RECORDING_STATUS_DONE;
+
+    } else if (src->video_rec_status == GST_VIDEO_RECORDING_STATUS_RUNNING) {
+      GST_DEBUG_OBJECT (src, "Marking video recording as finishing");
+      src->video_rec_status = GST_VIDEO_RECORDING_STATUS_FINISHING;
+    }
+  } else {
+    src->image_capture_count = 0;
+    src->capturing = FALSE;
+  }
+  g_mutex_unlock (src->capturing_mutex);
+}
+
 static void
 gst_v4l2_camera_src_base_init (gpointer g_class)
 {
@@ -1078,14 +1114,31 @@ gst_v4l2_camera_src_class_init (GstV4l2CameraSrcClass * klass)
   gobject_class->set_property = gst_v4l2_camera_src_set_property;
   gobject_class->get_property = gst_v4l2_camera_src_get_property;
 
-  // g_object_class_install_property ....
+  /* g_object_class_install_property .... */
   g_object_class_install_property (gobject_class, ARG_MODE,
       g_param_spec_enum ("mode", "Mode",
-          "The capture mode (still image capture, video recording or "
+          "The capture mode (still image capture or video recording)",
-          "viewfinder)",
+          GST_TYPE_CAMERABIN_MODE, MODE_IMAGE,
-          GST_TYPE_CAMERABIN_MODE, MODE_PREVIEW,
           G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS));
 
+  /* Signals */
+  v4l2camerasrc_signals[START_CAPTURE_SIGNAL] =
+      g_signal_new ("start-capture",
+      G_TYPE_FROM_CLASS (klass),
+      G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION,
+      G_STRUCT_OFFSET (GstV4l2CameraSrcClass, start_capture),
+      NULL, NULL, g_cclosure_marshal_VOID__VOID, G_TYPE_NONE, 0);
+
+  v4l2camerasrc_signals[STOP_CAPTURE_SIGNAL] =
+      g_signal_new ("stop-capture",
+      G_TYPE_FROM_CLASS (klass),
+      G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION,
+      G_STRUCT_OFFSET (GstV4l2CameraSrcClass, stop_capture),
+      NULL, NULL, g_cclosure_marshal_VOID__VOID, G_TYPE_NONE, 0);
+
+  klass->start_capture = gst_v4l2_camera_src_start_capture;
+  klass->stop_capture = gst_v4l2_camera_src_stop_capture;
+
   gstbasecamerasrc_class->construct_pipeline =
       gst_v4l2_camera_src_construct_pipeline;
   gstbasecamerasrc_class->setup_pipeline = gst_v4l2_camera_src_setup_pipeline;
@@ -1102,8 +1155,11 @@ gst_v4l2_camera_src_init (GstV4l2CameraSrc * self,
     GstV4l2CameraSrcClass * klass)
 {
   /* TODO where are variables reset? */
-  self->mode = MODE_PREVIEW;
+  self->mode = MODE_IMAGE;
+  self->image_capture_count = 0;
   self->video_rec_status = GST_VIDEO_RECORDING_STATUS_DONE;
+  self->capturing_mutex = g_mutex_new ();
+  self->capturing = FALSE;
 }
 
 gboolean

gst/camerabin2/gstv4l2camerasrc.h

@@ -59,9 +59,15 @@ struct _GstV4l2CameraSrc
 
   GstCameraBinMode mode;
 
+  gboolean capturing;
+  GMutex *capturing_mutex;
+
   /* video recording controls */
   gint video_rec_status;
 
+  /* image capture controls */
+  gint image_capture_count;
+
   /* source elements */
   GstElement *src_vid_src;
   GstElement *src_filter;
@@ -104,6 +110,9 @@ struct _GstV4l2CameraSrc
 struct _GstV4l2CameraSrcClass
 {
   GstBaseCameraSrcClass parent;
+
+  void (*start_capture) (GstV4l2CameraSrc * src);
+  void (*stop_capture) (GstV4l2CameraSrc * src);
 };
 
 gboolean gst_v4l2_camera_src_plugin_init (GstPlugin * plugin);