OpenCV: Adapt to API changes in OpenCV 4.5.2+

[ferdnyc]

Seems that OpenCV 4.5.1 has massively altered the cv::Tracker API, with the one our code uses being moved to cv::legacy::Tracker, and a new implementation taking its place in cv::Tracker.

Why this trigger was pulled with only a revision bump from 4.5.0 to 4.5.1, I cannot for the life of me understand, but cv::TrackerBoosted and friends are present in 4.5.0, but suddenly gone in 4.5.1. I encountered this when trying to build on Windows 10, now that MSYS2 is packaging OpenCV 4.5.1.

A function cv::legacy::upgradeTrackingAPI() is provided which can seemingly be used to wrap old code. But, since that function is only present in 4.5.1+, it doesn't help us with simultaneous support of OpenCV versions before and after 4.5.1.

Supporting OpenCV 4.5.1, specifically, turned out to be impossible as the released code is horribly broken for the "legacy" tracking API. There are headers missing from the install that make it unusable. Fortunately, 4.5.2 is since released that corrects this.

So, this PR blocks use of OpenCV verson 4.5.1 with a warning about the problem, and adds an OpenCVUtilities.h header which normalizes our code to be compatible with both the previous 4.5.0-and-earlier API, as well as the new-legacy 4.5.2+, in the manner of our other *Utilities.h headers.

The CMake build will define USE_LEGACY_TRACKER on the build command line if the OpenCV version is greater than 4.5.1, and if that macro is defined:

• OPENCV_TRACKER_TYPE is defined to be cv::legacy::Tracker
• OPENCV_TRACKER_NS is defined as cv::legacy

Otherwise, they are cv::Tracker and cv, respectively.

The rest of the code uses cv::Ptr<OPENCV_TRACKER_TYPE> as the pointer type for tracker, and OPENCV_TRACKER_NS::TrackerImplementation::create() to instantiate trackers.

Other included changes

1. I moved the ClipProcessingJobs class into the openshot namespace, because for some reason it wasn't.
2. I moved some includes around between .h and .cpp files, to avoid over-including headers, and dropped some unnecessary includes
3. I dropped the using namespace std; from CVTracker.cpp, adding std:: prefixes where needed.

[codecov]

Codecov Report

Merging #639 (339d194) into develop (a00bbb7) will decrease coverage by 0.04%.
The diff coverage is 16.00%.

@@             Coverage Diff             @@
##           develop     #639      +/-   ##
===========================================
- Coverage    52.42%   52.38%   -0.05%     
===========================================
  Files          151      151              
  Lines        12346    12345       -1     
===========================================
- Hits          6473     6467       -6     
- Misses        5873     5878       +5     

Impacted Files	Coverage Δ
src/CVObjectDetection.cpp	0.00% <ø> (ø)
src/CVTracker.h	81.81% <ø> (ø)
src/ClipProcessingJobs.cpp	0.00% <0.00%> (ø)
src/effects/ObjectDetection.cpp	0.00% <0.00%> (ø)
src/effects/Stabilizer.cpp	0.00% <0.00%> (ø)
src/effects/Tracker.cpp	0.00% <0.00%> (ø)
src/effects/Tracker.h	0.00% <ø> (ø)
tests/Point.cpp	100.00% <ø> (ø)
src/CVTracker.cpp	77.39% <25.00%> (-3.56%)	⬇️
src/CVStabilization.cpp	88.17% <33.33%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a00bbb7...339d194. Read the comment docs.

[ferdnyc]

Open bug report regarding the private-header issue: opencv/opencv#19260

[BrennoCaldato]

Thanks @ferdnyc. I'll check the tracking_internals.hpp problem

[ferdnyc]

(FTR, as expected doing this in a MinGW shell on Windows does fix the problem...)

wget 'https://repo.msys2.org/mingw/x86_64/mingw-w64-x86_64-opencv-4.5.0-2-any.pkg.tar.zst'
pacman -U ./mingw-w64-x86_64-opencv-4.5.0-2-any.pkg.tar.zst

[github-actions]

Merge conflicts have been detected on this PR, please resolve.

[github-actions]

Merge conflicts have been detected on this PR, please resolve.

[BrennoCaldato]

