search

Lauszus / FaceRecognitionApp

Face Recognition Android App
GNU General Public License v2.0
504 stars 238 forks source link

Update application for all version of opencv sdk #13

Closed SantoshKumar3295 closed 6 years ago

SantoshKumar3295 commented 6 years ago

Hello Team,

Great work! In order to improve this repo i have few suggestion :

I have struggled few hour just because i have not downloaded exact sdk version of opencv. Provide hardcode way instead of class path and in comment provide information of make it classpath.

Thanks

yohanes-ai commented 6 years ago

@SantoshKumar3295 Please read build instructions carefully. By the way, the build instruction is very clear told that you will need opencv library version 3.2.0 by using this command 

wget https://github.com/opencv/opencv/releases/download/3.2.0/opencv-3.2.0-android-sdk.zip -O opencv-android-sdk.zip

Hard coded isn't a best practices on real world programming. @Lauszus make this application as flexible as possible.

SantoshKumar3295 commented 6 years ago

@JhonImmanuel I know that bro, but the thing is, u never know how much the level of knowledge that person has who is using your repo eg; this was my first test on a ndk project. All though i understand, that doesn't matter and as i said, for improvement of this repo (not necessary) i had this suggestion.

Thanks for your time.

Lauszus commented 6 years ago

@SantoshKumar3295 I have updated the README and made it more clear to use those exact versions: https://github.com/Lauszus/FaceRecognitionApp/commit/ff5346b2bc419b5434d1ab104cdd817a5bcb75bf.

However I agree with @JhonImmanuel regarding hardcoding of the paths. What would you even hardcode them to be? If I hardcoded them it would only work on my system, that is why I am using environmental variables.

SantoshKumar3295 commented 6 years ago

Include OpenCV as module in your application, if it is that much dependent with version.

Lauszus commented 6 years ago

@SantoshKumar3295 I need to compile it, as I want to compile OpenCV statically, so the end user does not have to install the OpenCV Manager.

If you can come up with a better way of doing, then you are more than welcome to send a pull request.

Lauszus commented 6 years ago

@SantoshKumar3295 btw please see the following issue: https://github.com/opencv/opencv/issues/5003.

We could use this repository: https://github.com/floe/opencv-android, but then people would have to install the OpenCV Manager as well.

Lauszus commented 6 years ago

@SantoshKumar3295 I have now setup a Travis build to automatically upload a zip directory of the project including all the dependencies. This should make much easier for people to compile the project.

You can try it out at the following link: https://github.com/Lauszus/FaceRecognitionApp/releases/download/1.2.1/FaceRecognitionApp-1.2.1.zip.