wrappercamerabinsrc: Refactor internal pipeline

Remove tee and output-selector and just link the source
pad to the outputs we want as needed.

The way we need to prioritize caps negotiation and allocation
queries depending on the mode enabled is too custom to be
handled using tee and output-selector.
This provides more flexibility and doesn't get in the way of proper
handling of negotiation and allocation queries.

2 files changed, 133 insertions, 155 deletions

diff --git a/gst/camerabin2/gstwrappercamerabinsrc.c b/gst/camerabin2/gstwrappercamerabinsrc.c
index 867b207e3..731d91eb4 100644
--- a/gst/camerabin2/gstwrappercamerabinsrc.c
+++ b/gst/camerabin2/gstwrappercamerabinsrc.c
@@ -76,13 +76,17 @@ gst_wrapper_camera_bin_src_dispose (GObject * object)
 {
   GstWrapperCameraBinSrc *self = GST_WRAPPER_CAMERA_BIN_SRC (object);
 
-  if (self->outsel_imgpad) {
-    gst_object_unref (self->outsel_imgpad);
-    self->outsel_imgpad = NULL;
+  if (self->src_pad) {
+    gst_object_unref (self->src_pad);
+    self->src_pad = NULL;
   }
-  if (self->outsel_vidpad) {
-    gst_object_unref (self->outsel_vidpad);
-    self->outsel_vidpad = NULL;
+  if (self->video_tee_sink) {
+    gst_object_unref (self->video_tee_sink);
+    self->video_tee_sink = NULL;
+  }
+  if (self->video_tee_vf_pad) {
+    gst_object_unref (self->video_tee_vf_pad);
+    self->video_tee_vf_pad = NULL;
   }
   if (self->app_vid_src) {
     gst_object_unref (self->app_vid_src);
@@ -215,6 +219,17 @@ gst_wrapper_camera_bin_reset_video_src_caps (GstWrapperCameraBinSrc * self,
   }
 }
 
+static void
+gst_wrapper_camera_bin_src_set_output (GstWrapperCameraBinSrc * self,
+    GstPad * old_pad, GstPad * output_pad)
+{
+  gst_pad_peer_query (self->src_pad, gst_query_new_drain ());
+  if (old_pad)
+    gst_ghost_pad_set_target (GST_GHOST_PAD (old_pad), NULL);
+  if (output_pad)
+    gst_ghost_pad_set_target (GST_GHOST_PAD (output_pad), self->src_pad);
+}
+
 /**
  * gst_wrapper_camera_bin_src_imgsrc_probe:
  *
@@ -251,6 +266,8 @@ gst_wrapper_camera_bin_src_imgsrc_probe (GstPad * pad, GstPadProbeInfo * info,
     gst_sample_unref (sample);
 
     if (self->image_capture_count == 0) {
+      /* Get back to viewfinder */
+      gst_wrapper_camera_bin_src_set_output (self, self->imgsrc, self->vfsrc);
       gst_base_camera_src_finish_capture (camerasrc);
     }
   }
@@ -261,7 +278,7 @@ gst_wrapper_camera_bin_src_imgsrc_probe (GstPad * pad, GstPadProbeInfo * info,
 /**
  * gst_wrapper_camera_bin_src_vidsrc_probe:
  *
- * Buffer probe called before sending each buffer to image queue.
+ * Buffer probe called before sending each buffer to video queue.
  */
 static GstPadProbeReturn
 gst_wrapper_camera_bin_src_vidsrc_probe (GstPad * pad, GstPadProbeInfo * info,
@@ -325,6 +342,9 @@ gst_wrapper_camera_bin_src_vidsrc_probe (GstPad * pad, GstPadProbeInfo * info,
       GST_WARNING_OBJECT (camerasrc, "No peer pad for vidsrc");
     }
     self->video_rec_status = GST_VIDEO_RECORDING_STATUS_DONE;
+
+    gst_pad_unlink (self->src_pad, self->video_tee_sink);
+    gst_wrapper_camera_bin_src_set_output (self, self->vfsrc, self->vfsrc);
     gst_base_camera_src_finish_capture (camerasrc);
   } else {
     ret = GST_PAD_PROBE_OK;
@@ -388,14 +408,12 @@ gst_wrapper_camera_bin_src_max_zoom_cb (GObject * self, GParamSpec * pspec,
   g_object_notify (G_OBJECT (bcamsrc), "max-zoom");
 }
 
-
 static gboolean
 gst_wrapper_camera_bin_src_src_event (GstPad * pad, GstObject * parent,
     GstEvent * event)
 {
   gboolean ret = TRUE;
   GstWrapperCameraBinSrc *self = GST_WRAPPER_CAMERA_BIN_SRC (parent);
-  GstPad *upstream_pad = NULL;
 
   GST_DEBUG_OBJECT (self, "Handling event %p %" GST_PTR_FORMAT, event, event);
 
@@ -414,24 +432,10 @@ gst_wrapper_camera_bin_src_src_event (GstPad * pad, GstObject * parent,
       }
       break;
     default:
+      ret = gst_pad_event_default (pad, parent, event);
       break;
   }
 
-  if (pad == self->imgsrc) {
-    upstream_pad = self->outsel_imgpad;
-  } else if (pad == self->vidsrc) {
-    upstream_pad = self->outsel_vidpad;
-  }
-
-  if (upstream_pad) {
-    ret = gst_pad_send_event (upstream_pad, event);
-  } else {
-    GST_WARNING_OBJECT (self, "Event caught that doesn't have an upstream pad -"
-        "this shouldn't be possible!");
-    gst_event_unref (event);
-    ret = FALSE;
-  }
-
   return ret;
 }
 
@@ -512,11 +516,7 @@ check_and_replace_src (GstWrapperCameraBinSrc * self)
  *
  * This function creates and links the elements of the camerasrc bin
  * videosrc ! cspconv ! srcfilter ! cspconv ! capsfilter ! crop ! scale ! \
- * capsfilter ! tee name=t
- *    t. ! ... (viewfinder pad)
- *    t. ! output-selector name=outsel
- *        outsel. ! (image pad)
- *        outsel. ! (video pad)
+ * capsfilter
  *
  * Returns: TRUE, if elements were successfully created, FALSE otherwise
  */
@@ -525,13 +525,12 @@ gst_wrapper_camera_bin_src_construct_pipeline (GstBaseCameraSrc * bcamsrc)
 {
   GstWrapperCameraBinSrc *self = GST_WRAPPER_CAMERA_BIN_SRC (bcamsrc);
   GstBin *cbin = GST_BIN (bcamsrc);
-  GstElement *tee;
   GstElement *filter_csp;
   GstElement *src_csp;
   GstElement *capsfilter;
+  GstElement *video_recording_tee;
   gboolean ret = FALSE;
-  GstPad *vf_pad;
-  GstPad *tee_capture_pad;
+  GstPad *tee_pad;
 
   /* checks and adds a new video src if needed */
   if (!check_and_replace_src (self))
@@ -580,64 +579,31 @@ gst_wrapper_camera_bin_src_construct_pipeline (GstBaseCameraSrc * bcamsrc)
                 "zoom-capsfilter")))
       goto done;
 
-    if (!(tee =
-            gst_camerabin_create_and_add_element (cbin, "tee",
-                "camerasrc-tee")))
-      goto done;
+    /* keep a 'tee' element that has 2 source pads, one is linked to the
+     * vidsrc pad and the other is linked as needed to the viewfinder
+     * when video recording is hapenning */
+    video_recording_tee = gst_element_factory_make ("tee", "video_rec_tee");
+    gst_bin_add (GST_BIN_CAST (self), video_recording_tee);     /* TODO check returns */
+    self->video_tee_vf_pad =
+        gst_element_get_request_pad (video_recording_tee, "src_%u");
+    self->video_tee_sink =
+        gst_element_get_static_pad (video_recording_tee, "sink");
+    tee_pad = gst_element_get_request_pad (video_recording_tee, "src_%u");
+    gst_ghost_pad_set_target (GST_GHOST_PAD (self->vidsrc), tee_pad);
+    gst_object_unref (tee_pad);
 
     /* viewfinder pad */
-    vf_pad = gst_element_get_request_pad (tee, "src_%u");
-    g_object_set (tee, "alloc-pad", vf_pad, NULL);
-    gst_ghost_pad_set_target (GST_GHOST_PAD (self->vfsrc), vf_pad);
-    gst_object_unref (vf_pad);
-
-    /* image/video pad from tee */
-    tee_capture_pad = gst_element_get_request_pad (tee, "src_%u");
-
-    self->output_selector =
-        gst_element_factory_make ("output-selector", "outsel");
-    g_object_set (self->output_selector, "pad-negotiation-mode", 2, NULL);
-    gst_bin_add (GST_BIN (self), self->output_selector);
-    {
-      GstPad *pad = gst_element_get_static_pad (self->output_selector, "sink");
-
-      /* check return TODO */
-      gst_pad_link (tee_capture_pad, pad);
-      gst_object_unref (pad);
-    }
-    gst_object_unref (tee_capture_pad);
-
-    /* Create the 2 output pads for video and image */
-    self->outsel_vidpad =
-        gst_element_get_request_pad (self->output_selector, "src_%u");
-    self->outsel_imgpad =
-        gst_element_get_request_pad (self->output_selector, "src_%u");
-
-    g_assert (self->outsel_vidpad != NULL);
-    g_assert (self->outsel_imgpad != NULL);
-
-    gst_pad_add_probe (self->outsel_imgpad, GST_PAD_PROBE_TYPE_BUFFER,
-        gst_wrapper_camera_bin_src_imgsrc_probe, self, NULL);
-    gst_pad_add_probe (self->outsel_vidpad, GST_PAD_PROBE_TYPE_BUFFER,
-        gst_wrapper_camera_bin_src_vidsrc_probe, self, NULL);
-    gst_ghost_pad_set_target (GST_GHOST_PAD (self->imgsrc),
-        self->outsel_imgpad);
-    gst_ghost_pad_set_target (GST_GHOST_PAD (self->vidsrc),
-        self->outsel_vidpad);
-
-    if (bcamsrc->mode == MODE_IMAGE) {
-      g_object_set (self->output_selector, "active-pad", self->outsel_imgpad,
-          NULL);
-    } else {
-      g_object_set (self->output_selector, "active-pad", self->outsel_vidpad,
-          NULL);
-    }
-
-
+    self->src_pad = gst_element_get_static_pad (self->src_zoom_filter, "src");
+    gst_ghost_pad_set_target (GST_GHOST_PAD (self->vfsrc), self->src_pad);
 
     gst_pad_set_active (self->vfsrc, TRUE);
     gst_pad_set_active (self->imgsrc, TRUE);    /* XXX ??? */
     gst_pad_set_active (self->vidsrc, TRUE);    /* XXX ??? */
+
+    gst_pad_add_probe (self->imgsrc, GST_PAD_PROBE_TYPE_BUFFER,
+        gst_wrapper_camera_bin_src_imgsrc_probe, self, NULL);
+    gst_pad_add_probe (self->video_tee_sink, GST_PAD_PROBE_TYPE_BUFFER,
+        gst_wrapper_camera_bin_src_vidsrc_probe, self, NULL);
   }
 
   /* Do this even if pipeline is constructed */
@@ -809,45 +775,34 @@ img_capture_prepared (gpointer data, GstCaps * caps)
   }
 }
 
-/**
- *
- */
-static gboolean
-start_image_capture (GstWrapperCameraBinSrc * self)
+static GstPadProbeReturn
+start_image_capture (GstPad * pad, GstPadProbeInfo * info, gpointer udata)
 {
+  GstWrapperCameraBinSrc *self = udata;
   GstBaseCameraSrc *bcamsrc = GST_BASE_CAMERA_SRC (self);
   GstPhotography *photography =
       (GstPhotography *) gst_bin_get_by_interface (GST_BIN_CAST (bcamsrc),
       GST_TYPE_PHOTOGRAPHY);
-  gboolean ret = FALSE;
   GstCaps *caps;
 
   GST_DEBUG_OBJECT (self, "Starting image capture");
 
+  /* unlink from the viewfinder, link to the imagesrc pad to wait for
+   * the buffer to pass */
+  gst_wrapper_camera_bin_src_set_output (self, self->vfsrc, self->imgsrc);
+
   /* V4L2 source will not close the device until all buffers have came
    * back. Draining the pipeline, will ensure it's properly closed, and that
    * setting it back to PLAYING will work. */
   if (self->image_renegotiate) {
-    GstPad *pad, *peer;
-
-    g_mutex_unlock (&bcamsrc->capturing_mutex);
-    pad = gst_element_get_static_pad (self->src_vid_src, "src");
-    peer = gst_pad_get_peer (pad);
-    gst_object_unref (pad);
-    gst_pad_query (peer, gst_query_new_drain ());
-    gst_object_unref (peer);
-
     self->image_renegotiate = FALSE;
 
-    gst_element_set_state (self->src_vid_src, GST_STATE_READY);
-
     /* clean capsfilter caps so they don't interfere here */
     g_object_set (self->src_filter, "caps", NULL, NULL);
     if (self->src_zoom_filter)
       g_object_set (self->src_zoom_filter, "caps", NULL, NULL);
 
     caps = gst_pad_get_allowed_caps (self->imgsrc);
-
     gst_caps_replace (&self->image_capture_caps, caps);
     gst_caps_unref (caps);
 
@@ -857,28 +812,65 @@ start_image_capture (GstWrapperCameraBinSrc * self)
 
     /* We caught this event in the src pad event handler and now we want to
      * actually push it upstream */
-    gst_pad_send_event (self->outsel_imgpad, gst_event_new_reconfigure ());
-    g_mutex_lock (&bcamsrc->capturing_mutex);
+    gst_pad_mark_reconfigure (pad);
   }
 
   if (photography) {
     gst_element_set_state (self->src_vid_src, GST_STATE_PLAYING);
     GST_DEBUG_OBJECT (self, "prepare image capture caps %" GST_PTR_FORMAT,
         self->image_capture_caps);
-    ret = gst_photography_prepare_for_capture (photography,
-        (GstPhotographyCapturePrepared) img_capture_prepared,
-        self->image_capture_caps, self);
+    if (!gst_photography_prepare_for_capture (photography,
+            (GstPhotographyCapturePrepared) img_capture_prepared,
+            self->image_capture_caps, self))
+      self->image_capture_count = 0;    /* TODO post an error */
     gst_object_unref (photography);
   } else {
-    g_mutex_unlock (&bcamsrc->capturing_mutex);
     gst_wrapper_camera_bin_reset_video_src_caps (self,
         self->image_capture_caps);
-    g_mutex_lock (&bcamsrc->capturing_mutex);
-    ret = TRUE;
-    gst_element_set_state (self->src_vid_src, GST_STATE_PLAYING);
   }
 
-  return ret;
+  self->image_capture_probe = 0;
+  return GST_PAD_PROBE_REMOVE;
+}
+
+static GstPadProbeReturn
+start_video_capture (GstPad * pad, GstPadProbeInfo * info, gpointer udata)
+{
+  GstWrapperCameraBinSrc *self = udata;
+  GstCaps *caps;
+
+  GST_DEBUG_OBJECT (self, "Starting video capture");
+
+  if (self->video_renegotiate) {
+    GstCaps *anycaps = gst_caps_new_any ();
+    gst_wrapper_camera_bin_reset_video_src_caps (self, anycaps);
+    gst_caps_unref (anycaps);
+
+    /* clean capsfilter caps so they don't interfere here */
+    g_object_set (self->src_filter, "caps", NULL, NULL);
+    if (self->src_zoom_filter)
+      g_object_set (self->src_zoom_filter, "caps", NULL, NULL);
+  }
+
+  /* unlink from the viewfinder, link to the imagesrc pad, wait for
+   * the buffer to pass */
+  gst_wrapper_camera_bin_src_set_output (self, self->vfsrc, NULL);
+  gst_pad_link (self->src_pad, self->video_tee_sink);
+  gst_ghost_pad_set_target (GST_GHOST_PAD (self->vfsrc),
+      self->video_tee_vf_pad);
+
+  if (self->video_renegotiate) {
+    GST_DEBUG_OBJECT (self, "Getting allowed videosrc caps");
+    caps = gst_pad_get_allowed_caps (self->vidsrc);
+    GST_DEBUG_OBJECT (self, "Video src caps %" GST_PTR_FORMAT, caps);
+
+    self->video_renegotiate = FALSE;
+    gst_wrapper_camera_bin_reset_video_src_caps (self, caps);
+    gst_caps_unref (caps);
+  }
+  self->video_capture_probe = 0;
+
+  return GST_PAD_PROBE_REMOVE;
 }
 
 static gboolean
