Add frame impl for future rig support #2698
Conversation
|
Once we are happy with the class design, I could try to start removing the use of |
|
Thanks @B1ueber2y. On the high-level, this looks good to me. I suggest to also gain @sarlinpe's feedback 8000 before we move ahead. Cheers. |
Thanks. @sarlinpe PTAL when time permits : ) |
There was a problem hiding this comment.
Looks great to me, some final comments from my side.
In terms of next steps, I was hoping that, in addition to the IMU/frame BA, we can also adjust database and reconstruction I/O and then also incremental mapper to support rigs. This will be a longer term project. It may make sense to catch up in person before embarking on this journey. :-)
Thanks for all the comments. I have just addressed them all and added some tests. Actually after thinking about it more, the next step would actually already be to adjust database and reconstruction I/O rather than to support the BA. We would love to store the rig calibration ( On the other hand, do you think we should first merge this PR as a quote and then develop the I/O and BA, and do it in separate PRs one after another? Yes. Supporting incremental mapper and global mapper on top of rigs will be very exciting but effort demanding. Lets find some time to catch up in person and discuss in more details : ) |
|
As for next steps, I suggest decoupling changes to I/O format from rig support, unless we think that it's fundamentally not possible to do this with the existing custom format. I am all in for protobuf serialization but want to be very careful here in the sense that I don't want to break existing users. Ideally, we can be fully backwards compatible to existing models. I guess we could create a new frames.bin file and a new rigs.bin file. The existing cameras/images/points3D could stay identical? |
Sounds good. We can do either of the following two regarding the frame association in the I/O:
|
|
I think it is fine to let images have no idea about their association to rigs/frames. Frames are the concept that ties together sensor measurements (ie images) and rig calibrations. Do you see a problem with that other than having to read frame and rig files first, so we can correctly populate the data structures? |
Sounds good. This will the next step, and should not be that difficult. I will find time for this but probably only next week. The short-term roadmap for this will be:
We could discuss about long-term rig sfm offline : ) |
A draft implementation of the frame class for the rig support, with backward compatibility. However, with the new design CamFromWorld() wont make sense as a reference anymore when the frame is nontrivial, where we need to composite cam_from_world from the rigs rather than directly using the member variable. I currently throw error on this, which will potentially break a lot of implementations with CamFromWorld() when we really use the rig as input (which should break since they dont currently support rigs).
Feedbacks on this will be helpful. Marked as draft for now. @ahojnnes @sarlinpe