Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Remove check in database_cache.cc that breaks backwards compatibility with image_list_file_path#3478

Merged
ahojnnes merged 7 commits into
colmap:maincolmap/colmap:mainfrom
pd-karoly-harsanyi:patch-image-list-backwards-compatibilitypd-karoly-harsanyi/colmap:patch-image-list-backwards-compatibilityCopy head branch name to clipboard
Jul 14, 2025
Merged

Remove check in database_cache.cc that breaks backwards compatibility with image_list_file_path#3478
ahojnnes merged 7 commits into
colmap:maincolmap/colmap:mainfrom
pd-karoly-harsanyi:patch-image-list-backwards-compatibilitypd-karoly-harsanyi/colmap:patch-image-list-backwards-compatibilityCopy head branch name to clipboard

Conversation

@pd-karoly-harsanyi

@pd-karoly-harsanyi pd-karoly-harsanyi commented Jul 11, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

If we try to load a pre-3.12 (pre sensor rig support) db, and we also pass --image_list_path, the db loading will fail mainly due to the lines I am removing in this PR. Not sure why we have those in there.

The error that pops up without this change:

I20250711 11:43:48.726486 140208169588800 incremental_pipeline.cc:253] Loading database
I20250711 11:43:48.728317 140208169588800 database_cache.cc:66] Loading rigs...
I20250711 11:43:48.728328 140208169588800 database_cache.cc:76]  0 in 0.000s
I20250711 11:43:48.728333 140208169588800 database_cache.cc:84] Loading cameras...
I20250711 11:43:48.728343 140208169588800 database_cache.cc:102]  1 in 0.000s
I20250711 11:43:48.728346 140208169588800 database_cache.cc:110] Loading frames...
I20250711 11:43:48.728351 140208169588800 database_cache.cc:127]  0 in 0.000s
I20250711 11:43:48.728355 140208169588800 database_cache.cc:135] Loading matches...
I20250711 11:43:48.756288 140208169588800 database_cache.cc:140]  9644 in 0.028s
I20250711 11:43:48.756310 140208169588800 database_cache.cc:156] Loading images...
terminate called after throwing an instance of 'std::out_of_range'
  what():  unordered_map::at
*** Aborted at 1752227028 (unix time) try "date -d @1752227028" if you are using GNU date ***
PC: @     0x7f84c62ef00b signal

The actual error is coming from here:

        const frame_t frame_id1 = image_to_frame_id.at(image_id1);
        const frame_t frame_id2 = image_to_frame_id.at(image_id2);

line 205, 206

@sarlinpe

sarlinpe commented Jul 11, 2025

Copy link
Copy Markdown
Member

Thank you for the report! This was added in #3419, I'm not sure why @ahojnnes -- we might need to patch 3.12 again

@pd-karoly-harsanyi pd-karoly-harsanyi changed the title Remove check in database_cache.cc that break backwards compatibility with image_list_file_path Remove check in database_cache.cc that breaks backwards compatibility with image_list_file_path Jul 11, 2025
@B1ueber2y

B1ueber2y commented Jul 11, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Hey. Thanks for reporting! Could you share the database and the command? I cannot reproduce this error with my legacy database, which seems to work fine for me. Are you sure that your image is not missing in the image_list_path?

@B1ueber2y

B1ueber2y commented Jul 11, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

I see the issue now. It happens when you specify image_list_path and has only partial set of images in the image_names. Rather than removing the check, I would move this line up before the check, which simply becomes:

180         image_to_frame_id.emplace(image.ImageId(), image.ImageId());

Indeed, this seems to be a big issue in terms of backwards compatibility and needs attention.

@pd-karoly-harsanyi

pd-karoly-harsanyi commented Jul 13, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

So, the else block would look like this?

else {
      for (auto& image : images) {
        image_to_frame_id.emplace(image.ImageId(), image.ImageId());

        if (!image_names.empty() && image_names.count(image.Name()) == 0) {
          continue;
        }
        // For backwards compatibility with old databases from before having
        // support for rigs/frames, we create a frame for each image.
        class Frame frame;
        frame.SetFrameId(image.ImageId());
        frame.SetRigId(image.CameraId());
        frame.AddDataId(image.DataId());
        image.SetFrameId(frame.FrameId());
        frames_.emplace(frame.FrameId(), std::move(frame));
      }
    }

I changed the code as suggested. I re-ran the code that failed here, and the issue seems resolved.

@B1ueber2y
B1ueber2y requested a review from ahojnnes July 13, 2025 21:50
ahojnnes
ahojnnes approved these changes Jul 14, 2025
View reviewed changes

@ahojnnes ahojnnes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, LGTM.

@ahojnnes

ahojnnes commented Jul 14, 2025

Copy link
Copy Markdown
Contributor

If at all possible, it can be good to have a unit test for this bug.

@ahojnnes

ahojnnes commented Jul 14, 2025

Copy link
Copy Markdown
Contributor

If at all possible, it can be good to have a unit test for this bug.

I added a unit test.

@ahojnnes
ahojnnes enabled auto-merge (squash) July 14, 2025 12:11
@ahojnnes

ahojnnes commented Jul 14, 2025

Copy link
Copy Markdown
Contributor

Thank you for the fix.

@ahojnnes
ahojnnes disabled auto-merge July 14, 2025 12:20
@ahojnnes
ahojnnes enabled auto-merge (squash) July 14, 2025 12:22
@ahojnnes
ahojnnes merged commit 65982d2 into colmap:main Jul 14, 2025
14 checks passed
ahojnnes added a commit that referenced this pull request Jul 16, 2025
@pd-karoly-harsanyi @B1ueber2y @ahojnnes
Remove check in database_cache.cc that breaks backwards compatibility…
217bebd 
… with image_list_file_path (#3478)

If we try to load a pre-3.12 (pre sensor rig support) db, and we also
pass `--image_list_path`, the db loading will fail mainly due to the
lines I am removing in this PR. Not sure why we have those in there.

The error that pops up without this change:
```
I20250711 11:43:48.726486 140208169588800 incremental_pipeline.cc:253] Loading database
I20250711 11:43:48.728317 140208169588800 database_cache.cc:66] Loading rigs...
I20250711 11:43:48.728328 140208169588800 database_cache.cc:76]  0 in 0.000s
I20250711 11:43:48.728333 140208169588800 database_cache.cc:84] Loading cameras...
I20250711 11:43:48.728343 140208169588800 database_cache.cc:102]  1 in 0.000s
I20250711 11:43:48.728346 140208169588800 database_cache.cc:110] Loading frames...
I20250711 11:43:48.728351 140208169588800 database_cache.cc:127]  0 in 0.000s
I20250711 11:43:48.728355 140208169588800 database_cache.cc:135] Loading matches...
I20250711 11:43:48.756288 140208169588800 database_cache.cc:140]  9644 in 0.028s
I20250711 11:43:48.756310 140208169588800 database_cache.cc:156] Loading images...
terminate called after throwing an instance of 'std::out_of_range'
  what():  unordered_map::at
*** Aborted at 1752227028 (unix time) try "date -d @1752227028" if you are using GNU date ***
PC: @     0x7f84c62ef00b signal
```

The actual error is coming from here:
```
        const frame_t frame_id1 = image_to_frame_id.at(image_id1);
        const frame_t frame_id2 = image_to_frame_id.at(image_id2);
```
line 205, 206

---------

Co-authored-by: Shaohui Liu <b1ueber2y@gmail.com>
Co-authored-by: Johannes Schönberger <jsch@demuc.de>
Co-authored-by: Johannes Schönberger <jsch@meta.com>
@BrewTestBot BrewTestBot mentioned this pull request Nov 7, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ahojnnes ahojnnes ahojnnes approved these changes
@B1ueber2y B1ueber2y B1ueber2y approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@pd-karoly-harsanyi @sarlinpe @B1ueber2y @ahojnnes
Morty Proxy This is a proxified and sanitized view of the page, visit original site.