@ferdnyc The issue with headers installation seems fixed in this PR (opencv/opencv#19817) and incorporated to opencv-contrib 4.5.2

We can move forward with this PR. I merged with Develop and tested it already but I cannot push it to your fork. If you prefer I can push directly to libopenshot and make a new PR

Cc: @jonoomph

[ferdnyc]

@BrennoCaldato I'll give you push access, hold on a second while I remember how to invite collaborators... 😉

[ferdnyc]

@BrennoCaldato Invite sent.

[ferdnyc]

I'm wondering if the version check should enable the legacy tracking code starting from OpenCV version 4.5.2, and if it detects 4.5.1 either emit a fatal config error, or emit a warning and disable OpenCV in the build. 'Cause... it just ain't gonna work. (And at least MSYS2 is, last I checked, still on 4.5.1 as their latest/current packaged release, for now.)

[ferdnyc]

Yeah, in fact 4.5.2 isn't even in the MSYS2 pending queue, so far. There's a notice up about the new version, but nobody's gotten around to packaging it yet. https://packages.msys2.org/base/mingw-w64-opencv

[BrennoCaldato]

I'm wondering if the version check should enable the legacy tracking code starting from OpenCV version 4.5.2, and if it detects 4.5.1 either emit a fatal config error, or emit a warning and disable OpenCV in the build. 'Cause... it just ain't gonna work. (And at least MSYS2 is, last I checked, still on 4.5.1 as their latest/current packaged release, for now.)

Totally agree.

I'm still not being able to push. I have already pulled the repo and it's up to date

To https://github.com/ferdnyc/libopenshot.git
 ! [remote rejected] legacy-tracker -> legacy-tracker (protected branch hook declined)
error: failed to push some refs to 'https://github.com/ferdnyc/libopenshot.git'

[ferdnyc]

@BrennoCaldato You should've gotten a collab invite for my repo through Github, I sent it yesterday.

The other option, which I feel dumb for not thinking of sooner, is to pull one of my favorite tricks and open a PR against the PR. If you were to open a PR targeting the legacy-tracker branch in my fork, when merged those changes would become part of this PR.

[ferdnyc]

@BrennoCaldato @jonoomph

Meanwhile, because I'm a masochist I'm currently working on building OpenCV 4.5.2 as a MinGW package, so I can submit an update to MSYS2. It's slow going, though, because it's building in Win10 running in a VM on my Linux box.

[BrennoCaldato]

I feel dumb for not thinking of sooner, is to pull one of my favorite tricks and open a PR against the PR. If you were to open a PR targeting the legacy-tracker branch in my fork, when merged those changes would become part of this PR.

Done with the PR against the PR :)

[ferdnyc]

Added a message to freak out and disable OpenCV, if evil version 4.5.1 is detected.

[ferdnyc]

I'm still not being able to push. I have already pulled the repo and it's up to date

To https://github.com/ferdnyc/libopenshot.git
 ! [remote rejected] legacy-tracker -> legacy-tracker (protected branch hook declined)
error: failed to push some refs to 'https://github.com/ferdnyc/libopenshot.git'

Hmm. I'll check on those protection rules, I thought they only applied to develop. (Glad to see the PR worked, though.)

[ferdnyc]

@BrennoCaldato @jonoomph

Ehhhhhhxcellent. I now have a MinGW{32,64} packaged build of OpenCV 4.5.2 for which, when libopenshot [ETA: this PR's branch of libopenshot, I mean] is built with it, all of our unit tests still pass. Just have to polish up the configs and the commits, and I can open a PR to MSYS2 and get it updated there.

I also have to submit a PR to opencv, because they once again released broken code in the opencv_community package. (Though minorly, this time; a new module assumes that iconv can be used without configuration, which is true on POSIX where it's part of libc... but MinGW has a separate libiconv that has be be linked in.)

[BrennoCaldato]

Great @ferdnyc. This means we can upgrade opencv after the effect-parenting is merged

[ferdnyc]

Hahaha! So, one minor issue I discovered with the Catch2/CTest "separate executable for each class" style of unit test runners: If you actually run the unit tests on a desktop Windows machine, you get a Firewall access request for each of the... 19 unit test runners! They're treated as separate applications.

Which makes me think we probably need to do something akin to the mode=unittest special-casing in the OpenShotApp and MainWindow classes of openshot-qt, and have the unit tests build with ZeroMQ's network connectivity auto-disabled through a #define or something.

(And part of me thinks it should always start disabled, unless it's explicitly activated by means of an environment variable, a config file setting, an API call, or some other mechanism. OpenShot actually uses libopenshot's ZmqLogger API to discover its connectivity parameters, I was recently reminded, so there's no reason it couldn't also trigger an explicit activation of the network endpoint. And it could also hold off, and do that IFF it actually intends to receive logging.)

...But, this is all off topic.

cc: @jonoomph

[ferdnyc]

OpenCV 2.5.2 packaging update submitted as msys2/MINGW-packages#8457.

OpenCV module changes not submitted, because I discovered prior (and conflicting) PR opencv/opencv_contrib#2916. So now I'm hoping to get a response from the submitter there, as to why they chose their approach vs. mine, to see whether I should push forward with my changes or abandon them and adopt the ones from that PR instead.

[ferdnyc]

I'm going to extract just he CMake code that disables OpenCV support under 4.5.1, and merge that into develop from a new branch. Turns out, Ubuntu Impish (future 21.10) currently has 4.5.1 in its repos, and it's killing our PPA builds.

The rest of what's here, actually I think we're pretty good on, so maybe it can all be merged. But I want to kind of "go around the room" to get everyone's take on that one more time, before merging... and I don't want to delay the 4.5.1-mitigating stuff even that long.

[github-actions]

Merge conflicts have been detected on this PR, please resolve.

[ferdnyc]

Whoops! Well, it seems the newly-released Fedora 34 includes OpenCV 4.5.2, so it's definitely time to merge this one. Going to do that now.