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

[FIX][MOTION ESTIMATION] Fix error on homography calculation #278

Merged
merged 11 commits into from
Jan 30, 2024

Conversation

gfugante
Copy link
Contributor

@gfugante gfugante commented Sep 15, 2023

When estimating the motion transformation with the following code:

motion_estimator = MotionEstimator()
while True:
  frame = get_frame()
  detections = detect_frame(frame)
  mask = build_mask(frame, detection)
  coord_transformations = motion_estimator.update(frame=frame, mask=mask)

where frame can be any captured RGB frame and motion_estimation_mask is a mask representing all the bounding box of the people detected in the frame, the following two errors are sometimes raised:

cv2.error: OpenCV(4.8.0) /io/opencv/modules/calib3d/src/fundam.cpp:385: error: (-28:Unknown error code -28) The input arrays should have at least 4 corresponding point sets to calculate Homography in function 'findHomography'

and:

cv2.error: OpenCV(4.8.0) /io/opencv/modules/video/src/lkpyramid.cpp:1260: error: (-215:Assertion failed) (npoints = prevPtsMat.checkVector(2, CV_32F, true)) >= 0 in function 'calc'

This becomes increasingly frequent when there are lots of detections, making the tracker unusable.

The issue can be easily reproduced by istantiating the motion estimator as MotionEstimator(max_points=3), as the cv2.findHomography needs at least 4 points.

# error: (-28:Unknown error code -28) The input arrays should have
# at least 4 corresponding point sets to calculate Homography
# in function 'findHomography'
return True, None
Copy link
Collaborator

@aguscas aguscas Oct 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of returning None for the transformation when it couldn't be updated, I would prefer to return HomographyTransformation(self.data) so that at least the MotionEstimator still returns the coordinate transformation corresponding to the previous frame. So the last line should be

return True, HomographyTransformation(self.data)

@gfugante
Copy link
Contributor Author

Hi @aguscas! I tried to use the motion estimation a lot more and I found out that it breaks very easily even with the previous fix. Have a look at the new one! It's much more stable as it handles better the _get_sparse_flow function and the transformations_getter: on the second one the new handling is pretty strong, as I delete it and reinstantiate a copy, this has worked very well even in my usage code, where I delete the MotionEstimator when it break to instantiate a new one.

Anyways, the code is now stable even when you cover the camera, wave in front of it or have a mask that covers the entire frame. As said this is the best solution I found, it actually took a long time to get it and if you can break it please let me know how, also because touching these logics got me to Nan distances, and consequently to a very hard exit() in tracker.py, which is game over for the user.

@gfugante gfugante requested a review from aguscas October 15, 2023 10:16
@gfugante
Copy link
Contributor Author

Hi @aguscas! Did you have any time to look or test this?

Copy link
Collaborator

@aguscas aguscas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! I wrote a couple comments regarding the nan/inf error you mentioned. Sorry for the late responses by the way, didn't see the notification last time, so thanks for asking again for the review!

if np.isnan(new_points_transformed).any() or np.isinf(new_points_transformed).any():
new_points_transformed = np.array([[0, 0], [0, 0]])

return new_points_transformed
Copy link
Collaborator

@aguscas aguscas Nov 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch. I assume this is happening when dividing by the last column of the points_transformed array, since it might contain zero values (really weird case, but sure, can happen). Tell me if you think the source of the problem might be other, because I actually couldn't reproduce the nan or infs error.

Anyway, I'm not exactly convinced that this is the best way of fixing it if you get infinite values. The two main reasons I say that are

  1. You are just returning an array of two points, but take into account that the number of points that this function is receiving can be arbitrary. The expected shape of the output array should be the same as the input.
  2. The 'arbitrarily' chosen 0 coordinates for the output also doesn't sit well with me. We could instead just add some really small value to the last column of the points_transformed array whenever it has some zero entry, so that when we compute the quotient we are not getting a division by zero. I think at least that feels somewhat less arbitrary than just returning the zero point.

Could you please try the suggestion of my second point and check if you still can reproduce the nan/inf error or if that would also solve it?

if np.isnan(new_points_transformed).any() or np.isinf(new_points_transformed).any():
new_points_transformed = np.array([[0, 0], [0, 0]])

return new_points_transformed

def rel_to_abs(self, points: np.ndarray):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the same error that happened in abs_to_rel could also happen in rel_to_abs because of doing essentially the same computation but with the inverse matrix, so the fix should also be applied here. Is there any reason for not fixing it here that I am missing?

@aguscas aguscas self-requested a review January 29, 2024 18:21
@aguscas aguscas force-pushed the fix-homography-error branch from a8a14a2 to 18920ae Compare January 30, 2024 14:22
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