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

WIP - converting to OpenCL - 1 test #3

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

Conversation

kalwalt
Copy link
Collaborator

@kalwalt kalwalt commented Sep 2, 2019

i'm testing the conversion to OpenCL: basically converting the part of the code where OpenCL can improve the performances, it should only necessary change some cv::Mat to cv::UMat.
This PR is most a trial and error, i will report in the comments the problems that may arise.

@kalwalt kalwalt added enhancement New feature or request OpenCV all regarding OpenCV OpenCL all about OpenCL C/C++ about C and C++ code labels Sep 2, 2019
@kalwalt
Copy link
Collaborator Author

kalwalt commented Sep 2, 2019

Whit the latest i get this error:

/home/walter/kalwalt-github/artoolkitX_em_2d/Source/ARX/OCVT/PlanarTracker.cpp:415:20: error: 
      no matching constructor for initialization of 'cv::UMat'
  ...colorFrame(_frameSizeY, _frameSizeX, CV_8UC4, frame, cv::USAGE_DEFAULT);
     ^          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/walter/kalwalt-github/artoolkitX_em_2d/Source/depends/emscripten/opencv-4.1/modules/core/include/opencv2/core/mat.inl.hpp:3600:7: note: 
      candidate constructor not viable: no known conversion from
      'unsigned char *' to 'const cv::Scalar' (aka 'const Scalar_<double>') for
      4th argument
