opencv4nodejs icon indicating copy to clipboard operation
opencv4nodejs copied to clipboard

Fix #750 making it compatible with Opencv 4.4

Open wandenberg opened this issue 5 years ago • 8 comments

wandenberg avatar Sep 19 '20 20:09 wandenberg

@justadudewhohacks can confirm this fixes the problem with 4.4

adin234 avatar Oct 08 '20 16:10 adin234

@mykola-mokhnach sorry for the delay. Yes, just tested and it worked with the OpenCV 4.5

Thanks, I also checked with the OpenCV 4.5.0 and everything works fine 👍

pavds avatar Dec 24 '20 00:12 pavds

Definitely a much anticipated PR +1

Maximvdw avatar Sep 30 '21 09:09 Maximvdw

Now with support to Opencv4.5.2 https://github.com/wandenberg/opencv4nodejs/commit/91bc118a43da570638db466e51cf56dbbf615b9e

wandenberg avatar Nov 09 '21 22:11 wandenberg

Nice patch, I merged it to my fork If you want to make further changes try them from my fork, It's way faster to use, the build script log has been improved, and each OpenCV version build has its build directory.

to get the best result, define an OPENCV_BUILD_ROOT environment variable outside your node_modules directory; Personally, I use:

export OPENCV_BUILD_ROOT=~/opencv

The OpenCV build script can be called from the command line without an extra environment variable.

Tested on Linux / MacOS X M1 / Windows.

Currently, only 2 samples failed.

UrielCh avatar Jan 06 '22 15:01 UrielCh

@wandenberg Sorry for the delay, but since i'm having contributor rights, i will try to review this :-)

The PR is great, but there is a lack of testing coverage for OpenCV 4.5 on travis

I feel it would be good to add 4.5.x version in travis in the matrix in https://github.com/justadudewhohacks/opencv4nodejs/blob/master/.travis.yml

Can you please also add some tests specifically related to your changes on 4.5 using cvVersionGreaterEqual(4, 5, 0) you can find example in https://github.com/justadudewhohacks/opencv4nodejs/blob/master/test/tests/tracking/TrackerTests.js

Thanks for your help

piercus avatar Apr 12 '22 07:04 piercus

@piercus Sorry for the delay. I tried to change the version for OpenCV4.5.5 on travis.yml and on appveyour.yml, but this made the test fail. Probably I am missing something on the configuration.
Regarding adding tests, as far as I can see, it is not needed since the changes were only on how to call the C++ code and this is tested on the current suite, we're only missing running the tests with the correct OpenCV version like you requested.
I noticed that a new project was created and the code of this merge request is already incorporated there and tested with the proper versions, so I will let this one as is :)

wandenberg avatar Jun 30 '22 09:06 wandenberg

Ok thanks for the follow up !

piercus avatar Jun 30 '22 09:06 piercus