@@ -890,16 +882,10 @@ gst_wrapper_camera_bin_src_set_mode (GstBaseCameraSrc * bcamsrc,
       GST_TYPE_PHOTOGRAPHY);
   GstWrapperCameraBinSrc *self = GST_WRAPPER_CAMERA_BIN_SRC (bcamsrc);
 
-  if (self->output_selector) {
-    if (mode == MODE_IMAGE) {
-      self->image_renegotiate = TRUE;
-      g_object_set (self->output_selector, "active-pad", self->outsel_imgpad,
-          NULL);
-    } else {
-      self->video_renegotiate = TRUE;
-      g_object_set (self->output_selector, "active-pad", self->outsel_vidpad,
-          NULL);
-    }
+  if (mode == MODE_IMAGE) {
+    self->image_renegotiate = TRUE;
+  } else {
+    self->video_renegotiate = TRUE;
   }
   self->mode = mode;
 
@@ -1007,7 +993,7 @@ gst_wrapper_camera_bin_src_set_zoom (GstBaseCameraSrc * bcamsrc, gfloat zoom)
 static void
 update_aspect_filter (GstWrapperCameraBinSrc * self, GstCaps * new_caps)
 {
-  // XXX why not instead add a preserve-aspect-ratio property to videoscale?
+  /* XXX why not instead add a preserve-aspect-ratio property to videoscale? */
 #if 0
   if (camera->flags & GST_CAMERABIN_FLAG_VIEWFINDER_SCALE) {
     GstCaps *sink_caps, *ar_caps;
@@ -1113,44 +1099,31 @@ static gboolean
 gst_wrapper_camera_bin_src_start_capture (GstBaseCameraSrc * camerasrc)
 {
   GstWrapperCameraBinSrc *src = GST_WRAPPER_CAMERA_BIN_SRC (camerasrc);
+  GstPad *pad;
+  gboolean ret = TRUE;
+
+  pad = gst_element_get_static_pad (src->src_vid_src, "src");
 
   /* TODO should we access this directly? Maybe a macro is better? */
   if (src->mode == MODE_IMAGE) {
-    start_image_capture (src);
     src->image_capture_count = 1;
+
+    src->image_capture_probe =
+        gst_pad_add_probe (pad, GST_PAD_PROBE_TYPE_IDLE, start_image_capture,
+        src, NULL);
   } else if (src->mode == MODE_VIDEO) {
-    GstCaps *caps = NULL;
-
-    if (src->video_renegotiate) {
-      GstCaps *anycaps = gst_caps_new_any ();
-      g_mutex_unlock (&camerasrc->capturing_mutex);
-      gst_wrapper_camera_bin_reset_video_src_caps (src, anycaps);
-      g_mutex_lock (&camerasrc->capturing_mutex);
-
-      /* clean capsfilter caps so they don't interfere here */
-      g_object_set (src->src_filter, "caps", NULL, NULL);
-      if (src->src_zoom_filter)
-        g_object_set (src->src_zoom_filter, "caps", NULL, NULL);
-
-      GST_DEBUG_OBJECT (src, "Getting allowed videosrc caps");
-      caps = gst_pad_get_allowed_caps (src->vidsrc);
-      GST_DEBUG_OBJECT (src, "Video src caps %" GST_PTR_FORMAT, caps);
-
-      src->video_renegotiate = FALSE;
-      g_mutex_unlock (&camerasrc->capturing_mutex);
-      gst_wrapper_camera_bin_reset_video_src_caps (src, caps);
-      g_mutex_lock (&camerasrc->capturing_mutex);
-      gst_caps_unref (caps);
-      gst_caps_unref (anycaps);
-    }
     if (src->video_rec_status == GST_VIDEO_RECORDING_STATUS_DONE) {
       src->video_rec_status = GST_VIDEO_RECORDING_STATUS_STARTING;
+      src->video_capture_probe =
+          gst_pad_add_probe (pad, GST_PAD_PROBE_TYPE_IDLE, start_video_capture,
+          src, NULL);
     }
   } else {
     g_assert_not_reached ();
-    return FALSE;
+    ret = FALSE;
   }
-  return TRUE;
+  gst_object_unref (pad);
+  return ret;
 }
 
 static void
@@ -1169,7 +1142,7 @@ gst_wrapper_camera_bin_src_stop_capture (GstBaseCameraSrc * camerasrc)
       src->video_rec_status = GST_VIDEO_RECORDING_STATUS_FINISHING;
     }
   } else {
-    src->image_capture_count = 0;
+    /* TODO check what happens when we try to stop a image capture */
   }
 }
 
diff --git a/gst/camerabin2/gstwrappercamerabinsrc.h b/gst/camerabin2/gstwrappercamerabinsrc.h
index 050cc86d7..8976fcb31 100644
--- a/gst/camerabin2/gstwrappercamerabinsrc.h
+++ b/gst/camerabin2/gstwrappercamerabinsrc.h
@@ -81,15 +81,20 @@ struct _GstWrapperCameraBinSrc
   GstElement *src_zoom_crop;
   GstElement *src_zoom_scale;
   GstElement *src_zoom_filter;
-  GstElement *output_selector;
+
+  /* Pad from our last element that is linked
+   * with the output pads */
+  GstPad *src_pad;
+
+  GstPad *video_tee_vf_pad;
+  GstPad *video_tee_sink;
 
   gboolean elements_created;
 
   gulong src_event_probe_id;
   gulong src_max_zoom_signal_id;
-
-  GstPad *outsel_imgpad;
-  GstPad *outsel_vidpad;
+  gulong image_capture_probe;
+  gulong video_capture_probe;
 
   /* Application configurable elements */
   GstElement *app_vid_src;