UMat::UMat(int _rows, int _cols, int _type, const Scalar& _s, UMatUsageF...
      ^

cv::Mat instead may accept void* data

Mat (int rows, int cols, int type, void *data, size_t step=AUTO_STEP)

from the OpenCV docs

@kalwalt
Copy link
Collaborator Author

kalwalt commented Sep 2, 2019

I read this article https://www.learnopencv.com/opencv-transparent-api/ and i solved sending the data initially to a cv::Mat and after getting them with the getUmat function. Any way this is fixed only for the Emscripten part, won't work for the other platforms.

 void ProcessFrameData(unsigned char * frame)
   {
     cv::Mat data(_frameSizeY, _frameSizeX, CV_8UC4, frame);
     // When using emscripten the image comes in as RGB image from the browser
     // Convert it to Gray
     #if ARX_TARGET_PLATFORM_EMSCRIPTEN
     cv::UMat colorFrame = data.getUMat(cv::ACCESS_READ);

@kalwalt
Copy link
Collaborator Author

kalwalt commented Sep 2, 2019

This error with the latest commit:

/home/walter/kalwalt-github/artoolkitX_em_2d/Source/ARX/OCVT/PlanarTracker.cpp:724:52: error: 
      no member named 'ptr' in 'cv::UMat'
                memcpy(data, _trackables[i]._image.ptr(), _trackables[i]...
                             ~~~~~~~~~~~~~~~~~~~~~ ^
In file included from /home/walter/kalwalt-github/artoolkitX_em_2d/Source/ARX/OCVT/PlanarTracker.cpp:43:
In file included from /home/walter/kalwalt-github/artoolkitX_em_2d/Source/ARX/OCVT/OCVConfig.h:43:
In file included from /home/walter/kalwalt-github/artoolkitX_em_2d/Source/depends/emscripten/opencv-4.1/modules/core/include/opencv2/core.hpp:60:

Needs to think how to solve...

@@ -721,7 +721,10 @@ class PlanarTracker::PlanarTrackerImpl
info.fileName = _trackables[i]._fileName;
// Copy the image data and use a shared_ptr to refer to it.
unsigned char *data = (unsigned char *)malloc(_trackables[i]._width * _trackables[i]._height);
memcpy(data, _trackables[i]._image.ptr(), _trackables[i]._width * _trackables[i]._height);
/// this needs to be verifyed!!
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is only a test, completely not sure of.

@@ -504,7 +504,7 @@ class PlanarTracker::PlanarTrackerImpl
fs << "trackableId" + index << _trackables[i]._id;
fs << "trackableFileName" + index << _trackables[i]._fileName;
fs << "trackableScale" + index << _trackables[i]._scale;
fs << "trackableImage" + index << _trackables[i]._image;
//fs << "trackableImage" + index << _trackables[i]._image;
Copy link
Collaborator Author

@kalwalt kalwalt Sep 2, 2019

Choose a reason for hiding this comment

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

i commented out because an error (no viable conversion from UMat to another format, don't remember exactly)

@@ -543,7 +543,7 @@ class PlanarTracker::PlanarTrackerImpl
fs["trackableId" + index] >> newTrackable._id;
fs["trackableFileName" + index] >> newTrackable._fileName;
fs["trackableScale" + index] >> newTrackable._scale;
fs["trackableImage" + index] >> newTrackable._image;
//fs["trackableImage" + index] >> newTrackable._image;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

same as in the other comment, in this way the code compile completely, but save and load Trackables Database won't work.

@kalwalt
Copy link
Collaborator Author

kalwalt commented Sep 2, 2019

@ThorstenBux I have my Artoolkitx.js and Artoolkitx.wasm ready!

@kalwalt
Copy link
Collaborator Author

kalwalt commented Sep 2, 2019

The code do not detect the marker but i get an decisive increase in fps, probably the reason is in the changes in passing the data.
If you want to try i let here the compiled libs.
artoolkitxCL.zip

@kalwalt
Copy link
Collaborator Author

kalwalt commented Sep 2, 2019

sure the issue is in the PlanarTracker.cpp

TrackedImageInfo GetTrackableImageInfo(int trackableId)
    {
        TrackedImageInfo info;
        for(int i=0;i<_trackables.size(); i++) {
            if(_trackables[i]._id==trackableId) {
                info.uid = _trackables[i]._id;
                info.scale = _trackables[i]._scale;
                info.fileName = _trackables[i]._fileName;
                // Copy the image data and use a shared_ptr to refer to it.
                unsigned char *data = (unsigned char *)malloc(_trackables[i]._width * _trackables[i]._height);
                /// this needs to be verifyed!!
                cv::Mat dataPtr(_trackables[i]._width, _trackables[i]._height, CV_8UC4, data);
                _trackables[i]._image = dataPtr.getUMat(cv::ACCESS_READ);
                //memcpy(data, _trackables[i]._image.ptr(), _trackables[i]._width * _trackables[i]._height);
                info.imageData.reset(data, free);
                info.width = _trackables[i]._width;
                info.height = _trackables[i]._height;
                info.fileName = _trackables[i]._fileName;
                return info;
            }
        }
        return info;
    }

before was:

TrackedImageInfo GetTrackableImageInfo(int trackableId)
    {
        TrackedImageInfo info;
        for(int i=0;i<_trackables.size(); i++) {
            if(_trackables[i]._id==trackableId) {
                info.uid = _trackables[i]._id;
                info.scale = _trackables[i]._scale;
                info.fileName = _trackables[i]._fileName;
                // Copy the image data and use a shared_ptr to refer to it.
                unsigned char *data = (unsigned char *)malloc(_trackables[i]._width * _trackables[i]._height);
                memcpy(data, _trackables[i]._image.ptr(), _trackables[i]._width * _trackables[i]._height);
                info.imageData.reset(data, free);
                info.width = _trackables[i]._width;
                info.height = _trackables[i]._height;
                info.fileName = _trackables[i]._fileName;
                return info;
            }
        }
        return info;
    }

the problem is we can not have a ptr() for the UMat, that Mat instead has. If we fix this maybe the code will works correctly.
Proabably we can do other optimizations with Umat as for example in CameraPoseFromPoints and some other part of the code too.

@ThorstenBux
Copy link
Owner

ThorstenBux commented Sep 2, 2019 via email

@kalwalt
Copy link
Collaborator Author

kalwalt commented Sep 2, 2019

Yes, but i was very lucky too! The Transparent Api helps a lot!
We will see how to solve, no problem when. Good holiday!

@ThorstenBux
Copy link
Owner

I've pushed my branch of UMat things that I had done previously:
https://github.com/ThorstenBux/artoolkitX_em_2d/tree/perf_OpenCL_UMat

You can have a look at them if you like :).

@kalwalt
Copy link
Collaborator Author

kalwalt commented Sep 2, 2019

Ok thank you very much! For sure i will look at!

memcpy(data, _trackables[i]._image.ptr(), _trackables[i]._width * _trackables[i]._height);
/// this needs to be verifyed!!
cv::Mat dataPtr(_trackables[i]._width, _trackables[i]._height, CV_8UC4, data);
_trackables[i]._image = dataPtr.getUMat(cv::ACCESS_READ);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we should do the reverse!, we will have to fill in the data from the trackables to image data, so it doesn't work

cv::UMat grayImage(_frameSizeY, _frameSizeX, CV_8UC1);
cv::cvtColor(colorImage, grayImage, cv::COLOR_RGBA2GRAY);
std::cout << "grayImage ok!" << std::endl;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the problem seems located here i don't recieve this message: grayImage ok!

Copy link
Owner

Choose a reason for hiding this comment

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

The image should be coming in as luma image from the API directly as the conversion happens inside the API in JS for the video stream.

Not sure if the code refers to the JPG image used as trackable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It refers to 'newTrackable' i think the jpg image. It seems strange to me that fails at this point. Unlucky i will not have time to try other changes...

@ThorstenBux
Copy link
Owner

ThorstenBux commented Sep 4, 2019 via email

@kalwalt
Copy link
Collaborator Author

kalwalt commented Sep 4, 2019

i did but very quickly and not for all the code, i should do indeed !

@kalwalt
Copy link
Collaborator Author

kalwalt commented Sep 10, 2019

The code actually seems to fail when loading the data image to be tracked (in the addMarker function) not understand why it can not loaded, i don't see such critical code in addMarker or maybe it fails silently in another part but i can not get where.

newTrackable._image = grayImage;
cv::cvtColor(colorImage, grayImage, cv::COLOR_RGBA2GRAY);
std::cout << "grayImage ok 2!" << std::endl;
newTrackable._image = grayImage.getUMat(cv::ACCESS_RW);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Now it stops at this point: Issue while converting to UMat.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

or the step before...

@ThorstenBux
Copy link
Owner

Hi,

I start to feel that OpenCL won't work on mobile. Some things that I'm reading point into the direction that the drivers need to be written for the specific GPU and things. Which might explain why it starts failing. Basically, the browser doesn't have access to it but we tell the transpiled OpenCV that OpenCL is available so it tries to call it but fails because it isn't really there. The web counterpart would be WebCL which in turn isn't implemented inside any browser yet and the Emscripten polyfill is reportedly slower than not using it at all.

We might need to find another solution to tweak the performance

@kalwalt
Copy link
Collaborator Author

kalwalt commented Sep 12, 2019

yes i can understand this. But could be an issue the fact i build the OpenCV lib with "-DWITH_OPENCL=ON -DWITH_OPENCL_SVM=ON" with OpenCL enabled but also with OpenCL and the Shared Virtual Memory enabled? I can try to test without this last one, just to understand if it make a difference.

@ThorstenBux
Copy link
Owner

ThorstenBux commented Sep 12, 2019 via email

@ThorstenBux
Copy link
Owner

ThorstenBux commented Sep 12, 2019

We would need to find some other idea to improve performance. I fear

@kalwalt
Copy link
Collaborator Author

kalwalt commented Sep 12, 2019

ok good to know, and basically have you some ideas for other improvements?

@ThorstenBux
Copy link
Owner

Well, first would be to get it running profile it to see where the bottleneck is.

One idea would be to run a rectangle detection over the image first to reduce the size of the image that needs to run the recognition and tracking.

@ThorstenBux
Copy link
Owner

I see you are working with https://github.com/kalwalt/MarkerlessARJS maybe they did something different which could improve performance?

I recall that the performance might be solvePnP but not entirely sure. I'd say let us aim for a profiling run? Could you do that?

@kalwalt
Copy link
Collaborator Author

kalwalt commented Sep 12, 2019

Well, first would be to get it running profile it to see where the bottleneck is.

yes you are right, this is important.

One idea would be to run a rectangle detection over the image first to reduce the size of the image that needs to run the recognition and tracking.

Do you mean a rectangle detection thanks to OpenCV, like a blob detection algorithm?

I see you are working with https://github.com/kalwalt/MarkerlessARJS maybe they did something different which could improve performance?

yes i'm working on this but i'am far to make something that can be tested, The original code was developed with a oldest version of OpenCV. I hope that it can be something better in performances, i hope to go on tihs to compare with ArtoolkitX.js, The original code was only for windows I think, didn't tested the original one. The author says also in this issue ahmetozlu/augmented_reality#1 about Dimensionality Reduction maybe could be an idea?

I recall that the performance might be solvePnP but not entirely sure. I'd say let us aim for a profiling run? Could you do that?

I will try it i f i have time tomorrow or in the next days. Actually i am in France for job, i arrived today.

@kalwalt
Copy link
Collaborator Author

kalwalt commented Sep 12, 2019

also read this issue: ahmetozlu/augmented_reality#2

@ThorstenBux
Copy link
Owner

also read this issue: ahmetozlu/augmented_reality#2

Detection and tracking is already separated inside the C function as far as I know but it is not running as two threads that would be an idea.

@kalwalt
Copy link
Collaborator Author

kalwalt commented Sep 12, 2019

Detection and tracking is already separated inside the C function as far as I know but it is not running as two threads that would be an idea.

Can we do seperate threads with Emscripten? we can but enabling -s USE_PTHREADS=1 right?
as i did for jsartoolkit5 in kalwalt/jsartoolkit5#2 but it is needed to enable the shared memory option on the phone. Or maybe you mean something else.

@ThorstenBux
Copy link
Owner

I've compiled with PTHREADS before but didn't know about the shared memory option?

@kalwalt
Copy link
Collaborator Author

kalwalt commented Sep 14, 2019

I've compiled with PTHREADS before but didn't know about the shared memory option?

yes it is. maybe you used before chrome and other browsers disabled the default. But maybe in a near future it will become a default option again.
I will try to do the profiling tomorrow.

@kalwalt
Copy link
Collaborator Author

kalwalt commented Sep 15, 2019

i m doing the profiling i will open a separate issue for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C/C++ about C and C++ code enhancement New feature or request OpenCL all about OpenCL OpenCV all regarding OpenCV
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants