Created attachment 181884 [details] [review]
always pass a 320x280 image to cvHaarDetectObjects ()

This is an idea I tested for improving facedetect performance in resolutions higher than 320x280. It always passes a 320x280 image to cvHaarDetectObjects () and the performance improvement in 640x480 or 800x600 is very noticeable.

Good idea. What about making it configurable with two gobject properties?

I have more idea how can be faster facedetect, my modification reduce cpu usage with 50 percent. I can send my patch if you needs it.

Of course, please upload it so I can take a look.

Hi Guys,

I fixed and tuning facedetect and faceblur elements.
I add a new bus msg for both of two elements if element couldnt load profile file,it send a bus msg what value is this : profile_load_failed it send this msg once.

after my modification these elements works fine with lower CPU usage with this pipeline :

gst-launch -m v4l2src ! videoscale ! video/x-raw-yuv,width=320,height=240 ! ffmpegcolorspace ! facedetect/faceblur ! ffmpegcolorspace ! xvimagesink

I attached my patches for these elements.

I'm waiting for your response.

Enjoy :)

Created attachment 186929 [details] [review]
facedetect and faceblur speed up patch

increase scale factor
use canny prunning
change profile
increase min size
add profile load failed bus msg

Sorry something small thing missing previous patch. 
I fixed and send again the latest patch.

Created attachment 186932 [details] [review]
facedetect and faceblur fixed speed up patch

fixed profile load failed bus msg

Laura any response ? 

or someone else ? 

Robert

Hi,

I tested Robert patch and it works fine for me,

Nicola

Review of attachment 186932 [details] [review]:

::: ext/opencv/gstfaceblur.c
@@ +397,3 @@
+     GstMessage *m = gst_message_new_element (GST_OBJECT (filter), s);
+	 gst_element_post_message (GST_ELEMENT (filter), m);
+	 filter->sent_profile_load_failed_msg = TRUE;

I think you could just use GST_ELEMENT_WARNING() here, something like this (please review the actual message and chage as appropriate):

GST_ELEMENT_WARNING (filter, RESOURCE, NOT_FOUND,
  (_("Profile %s is missing.",profile_name)),
  ("missing facedetect profile file %s",profile_name);

This will send a warning message and log a warning (with the 2nd text) as well.

Review of attachment 186932 [details] [review]:

::: ext/opencv/gstfaceblur.c
@@ +70,3 @@
 #define GST_CAT_DEFAULT gst_faceblur_debug
 
+#define DEFAULT_PROFILE "/usr/share/doc/opencv-doc/examples/haarcascades/haarcascades/haarcascade_frontalface_alt2.xml"

I think this should be:
/usr/share/opencv/haarcascades/haarcascade_frontalface_alt2.xml

Review of attachment 186932 [details] [review]:

::: ext/opencv/gstfaceblur.c
@@ +118,3 @@
+  };
+  *id = g_flags_register_static ("GstOpencvFaceDetectFlags", values);
+

*id = g_flags_register_static ("GstOpencvFaceBlurFlags", values);

otherwise you'll get
gst-plugin-scanner:7672): GLib-GObject-WARNING **: cannot register existing type `GstOpencvFaceDetectFlags'

please also rename the flags itself.

Stefan,

Thanks your guide.

It's an old code.
I learned/gave lot of useful advice about gstreamer for you recently,
so I'll update this patch too.

Robert

Test Pipeline:

gst-launch-0.10 -m v4l2src ! video/x-raw-yuv,width=320,height=240,framerate=20/1 ! ffmpegcolorspace ! facedetect profile=/usr/local/share/opencv/haarcascades/haarcascade_frontalface_alt2.xml display=true ! ffmpegcolorspace ! xvimagesink

Test notebook: Asus K42Jv

Without my patch

works all 4 thread about 70-80 % 

and display play ~ 1 fps :(

With my patch:

one thread ~ 25 % 
and other three thread ~ 15 %

Created attachment 192905 [details] [review]
facedetect and faceblur speed up patch2

Review of attachment 192905 [details] [review]:

::: ext/opencv/gstfacedetect.c
@@ +158,1 @@
 

why those arbitrary resolutions? I don't think we want to force everyone to such a low resolution. I would just recommend to use it with videoscale and capsfilter in case of too high cpu load in a example.

@@ +323,1 @@
 }

if you just lock/unlock the mutex in the property setter you can also remove it. The setters never run from different processes at the same time (if the app is done right). We should only use mutexes if we need to ensure that the processing function is getting a coherent set of properties.

Right now I have to use:
gst-launch-0.10 <camerasrc> ! colorspace ! videoscale ! facedetect ! colorspace ! xvimagesink

if I just run
gst-launch-0.10 <camerasrc> ! colorspace ! facedetect ! colorspace ! xvimagesink

it does not show anything, but should actually fail to negotiate.

Created attachment 193097 [details] [review]
facedetect and faceblur speed up patch 3

Stefan,

I fixed these things what we discussed.

Pls check this patch.

Thanks.

Robert

Review of attachment 193097 [details] [review]:

Just one small question. Otherwise this is ready to go.

::: ext/opencv/gstfaceblur.c
@@ +137,2 @@
 static GstStaticPadTemplate src_factory = GST_STATIC_PAD_TEMPLATE ("src",
+    GST_PAD_SRC, GST_PAD_ALWAYS, GST_STATIC_CAPS ("video/x-raw-rgb "));

Is this change needed? (GST_STATIC_CAPS (GST_VIDEO_CAPS_RGB) -> GST_STATIC_CAPS ("video/x-raw-rgb "))

Of course no, this indentation change was in caps update, when I removed it I dont undo this. Sorry for that.

Hi,

I attached this little modification.

Robert

Created attachment 196163 [details] [review]
replace static caps

Review of attachment 193097 [details] [review]:

::: ext/opencv/gstfaceblur.c
@@ +53,3 @@
  * <title>Example launch line</title>
  * |[
+ * gst-launch-0.10 <camerasrc> ! ffmpegcolorspace ! faceblur ! ffmpegcolorspace ! xvimagesink

we have "autovideosrc" for this.

Robert, if you could update the patch, I'll apply it right away. Sorry for not getting back to this any earlier.

Hi Stefan,

No problem, I'll have time on this weekend and I can do that.
It's okay for you ? 

Robert

Yes, that will work. I hope we won't freeze before.

Robert, any progress on this? A new patch against latest git master of gst-plugins-bad would be good to have here

Created attachment 281892 [details] [review]
facedetect: faceblur: new properties for speed up

Updated for latest master, I didn't incorporate the change in the haar
profile file as I think the one named 'default' should be the default unless
someone give me a good reason for the change.

Review of attachment 281892 [details] [review]:

Looks good to me. I would suggest to split this though 1) Port gst 0.10 doc 2) send warning if cascade isn't found and 3) Well this one, but with a more descriptive comment.

::: ext/opencv/gstfaceblur.c
@@ +354,3 @@
+          ("missing faceblur profile file %s", filter->profile));
+      filter->sent_profile_load_failed_msg = TRUE;
+    }

This is slightly unrelated, could have been a seperate patch.

Updated and pushed. Thanks for the review.
As I said I didn't update the default profile as there was no reasoning to do so. If there is one, please open a new bug about it.


commit bd3cefeecc732cb0aa376a508165bcd04e9f950f
Author: Robert Jobbagy <jobbagy.robert@gmail.com>
Date:   Wed Jul 30 16:32:01 2014 -0300

    faceblur: facedetect: add properties to faceblur
    
    Makes faceblur have the same properties as facedetect. Also improves
    the standard defaults and makes them the same in the 2 elements

commit 43a4498569f1939e409a08b88445cafe10e26f94
Author: Robert Jobbagy <jobbagy.robert@gmail.com>
Date:   Wed Jul 30 16:30:39 2014 -0300

    faceblur: post message when profile loading fails

commit 555d29681bd661cf8fd5d1681a695f2f2196874a
Author: Robert Jobbagy <jobbagy.robert@gmail.com>
Date:   Wed Jul 30 16:07:17 2014 -0300

    facedetect: faceblur: update launch line examples
    
    Reflect 1.0 and more useful examples