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

Update representation.py #1427

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dakotah-jones
Copy link

Add explicit "color_face" argument.

Tickets

#1426

What has been done

With this PR I've added color_face="bgr" to detection.extract_faces call.

Previously: When using a detector_backend the final image sent to the model for embedding was BGR" format.
Now: When using a detector_backend the final image sent to the model for embedding is "RGB" format.

How to test

make lint && make test

Add explicit "color_face" argument.
@serengil
Copy link
Owner

I think we should change the skip case's behaviour, not default branch. All thresholds are tuned for default behaviour.

@dakotah-jones
Copy link
Author

I think that would depend on the expected input of embedding = model.forward(img). (here)

Is the expected input to the model supposed to be RGB or BGR format?
Please clarify this for me.

If img is expected to be in BGR format, then I agree with you and I'll update the PR.
If img is expected to be in RGB format, then I disagree. The default case is incorrectly sending in img as BGR.

@dakotah-jones
Copy link
Author

dakotah-jones commented Jan 15, 2025

The comments seem to suggest that img should be in RGB format when the model is called. (here)

@serengil
Copy link
Owner

Then, that comment should be fixed, too.

That line was updated in this PR. I missed it.

#1402

@dakotah-jones
Copy link
Author

For clarification sake.
Is the expected input to the model supposed to be RGB or BGR format?

It seems as though you are inferring that img sent for embedding should be in BGR format.

I ask because this is the opposite of my understanding.
I'm using this to generate FaceNet image embeddings. It's my understanding that FaceNet was trained on 160x160 images in RGB format.
It then seems strange that we would be calculating the embeddings on images in BGR format

@dakotah-jones
Copy link
Author

To be clear, I'm talking about the final state of img, not the initial state of the image loaded in.

@serengil
Copy link
Owner

BGR

@dakotah-jones
Copy link
Author

dakotah-jones commented Jan 15, 2025

Thank you for the clarification.

The changes have been reverted.
The skip case has been updated.
The comment you mentioned from #1402 has been reverted.

@serengil
Copy link
Owner

Did you run the tests after your change? Seems tests are broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants