-
-
Notifications
You must be signed in to change notification settings - Fork 824
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
GPS alignment from exif metadata #1069
Conversation
@@ -200,7 +213,7 @@ class View | |||
*/ | |||
bool isPoseIndependant() const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fabiencastan I can fix the typo in this one as well but I'd rather do it before merging to avoid introducing too much noise, as there are quite few instances to change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you mean after merging?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, just before the PR is ready to be merged, i can make the refactoring. Just to avoid noise in the review
@@ -32,6 +33,18 @@ enum class EEXIFOrientation | |||
, UNKNOWN = -1 | |||
}; | |||
|
|||
struct GPSExifTags |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fabiencastan I don't know if it is the correct or proper way to do it, I just wanted to avoid repeating the raw strings for the GPS tags in many parts of the code (better maintainability).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would keep it shorter:
struct GPSExifTags
{
static const std::string latitude = "GPS:Latitude";
...
```
And "static const std::string" could be replaced more properly by "constexpr std::string" with C++20.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes I thought about that at first, the only thing is that you cannot initialize the static member inline, you have to do it in the cpp. I went for the functions to avoid any sort of problems with the initialization etc
But I can go back to it if it is better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're right. We will change it one day when we will move to C++20 ;)
Need to add "from_gps" option on the meshroom side too. |
Co-authored-by: Fabien Castan <fabcastan@gmail.com> [numeric] fix double Co-authored-by: Fabien Castan <fabcastan@gmail.com>
Shall I also change the software (minor) version for SfMTransform? |
@fabiencastan @simogasp @natowi when is the next release planned? I actually really need this feature. I attempted to compile the |
Description
It introduces another option to sfmTransform to align an existing sfmdata to the gps position available as exif metadata.
It computes the closest transformation that align the existing sfmdata to the GPS reference system (cartesian x,y,z ref system)
Features list
View
to check for the gps data