deepinsight / insightface

State-of-the-art 2D and 3D Face Analysis Project
https://insightface.ai
23.61k stars 5.44k forks source link

parameter swapped in SimilarityTransform's estimate method #1002

Open photoszzt opened 5 years ago

photoszzt commented 5 years ago

From the documentation(https://scikit-image.org/docs/dev/api/skimage.transform.html#skimage.transform.SimilarityTransform), I notice that the order given to estimate is swapped.

https://github.com/deepinsight/insightface/blob/4a4b8d03fec981912fdef5b3232a37a827cbeed6/src/common/face_preprocess.py#L61-L72

Is there any reason to swap the dst and src? Is that a mistake? If it's intended, it would be more clear if it's written as estimate(src=dst, dst=src) to avoid any confusion.

nttstar commented 5 years ago

Yes we have this problem from the beginning. We can have a warper function called norm_crop to hide this coding error inside.

photoszzt commented 5 years ago

Is that mean estimate(src=dst, dst=src) is the right order to pass in parameter?

golunovas commented 5 years ago

It seems like there is a confusion with namings. src array should be called dst because it contains positions that we want to achieve after the transformation. and following the same logic, dst array should be called src.