-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
calib3d: bindings-friendly solvePnPExtrinsicGuess() #14181
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
Conversation
Or maybe There is still an issue about how the logic behind the function works in my opinion.
will not use the initial opencv/modules/calib3d/src/solvepnp.cpp Lines 95 to 96 in b761ec0
Indeed,
In my opinion, a better way would be to have (not in the current plan since this break how
|
@catree Thank you for looking on this! I believe, longer name is not a problem with modern IDEs. In theory there are several different implementations of iterative algorithms. OpenCV just have this one at this moment. Possible new algorithms can be added via:
The second way is preferable if additionally we need to expose some algorithm parameters. The first way sometimes produce monsters like this with According the second way, I see these alternative:
There is similar problem with |
What are the options?
I like having one function with a flag to select the method. Otherwise, we could play safe and introduce a new method for each new approach and try to minimize duplicate code internally? For But if we have an initial guess, I would say it makes sense to use it also to determine the pose for the consensus step. Also, why not use the pose that allows getting the biggest consensus set and just refine the pose with an iterative algorithm and all the inliers? |
Well, this is the trade-off between api simplicity and exposure to method specific parameters. I agree it's too restrictive to assume refinement methods should have the same parameters. Furthermore, with the current api we have no exposure to LM refinement parameters, which is limiting. The most important parameters are stopping criteria and loss function (currently L2 but other robust losses are also used in the research literature to deal with outliers such as Huber or L1). I'm of course not suggesting different losses be implemented in this issue but it's worth knowing about. Stepping back, the fundamental division between pnp methods is whether they are for locally improving an initial pose estimate (a PnpRefiner) or not (a PnpSolver). As you've said, it doesn't make sense to use a PnPSolver e.g. IPPE or EPnP with a "use extrinsicGuess" option. Similarly it doesn't make sense to use a PnPRefiner with useExtrinsicGuess as an option. I would propose that Pnp methods be implemented as objects that inherit from two base classes (either PnPSolver or PnpRefiner). Method specific parameters would be passed at construction time. We nevertheless need to avoid sacrificing simplicity for users who don't want to worry about specific solvers or custom parameters. This could be done with an encapsulator that constructs and executes default PnPSolver and PnPRefiner objects (chosen depending on whether the points are planar etc.) with default parameters. |
@catree re "Also, why not use the pose that allows getting the biggest consensus set and just refine the pose with an iterative algorithm and all the inliers?" The only problem with that is when pose estimation is ambiguous such as planar cases. The pose that provides the biggest consensus is not necessary the correct one in those situations. |
@tobycollins I agree that using classes would provide more flexibility and cleaner interface, something similar to the I have proposed #14431 to add functions to refine a pose. This could be a first step toward something cleaner and useful. |
relates #12334
WIP
force_builders=linux,docs