ImageReder new option and bug fix in GPS priors #1146
Conversation
src/base/image_reader.cc
Outdated
| // Read camera model and check for consistency if it exists | ||
| ////////////////////////////////////////////////////////////////////////////// | ||
| std::string camera_model = "Undefined"; | ||
| bitmap->ExifCameraModel(&camera_model); |
There was a problem hiding this comment.
If I read this code here correctly, then this would always select a single camera, if the camera_model == "Undefined"? This doesn't seem right. In case the camera_model is undefined, we want to instantiate a new camera for each image.
There was a problem hiding this comment.
We should also check whether we successfully retrieved the camera_model by checking the return value of ExifCamerModel
There was a problem hiding this comment.
Due to the "hashing" by make, model, focal length, and width+height the call to ExifCameraModel will always return something meaningful even if it's just the "WidthxHeight" descriptor. Thus, even for missing EXIF tags it will still only group together images with the same dimensions (which I think is fine in this case) and won't force a single camera for the whole model.
Returning false from ExifCameraModel when an EXIF tag is missing will break the setup since the cameras_ is a map indexed by camera_model so we do want this to always succeed for the AUTO mode to work correctly. Returning an empty string would essentially be forcing the use of a single camera model in the AUTO mode when EXIF tags are missing. But, maybe that's a more acceptable behavior; I'm open to handle this in whichever way makes most sense to you.
There was a problem hiding this comment.
Okay, so correct me if I am wrong, but the way I understand the current behavior: if only width_height are available, then it will use the same camera for all images that only have width_height EXIF fields and where they are equal. The behavior I want is that: if any of the make/model/focal/width/height fields are missing, instantiate a new/independent camera for that image. I really only want to group them to a single camera, if all the fields are present and their values match exactly.
There was a problem hiding this comment.
OK I see your take now; I can certainly do it this way. It will take a bit of restructuring but it will get there.
There was a problem hiding this comment.
Now the logic should work correctly; tested locally and results agreed with expectation of camera models. The map used to keep track of existing camera models now gets populated only if the camera model is valid, thus cameras where the model cannot be found will not get grouped together.
| std::string model_str; | ||
| std::string focal_length; | ||
| *camera_model = ""; | ||
| if (ReadExifTag(FIMD_EXIF_MAIN, "Make", &make_str)) { |
There was a problem hiding this comment.
I think each if block here should have an else { return false } and in the negative case also set camera_model = ""
| std::string model_str; | ||
| std::string focal_length; | ||
| *camera_model = ""; | ||
| if (ReadExifTag(FIMD_EXIF_MAIN, "Make", &make_str)) { |
There was a problem hiding this comment.
I think each if block here should have an else { return false } and in the negative case also set camera_model = ""
Merge from main repo
Merge from main
* ImageReder new option and bug fix in GPS priors * Add camera_mode option to feature_importer * Minor feature_importer update * Update Bitmap and CameraMode documentation * Fix typo in coment * Fix another small typo * Update image_reader.h * Update bitmap.cc * Update colmap.cc * Change camera mode handling in ImageReader * Update CameraMode documentation Co-authored-by: Antonios Matakos <anmatako@dss.microsoft.us> Co-authored-by: Johannes Schönberger <jsch@demuc.de>
Default behavior of
ImageReaderis now changed and it will attempt to reuse the same camera model if the model exif tag and image size remains the same. The old default of one camera per image can now be used through the explicit option--ImageReader.single_camera_per_image 1. Convenience input--camera_modewas added infeature_extractorto set the correct flags forImageReader.Fixed bug when reading GPS coordinates from EXIF tag where the Lat/Lon reference tag (N or S, and E or W) was being ignored resulting in incorrect GPS coordinates for locations outside the NE quadrant; coordinates were always positive instead of negative Lat for south hemisphere and negative Lon for West hemisphere.