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

OpenCV Installation improvements & fixes #53

Merged
merged 4 commits into from
Dec 18, 2019

Conversation

rajat2004
Copy link
Collaborator

One of the fixes from the dicussion in #52

@rajat2004 rajat2004 requested a review from rsnk96 December 10, 2019 05:33
@rajat2004
Copy link
Collaborator Author

Travis is passing the opencv builds, but I think it'll be good to do a double check in a Docker container

@rajat2004 rajat2004 changed the title Export FFmpeg path and then echo to SHELLRC OpenCV Installation improvements & fixes Dec 15, 2019
@rajat2004
Copy link
Collaborator Author

@rsnk96 By any chance, were you able to test these changes in a docker container?
I think both these changes should be good to go, but just confirming

@rsnk96
Copy link
Owner

rsnk96 commented Dec 17, 2019

Tested once, didn't work out of the box, primarily because run_and_echo() reloads the environment within the bash script effectively, but doesn't update the environment which started the script in the first place (i.e. the shell process). Easy way around this is to just start a new sub-process with the updated SHELLRC file like here just before the script exits (and this has to be done within an if [[ ! -n $CIINSTALL ]]; then....fi so that Travis doesn't fail with this addition)

Also need to check if OpenCV built with the updated version of FFmpeg, will post an update on that in a while

@rajat2004
Copy link
Collaborator Author

@rsnk96 Ohk, thanks for the detailed explanation and testing, otherwise wouldn't have caught the problem. Would have tested it myself but don't have a very good internet connection right now.
I've Pushed a fix as you described

@rajat2004
Copy link
Collaborator Author

Things are definitely wrong right now -

./opencvDirectInstall.sh: line 30: export: `--variable': not a valid identifier
./opencvDirectInstall.sh: line 30: export: `pkg-config)': not a valid identifier

https://travis-ci.org/rsnk96/Ubuntu-Setup-Scripts/jobs/626129056#L1195
Need to look into this and fix it

@rajat2004
Copy link
Collaborator Author

Error's not there now, changed run_and_echo () { $1 ... } to run_and_echo () { eval $1 ... } to force it to run the embedded commands

@rajat2004
Copy link
Collaborator Author

Lapacke change fixes the Atlas not found problem, the OpenBLAS ones generally need code change
Will try to have a go at some of the other errors in #20

@rsnk96
Copy link
Owner

rsnk96 commented Dec 18, 2019

Updates looks good, and haha didn't know that there was a version of lapack with an e that has more features. Whenever you feel you've hit a wall wrt fixing the issues in #20, feel free to merge this PR

@rajat2004
Copy link
Collaborator Author

@rsnk96 Fixing the other errors will need more time, will try to do that in another PR, this one has enough things in it already
It'll be good to have another test in a docker container soon, but will merge this for now. Also will update the issue with the current errors

@rajat2004 rajat2004 merged commit eef7fdb into rsnk96:master Dec 18, 2019
@rajat2004 rajat2004 deleted the ffmpeg-export branch December 18, 2019 16:20
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