Skip to content

Tracking CSRT and MOSSE#537

Merged
justadudewhohacks merged 20 commits into
justadudewhohacks:masterfrom
piercus:CSRT-and-MOSSE
Apr 20, 2019
Merged

Tracking CSRT and MOSSE#537
justadudewhohacks merged 20 commits into
justadudewhohacks:masterfrom
piercus:CSRT-and-MOSSE

Conversation

@piercus

@piercus piercus commented Apr 10, 2019

Copy link
Copy Markdown
Collaborator

Implements CSRT and MOSSE trackers.

This needs to be tested by CI on other opencv build since MOSSE and CSRT are only in OpenCV since 3.4.2 and 3.4.1

@justadudewhohacks Should we test for cv.version.patch > 1 && cv.version.minor > 3 or do we consider cv.version.minor > 3 is enough ?

@piercus piercus removed the request for review from justadudewhohacks April 10, 2019 14:47
@piercus

piercus commented Apr 10, 2019

Copy link
Copy Markdown
Collaborator Author

For now, i'm not able to add psr_threshold into TrackerCSRTParams.h (commented in current version)

I'm getting following error when i uncomment corresponding lines

 CXX(target) Release/obj.target/opencv4nodejs/cc/modules/tracking/Trackers/TrackerCSRT.o
In file included from ../cc/modules/tracking/Trackers/../Tracker.h:1:0,
                 from ../cc/modules/tracking/Trackers/TrackerCSRT.h:1,
                 from ../cc/modules/tracking/Trackers/TrackerCSRT.cc:3:
../cc/modules/tracking/Trackers/TrackerCSRTParams.h: In static member function ‘static Nan::NAN_GETTER_RETURN_TYPE TrackerCSRTParams::psr_thresholdGet(v8::Local<v8::String>, Nan::NAN_GETTER_ARGS_TYPE)’:
../cc/modules/tracking/Trackers/TrackerCSRTParams.h:42:63: error: ‘struct cv::TrackerCSRT::Params’ has no member named ‘psr_threshold’
  static FF_GETTER(TrackerCSRTParams, psr_thresholdGet, params.psr_threshold);

I'm not sure if this is related to a specific OpenCV version or if this is related to the implementation.
Any help would be apprecaited, but i think we can merge it anyway.

@justadudewhohacks

Copy link
Copy Markdown
Owner

Thanks for the PR!

@justadudewhohacks Should we test for cv.version.patch > 1 && cv.version.minor > 3 or do we consider cv.version.minor > 3 is enough ?

Hmm if these trackers have been introduced with a patch version then it would be better yeah. You can simply add the patch version to the cv.version object here.

For now, i'm not able to add psr_threshold into TrackerCSRTParams.h (commented in current version)

A quick look in the docs revealed, that psr_threshold has been added with 3.4.4 apparently. Might need a version guard there as well.

@piercus

piercus commented Apr 12, 2019

Copy link
Copy Markdown
Collaborator Author

Thanks for your feedbacks.

psr_threshold has been added with 3.4.4 apparently

I have added the version checking but also the ci build environments for 3.4.5, 3.4.4 and 3.4.6, and i'm voting to move the default version to 3.4.6, see justadudewhohacks/npm-opencv-build#31

@justadudewhohacks

Copy link
Copy Markdown
Owner

Great! Looks like you made a small typo in trackerParamTests.js:

C:\projects\opencv4nodejs\test\tests\modules\tracking\trackerParamTests.js:75
if(cv.version.minor > 4 || (cv.version.minor === 4 && cv.version.subminor > 3){
^
SyntaxError: Unexpected token {

@justadudewhohacks

justadudewhohacks commented Apr 15, 2019

Copy link
Copy Markdown
Owner

I have added the version checking but also the ci build environments for 3.4.5, 3.4.4 and 3.4.6, and i'm

Ahh didn't notice that one. The docker images do not exist, I have to build them first. For now I would suggest reverting the travis.yml, I will add the CI for the latest builds later on.

Other than that, there still seem to be some buld jobs failing.

@piercus

piercus commented Apr 19, 2019

Copy link
Copy Markdown
Collaborator Author

@justadudewhohack it's ready now !

Thanks for your help

@justadudewhohacks

Copy link
Copy Markdown
Owner

Great stuff. Thank you!

@justadudewhohacks justadudewhohacks merged commit bc69acf into justadudewhohacks:master Apr 20, 2019
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