[Bug 629244] [opencv] Add motion detection element

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=629244
  GStreamer | gst-plugins-bad | git

Stefan Kost (gstreamer, gtkdoc dev) <ensonic> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #192016|none                        |reviewed
             status|                            |

--- Comment #137 from Stefan Kost (gstreamer, gtkdoc dev) <ensonic at sonicpulse.de> 2011-07-18 21:35:54 UTC ---
Review of attachment 192016:
 --> (https://bugzilla.gnome.org/review?bug=629244&attachment=192016)

A few random comments. It would really be good to just run gst-indent (from
core) over the code before making the patch too.

::: orig/ext/opencv/gstmotioncells.c
@@ +390,3 @@
+    case PROP_GRID_X:
+      retgridx = gst_element_get_state (GST_ELEMENT (filter),
+          &filter->state, &filter->pending, 250 * GST_NSECOND);

If you don't need pending, you can pass NULL. Also it seems that instead of
filter->state you could use a local variable.

@@ +398,3 @@
+        filter->prevgridx = filter->gridx;
+      } else {
+        filter->prevgridx = filter->gridx;

If you assign filter->prevgridx = filter->gridix in both cases, I would move it
out of the branches.

@@ +493,3 @@
+     if (tmpux > -1 && tmpuy > -1 && tmplx > -1 && tmply > -1) {
+       filter->motionmaskcoords =
+           (motionmaskcoordrect *) g_malloc (filter->motionmaskcoord_count *

you can use g_new for better readability here.

@@ +506,3 @@
+       filter->motionmaskcoord_count = 0;
+     }
+   }

indentation seems to be off.

@@ +550,3 @@
+      sscanf (colorstr[2], "%d", &b);
+      //check right RGB color format
+      (r < 0) ? r = 0 : (r > 255) ? r = 255 : 0;

r=CLAMP(r, 0, 255)

@@ +683,3 @@
+        else
+          g_string_append_c (str, ',');
+        g_string_append (str, substr->str);

just do:
  g_string_append_printf(str, "%d:%d", filter->motionmaskcellsidx[i].lineidx,
                filter->motionmaskcellsidx[i].columnidx);

-- 
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.