x,y swapped in flandmarkdetector.cpp

[berak]

flandmarkdetector.cpp, Line 1100: x and y are in reverse here (at the end):

face_img[INDEX(x, y, model->data.options.bw[1])] = (uint8_t)((resizedImage->imageData + resizedImage->widthStep*x)[y]);

it's not too bad, since the cropped region is a square

but am i right, that i can't just change it without also changing the training code and recalculating the flandmark_model.dat ?

btw, found this, while replacing the deprecated IplImages with cv::Mat ( i have to use flandmark with opencv master(3.0), and they made it impossible to use IplImages outside the library )

still, thanks a lot for coming up with this extremely useful library !

[uricamic]

Hi, thanks for notification.

I think it might work even without re-training, since as you mentioned the region is squared. However, it is just a hypothesis and it would require deeper check.

We are now developing new version of the library which is much more general - the set of landmarks and their relation in form of graph can be specified by the user. And this new version (which will be hopefully also published as an open source soon) is OpenCV independent, so there will be much easier compilation and installation. The OpenCV is there used just for examples.

[berak]

hi, thanks for the quick feedback.

"I think it might work even without re-training"

i'm getting different landmark points, if i swap x and y.

"We are now developing new version"

oh, nice, definitely looking forward to that !

[sthalik]

Is this the case that widthstep x/y is swapped too?

[berak]

oh, forgot to clarify: i'm only talking about this part:

((resizedImage->imageData + resizedImage->widthStep*x)[y]);

so, if i'm using a cv::Mat here instead, i'd write:

 resizedImage.at<uchar>(x,y);

and at least get the same resulting landmarks

[sthalik]

But in INDEX definition, row goes first. Are you sure? It actually works without retraining if both x/y pairs are swapped.

[berak]

ah, my bad. ofc. you're right about swapping both

[sthalik]

@uricamic can you merge the change, please?

[uricamic]

@sthalik Hi, I don't see any pull request

[berak]

well i can do one, if you want, but i can't decide if it's feasible.

[uricamic]

I see, it was my misunderstanding - I thought that you made some changes... Well, it is a question, I am now focused mainly on the new version of the library, where this problem due to independence on OpenCV does not arise at all...

[berak]

yes, i got that (new version on the way ++). that's why i was hesitating

so either just close it here (fine with me) or say 'do it!' and i'll push the button.

[uricamic]

Ok, closing.

[berak]

ok . looking forward to next version ;)