Skip to content
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

Better colmap2nerf script #1051

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

VanIseghemThomas
Copy link

colmap2nerf overhaul

Today I was about to train my own dataset. For this you need to pass it through the colmap2nerf script but I encountered some problems with generating these.

File path handling

From what I understand you are able to run the script either:

  • from the image parent directory.
  • or define the the images/out directory using the --images and --out flags

The first one works like expected, but the problem here is that you need to run the script from the actual image parent directory, linking to the script by defining the absolute path. This is far from ideal.

The second one does not function like expected, because it fills the transforms.json file with incorrect file paths. These are also partly in the UNIX format and this messes things up in Windows. Another problem with this, is that you always need to end the --out flag with "transforms.json". If not, Python goes in error because it is trying to access a directory instead of a file.

Another problem I encountered was that colmap defaulted to use all threads, resulting in the system freezing due to using way to much resources. This was easily fixed by adding an argument defining the amount of threads.

What was fixed

  • Running the script is now possible from the instant-ngp root directory + allocation of threads resulting in stable system.
    Example usage:
python scripts\colmap2nerf.py --colmap_matcher exhaustive --run_colmap --aabb_scale
 8 --images .\data\nerf\custom\guitar\images\ --num_threads 6
  • File paths are now formatted correctly using os.path.normpath()
  • transform.json get's appended to the path given in --out
  • Added a "custom folder" that doesn't push to version control

When using the script, colmap generates some files inside the root directory. I don't think these are ment for version control so I added a patern to ignore these.

Fantastic project by the way! ;)

Without this, colmap will use up all available resources. This results in the system freezing due to no resources available. Adding this argument caps the resources and allows the system to function as normal.
Before you had to specify the path with "transform.json" at the end, else the script went in error. Now "transform.json" will be appended to the path resulting in it being created without specifying it every time.
All path locations are handled to put the transforms.json in the right location and populate the frames-object with the right image file_path. Also wrapped them is a os.normpath to ensure cross platform functionallity. This might not be necesarry but is a good assurance.
@Tom94
Copy link
Collaborator

Tom94 commented Nov 7, 2022

Hi there; thank you for the PR & thorough description!

Just a few minor comments:

  • Posix paths were intentional for portability reasons. You'd want to be able to zip up a dataset generated in Windows and be able to use it on Linux. Your contribution to make normalization consistent is great, but it should target posix rather than the OS-native format
  • It looks like the --out argument went away. I'm all for appending transforms.json to the argument if it happens to be a folder, but otherwise, in case where it worked, the behavior of colmap2nerf should stay identical to before. I agree that broken paths within the json need to be fixed, but I reckon this can be done with proper usage of os.path.relpath
  • Could you remove the gitignore entry for the custom folder again? In my opinion, if no version control for a generated dataset is desired, then that dataset should live outside of the repo. (The colmap-related entries are great, though!)

@VanIseghemThomas
Copy link
Author

Good point about the portability, didn't think of that! Regarding the appending of transforms.json, I could certainly write a check if for if the path links to a folder or a file.
The --out flag was done intentionally, since it always want's to live in the parent directory of images due to how the file path get's constructed. Might be me being dumb but at this point I don't see the point of having it in the first place. Correct me if I'm wrong by all means :p
Regarding the custom directory, that's a preference thing. But I'll remove it so you can use utilize this version ;)

@Tom94
Copy link
Collaborator

Tom94 commented Nov 7, 2022

Agree on all accounts.

I'm with you RE --out being somewhat redundant, but people have set up workflows that use colmap2nerf under the hood, and I don't want those to break. Imho, the correct approach is to fix the --out cases where the relative paths are constructed incorrectly rather than to remove the flag entirely.

@VanIseghemThomas
Copy link
Author

Sorry for the silence, was quite occupied the last few weeks. So I went and implemented the requested changes. The --out flag functions like before but is now optional. When empty, it will default to my implementation.
I kept the os.path.normpath for safety (it's something I always do to ensure proper paths) and in the end it gets converted to the posixpath. Finally I got rid of the custom folder entry in the gitignore file. So everything should be good now!

@Tom94 Tom94 force-pushed the master branch 3 times, most recently from b04280e to c1bd002 Compare January 8, 2023 08:34
@TanOneDeg
Copy link

Since I'm not that familiar with GitHub, I'd like to ask a question about the modified program from an end-user perspective and not as a programmer. Is that acceptable?

-JW:

@VanIseghemThomas
Copy link
Author

@TanOneDeg is that a question for me? If so, go ahead I'd say.

@TanOneDeg
Copy link

Finding help is difficult, so I appreciate your time. I hope you can help me out here. I am not saying that my workflow is the correct protocol. My o/s is Windows 10. My super old Linux laptops aren't capable of running instant-ngp. So, I downloaded instant-ngp binaries based on my RTX 3070 graphics card and installed the program on my D: drive. Oh, and I did not install anaconda and know nothing about it. In my workflow, I make a temporary work directory on my desktop, for example, G9Test, and place my A.mp4 video there, cd to that directory, then run the python example below from the Windows command prompt.

python D:\InstantNGPv1\scripts/colmap2nerf.py --video_in A.mp4 --video_fps 1 --run_colmap --aabb_scale 16

Once that's completed, I create a directory inside my work directory called data and place the image directory and transforms.json file into the data directory. I have a shortcut on my desktop to run Instant-ngp and drag the data directory into instand-ngp. So that's been working for me. I would like to use individual images taken with my DSLR instead of a video. I am trying to understand what the process would be to accomplish that with my workflow.

@VanIseghemThomas
Copy link
Author

You would use the --images flag instead of --video_in with colmap2nerf.py to create the dataset. Should be as simple as that if I understand correctly.

@TanOneDeg
Copy link

Thanks for taking the time to respond. I figured it out late last night after reading the documentation more carefully.

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.

3 participants