[Bug 704760] opencv: disparity-map calculation element
GStreamer (bugzilla.gnome.org)
bugzilla at gnome.org
Mon Aug 19 03:11:18 PDT 2013
https://bugzilla.gnome.org/show_bug.cgi?id=704760
GStreamer | gst-plugins-bad | 1.x
Sebastian Dröge (slomo) <slomo> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #250442|none |needs-work
status| |
--- Comment #12 from Sebastian Dröge (slomo) <slomo at circular-chaos.org> 2013-08-19 10:11:13 UTC ---
Review of attachment 250442:
--> (https://bugzilla.gnome.org/review?bug=704760&attachment=250442)
::: ext/opencv/gstdisparity.cpp
@@ +324,3 @@
+ GstStateChangeReturn ret = GST_STATE_CHANGE_SUCCESS;
+ GstDisparity *fs = GST_DISPARITY (element);
+ g_mutex_lock (fs->lock);
Better take the lock just around the lines that need it, someone might add
something else to the switch statement later
@@ +342,3 @@
+ transition);
+
+ g_mutex_lock (fs->lock);
Same here
@@ +373,3 @@
+ /* Critical section since both pads handle event sinking simultaneously
*/
+ g_mutex_lock (fs->lock);
+ GstVideoInfo info;
Put all variable declarations at the beginning of a block
@@ +387,3 @@
+ "format", G_TYPE_STRING, "RGB",
+ "width", G_TYPE_INT, fs->width,
+ "height", G_TYPE_INT, fs->height, NULL);
Use gst_video_info_to_caps() to create raw video caps
@@ +396,3 @@
+
+ GST_INFO_OBJECT (pad, " Negotiated caps via event, %" GST_PTR_FORMAT,
+ caps);
Put ret in the debug output too
@@ +402,3 @@
+ break;
+ }
+ ret = gst_pad_event_default (pad, parent, event);
You probably don't want to forward the event if above the caps equality check
has failed
@@ +418,3 @@
+ else
+ gst_query_set_caps_result (query,
+ gst_pad_get_current_caps (fs->srcpad));
This is leaking the return values of gst_pad_get_pad_template_caps() and
gst_pad_get_current_caps(), also needs locking
@@ +448,3 @@
+ filter = GST_DISPARITY (object);
+ gst_disparity_release_all_pointers (filter);
+ filter->width = filter->height = filter->actualChannels = 0;
Not necessary to reset these to 0 in finalize
@@ +469,3 @@
+ GST_DEBUG_OBJECT (pad, "processing frame from left");
+ if (fs->flushing)
+ return GST_FLOW_FLUSHING;
Check flushing after taking the mutex
@@ +476,3 @@
+ GST_DEBUG_OBJECT (pad, " right is free, continuing");
+ if (fs->flushing)
+ return GST_FLOW_FLUSHING;
Returning without unlocking the mutex
@@ +481,3 @@
+
+ if (!gst_buffer_map (buffer, &info, (GstMapFlags) GST_MAP_READWRITE)) {
+ return GST_FLOW_ERROR;
Returning without unlocking the mutex
@@ +504,3 @@
+ GST_DEBUG_OBJECT (pad, "processing frame from right");
+ if (fs->flushing)
+ return GST_FLOW_FLUSHING;
Check flushing after taking the mutex
@@ +511,3 @@
+ GST_DEBUG_OBJECT (pad, " left has just provided a frame, continuing");
+ if (fs->flushing)
+ return GST_FLOW_FLUSHING;
Returning without unlocking the mutex
@@ +587,3 @@
+
+ GST_DEBUG_OBJECT (pad, " right has finished");
+ fs->buffer_left = NULL;
Unmap and unref the left buffer
@@ +591,3 @@
+ g_mutex_unlock (fs->lock);
+
+ ret = gst_pad_push (fs->srcpad, buffer);
As the right buffer is always the one that is forwarded it would make sense to
proxy the allocation query from the right sinkpad to the srcpad
--
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.
More information about the gstreamer-bugs
mailing list