From 5ca3c9477e766ffa1e13be924432cc783da2ba4d Mon Sep 17 00:00:00 2001 From: Lasse Laukkanen Date: Wed, 9 Nov 2011 09:41:44 -0300 Subject: [PATCH 1/4] camerabin2: Avoid blocking in start-capture and send application tags later Tags are currently sent from start-capture, which is run in the application thread. For images we can delay the tags pushing to the buffer probe and push the tags with the location event and reduce start-capture time. --- gst/camerabin2/gstcamerabin2.c | 49 +++++++++++++++++++++++++++++----- gst/camerabin2/gstcamerabin2.h | 3 +++ 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/gst/camerabin2/gstcamerabin2.c b/gst/camerabin2/gstcamerabin2.c index 62f29a1b26..8ba8a4ecfa 100644 --- a/gst/camerabin2/gstcamerabin2.c +++ b/gst/camerabin2/gstcamerabin2.c @@ -429,21 +429,27 @@ gst_camera_bin_start_capture (GstCameraBin2 * camerabin) if (taglist) { GstPad *active_pad; - GST_DEBUG_OBJECT (camerabin, "Pushing tags from application: %" + GST_DEBUG_OBJECT (camerabin, "Have tags from application: %" GST_PTR_FORMAT, taglist); if (camerabin->mode == MODE_IMAGE) { - active_pad = gst_element_get_static_pad (camerabin->src, - GST_BASE_CAMERA_SRC_IMAGE_PAD_NAME); + /* Store image tags in a list and push them later, this prevents + start_capture() from blocking in pad_push_event call */ + g_mutex_lock (camerabin->image_tags_mutex); + camerabin->image_tags_list = + g_slist_append (camerabin->image_tags_list, + gst_tag_list_copy (taglist)); + g_mutex_unlock (camerabin->image_tags_mutex); } else { active_pad = gst_element_get_static_pad (camerabin->src, GST_BASE_CAMERA_SRC_VIDEO_PAD_NAME); - } + gst_pad_push_event (active_pad, + gst_event_new_tag (gst_tag_list_copy (taglist))); - gst_pad_push_event (active_pad, - gst_event_new_tag (gst_tag_list_copy (taglist))); - gst_object_unref (active_pad); + gst_object_unref (active_pad); + } } + GST_DEBUG_OBJECT (camerabin, "Start-capture end"); } static void @@ -526,6 +532,7 @@ gst_camera_bin_dispose (GObject * object) g_free (camerabin->location); g_mutex_free (camerabin->preview_list_mutex); + g_mutex_free (camerabin->image_tags_mutex); if (camerabin->src_capture_notify_id) g_signal_handler_disconnect (camerabin->src, @@ -882,6 +889,7 @@ gst_camera_bin_init (GstCameraBin2 * camera) camera->max_zoom = MAX_ZOOM; camera->flags = DEFAULT_FLAGS; camera->preview_list_mutex = g_mutex_new (); + camera->image_tags_mutex = g_mutex_new (); /* capsfilters are created here as we proxy their caps properties and * this way we avoid having to store the caps while on NULL state to @@ -1263,7 +1271,27 @@ gst_camera_bin_image_src_buffer_probe (GstPad * pad, GstBuffer * buf, GstEvent *evt; gchar *location = NULL; GstPad *peer; + GstTagList *tags; + /* Push pending image tags */ + g_mutex_lock (camerabin->image_tags_mutex); + if (camerabin->image_tags_list) { + tags = camerabin->image_tags_list->data; + camerabin->image_tags_list = + g_slist_delete_link (camerabin->image_tags_list, + camerabin->image_tags_list); + g_mutex_unlock (camerabin->image_tags_mutex); + GST_DEBUG_OBJECT (camerabin, "Pushing tags from application: %" + GST_PTR_FORMAT, tags); + peer = gst_pad_get_peer (pad); + gst_pad_send_event (peer, gst_event_new_tag (tags)); + gst_object_unref (peer); + } else { + g_mutex_unlock (camerabin->image_tags_mutex); + GST_DEBUG_OBJECT (camerabin, "No tags from application to send"); + } + + /* Push image location event */ if (camerabin->image_location_list) { location = camerabin->image_location_list->data; camerabin->image_location_list = @@ -1775,6 +1803,13 @@ gst_camera_bin_change_state (GstElement * element, GstStateChange trans) g_slist_free (camera->image_location_list); camera->image_location_list = NULL; + g_mutex_lock (camera->image_tags_mutex); + g_slist_foreach (camera->image_tags_list, (GFunc) gst_tag_list_free, + NULL); + g_slist_free (camera->image_tags_list); + camera->image_tags_list = NULL; + g_mutex_unlock (camera->image_tags_mutex); + g_mutex_lock (camera->preview_list_mutex); g_slist_foreach (camera->preview_location_list, (GFunc) g_free, NULL); g_slist_free (camera->preview_location_list); diff --git a/gst/camerabin2/gstcamerabin2.h b/gst/camerabin2/gstcamerabin2.h index 3b4c9c92fd..27115bdf45 100644 --- a/gst/camerabin2/gstcamerabin2.h +++ b/gst/camerabin2/gstcamerabin2.h @@ -93,6 +93,9 @@ struct _GstCameraBin2 * as file location change notifications, they are pushed before * each buffer capture */ GSList *image_location_list; + /* Store also tags and push them before each captured image */ + GMutex *image_tags_mutex; + GSList *image_tags_list; /* * Similar to above, but used for giving names to previews From 9ab6406f23e4374c5d5299537546db833d3bb3b8 Mon Sep 17 00:00:00 2001 From: Thiago Santos Date: Wed, 9 Nov 2011 11:17:15 -0300 Subject: [PATCH 2/4] camerabin2: Don't store preview location if preview isn't requested Do not store preview location is post-previews is false, this would mess up preview naming in case application switches between enabling and disabling previews --- gst/camerabin2/gstcamerabin2.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/gst/camerabin2/gstcamerabin2.c b/gst/camerabin2/gstcamerabin2.c index 8ba8a4ecfa..693af1f144 100644 --- a/gst/camerabin2/gstcamerabin2.c +++ b/gst/camerabin2/gstcamerabin2.c @@ -377,11 +377,6 @@ gst_camera_bin_start_capture (GstCameraBin2 * camerabin) GST_CAMERA_BIN2_PROCESSING_INC (camerabin); - if (camerabin->post_previews) { - /* Count processing of preview images too */ - GST_CAMERA_BIN2_PROCESSING_INC (camerabin); - } - if (camerabin->location) location = g_strdup_printf (camerabin->location, capture_index); @@ -410,11 +405,17 @@ gst_camera_bin_start_capture (GstCameraBin2 * camerabin) g_slist_append (camerabin->image_location_list, g_strdup (location)); } - /* store the next preview filename */ - g_mutex_lock (camerabin->preview_list_mutex); - camerabin->preview_location_list = - g_slist_append (camerabin->preview_location_list, location); - g_mutex_unlock (camerabin->preview_list_mutex); + if (camerabin->post_previews) { + /* Count processing of preview images too */ + GST_CAMERA_BIN2_PROCESSING_INC (camerabin); + /* store the next preview filename */ + g_mutex_lock (camerabin->preview_list_mutex); + camerabin->preview_location_list = + g_slist_append (camerabin->preview_location_list, location); + g_mutex_unlock (camerabin->preview_list_mutex); + } else { + g_free (location); + } g_signal_emit_by_name (camerabin->src, "start-capture", NULL); if (camerabin->mode == MODE_VIDEO && camerabin->audio_src) From 27e01e02a4c7470b60d31e38f9d452a8277ac456 Mon Sep 17 00:00:00 2001 From: Thiago Santos Date: Wed, 9 Nov 2011 11:45:27 -0300 Subject: [PATCH 3/4] camerabin2: Handle null taglists for images Add NULL and check for them to the image capture taglist list, representing that a capture has no application tags set. --- gst/camerabin2/gstcamerabin2.c | 56 +++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/gst/camerabin2/gstcamerabin2.c b/gst/camerabin2/gstcamerabin2.c index 693af1f144..d53b0da88f 100644 --- a/gst/camerabin2/gstcamerabin2.c +++ b/gst/camerabin2/gstcamerabin2.c @@ -427,29 +427,28 @@ gst_camera_bin_start_capture (GstCameraBin2 * camerabin) * notify from ready for capture going to FALSE */ taglist = gst_tag_setter_get_tag_list (GST_TAG_SETTER (camerabin)); - if (taglist) { + GST_DEBUG_OBJECT (camerabin, "Have tags from application: %" + GST_PTR_FORMAT, taglist); + + if (camerabin->mode == MODE_IMAGE) { + /* Store image tags in a list and push them later, this prevents + start_capture() from blocking in pad_push_event call */ + g_mutex_lock (camerabin->image_tags_mutex); + camerabin->image_tags_list = + g_slist_append (camerabin->image_tags_list, + taglist ? gst_tag_list_copy (taglist) : NULL); + g_mutex_unlock (camerabin->image_tags_mutex); + } else if (taglist) { GstPad *active_pad; - GST_DEBUG_OBJECT (camerabin, "Have tags from application: %" - GST_PTR_FORMAT, taglist); + active_pad = gst_element_get_static_pad (camerabin->src, + GST_BASE_CAMERA_SRC_VIDEO_PAD_NAME); + gst_pad_push_event (active_pad, + gst_event_new_tag (gst_tag_list_copy (taglist))); - if (camerabin->mode == MODE_IMAGE) { - /* Store image tags in a list and push them later, this prevents - start_capture() from blocking in pad_push_event call */ - g_mutex_lock (camerabin->image_tags_mutex); - camerabin->image_tags_list = - g_slist_append (camerabin->image_tags_list, - gst_tag_list_copy (taglist)); - g_mutex_unlock (camerabin->image_tags_mutex); - } else { - active_pad = gst_element_get_static_pad (camerabin->src, - GST_BASE_CAMERA_SRC_VIDEO_PAD_NAME); - gst_pad_push_event (active_pad, - gst_event_new_tag (gst_tag_list_copy (taglist))); - - gst_object_unref (active_pad); - } + gst_object_unref (active_pad); } + GST_DEBUG_OBJECT (camerabin, "Start-capture end"); } @@ -1284,9 +1283,11 @@ gst_camera_bin_image_src_buffer_probe (GstPad * pad, GstBuffer * buf, g_mutex_unlock (camerabin->image_tags_mutex); GST_DEBUG_OBJECT (camerabin, "Pushing tags from application: %" GST_PTR_FORMAT, tags); - peer = gst_pad_get_peer (pad); - gst_pad_send_event (peer, gst_event_new_tag (tags)); - gst_object_unref (peer); + if (tags) { + peer = gst_pad_get_peer (pad); + gst_pad_send_event (peer, gst_event_new_tag (tags)); + gst_object_unref (peer); + } } else { g_mutex_unlock (camerabin->image_tags_mutex); GST_DEBUG_OBJECT (camerabin, "No tags from application to send"); @@ -1759,6 +1760,13 @@ fail: return FALSE; } +static void +_gst_tag_list_free_maybe (GstTagList * taglist) +{ + if (taglist) + gst_tag_list_free (taglist); +} + static GstStateChangeReturn gst_camera_bin_change_state (GstElement * element, GstStateChange trans) { @@ -1805,8 +1813,8 @@ gst_camera_bin_change_state (GstElement * element, GstStateChange trans) camera->image_location_list = NULL; g_mutex_lock (camera->image_tags_mutex); - g_slist_foreach (camera->image_tags_list, (GFunc) gst_tag_list_free, - NULL); + g_slist_foreach (camera->image_tags_list, + (GFunc) _gst_tag_list_free_maybe, NULL); g_slist_free (camera->image_tags_list); camera->image_tags_list = NULL; g_mutex_unlock (camera->image_tags_mutex); From c7db5db63224bdb0ae09c4980ad96488e9e2b9bc Mon Sep 17 00:00:00 2001 From: Thiago Santos Date: Wed, 9 Nov 2011 12:21:37 -0300 Subject: [PATCH 4/4] camerabin2: protect image location list with mutex Rename the image taglists' mutex into image capture mutex and use it also for the image capture list to prevent concurrent access from different threads (application and capture threads). --- gst/camerabin2/gstcamerabin2.c | 21 ++++++++++++--------- gst/camerabin2/gstcamerabin2.h | 2 +- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/gst/camerabin2/gstcamerabin2.c b/gst/camerabin2/gstcamerabin2.c index d53b0da88f..afae1014a3 100644 --- a/gst/camerabin2/gstcamerabin2.c +++ b/gst/camerabin2/gstcamerabin2.c @@ -401,8 +401,10 @@ gst_camera_bin_start_capture (GstCameraBin2 * camerabin) } } else { /* store the next capture buffer filename */ + g_mutex_lock (camerabin->image_capture_mutex); camerabin->image_location_list = g_slist_append (camerabin->image_location_list, g_strdup (location)); + g_mutex_unlock (camerabin->image_capture_mutex); } if (camerabin->post_previews) { @@ -433,11 +435,11 @@ gst_camera_bin_start_capture (GstCameraBin2 * camerabin) if (camerabin->mode == MODE_IMAGE) { /* Store image tags in a list and push them later, this prevents start_capture() from blocking in pad_push_event call */ - g_mutex_lock (camerabin->image_tags_mutex); + g_mutex_lock (camerabin->image_capture_mutex); camerabin->image_tags_list = g_slist_append (camerabin->image_tags_list, taglist ? gst_tag_list_copy (taglist) : NULL); - g_mutex_unlock (camerabin->image_tags_mutex); + g_mutex_unlock (camerabin->image_capture_mutex); } else if (taglist) { GstPad *active_pad; @@ -532,7 +534,7 @@ gst_camera_bin_dispose (GObject * object) g_free (camerabin->location); g_mutex_free (camerabin->preview_list_mutex); - g_mutex_free (camerabin->image_tags_mutex); + g_mutex_free (camerabin->image_capture_mutex); if (camerabin->src_capture_notify_id) g_signal_handler_disconnect (camerabin->src, @@ -889,7 +891,7 @@ gst_camera_bin_init (GstCameraBin2 * camera) camera->max_zoom = MAX_ZOOM; camera->flags = DEFAULT_FLAGS; camera->preview_list_mutex = g_mutex_new (); - camera->image_tags_mutex = g_mutex_new (); + camera->image_capture_mutex = g_mutex_new (); /* capsfilters are created here as we proxy their caps properties and * this way we avoid having to store the caps while on NULL state to @@ -1273,14 +1275,14 @@ gst_camera_bin_image_src_buffer_probe (GstPad * pad, GstBuffer * buf, GstPad *peer; GstTagList *tags; + g_mutex_lock (camerabin->image_capture_mutex); + /* Push pending image tags */ - g_mutex_lock (camerabin->image_tags_mutex); if (camerabin->image_tags_list) { tags = camerabin->image_tags_list->data; camerabin->image_tags_list = g_slist_delete_link (camerabin->image_tags_list, camerabin->image_tags_list); - g_mutex_unlock (camerabin->image_tags_mutex); GST_DEBUG_OBJECT (camerabin, "Pushing tags from application: %" GST_PTR_FORMAT, tags); if (tags) { @@ -1289,7 +1291,6 @@ gst_camera_bin_image_src_buffer_probe (GstPad * pad, GstBuffer * buf, gst_object_unref (peer); } } else { - g_mutex_unlock (camerabin->image_tags_mutex); GST_DEBUG_OBJECT (camerabin, "No tags from application to send"); } @@ -1303,8 +1304,10 @@ gst_camera_bin_image_src_buffer_probe (GstPad * pad, GstBuffer * buf, location); } else { GST_DEBUG_OBJECT (camerabin, "No filename location change to send"); + g_mutex_unlock (camerabin->image_capture_mutex); return ret; } + g_mutex_unlock (camerabin->image_capture_mutex); if (location) { evt = gst_camera_bin_new_event_file_location (location); @@ -1808,16 +1811,16 @@ gst_camera_bin_change_state (GstElement * element, GstStateChange trans) gst_tag_setter_reset_tags (GST_TAG_SETTER (camera)); GST_CAMERA_BIN2_RESET_PROCESSING_COUNTER (camera); + g_mutex_lock (camera->image_capture_mutex); g_slist_foreach (camera->image_location_list, (GFunc) g_free, NULL); g_slist_free (camera->image_location_list); camera->image_location_list = NULL; - g_mutex_lock (camera->image_tags_mutex); g_slist_foreach (camera->image_tags_list, (GFunc) _gst_tag_list_free_maybe, NULL); g_slist_free (camera->image_tags_list); camera->image_tags_list = NULL; - g_mutex_unlock (camera->image_tags_mutex); + g_mutex_unlock (camera->image_capture_mutex); g_mutex_lock (camera->preview_list_mutex); g_slist_foreach (camera->preview_location_list, (GFunc) g_free, NULL); diff --git a/gst/camerabin2/gstcamerabin2.h b/gst/camerabin2/gstcamerabin2.h index 27115bdf45..ec740a57f4 100644 --- a/gst/camerabin2/gstcamerabin2.h +++ b/gst/camerabin2/gstcamerabin2.h @@ -89,12 +89,12 @@ struct _GstCameraBin2 /* Index of the auto incrementing file index for captures */ gint capture_index; + GMutex *image_capture_mutex; /* stores list of image locations to be pushed to the image sink * as file location change notifications, they are pushed before * each buffer capture */ GSList *image_location_list; /* Store also tags and push them before each captured image */ - GMutex *image_tags_mutex; GSList *image_tags_list; /*