I just went through a debugging exercise that was so ridiculous, I just had to
write it up. Some of this probably should go into a bug report instead of a
rant, but I'm tired. And clearly I don't care anymore.
OK, so I'm doing computer vision work. OpenCV has been providing basic functions
in this area, so I have been using them for a while. Just for really, really
basic stuff, like
projection. The C API was kinda weird, and their error
handling is a bit ridiculous (if you give it arguments it doesn't like, it
asserts!), but it has been working fine for a while.
At some point (around OpenCV 3.0) somebody over there decided that they didn't
like their C API, and that this was now a C++ library. Except the docs still
documented the C API, and the website said it supported C, and the code wasn't
actually removed. They just kinda stopped testing it and thinking about it. So
it would mostly continue to work, except some poor saps would see weird
failures; like
this and
this, for instance. OpenCV 3.2 was the last version
where it was mostly possible to keep using the old C code,
even when compiling
without optimizations. So I was doing that for years.
So now, in 2020, Debian is finally shipping a version of OpenCV that
definitively does not work with the old code, so I had to do something. Over
time I stopped using everything about OpenCV, except a few
cvProjectPoints2()
calls. So I decided to just write a small C++ shim to call the new version of
that function, expose that with =extern "C"= to the rest of my world, and I'd be
done. And normally I would be, but this is OpenCV we're talking about. I wrote
the shim, and it didn't work. The code built and ran, but the results were
wrong. After some pointless debugging, I boiled the problem down to this test
program:
#include <opencv2/calib3d.hpp>
#include <stdio.h>
int main(void)
double fx = 1000.0;
double fy = 1000.0;
double cx = 1000.0;
double cy = 1000.0;
double _camera_matrix[] =
fx, 0, cx,
0, fy, cy,
0, 0, 1 ;
cv::Mat camera_matrix(3,3, CV_64FC1, _camera_matrix);
double pp[3] = 1., 2., 10. ;
double qq[2] = 444, 555 ;
int N=1;
cv::Mat object_points(N,3, CV_64FC1, pp);
cv::Mat image_points (N,2, CV_64FC1, qq);
// rvec,tvec
double _zero3[3] = ;
cv::Mat zero3(1,3,CV_64FC1, _zero3);
cv::projectPoints( object_points,
zero3,zero3,
camera_matrix,
cv::noArray(),
image_points,
cv::noArray(), 0.0);
fprintf(stderr, "manually-projected no-distortion: %f %f\n",
pp[0]/pp[2] * fx + cx,
pp[1]/pp[2] * fy + cy);
fprintf(stderr, "opencv says: %f %f\n", qq[0], qq[1]);
return 0;
This is as trivial as it gets. I project one point through a pinhole camera, and
print out the right answer (that I can easily compute, since this is trivial),
and what OpenCV reports:
$ g++ -I/usr/include/opencv4 -o tst tst.cc -lopencv_calib3d -lopencv_core && ./tst
manually-projected no-distortion: 1100.000000 1200.000000
opencv says: 444.000000 555.000000
Well that's no good. The answer is wrong, but it looks like it didn't even write
anything into the output array. Since this is supposed to be a thin shim to C
code, I want this thing to be filling in C arrays, which is what I'm doing here:
double qq[2] = 444, 555 ;
int N=1;
cv::Mat image_points (N,2, CV_64FC1, qq);
This is how the C API has worked forever, and their C++ API works the same way,
I thought. Nothing barfed, not at build time, or run time. Fine. So I went to
figure this out. In the true spirit of C++, the
new API is inscrutable. I'm
passing in
cv::Mat, but the API wants
cv::InputArray for some arguments and
cv::OutputArray for others. Clearly
cv::Mat can be coerced into either of
those types (and that's what you're supposed to do), but the details are not
meant to be understood. You can read the snazzy C++-style documentation.
Clicking on "
OutputArray" in the doxygen gets you
here. Then I guess you're
supposed to click on "
_OutputArray", and you get
here. Understand what's going
on now? Me neither.
Stepping through the code revealed the problem.
cv::projectPoints() looks like
this:
void cv::projectPoints( InputArray _opoints,
InputArray _rvec,
InputArray _tvec,
InputArray _cameraMatrix,
InputArray _distCoeffs,
OutputArray _ipoints,
OutputArray _jacobian,
double aspectRatio )
....
_ipoints.create(npoints, 1, CV_MAKETYPE(depth, 2), -1, true);
....
I.e. they're allocating a new data buffer for the output, and giving it back to
me via the
OutputArray object. This object already had a buffer, and that's
where I was expecting the output to go. Instead it went to the brand-new buffer
I didn't want. Issues:
- The
OutputArray object knows it already has a buffer, and they could just
use it instead of allocating a new one
- If for some reason my buffer smells bad, they could complain to tell me
they're ignoring it to save me the trouble of debugging, and then bitching
about it on the internet
- I think dynamic memory allocation smells bad
- Doing it this way means the new function isn't a drop-in replacement for the
old function
Well that's just super. I can call the C++ function, copy the data into the
place it's supposed to go to, and then deallocate the extra buffer. Or I can
pull out the meat of the function I want into my project, and then I can drop
the OpenCV dependency entirely. Clearly that's the way to go.
So I go poking back into their code to grab what I need, and here's what I see:
static void cvProjectPoints2Internal( const CvMat* objectPoints,
const CvMat* r_vec,
const CvMat* t_vec,
const CvMat* A,
const CvMat* distCoeffs,
CvMat* imagePoints, CvMat* dpdr CV_DEFAULT(NULL),
CvMat* dpdt CV_DEFAULT(NULL), CvMat* dpdf CV_DEFAULT(NULL),
CvMat* dpdc CV_DEFAULT(NULL), CvMat* dpdk CV_DEFAULT(NULL),
CvMat* dpdo CV_DEFAULT(NULL),
double aspectRatio CV_DEFAULT(0) )
...
Looks familiar? It should. Because this is the original C-API function they
replaced. So in their quest to move to C++, they left the original code intact,
C API and everything, un-exposed it so you couldn't call it anymore, and made
a new, shitty C++ wrapper for people to call instead.
CvMat is still there. I
have no words.
Yes, this is a massive library, and maybe other parts of it indeed did make some
sort of non-token transition, but this thing is ridiculous. In the end, here's
the function I ended up with (licensed as OpenCV; see the comment)
// The implementation of project_opencv is based on opencv. The sources have
// been heavily modified, but the opencv logic remains. This function is a
// cut-down cvProjectPoints2Internal() to keep only the functionality I want and
// to use my interfaces. Putting this here allows me to drop the C dependency on
// opencv. Which is a good thing, since opencv dropped their C API
//
// from opencv-4.2.0+dfsg/modules/calib3d/src/calibration.cpp
//
// Copyright (C) 2000-2008, Intel Corporation, all rights reserved.
// Copyright (C) 2009, Willow Garage Inc., all rights reserved.
// Third party copyrights are property of their respective owners.
//
// Redistribution and use in source and binary forms, with or without modification,
// are permitted provided that the following conditions are met:
//
// * Redistribution's of source code must retain the above copyright notice,
// this list of conditions and the following disclaimer.
//
// * Redistribution's in binary form must reproduce the above copyright notice,
// this list of conditions and the following disclaimer in the documentation
// and/or other materials provided with the distribution.
//
// * The name of the copyright holders may not be used to endorse or promote products
// derived from this software without specific prior written permission.
//
// This software is provided by the copyright holders and contributors "as is" and
// any express or implied warranties, including, but not limited to, the implied
// warranties of merchantability and fitness for a particular purpose are disclaimed.
// In no event shall the Intel Corporation or contributors be liable for any direct,
// indirect, incidental, special, exemplary, or consequential damages
// (including, but not limited to, procurement of substitute goods or services;
// loss of use, data, or profits; or business interruption) however caused
// and on any theory of liability, whether in contract, strict liability,
// or tort (including negligence or otherwise) arising in any way out of
typedef union
struct
double x,y;
;
double xy[2];
point2_t;
typedef union
struct
double x,y,z;
;
double xyz[3];
point3_t;
void project_opencv( // outputs
point2_t* q,
point3_t* dq_dp, // may be NULL
double* dq_dintrinsics_nocore, // may be NULL
// inputs
const point3_t* p,
int N,
const double* intrinsics,
int Nintrinsics)
const double fx = intrinsics[0];
const double fy = intrinsics[1];
const double cx = intrinsics[2];
const double cy = intrinsics[3];
double k[12] = ;
for(int i=0; i<Nintrinsics-4; i++)
k[i] = intrinsics[i+4];
for( int i = 0; i < N; i++ )
double z_recip = 1./p[i].z;
double x = p[i].x * z_recip;
double y = p[i].y * z_recip;
double r2 = x*x + y*y;
double r4 = r2*r2;
double r6 = r4*r2;
double a1 = 2*x*y;
double a2 = r2 + 2*x*x;
double a3 = r2 + 2*y*y;
double cdist = 1 + k[0]*r2 + k[1]*r4 + k[4]*r6;
double icdist2 = 1./(1 + k[5]*r2 + k[6]*r4 + k[7]*r6);
double xd = x*cdist*icdist2 + k[2]*a1 + k[3]*a2 + k[8]*r2+k[9]*r4;
double yd = y*cdist*icdist2 + k[2]*a3 + k[3]*a1 + k[10]*r2+k[11]*r4;
q[i].x = xd*fx + cx;
q[i].y = yd*fy + cy;
if( dq_dp )
double dx_dp[] = z_recip, 0, -x*z_recip ;
double dy_dp[] = 0, z_recip, -y*z_recip ;
for( int j = 0; j < 3; j++ )
double dr2_dp = 2*x*dx_dp[j] + 2*y*dy_dp[j];
double dcdist_dp = k[0]*dr2_dp + 2*k[1]*r2*dr2_dp + 3*k[4]*r4*dr2_dp;
double dicdist2_dp = -icdist2*icdist2*(k[5]*dr2_dp + 2*k[6]*r2*dr2_dp + 3*k[7]*r4*dr2_dp);
double da1_dp = 2*(x*dy_dp[j] + y*dx_dp[j]);
double dmx_dp = (dx_dp[j]*cdist*icdist2 + x*dcdist_dp*icdist2 + x*cdist*dicdist2_dp +
k[2]*da1_dp + k[3]*(dr2_dp + 4*x*dx_dp[j]) + k[8]*dr2_dp + 2*r2*k[9]*dr2_dp);
double dmy_dp = (dy_dp[j]*cdist*icdist2 + y*dcdist_dp*icdist2 + y*cdist*dicdist2_dp +
k[2]*(dr2_dp + 4*y*dy_dp[j]) + k[3]*da1_dp + k[10]*dr2_dp + 2*r2*k[11]*dr2_dp);
dq_dp[i*2 + 0].xyz[j] = fx*dmx_dp;
dq_dp[i*2 + 1].xyz[j] = fy*dmy_dp;
if( dq_dintrinsics_nocore )
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 0) + 0] = fx*x*icdist2*r2;
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 1) + 0] = fy*(y*icdist2*r2);
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 0) + 1] = fx*x*icdist2*r4;
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 1) + 1] = fy*y*icdist2*r4;
if( Nintrinsics-4 > 2 )
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 0) + 2] = fx*a1;
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 1) + 2] = fy*a3;
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 0) + 3] = fx*a2;
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 1) + 3] = fy*a1;
if( Nintrinsics-4 > 4 )
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 0) + 4] = fx*x*icdist2*r6;
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 1) + 4] = fy*y*icdist2*r6;
if( Nintrinsics-4 > 5 )
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 0) + 5] = fx*x*cdist*(-icdist2)*icdist2*r2;
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 1) + 5] = fy*y*cdist*(-icdist2)*icdist2*r2;
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 0) + 6] = fx*x*cdist*(-icdist2)*icdist2*r4;
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 1) + 6] = fy*y*cdist*(-icdist2)*icdist2*r4;
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 0) + 7] = fx*x*cdist*(-icdist2)*icdist2*r6;
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 1) + 7] = fy*y*cdist*(-icdist2)*icdist2*r6;
if( Nintrinsics-4 > 8 )
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 0) + 8] = fx*r2; //s1
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 1) + 8] = fy*0; //s1
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 0) + 9] = fx*r4; //s2
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 1) + 9] = fy*0; //s2
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 0) + 10] = fx*0;//s3
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 1) + 10] = fy*r2; //s3
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 0) + 11] = fx*0;//s4
dq_dintrinsics_nocore[(Nintrinsics-4)*(2*i + 1) + 11] = fy*r4; //s4
This does only the stuff I need: projection only (no geometric
transformation), and gradients in respect to the point coordinates and
distortions only. Gradients in respect to fxy and cxy are trivial, and I don't
bother reporting them.
So now I don't compile or link against OpenCV, my code builds and runs on
Debian/sid and (surprisingly) it runs much faster than before. Apparently there
was a lot of pointless overhead happening.
Alright. Rant over.
