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

Feature: Enhance head pose estimation #464

Closed
saeidn95 opened this issue May 16, 2024 · 7 comments
Closed

Feature: Enhance head pose estimation #464

saeidn95 opened this issue May 16, 2024 · 7 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@saeidn95
Copy link

Changes to rotationMatrixToEulerAngle

Need for dropping the factor of 2 in return values (pitch, yaw and roll) in the function rotationMatrixToEulerAngle (

const rotationMatrixToEulerAngle = (r: number[]): { pitch: number, yaw: number, roll: number } => {
)

The formulation can be modified as follows;
var cosThetaZ = math.sqrt(r00 * r00 + _r20 * _r20);
// thetaZ = Math.asin(r10);
thetaZ = Math.tan2(r10, cosThetaZ);

what this change thetaX, thetaY, thetaZ, are within the range of -pi to +pi, and there is no need to scale the return values by 2.

@vladmandic
Copy link
Owner

why not submit a PR?

@saeidn95
Copy link
Author

I could not quite figure how to do that. Let me try again.

@vladmandic
Copy link
Owner

it would be better as then your name goes to contributors list. if you can't, I'll deal with it next week.

@saeidn95
Copy link
Author

Not sure if I did it right, but I did a pull request now.

@vladmandic
Copy link
Owner

sorry for a long delay, somehow this dropped off the radar.
anyhow, you did a pull request on your fork of this repo, you never submitted it to actual upstream (this) repo.
go to main...saeidn95:human:main and you can click create pr there.

but...your code is non-functional as it is:

var cosThetaZ = math.sqrt(r00 * r00 + _r20 * _r20);

there is no math, it should be Math and there are no _r20 values, so which ones you wanted to use?
i can test on my side and do the changes if you don't want to deal with pull request, but at give me a working code :)

@vladmandic vladmandic added question Further information is requested enhancement New feature or request labels Sep 11, 2024
@vladmandic vladmandic changed the title Head Pose Estimation Feature: Enhance head pose estimation Sep 11, 2024
@vladmandic vladmandic removed their assignment Sep 11, 2024
@saeidn95
Copy link
Author

Apologies for typos. Please replace _r20 with r20), The equation should be
var cosThetaZ = math.sqrt(r00 * r00 + r20 * r20);

You can double check that by looking at the formulation that you use in section 2.4 in reference https://www.geometrictools.com/Documentation/EulerAngles.pdf

Please go ahead and do the changes yourself. I would appreciate that. That saves doing a pull request on a code that is wong anyway!

BTW: is there a reason you use _r01, and _r02 instead of r01, and r02 in

const rotationMatrixToEulerAngle = (r: number[]): { pitch: number, yaw: number, roll: number } => {

@vladmandic
Copy link
Owner

BTW: is there a reason you use _r01, and _r02 instead of r01, and r02 in

underscore in front of variable visually indicates that variable is not actually used in the code.

Please go ahead and do the changes yourself. I would appreciate that

done, will be pushed to github soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants