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.

Sign up for GitHub

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

Jump to bottom

Feature/refactor orb slam2 front end to accept arbitary data sources and data sinks #68

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

Feature/refactor orb slam2 front end to accept arbitary data sources and data sinks #68

wants to merge 28 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 6, 2018

Currently kitty data source , semantic data source is supported.
Following design is implemented.
l
slam_application_process

@ghost ghost mentioned this pull request Apr 6, 2018
Open
paul-michalik
paul-michalik requested changes Apr 9, 2018
View reviewed changes
ext/data_source.hpp Outdated

namespace ORB_SLAM2 {
namespace ext {

Copy link
Owner

Choose a reason for hiding this comment

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

The types of data source should not be known here at all. Assume we want to assist source implementers. Then, look at boost::iterator_facade pattern (= CRTP). One possibility would be to create a simplified façade for users to define own iterators for own data sources:

template<class TDerived>
class data_source_iterator_facade: public boost::iterator_facade<
    TDerived,
    ext::tuple<ext::time_point_t, ...> // etc...
    boost::forward_traversal_tag
> {
// ...
};

Then the implementation of a new data source would consist of implementing a new iterator using the data_source_iterator_facade<my_data_source_iterator>. However the benefits from this would be marginal, I'd prefer to expose iterator_facade directly.

Second possibility would be to provide a façade for a data source, i.e. data_source_facade<TDerived> which would basically implement a SinglePassRange concept. However I am not sure if implementers could benefit from this too much either.

Either of the possibilities would make sense if we provided a concept check for user implementation but even that would have to be a specialization of SinglePassRangeConcept

ext/run.cpp Outdated
#include "ext/ds_kitty.hpp"
#include "ext/ds_dashcam.hpp"
#include "ext/run_tracking.hpp"

Copy link
Owner

@paul-michalik paul-michalik Apr 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Imagine we have a set of data sources implemented following the pattern described above - i.e. each data source implements a boost::iterator_facade<ext::ds_kitty_iterator, ...>. Then the implementation of a program which uses specific data source looks like this:

#include "ext/run_tracking.h"
#include "ext/ds_kitty.h"
#include "boost/ranges.hpp"

int main(argc, argv)
{
    ext::ds_kitty_args args{argc, argv};
    ext::run_tracking(boost::make_iterator_range(ext::ds_kitty_iterator{args}, ext::ds_kitty_iterator{}));
}

Alternatively, if a data source is modeled by a specific range:

#include "ext/run_tracking.h"
#include "ext/ds_kitty.h"
#include "boost/ranges.hpp"

int main(argc, argv)
{
    ext::ds_kitty_args args{argc, argv};
    ext::run_tracking(ext::ds_kitty{args});
}

So no need for case distinctions, just have a distinct runner for any ds type...

Copy link
Author

Choose a reason for hiding this comment

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

@paul-michalik
I have the following suggestion:
As we already made orb-slam2-ext has separate library,semantic_monocular.exe as separate application, cant we make orb-slam2-ext to handle all user input so that from application point of view (semantic_monocular.exe), the particular application should not know which data source it use, more specifically its implementation. so that we have control over is data source to change at latter timer, if any changes required.
which means from application point of view: the pseducode can be
#include "ext/run_tracking.h"

int main(argc, argv)
{
ext::slam_helper slam(args,argv); //it is wrapper class which will handle calling orb_slam2 object
//currently it is created as ext/slam_object
slam.run_tracking();
}

  1. I believe , though implementation is same for both cases, but with this we have same application which can run any type of application irrespective of input data type.
  2. in code wise what you described , we can move to orb-slam2-ext library only. so in most of time we will change only orb-slam2-ext library only. (close for modification and open for extension)
    Let me know if it make any valuable meaning.

@paul-michalik
Copy link
Owner

paul-michalik commented Apr 10, 2018 via email •
edited
Loading

@paul-michalik
Copy link
Owner

Version with slam_object created outside of run_tracking..

#include "ext/run_tracking.h"
#include "ext/slam_object.h"
#include "ext/ds_kitty.h"
#include "boost/ranges.hpp"

int main(argc, argv)
{
    ext::ds_kitty_args args{argc, argv};
    ext::slam_object slam{argc, argv};
    ext::run_tracking(slam, boost::make_iterator_range(ext::ds_kitty_iterator{args}, ext::ds_kitty_iterator{}));
}

@ghost
Copy link
Author

ghost commented Apr 10, 2018

@paul-michalik
I tried to implement your comments. Please review the changes.

paul-michalik
paul-michalik requested changes Apr 13, 2018
View reviewed changes
CMakeLists.txt Outdated
@@ -53,6 +53,16 @@ add_library(orb-slam2-ext STATIC
ext/app_monitor_api.h
ext/app_monitor_api_impl.h
ext/messages.h
ext/kitty_data_source/ds_kitty.hpp
Copy link
Owner

Choose a reason for hiding this comment

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

Make it shorter: ext/ds_kitty/.... But do we really need a folder?

CMakeLists.txt Outdated

add_executable(semantic_monocular_video Examples/Monocular/semantic_monocular_video.cpp)
target_link_libraries(semantic_monocular_video orb-slam2 ${Boost_LIBRARIES})
elseif(BUILD_EXAMPLES)
add_executable(semantic_monocular Examples/Monocular/semantic_monocular.cc Examples/Monocular/semantic_monocular.hpp)
target_link_libraries(semantic_monocular
add_executable(run_kitty_monocular Examples/Monocular/run_kitty_monocular.cc)
Copy link
Owner

Choose a reason for hiding this comment

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

Please use the name of the data source in the target and source file names: run_ds_kitti.cpp, run_ds_video.cpp, ...

Examples/Monocular/run_dashcam_monocular.cc Outdated
@@ -0,0 +1,66 @@

Copy link
Owner

@paul-michalik paul-michalik Apr 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

  1. Factor the common parts of each runner implementation into a new module.
  2. Data source implementation must export an API to create itself. A function template to create a data source:
// TDataSource is a trait class which describes the data source and is provided by a data source implementation
auto make_data_source<TDataSource>(argc, argv)
{
    return boost::range::make_iterator_range<
        typename TDataSource::iterator_t, ... // etc, you get the point
    >(argc, argv);
}
  1. Rewrite runners according to this API

Copy link
Author

Choose a reason for hiding this comment

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

small doubt on this.

  1. do you want to make a common iterator template class for all data source?
  2. from data source, do you want to implement begin() and end() , so that it can be used as stl container (example: TDataSource::iterator = TDataSourceObj.begin())
  3. in run_tracking.hpp, we are going to add make_data_sourcec() which will return range for specific data source.

paul-michalik
paul-michalik requested changes Apr 17, 2018
View reviewed changes
Copy link
Owner

@paul-michalik paul-michalik left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Please rename *. cc to *. cpp and stay consistent. The same for *.h or *. hpp. Please use either of them but not both.

paul-michalik
paul-michalik requested changes Apr 23, 2018
View reviewed changes
Copy link
Owner

@paul-michalik paul-michalik left a comment

Choose a reason for hiding this comment

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

The runners are obviously still far away from the ideal we have set up a while ago. There's too much code in them, most of it repeated over all nrunners.

Examples/Monocular/testJson.json Outdated
{
"000000": {
"traffic_signs": [
[
Copy link
Owner

Choose a reason for hiding this comment

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

Please move this into a new test folder. I suggest tests/tests_ds_semantic.json and all other unit test shall go there and shall be named following a unified schema.

ext/ds_dashcam.hpp Outdated
namespace fs = boost::filesystem;
using namespace std;
using boost::property_tree::ptree;

Copy link
Owner

Choose a reason for hiding this comment

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

Are we reading the csv file here? I'd prefer to red the mpeg stream directly!

Copy link
Author

Choose a reason for hiding this comment

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

@paul-michalik

  • I am reading mpeg stream. and also csv file.
  • mpeg steam for extracting images
  • csv file for extracting gps information.

let me know if we any other ways are there to extract gps.

ext/run.cpp Outdated
#include "ext/ds_dashcam.hpp"
#include "ext/run_tracking.hpp"

namespace ORB_SLAM2 {
Copy link
Owner

Choose a reason for hiding this comment

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

Why is this still there? Please remove...

scripts/windows/run_semantic_monocular.bat Outdated
@@ -16,11 +16,11 @@ if not exist "%OrbSlam2_ProjectDir%\Vocabulary\ORBvoc.bin" (
del /f /q "%OrbSlam2_ProjectDir%\Vocabulary\ORBvoc.bin.tar"
)

set "OrbSlam2_App=%OrbSlam2_ProductsDir%\Release\semantic_monocular.exe"
set "OrbSlam2_App=%OrbSlam2_ProductsDir%\Release\run_semantic_monocular.exe"
Copy link
Owner

Choose a reason for hiding this comment

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

This file needs to be completely removed too, or am I misunderstanding something?

@paul-michalik
Copy link
Owner

paul-michalik commented Apr 23, 2018 •
edited
Loading

The ideal form of a runner I mentioned in the review comment is this (grabbing "kitti" data source as an example)...

//  in run_ds_kitty.cpp:
#include "ext/run_tracking.h"
#include "ext/slam_object.h"
#include "ext/ds_kitty.h"

int main(int argc, char** argv)
{
    ext::ds_kitty_args args{argc, argv};
    ext::slam_object slam{argc, argv};
    ext::run_tracking(slam, ext::ds_kitty{args});
}

Alternative is the iterator range based version. Let us work towards this result step by step!

@ghost ghost force-pushed the feature/Refactor-ORB_SLAM2-front-end-to-accept-arbitary-data-sources-and-data-sinks branch from 14f2521 to b3b3d43 Compare May 2, 2018 09:25
@ghost
Copy link
Author

ghost commented May 2, 2018

@paul-michalik
following changes are made:

  1. rebase master
  2. use consistent header and source file extension(.cc and .h)
  3. remove run_semantic_monocular.bat
  4. moved Examples/Monocular/testJson.json to tests/testJson.json
  5. remove ext/run.cpp
  6. change run_ds_kitty.cpp ,run_ds_dashcam.cc and run_ds_semantic.cc to the following:
//  in run_ds_kitty.cpp:
#include "ext/run_tracking.h"
#include "ext/slam_object.h"
#include "ext/ds_kitty.h"

int main(int argc, char** argv)
{
    ext::ds_kitty_args args{argc, argv};
    ext::slam_object slam{argc, argv};
    ext::run_tracking(slam, ext::ds_kitty_iter{args});
}

please note:

  • though we don't need separate iterator class ds_kitty_iter (ds_kitty_iter.h) for class ds_kitty as we can make class ds_kitty iterator class, but with class ds_kitty_iter, it makes more simple and clean.

to be done:

  1. provide iterator range based version.
  2. update ds_dashcam.h for reading mpeg stream directly

@paul-michalik let me know your input on this.

paul-michalik
paul-michalik reviewed May 2, 2018
View reviewed changes
ext/ds_kitty.h Outdated
}
gpsfile.close();
_gps_index++;
//slow down driving speed for avoiding tracking lost
Copy link
Owner

Choose a reason for hiding this comment

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

What is this for?

Copy link
Author

@ghost ghost May 2, 2018 •
edited by ghost
Loading

Choose a reason for hiding this comment

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

  • while running kitty inputs, tracking lost is coming almost every time.
  • Hence by making tracking thread slow ,local mapping will get time to process the current keyframe which in turn process more keyframe.
  • The time is added as random and hence just used for practical purpose.

Copy link
Owner

Choose a reason for hiding this comment

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

This isn't good at all. Has Poorva run into this when testing against Kitti datasets for the first time?

Copy link
Owner

@paul-michalik paul-michalik May 3, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

We need to investigate this. @apattnaik0721013's theory is that the mapping thread takes too much time and in consequence the next frame from the tracking differs too much from the last seen frame (=loss of coherence).

  1. Why does the mapping thread take much longer than with the other data sets? Especially the Garching data appears to be more complex than the Kitti data...
  2. I propose to enlarge the size of the queue which is used for communication between tracking and mapping threads. This should help amend the loss of tracking...
  3. Are the configuration data used for the feature detector the same as for other data sets? Have you tried we lower the number of detected features?

Copy link
Author

Choose a reason for hiding this comment

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

q. why tracking lost occurs?
Ans. unrelated neighbor images are encountered.
Description ->

  • neighbor images can be unrelated if the matching of features(key-points) are less among neighbor images.
  • In monocular orb-slam2, map-points are considered if there is at-least 2 observers which means at least 3 neighbor key-frames.
  • each key-frame is constructed after certain frames is passed. so there may be 2 or more frames(images) should be passed for each key-frame construction.
  • so in assumption, features(key-points) of one images should have more matches with its 4th image.
    causes of unrelated neighbor images ->
  1. vehicle is moving very fast which may create more difference in terms of matching of features(key-points) among neighbor images or frames.
  2. features(key-points) extraction is less in images.
  3. creation of key-frames becomes less as local mapping takes huge time to process each key-frame.
  • local mapping perform following task when it encounters a key-frame in the queue->
    -- MapPointCulling() - remove redundant local map-points
    -- CreateNewMapPoints() - create map-point from current key-frame
    -- SearchInNeighbors(); - replace duplicate map-points
    -- Optimizer::LocalBundleAdjustment - optimizations of map-points
    -- KeyFrameCulling() - remove redundant redundant local Key-frames

q1.Why does the mapping thread take much longer than with the other data sets? Especially the Garching data appears to be more complex than the Kitti data...
Ans.

  • There cant be any particular answer as it depends on matching of features(key-points) among images .
  • But my assumption is that in some kitty images, either vehicle is move very fast and kitty fps is 10 per sec which my casuse more difference among images in terms of features(key-points).

q2. I propose to enlarge the size of the queue which is used for communication between tracking and mapping threads. This should help amend the loss of tracking...
Ans

  • yes we can enlarge the size of the queue to avoid tracking lost. Though we can use sleep() in tracking which also increase more keyframe. but use queue will give more control to select particular frame to become key-frame.
  • in orb-slam2 (raulmur version) , sleep() is used to avoid tracking lost.
  • the key-frame is filtered by orb-slam2 in order to view the result in sync with the current input though run tracking , local mapping and viewer thread are run asynchronously.
  • Though we can make more key-frames or all the key-frames from frames and process them all in local mapping, but this will make orb-slam2 to run tracking and local mapping thread synchronously and viewer thread more out-of-sync with the current input. so the lag will seen between current input frame and map-viewer.
  • so it is important to select key-frames more carefully to avoid performance issue.

q3.Are the configuration data used for the feature detector the same as for other data sets? Have you tried we lower the number of detected features?
Ans

  • i tried to run kitty data set with lower features and higher features , but result is same.

@paul-michalik
Copy link
Owner

The runners look very good now. Now looking into the implementation of the data sources strange things are coming up...: Which of the data sources are really functional? How do you test this? More ds specific questions in the comments...

@paul-michalik
Copy link
Owner

paul-michalik commented May 3, 2018 via email

@paul-michalik
Copy link
Owner

Please add an (intermediate) Readme in the ext with brief description how to use the runners, which CL arguments have to set what is their semantics.

paul-michalik
paul-michalik reviewed May 3, 2018
View reviewed changes
Examples/Monocular/testJson.json
@@ -1,156 +0,0 @@
{
Copy link
Owner

Choose a reason for hiding this comment

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

Don't use CamelCase in file names. Prefer some_meaningful_name.json

Copy link
Owner

Choose a reason for hiding this comment

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

If you have test data, please add it to a new folder called ORB_SLAM2/tests. If you feel confident and want to make me happy add also unit tests which verify the correctnes of the current implementation if the respective data sources.

@ghost
Copy link
Author

ghost commented May 3, 2018

@paul-michalik

  1. add readme to use the application
  2. add script for the extended application.

paul-michalik
paul-michalik requested changes May 4, 2018
View reviewed changes
ext/Readme.md Outdated
- dashcam: The MPEG transport stream generated by Ausdom A261 DashCam. [Format description and analysis](https://jira.harman.com/jira/browse/INTHAD-183?filter=53275&jql=project%20%3D%20%22Highly%20Automated%20Driving%22%20AND%20%20labels%20in%20(Macs-SLAM-ORB2)%20AND%20description%20~%20Ausdom%20ORDER%20BY%20created%20DESC)

## Bootstrap and build

Copy link
Owner

Choose a reason for hiding this comment

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

Please add the usual description about using the project API

ext/Readme.md Outdated

### Run kitti
* `run_ds_kitti.bat <path-to-image-folder> <path-to-oxts-folder> <path-to-settings-file>`
* download Kitti dataset with GPS support http://kitti.is.tue.mpg.de/kitti/raw_data/2011_09_26_drive_0009/2011_09_26_drive_0009_sync.zip and unzip into a folder `kitti_data_dir`. To run the data set `00` invoke script as follows: `run_ds_kitti.bat kitti_data_dir\image_00\data kitti_data_dir\oxts\data`
Copy link
Owner

Choose a reason for hiding this comment

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

How can this work? image_00\data contains images from one specific track. How is oxts\data related to image_00?

Copy link
Owner

Choose a reason for hiding this comment

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

You need to document the connection between the input sequence and the raw data which contains the "oxts" readings. Please consult with @PTavse. Obviously not each input sequence will be compatible with arbitrary raw data folder!

@ghost
Copy link
Author

ghost commented May 10, 2018

@paul-michalik
following changes are done:
in run_tracking.h->

  • waiting tracking time is calculated:
std::chrono::steady_clock::time_point start_time = std::chrono::steady_clock::now();
sys_.track(item);
std::chrono::steady_clock::time_point stop_time = std::chrono::steady_clock::now();
double actual_tracking_time = std::chrono::duration_cast<std::chrono::duration<double> >(stop_time - start_time).count();
std::this_thread::sleep_for(std::chrono::microseconds(wait_time));
  • waiting tracking time is required in order to avoid tracking lost.
  • this condition is already handled in orginal orb-slam2 of raulmur.
  • reason of tracking lost is already mentioned in previous comment.
  • so data-source classes (ds_semantic.h,ds_dashcam.h,ds_kitty.h) is changed to calculate timestamp between two frames.

kitty data set -:
There two sets of kitty datasets

  1. standard kitt datasets which contain images set , timestamp , and its ground truth.
    (http://www.cvlibs.net/datasets/kitti/eval_odometry.php)
  2. raw kitt datasets which contain images set , timestamp and gps data with ground truth.
    (http://www.cvlibs.net/datasets/kitti/raw_data.php)

@ghost
Copy link
Author

ghost commented May 15, 2018

@paul-michalik

  • removed ds_kitty_iter.h , ds_semantic_iter.h , ds_dashcam_iter.h.
  • iterator functionalities are handled in ds_kitty.h , ds_semantic.h , ds_dashcam.h

@ghost ghost force-pushed the feature/Refactor-ORB_SLAM2-front-end-to-accept-arbitary-data-sources-and-data-sinks branch from 554d80a to ddd52a6 Compare June 4, 2018 07:03
@ghost ghost force-pushed the feature/Refactor-ORB_SLAM2-front-end-to-accept-arbitary-data-sources-and-data-sinks branch 2 times, most recently from a8c6ad7 to 7ee9f48 Compare June 29, 2018 07:56
@paul-michalik paul-michalik mentioned this pull request Jul 3, 2018
Merged
@ghost ghost force-pushed the feature/Refactor-ORB_SLAM2-front-end-to-accept-arbitary-data-sources-and-data-sinks branch 2 times, most recently from bad24a7 to dbc1446 Compare July 5, 2018 08:16
@shanmukhananda shanmukhananda force-pushed the master branch 2 times, most recently from 971244c to 5d30f54 Compare July 9, 2018 09:39
@paul-michalik
Copy link
Owner

paul-michalik commented Jul 31, 2018 via email

@paul-michalik
Copy link
Owner

No, please pull this back! You overwrote the renames "traficsign" -> "tsr"...! Just remove this whole commit and start over!

@ghost
Copy link
Author

ghost commented Aug 1, 2018

@paul-michalik
[Status]

  • renames ds_trafficsign to ds_tsr
  • regarding unit testing, we already have created this issue.
  • so we can address this in separate branch
  • currently ds_tsr.bat takes only images along with txt file

[Need feedback]

  • do you want to change ds_tsr to handle both image and video input where default is video input.
  • do i create test cases by using the graching_loopcloser_5 dataset in this branch

@paul-michalik
Copy link
Owner

paul-michalik commented Aug 1, 2018 via email •
edited
Loading

@ghost ghost force-pushed the feature/Refactor-ORB_SLAM2-front-end-to-accept-arbitary-data-sources-and-data-sinks branch from 826ffcc to 6c202eb Compare August 2, 2018 09:42
@paul-michalik
Copy link
Owner

@apattnaik0721013

Adding boost-parameters is a good thing, but a CLI should be designed. In your implementation there's a lot of typos and no recognizable system. The idea of the arguments types is to have a semantic type which represents the arguments of a specific data source and provides an semantic type safe API to retrieve the parameters independently of how they were named on the command line. I am going to do this for ds_tsr_args and ds_tsr. Please do this for all other data sources after I commit the changes!

@paul-michalik
Copy link
Owner

@apattnaik0721013

Stop adding using namespace directives to global namespace!

@ghost
Copy link
Author

ghost commented Aug 2, 2018

@paul-michalik
sorry paul , i pushed my changes before seeing your latest comments.

@paul-michalik
Copy link
Owner

@apattnaik0721013

  1. I am also going to change the code formatting. Please stick with using this formatting style!
  2. Don't use BOOST_FOREACH. There is range-base for loop!

@ghost
Copy link
Author

ghost commented Aug 2, 2018 •
edited by ghost
Loading

@paul-michalik

  • paul please have a look new changes.
    [changes]
  • added graching data set
  • ad new iterator ds_image which will handle both image and video input in util
  • ds_kitti, ds_tsr, ds_dashcam use ds_image to handle both image and video input and provide slam input.
  • change launch.vs.json for launch run_dashcam and run_tsr.
  • run_ds_dashcam.bat and run_tsr.bat will launch with default graching dataset if not parameter is given.
  • so ds_tsr support both image and video based on input given.
    data_source

@paul-michalik
(1) Add precompiled header support for run_ds_tsr (2) rename wait to …
8146c47 
…throttle (2) Extracted semantic type for arguments to ds_tsr_args (3) Fixed typos
@paul-michalik
(1) Remove duplicated code in run_tracking (2) Rename throttle to wai…
2129385 
…t_for with simpler signature
@ghost
Copy link
Author

ghost commented Aug 3, 2018

@paul-michalik

  • i will remove BOOST_FOREACH and using namespace.
  • as you mentioned , you are going to change ds_tsr args and ds_tsr, so after this i will address this changes.

@paul-michalik
Copy link
Owner

paul-michalik commented Aug 3, 2018 via email

@ghost
Copy link
Author

ghost commented Aug 8, 2018

@paul-michalik

  • sorry for this trouble.
  • but i didnt update any code changes after your previous comments

@apattnaik0721013
I am also going to change the code formatting. Please stick with using this formatting style!
Don't use BOOST_FOREACH. There is range-base for loop!

  • so the last commit is 2a436fe.
  • but i added only the comments about the this commit
  • just for information, this commit has following changes:
    • renames ds_trafficsign to ds_tsr
    • added graching data set
    • ad new iterator ds_image which will handle both image and video input in util
    • ds_kitti, ds_tsr, ds_dashcam use ds_image to handle both image and video input and provide slam input.
    • change launch.vs.json for launch run_dashcam and run_tsr.
    • run_ds_dashcam.bat and run_tsr.bat will launch with default graching dataset if not parameter is given
  • let me know how to resolve this now.

@paul-michalik
Copy link
Owner

paul-michalik commented Aug 8, 2018 via email •
edited
Loading

@ghost ghost force-pushed the feature/Refactor-ORB_SLAM2-front-end-to-accept-arbitary-data-sources-and-data-sinks branch from 2a436fe to 6c202eb Compare August 8, 2018 10:29
@ghost
Copy link
Author

ghost commented Aug 8, 2018

@paul-michalik

shall i add the following changes:

  • add Garching_LoopClosure-5-images dataset to data folder
  • support boost-parameters in data source.

@paul-michalik
Copy link
Owner

paul-michalik commented Aug 8, 2018 •
edited
Loading

@apattnaik0721013, I am giving up, you win. You obviously do not read what I write... So let me push the changes to this branch. You can add Garching_LoopClosure-5-images to the feature/temp/Refactor-ORB_SLAM2-front-end-to-accept-arbitary-data-sources-and-data-sinks-temporary branch. Please go and create a new pull request for the temporary branch and we are going to merge it onto this branch. But: please do not merge anything until I tell you to do so. You will need to rewrite a lot of code and add unit tests for each single data source

@ghost
Copy link
Author

ghost commented Aug 10, 2018

@paul-michalik
summarizing the list of task that i am going to do in feature/temp/Refactor-ORB_SLAM2-front-end-to-accept-arbitary-data-sources-and-data-sinks-temporary:

  • add Garching_LoopClosure-5-images
  • change the structural representation of the branch feature/temp/Refactor-ORB_SLAM2-front-end-to-accept-arbitary-data-sources-and-data-sinks-temporary as you did in this current branch:
    • move all the data source to ext_ds
    • all data source unit tests to ext_tests
  • retaining the ds_image support in feature/temp/Refactor-ORB_SLAM2-front-end-to-accept-arbitary-data-sources-and-data-sinks-temporary which makes data source supporting both image and video inputs.

@paul-michalik
Copy link
Owner

paul-michalik commented Aug 10, 2018 via email

@paul-michalik
Copy link
Owner

paul-michalik commented Aug 10, 2018 via email

@ghost
Copy link
Author

ghost commented Aug 10, 2018

Yes paul, we don't need separate tsr data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@paul-michalik