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

[OpenCV2] add new "old" port #7849

Merged
merged 25 commits into from
May 6, 2020
Merged

Conversation

cenit
Copy link
Contributor

@cenit cenit commented Aug 23, 2019

Reopening here #3360, which is locked down

@cenit
Copy link
Contributor Author

cenit commented Aug 23, 2019

Some work is still necessary (update to latest 2.x version, conforming to latest opencv3/4 patching style, etc), but I remember the base port was functional

@cenit
Copy link
Contributor Author

cenit commented Aug 23, 2019

@vicroms Default features and external ffmpeg integration (yay! no more prebuilt dlls anywhere!) are ok and do not exhibit any problem. I still have to check cuda, it will be next (and last) effort here for now

any feedback is welcome! :)

@cenit
Copy link
Contributor Author

cenit commented Aug 23, 2019

I am explicitly avoiding adding cross-check on opencv3 and opencv4 to verify that opencv2 is not installed to not trigger any build there. It would just add noise to CI for now. We can always add those checks later, when CI won't be interesting anymore

@cenit
Copy link
Contributor Author

cenit commented Aug 23, 2019

Finally, as a note, the build system of OpenCV2 is very archaic and has problems with the debug;libnamed.lib;optimized;libname.lib structure of modern cmake symbols for libraries. Luckily if you use a target instead of a complex symbol, those machinery is hidden away from the build system thanks to cmake itself. That’s why I explicitly modified those symbols whenever necessary in the source code. Two features are still missing this treatment (can be seen from the commit messages): they are jasper and openexr. The former would require a patched module instead of the official one, the latter would require some improvements to our own supplied module to expose also targets. If it is necessary I can do that, otherwise if anyone is interested, it would be terrific.
The same goes for UWP. It does not work atm, but I am not interested in it. It might be possible to fix it, maybe, without too much hassle.

@cenit
Copy link
Contributor Author

cenit commented Aug 26, 2019

I tested also cuda features and now they work properly.
I’d say it’s ready for merging

@cenit cenit changed the title [OpenCV2] is it interesting? [OpenCV2] add new "old" port Aug 26, 2019
@cenit
Copy link
Contributor Author

cenit commented Sep 10, 2019

@vicroms do you have any feedback?

@cenit
Copy link
Contributor Author

cenit commented Sep 23, 2019

any news here? pinging @ras0219-msft since he is marked as assignee

@vicroms
Copy link
Member

vicroms commented Sep 23, 2019

Hi @cenit,

At the moment we're holding back this PR, it is something that we want to merge, but are not ready to do so yet. Right now, we're in the research and design stages for a solution to the versioning problem (no definitive dates for when we'll implement it yet).

Besides versioning, we also have considered the idea of having a list of community ports (ports maintained outside the main vcpkg repo) that users can clone and use with --overlay-ports, that may be more readily implementable than actual versioning.

@cenit
Copy link
Contributor Author

cenit commented Sep 27, 2019

I also added a feature in darknet that enables the port to use opencv2, just to test it, like what I did for opencv3

@cenit
Copy link
Contributor Author

cenit commented Oct 10, 2019

merged with master to keep the branch up to date, sorry for the unnecessary CI trigger

@JackBoosY
Copy link
Contributor

/azp run

@cenit
Copy link
Contributor Author

cenit commented May 2, 2020

Any news here? I will keep up to date from time to time, hoping for merge

@JackBoosY
Copy link
Contributor

@strega-nil Could you please review this PR?

Thanks.

@JackBoosY
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JackBoosY
Copy link
Contributor

I think we should accept these new ports with older versions. There are always some developers using them.

@JackBoosY
Copy link
Contributor

Ping @strega-nil

@strega-nil
Copy link
Contributor

@cenit it seems opencv2 on x64-uwp fails; is that a thing you need to fix or do you want to baseline it?

@cenit
Copy link
Contributor Author

cenit commented May 6, 2020

It should be baselined. I fear fixing opencv2 for UWP would take too much time.
Shall I do it?

@strega-nil
Copy link
Contributor

strega-nil commented May 6, 2020

Yup, go ahead; might also add a Supports field, if you think that's wise

@cenit
Copy link
Contributor Author

cenit commented May 6, 2020

I just saw that opencv3 was skipped from CI, and I remember @vicroms explaining reasons to do so. I preferred to do the same also for OpenCV2

@strega-nil
Copy link
Contributor

Alright, fair enough! Thanks @cenit, merging now.

@strega-nil strega-nil merged commit 7a0c97b into microsoft:master May 6, 2020
@cenit cenit deleted the dev/cenit/opencv2 branch May 6, 2020 22:30
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.

7 participants