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

implement and test camera prototype #16

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

implement and test camera prototype #16

wants to merge 2 commits into from

Conversation

RogerQi
Copy link
Member

@RogerQi RogerQi commented Feb 23, 2020

No description provided.

Comment on lines +56 to +65
double TicTocStatsTime(std::string query) {
tictoc_t tictocs = global_tic_toc_bank.GetLCM();
// sort according to keys + find maximum channel name length
for (tictoc_channel_t &tictoc: tictocs.tictoc_channels) {
if (tictoc.name == query)
return ((TicTocStats)(tictoc.tictoc_stats)).TotalTime();
}
return -1; // not found
}

Copy link
Member

Choose a reason for hiding this comment

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

make this a TicTocBank method, and for generalization purposes, return the entire stats for the channel. Key access should be hash mapped not queried.

imwrite("./test.jpg", test_frame);
}

void BenchmarkTest() {
Copy link
Member

Choose a reason for hiding this comment

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

Is this test necessary? It does not seem to verify the correctness of anything.

Comment on lines +78 to +79
std::vector<cv::Mat> _buffer;
mutex _index_lock;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
std::vector<cv::Mat> _buffer;
mutex _index_lock;
std::vector<cv::Mat> buffer_;
std::mutex index_lock_;


include(${LCM_USE_FILE}) # lcm related cmake helper functions
include_directories(${OpenCV_INLUCDE_DIRS}) # OpenCV headers
Copy link
Member

Choose a reason for hiding this comment

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

include_directories should not be used unless this is required for every source file. use target_include_directories instead, for the module that actually depends on it.

* @param sleep time (in ms) to sleep between each capture. Default to zero
* @return none
*/
void _start_capture(unsigned int sleep = 0);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
void _start_capture(unsigned int sleep = 0);
void start_capture_(unsigned int sleep = 0);

}

void CameraBase::get_img(cv::Mat &dst) {
this->_index_lock.lock();
Copy link
Member

Choose a reason for hiding this comment

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

consider using modern C++ lock guard instead of this raw locking mechanism

// this can be potentially improved by taking (diminishing) tradeoff with thread safety
// but on Jetson TX2, this implementation already runs faster than 120fps
// which is faster than almost every camera...
this->_buffer[ind_to_read].copyTo(dst);
Copy link
Member

Choose a reason for hiding this comment

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

If the circular buffer is big enough and the processing frequency is faster than the circular capture frequency, we could get rid of this copy.

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