8000 Don't re-open files when reading and writing matrices by SomeAlphabetGuy · Pull Request #1926 · colmap/colmap · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Don't re-open files when reading and writing matrices #1926

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

Merged
merged 2 commits into from
May 19, 2023

Conversation

SomeAlphabetGuy
Copy link
Contributor

The previous version opens the matrix file in text mode, reads or writes the header, then re-opens the file in binary mode to read or write the data. This can cause issues on some network filesystems that do not guarantee immediate availability of the file after it is initially written. On such filesystems the Write() method may not see the file after writing the header, which results in overwriting the header instead of appending to it.

After this change the file is opened only once using binary mode. This is OK because the header does not contain any newline characters or non-ASCII characters.

The previous version opens the matrix file in text mode, reads or writes the header, then re-opens the file in binary mode to read or write the data. This can cause issues on some network filesystems that do not guarantee immediate availability of the file after it is initially written. On such filesystems the Write() method may not see the file after writing the header, which results in overwriting the header instead of appending to it.

After this change the file is opened only once using binary mode. This is OK because the header does not contain any newline characters or non-ASCII characters.
@ahojnnes ahojnnes merged commit 9eddf2e into colmap:dev May 19, 2023
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
0