Optional face detection with gocv #35
Conversation
…698500' Restore pieces of deleted face detection code, to its new location.
|
This looks fairly promising, thanks! Will try it out, see how it goes and provide feedback here. |
smartcrop.go
Outdated
| @@ -54,7 +54,7 @@ var ( | |||
|
|
|||
| const ( | |||
| detailWeight = 0.2 | |||
| skinBias = 0.01 | |||
| skinBias = 0.9 | |||
There was a problem hiding this comment.
This probably isn't the best idea, as it changes the current behavior (without face detection). We should introduce a separate bias here, maybe.
There was a problem hiding this comment.
Yes, that's a valid concern. I ended up with this since now the score function does not know whether skin or face was detected. Would it be useful to be able to override these values, or are the same values useful for all use cases? If it is useful some kind of API for providing overrides could solve this problem.
There was a problem hiding this comment.
Maybe each Detector should contain & provide its own bias, which the user could then manipulate, e.g. by manually initializing a Detector.
gocv/face.go
Outdated
| height := face.Dy() | ||
|
|
||
| if d.DebugMode == true { | ||
| log.Printf("Face: x: %d y: %d w: %d h: %d\n", x, y, width, height) |
There was a problem hiding this comment.
We should think about injecting the Logger here.
smartcrop.go
Outdated
| return &smartcropAnalyzer{Resizer: resizer, logger: logger} | ||
|
|
||
| // Set default detectors here | ||
| detectors := []Detector{ |
There was a problem hiding this comment.
This is a nice idea, but requires a bit more restructuring, I think. For example, if you initialize smartcrop with only the FaceDetector, the debug output looks a bit funky.
There was a problem hiding this comment.
Yeah, when this goes public the interface needs to be a bit stricter. I witnessed that by unintentionally changing the order of detectors, and got my detected face overwritten by the artist formerly known as edgeDetect. I'll try to figure something out.
|
Thanks for your feedback, I'll try to resolve the issues. |
|
@svkoskin If there's anything I can do to help, let me know! |
This reverts commit a3a40ff.
smartcropAnalyzer will soon have the configuraed detectors as its members, and a method can use them without having to pass them as parameters
EdgeDetector's results are handled a bit differently in the scoring phase, so introduce a new interface and a field for it
Revert skin bias to the value in master branch that users are depending on. This reverts commit e15f810.
…ctangle, like it used to before Detector interface changes
…fferent signature in current gocv
|
@muesli As usual, I have had a bit limited time for actually implementing the fixes. Detaching bias seemed to require quite a big refactoring, but I added providing bias and weights to Detector's responsibilities. Making detections [][]uint8-based made it a bit easier to detach things from each other. Nevertheless, without attaching the face detector this should now return same crops as the version in |
|
I have not forgotten about this! Just need to find some time to fix up the CI and integrate all your changes - sorry it's been taking so long! |
|
Any update on this? |
|
@Nazdroth I'm currently attempting to migrate the CI stuff over to GitHub actions for a bunch of my projects - this among them. This should hopefully make it easier to fix up the CI with the required dependencies. As @svkoskin mentioned themselves, this probably still requires a bit of work inside smartcrop to decouple the bias and weighting process. Other than that still a fantastic PR, I just need to find the time to tie up the loose ends. Actually looking forward to that, though! |
I restored face detection features using another set of bindings for OpenCV, called gocv. It should still be possible to build smartcrop without having OpenCV installed. I have no particular interest in maintaining or supporting a fork, so if there is any chance for this to get merged, I'd appreciate it 😸