Page MenuHome

GSoC 2016 - Multiview Reconstruction - autotrack revisions
Needs RevisionPublic

Authored by Tianwei Shen (hlzz001) on Mar 21 2017, 6:06 PM.

Details

Summary

This is first sub patch of the GSoC 2016 project: Multiview Reconstruction.
This sub patch contains only changes related to the internal changes in Libmv, to accommodate multiple cameras. It adapts most the code in simple_pipeline and should be self-contained.

Diff Detail

Event Timeline

Sergey Sharybin (sergey) requested changes to this revision.Mar 29 2017, 11:41 AM
Sergey Sharybin (sergey) added inline comments.
intern/libmv/intern/reconstructionN.cc
258

Style is: } // namespace

intern/libmv/intern/reconstructionN.h
19

Picky: file headers are to be updated.

intern/libmv/intern/tracksN.cc
34 ↗(On Diff #8457)

This is the biggest concern of the change.

This was exact idea of Marker::clip, so you can get all correspondences by iterating track's markers.

Adding extra entity for that is an unnecessary redundancy.

intern/libmv/libmv/autotrack/reconstruction.h
54

Why exactly we need to go from class to struct?

intern/libmv/libmv/autotrack/tracks.cc
219

Track covers markers from multiple clips. So don't see how you can set clip to a track.

This revision now requires changes to proceed.Mar 29 2017, 11:41 AM
Tianwei Shen (hlzz001) marked 3 inline comments as done.
Tianwei Shen (hlzz001) requested review of this revision.
Tianwei Shen (hlzz001) added inline comments.
intern/libmv/intern/reconstructionN.h
19

Do you mean the date?

intern/libmv/intern/tracksN.cc
34 ↗(On Diff #8457)

Yes, this is useless and needs to be removed.

intern/libmv/libmv/autotrack/reconstruction.h
54

I did this may because they are simple collections of data fields without methods. Or do we use them as class with public data members?

intern/libmv/libmv/autotrack/tracks.cc
219

Here 'clip number' actually means the total number of clips this Tracks structure is concerned with.

Tianwei Shen (hlzz001) marked 2 inline comments as done.Mar 29 2017, 6:35 PM
Tianwei Shen (hlzz001) updated this revision to Diff 8516.
Sergey Sharybin (sergey) requested changes to this revision.Apr 14 2017, 4:10 PM

Other round of review as requested in IRC..

This change is still adding clip number to the Tracks, which (as was already mentioned) causes redundant information because clip is already inside of Marker. This doesn't seem to be a left-over from previous version, because it is actually used in the solver.

I'm also not fully following the concept of intrinsics_map. Can you explain a bit what it is and why is it needed?

This revision now requires changes to proceed.Apr 14 2017, 4:10 PM

I get your point. It seems that GetClipNum and SetClipNum can be replaced by MaxClip. I will do this revision.

The idea of intrinsics_map is to let each frame has its own intrinsic parameters, which facilitates the idea of varying focal lengths. Also, movie clips can be captured with different cameras with different intrinsics. For now it is not fully functional, but it takes care of these possibilities.

This revision now requires changes to proceed.Mar 28 2019, 11:41 PM