Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement mtcnn detect method #10

Merged
merged 4 commits into from
Jun 25, 2019
Merged

Conversation

timesler
Copy link
Owner

@timesler timesler commented Jun 19, 2019

Instead of modifying the main detection-recognition pipeline that the repo was built for, I've exposed an additional method (MTCNN.detect(self, img)) that returns the bounding boxes and detection probabilities. The MTCNN forward method has the same functionality as before, but it now calls the new detect method to get bounding boxes. The main motivation for this is that I want to keep the main path through the code as easy to use as possible, while allowing customisations for those happy to dig a little deeper (though not much).

I think this structure makes sense for most use cases. For more general applications of MTCNN, such as face tracking, the new detect method should be ideal, and can be used in the following way (example also added to the docstring):

from PIL import Image, ImageDraw
from facenet_pytorch import MTCNN, extract_face
mtcnn = MTCNN(keep_all=True)
boxes, probs = mtcnn.detect(img)

# Draw boxes and save faces
img_draw = img.copy()
draw = ImageDraw.Draw(img_draw)
for i, box in enumerate(boxes):
    draw.rectangle(box.tolist())
img_draw.save('annotated_faces.png')

A combination of the MTCNN.detect method and the facenet_pytorch.extract_face function can be used to replicate the forward method, but gives more flexibility in terms of access to bounding boxes.

@ldulcic I've changed the way the bounding boxes are returned here a fair bit, apologies if it no longer allows you to do what you need. I wasn't able to make the changes directly onto your PR #9 since it was made from the master branch in your fork of the repo rather than a new feature branch (and I wasn't sure if you were using the fork for your own work). Let me know if this change works for your needs.

@codecov
Copy link

codecov bot commented Jun 19, 2019

Codecov Report

Merging #10 into master will increase coverage by 1.21%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #10      +/-   ##
=========================================
+ Coverage   98.09%   99.3%   +1.21%     
=========================================
  Files           4       4              
  Lines         577     577              
=========================================
+ Hits          566     573       +7     
+ Misses         11       4       -7
Impacted Files Coverage Δ
models/utils/detect_face.py 98.52% <100%> (-0.01%) ⬇️
models/mtcnn.py 100% <100%> (+4.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7a5077...55db1ad. Read the comment docs.

@timesler
Copy link
Owner Author

Closes #8
Closes #9

@timesler timesler added the enhancement New feature or request label Jun 19, 2019
@timesler timesler mentioned this pull request Jun 19, 2019
@timesler timesler merged commit 9cb20da into master Jun 25, 2019
@ldulcic
Copy link
Contributor

ldulcic commented Jun 26, 2019

Hi @timesler
Sorry for the late response, I was on vacation.
Thanks a lot for implementing this, these changes will work for me.

Just a thought, if you are planning on adding more face aligners in the future (other than MTCNN), maybe it would be a good idea to define a common interface for aligners where aligner forward method would return coordinates of bounding boxes and probabilities (same as detect now). Cropping images from bounding boxes and saving images could be implemented as some wrapper around aligner because it would be the same for every aligner. It may be overengineering to implement it now but once you add other aligner it would make sense. What do you think?

@timesler
Copy link
Owner Author

@ldulcic definitely a good idea in the future. In addition, it is probably worthwhile also returning facial landmarks in addition to the bounding boxes if we get around to implementing multiple detection algorithms. The landmarks can then be used to perform proper face alignment (affine transformations) following detection. I will create a placeholder issue for these enhancements so it's not forgotten.

Thanks for getting involved in the project!

@timesler timesler deleted the implement_mtcnn_detect_method branch August 17, 2019 01:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants