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

Adaptive simple threshold #34

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

Conversation

jvoigts
Copy link
Contributor

@jvoigts jvoigts commented Dec 17, 2016

No description provided.

for now this just optinally makes the thresholds relative 
to the darkest non-zero pixel
Copy link
Owner

@jonnew jonnew left a comment

Choose a reason for hiding this comment

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

Needs some formatting and logic fixes before merge.

//,
// ("mincomp,m", po::value<int>(),
// "Minimum compensation, optionally adjust thresholds by the darkest pixels in the region."),

Copy link
Owner

Choose a reason for hiding this comment

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

Get rid of these comments. space between function defs should be one line.

@@ -64,6 +70,10 @@ void SimpleThreshold::appendOptions(po::options_description &opts)
("area,a", po::value<std::string>(),
"Array of floats, [min,max], specifying the minimum and maximum "
"object contour area in pixels^2.")

Copy link
Owner

Choose a reason for hiding this comment

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

Get rid of white space around this option addition.

if (oat::config::getNumericValue<int>(vm, config_table, "mincomp", mincomp, 0))
set_mincomp_size(mincomp);


Copy link
Owner

Choose a reason for hiding this comment

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

Get rid of this line break.

@@ -259,6 +287,18 @@ void SimpleThreshold::set_dilate_size(int value)
}
}

void SimpleThreshold::set_mincomp_size(int value)
{
std::cout << "mincomp set to " << value<<"\n";
Copy link
Owner

Choose a reason for hiding this comment

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

Get rid of this debug output.

std::cout << "mincomp set to " << value<<"\n";
if (value != 0) {
mincomp_on_ = true;

Copy link
Owner

Choose a reason for hiding this comment

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

Get rid of line

@@ -54,18 +54,20 @@ class SimpleThreshold : public PositionDetector {
void set_max_object_area(double value) { max_object_area_ = value; }
void set_erode_size(int erode_px);
void set_dilate_size(int dilate_px);
void set_mincomp_size(int mincomp_val);
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 need this member function. Those were only needed for the erode and dilate options because I had to set up a little smoothing kernel each time the value is changed.


// Object detection
double object_area_ {0.0};

// Sizes of the erode and dilate blocks
int erode_px_ {0}, dilate_px_ {0};
bool erode_on_ {false}, dilate_on_ {false};
int erode_px_ {0}, dilate_px_ {0}, mincomp_val_ {0};
Copy link
Owner

Choose a reason for hiding this comment

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

int mincomp_ {0} on its own line with a an explanatory comment. It is not related to erode an dilate.

int erode_px_ {0}, dilate_px_ {0};
bool erode_on_ {false}, dilate_on_ {false};
int erode_px_ {0}, dilate_px_ {0}, mincomp_val_ {0};
bool erode_on_ {false}, dilate_on_ {false}, mincomp_on_ {false};
Copy link
Owner

Choose a reason for hiding this comment

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

Get rid of mincomp_on_ member.

nonmasked_frame_);

double mincomp_brightness =0;
if (mincomp_on_)
Copy link
Owner

@jonnew jonnew Dec 18, 2016

Choose a reason for hiding this comment

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

Get rid of mincomp_on variable and just do mincomp_ != 0 here.

Where is the value of mincomp_ being used? It looks like you are just always adding whatever the minimum of the non zero part of the frame to the threshold range.

In fact the if block should start before the first inRange call to get rid of the processing overhead if its not being used. It should end below the minMaxIdx function.

if (mincomp_on_)
cv::minMaxIdx(frame, &mincomp_brightness,NULL,NULL,NULL,nonmasked_frame_);

//std::cout << " br " << mincomp_brightness <<"\n";
Copy link
Owner

Choose a reason for hiding this comment

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

Get rid of temporary std::out stuff.

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

2 participants