Skip to content

Conversation

iclemens
Copy link

#2129

The proposed change would convert the training image to grayscale AFTER running the face detector. The default face detector already converts the images to grayscale and a custom detector should be able to decide if it want to use grayscale/color images.

I've kept the conversion itself as I'm not sure if the actual training requires grayscale images.

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

Rect box = getBBox(img, _shape);

if(img.channels()>1){
cvtColor(img,img,COLOR_BGR2GRAY);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps, it makes sense to convert color of cropped image only. Update the code below:

Mat crop = img(Rect((int)min_x, (int)min_y, (int)w, (int)h)).clone();

(BTW, avoid using of in-place src/dst parameters - they cause performance regressions)

Copy link
Author

@iclemens iclemens May 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I completely agree... as you can see I just moved some existing code around (i.e. tried to change as little as possible).

To be honest, I don't really get why the face detector needs to be called from this code in the first place. It would make more sense to me if the face rectangles were passed from the calling code as is done when fitting:

facemark = cv2.face.createFacemarkLBF()
facemark.fit(image, faces)

That would probably also make it easier to do the training from Python (#2071), as the main issue here seems to be that passing a function from Python to C++ is difficult.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it is OK to make API more friendly for Python or other bindings.

@iclemens iclemens changed the base branch from master to 3.4 May 16, 2021 02:04
@iclemens iclemens force-pushed the lbf_dont_convert_to_grayscale branch from 66f3da6 to f91b4d8 Compare May 16, 2021 02:04
@opencv-pushbot opencv-pushbot merged commit e0cfc92 into opencv:3.4 May 19, 2021
@alalek alalek mentioned this pull request May 23, 2021
@alalek alalek mentioned this pull request Jun 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants