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

Implementation of CSR-DCF tracker #1552

Merged
merged 6 commits into from
Feb 22, 2018
Merged

Implementation of CSR-DCF tracker #1552

merged 6 commits into from
Feb 22, 2018

Conversation

amuhic
Copy link
Contributor

@amuhic amuhic commented Feb 11, 2018

This pullrequest implements CSR-DCF tracker based on:
http://prints.vicos.si/publications/355
http://prints.vicos.si/publications/files/355

I also have one question. In modules/tracking/src/trackerCSRTUtils.cpp, I've copied function:
void computeHOG32D(const Mat &imageM, Mat &featM, const int sbin, const int pad_x, const int pad_y) from modules/dpm/src/dpm_feature.cpp (with some minor changes). This was mostly used for testing purposes, to see whether this implementation of FHOG could be used in this tracker. I'm asking for your advice on how to go on about this. I'm assuming that a dependency for dpm module should be added to tracking module and then use that function directly.

force_builders=Mac

Copy link
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

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

Thank you for contribution!

Could you add some simple test (to check that this code doesn't crash and return non-empty result)?

@@ -0,0 +1,187 @@
/*M///////////////////////////////////////////////////////////////////////////////////////
Copy link
Member

Choose a reason for hiding this comment

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

In OpenCV we usually don't add license header to sample code.

using namespace std;
using namespace cv;

#include <chrono>
Copy link
Member

Choose a reason for hiding this comment

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

chrono is C++11 header.

Consider using cv::getTickCount() call or TickMeter.


#include <math.h>

#include "opencv2/highgui.hpp"
Copy link
Member

Choose a reason for hiding this comment

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

Does it really required?

@@ -0,0 +1,779 @@
/*///////////////////////////////////////////////////////////////////////////////////////
Copy link
Member

Choose a reason for hiding this comment

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

Please use short OpenCV license header.

#ifndef OPENCV_TRACKER_CSRT_SCALE_ESTIMATION
#define OPENCV_TRACKER_CSRT_SCALE_ESTIMATION

#include "precomp.hpp"
Copy link
Member

Choose a reason for hiding this comment

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

precomp.hpp:

  • must be included from module's cpp files only (remove it from .hpp files)
  • must be the first include in module's .cpp files (please add it there)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand, though there seem to be trackers/files, already included in tracking module, that include precomp.hpp in their header file. (e.g. modules/tracking/src/tldDetector.hpp). Is there a reason for this?

Histogram hist_target, Histogram hist_background, int numBinsPerChannel)
{
//preprocess and normalize all data
assert(imgChannels.size() > 0);
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 CV_Assert() instead.

myfile.close();
}

#define pvar(obj) std::cout << "; " << #obj << " > " << obj << "\t call@: " << __func__ << std::endl;
Copy link
Member

Choose a reason for hiding this comment

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

Please avoid using of 'tab' symbols in code.

Mat TrackerCSRTImpl::calculate_response(const Mat &image, const std::vector<Mat> filter)
{
Mat patch = get_subwindow(image, object_center, floor(current_scale_factor * template_size.width),
floor(current_scale_factor * template_size.height));
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 OpenCV calls:

  • floor() -> cvFloor()
  • ceil() -> cvCeil()
  • round() -> cvRound()

@amuhic
Copy link
Contributor Author

amuhic commented Feb 12, 2018

Thanks for the comments!

I think I've fixed most of the problems you've pointed out. Except for the simple test part. Do you want me to add this tracker to 'test_trackers.cpp' or do you want me to send you a few or the tracking sequences, that I use for testing, together with a make file to build and test this?

@alalek
Copy link
Member

alalek commented Feb 12, 2018

Thank you!

I believe it is enough to add this tracker to 'test_trackers.cpp'.

Adding new datasets (tracking sequences) into OpenCV repositories requires testdata approval (license), so it is better to reuse existing testdata (anyway you could add an external link on your dataset into Pull request description or documentation).

@alalek
Copy link
Member

alalek commented Feb 14, 2018

Looks better, Please take a look on current build issues on Windows/Android builders.
BTW, Identifiers started with underscore ('_') and capital letter are reserved (platform specific).

Copy link
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

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

Well done!
Looks good to me 👍

@alalek alalek merged commit ced5aa7 into opencv:master Feb 22, 2018
@moi90
Copy link

moi90 commented Jul 7, 2022

@amuhic I know this pull request got merged a long time ago, but would it be possible to follow this up with more documentation and comments? The tracker has a lot of parameters and (at least to me) it is not obvious what they do or how to tune them, even after reading the paper.

@moi90
Copy link

moi90 commented Jul 7, 2022

I created a draft pull request (#3300). Could someone help me improve it?

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.

None yet

3 participants