Clever Geek Handbook

Checking OpenCvSharp wrapper over OpenCV using PVS-Studio





Figure 2






OpenCV is a library of computer vision algorithms, image processing and general purpose numerical algorithms with open source, familiar to many C ++ developers. In addition to C ++, OpenCV is also being developed for Python, Java, Ruby, Matlab, Lua, and other languages. Since among these languages ​​there is no my main one, C #, I decided to pay attention to the OpenCvSharp wrapper library under C # and check this project. What came of this can be found in this article.



Introduction



Earlier, before joining PVS-Studio, I was engaged in robotics at exhibitions. My tasks included the most basic fix (if a major breakdown occurred, then the robot was given to another person), as well as the development of a wide variety of software and utilities.







Picture 1






Tired and recently arrived in a new city, I along with a freshly unpacked KIKI robot.



Speaking of development. That was pretty funny. Every time an idea came up to someone from the team, what else would surprise the guests of the exhibitions, we brought this issue up for general discussion and, if the idea was a good one, we took up implementation. Once, the idea came to us to make a creature that responds to a human face with a welcoming speech.



After searching the Internet for a library for my needs, I came across the OpenCV website, libraries of computer vision algorithms. I was soon disappointed - OpenCV was implemented in C ++. My knowledge about the advantages that I got from college was clearly not enough. Therefore, by briefly googling, I came across OpenCvSharp - wrapper of this library under C #, my main language. Six months have passed since then, the program has been written and used for a long time, and I decided to get under the hood of OpenCvSharp and check its source code using the PVS-Studio static analyzer.



Audited Project



OpenCvSharp is a wrapper over OpenCV for using the library in C # projects. OpenCV library, by the way, we also checked . The advantages of OpenCvSharp include a large collection of code samples, cross-platform (can work on any platform that Mono supports) and ease of installation.



The wrapper is a small project and contains about 112,200 lines of code in C #. Of these, 1.2% are comments, which, incidentally, is somehow suspiciously small. But for such a small project there are a lot of mistakes. In the article, I wrote out more than 20 errors, but there were others that were not so interesting or obvious.



PVS-Studio code analyzer



PVS-Studio is a tool for detecting errors and potential vulnerabilities in the source code of programs written in C, C ++, C # and Java. Runs on Windows, Linux, and macOS. In addition to unattainable code, errors and typos, PVS-Studio can detect potential vulnerabilities, as noted above. Therefore, it can be considered as a tool for static application security testing (Static Application Security Testing, SAST).



Pieces of code that caught attention when examining the analyzer report



The WriteableBitmapConverter method attracts attention immediately with four of the same type of PVS-Studio warnings: 





static WriteableBitmapConverter() { optimumChannels = new Dictionary <PixelFormat, int>(); optimumChannels[PixelFormats.Indexed1] = // <= optimumChannels[PixelFormats.Indexed8] = // <= optimumChannels[PixelFormats.Gray2] = optimumChannels[PixelFormats.Gray4] = optimumChannels[PixelFormats.Gray8] = optimumChannels[PixelFormats.Gray16] = optimumChannels[PixelFormats.Gray32Float] = optimumChannels[PixelFormats.Indexed1] = // <= optimumChannels[PixelFormats.Indexed2] = optimumChannels[PixelFormats.Indexed4] = optimumChannels[PixelFormats.Indexed8] = // <= .... optimumTypes = new Dictionary <PixelFormat, MatType>(); optimumTypes[PixelFormats.Indexed1] = // <= optimumTypes[PixelFormats.Indexed8] = // <= optimumTypes[PixelFormats.Gray2] = optimumTypes[PixelFormats.Gray4] = optimumTypes[PixelFormats.Gray8] = optimumTypes[PixelFormats.Indexed1] = // <= optimumTypes[PixelFormats.Indexed2] = optimumTypes[PixelFormats.Indexed4] = optimumTypes[PixelFormats.Indexed8] = // <= optimumTypes[PixelFormats.BlackWhite] = .... } .... public static class PixelFormats { .... public static PixelFormat Indexed8 { get; } .... public static PixelFormat Indexed1 { get; } .... }


The PixelFormats class is defined in the System.Windows.Media namespace and is a collection of various pixel formats. The analyzer draws attention to the fact that the WriteableBitmapConverter method reassigns the values ​​to the optimumChannels [PixelFormats.Indexed1] and optimumChannels [PixelFormats.Indexed8] elements , which makes no practical sense. It is unclear whether this is a simple typo or something else was meant. By the way, this section of code clearly demonstrates the benefits of static analyzers. When you see a bunch of lines of the same type before your eyes, your eyes begin to “blur”, and your attentiveness disappears - it’s not surprising if even after code review a typo creeps into the program. And the static analyzer has no problems with attentiveness and it does not need to rest, therefore it is easier for it to find such errors.







Figure 5






Feel the power and power of static analysis.



PVS-Studio Warning : V3021 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains method return. This means that the second 'if' statement is senseless InputArray.cs 394 



 private static MatType EstimateType(Type t) { .... if (t == typeof(Vec2b)) return MatType.CV_8UC2; if (t == typeof(Vec3b)) return MatType.CV_8UC3; if (t == typeof(Vec4b)) return MatType.CV_8UC4; if (t == typeof(Vec6b)) return MatType.CV_8UC(6); if (t == typeof(Vec2s)) // <= return MatType.CV_16SC2; .... if (t == typeof(Vec2s)) // <= return MatType.CV_32SC2; .... }


This error is a bit like the previous one. The programmer made a situation where the same condition is checked twice. In this case, this does not make sense - then the branch of the "duplicated" if statement will not be executed, since:





The developer should double-check this piece of code. It is likely that in place of the second variable Vec2s should be some other.



PVS-Studio Warning : V3010 The return value of function 'ToString' is required to be utilized. ImgProcTest.cs 80 



 public static RectanglesIntersectTypes RotatedRectangleIntersection(RotatedRect rect1, RotatedRect rect2, out Point2f[] intersectingRegion) { using (var intersectingRegionVec = new VectorOfPoint2f()) { int ret = NativeMethods .imgproc_rotatedRectangleIntersection_vector( rect1, rect2, intersectingRegionVec.CvPtr); intersectingRegion = intersectingRegionVec.ToArray(); return (RectanglesIntersectTypes) ret; } } public void RotatedRectangleIntersectionVector() { var rr1 = new RotatedRect(new Point2f(100, 100), new Size2f(100, 100), 45); var rr2 = new RotatedRect(new Point2f(130, 100), new Size2f(100, 100), 0); Cv2.RotatedRectangleIntersection(rr1, rr2, out var intersectingRegion); .... intersectingRegion.ToString(); }


The RotatedRectangleIntersection method returns an array of Point2f elements through the intersectingRegion parameter. After the program fills in the intersectingRegion with values, the ToString () method is called on this array . With the elements of the array, no changes occur from this and no useful work is done in the last line, therefore there is reason to suspect the developer that he simply forgot to remove it.



PVS-Studio warnings : 





 public static double CalibrateCamera(....) { if (objectPoints == null) throw new ArgumentNullException(nameof(objectPoints)); if (objectPoints == null) throw new ArgumentNullException(nameof(objectPoints)); .... }


In this case, a code fragment was duplicated, due to which two warnings appeared. The first warning indicates that both if statements have the same conditional expression. If this expression is true, the method will exit from the then- branch of the first if statement . For this reason, the second condition will always be false, as indicated by the following warning. Apparently, the text was copied, but forgot to correct.







Figure 6






Cute Copy-Paste.



Other similar analyzer warnings:





PVS-Studio warning : V3022 Expression 'label == MarkerValue' is always false. Labeller.cs 135 



 internal static class Labeller { .... private const int MarkerValue = -1; public static int Perform(Mat img, CvBlobs blobs) { .... int label = 0; int lastLabel = 0; CvBlob lastBlob = null; for (int y = 0; y < h; y++) { for (int x = 0; x < w; x++) { if (imgIn[x + y * step] == 0) continue; bool labeled = labels[y, x] != 0; if (....) { labeled = true; // Label contour. label++; if (label == MarkerValue) // <= throw new Exception(); .... } .... } .... } } }


In this section of code, a label variable of zero is created. When a certain condition is met, one can be added to this variable. Moreover, in the code, the value of the variable label does not change downward. In the line marked with an arrow, this variable is compared with a constant equal to -1, which makes no practical sense.



PVS-Studio Warning : V3038 The argument was passed to method several times. It is possible that other argument should be passed instead. Cv2_photo.cs 124 



 public static void FastNlMeansDenoisingMulti(....) { .... NativeMethods.photo_fastNlMeansDenoisingMulti( srcImgPtrs, srcImgPtrs.Length, dst.CvPtr, imgToDenoiseIndex, templateWindowSize, h, templateWindowSize, searchWindowSize); .... }


To understand what the analyzer means, let's look at the parameters of the photo_fastNlMeansDenoisingMulti method: 



 public static extern void photo_fastNlMeansDenoisingMulti( IntPtr[] srcImgs, int srcImgsLength, IntPtr dst, int imgToDenoiseIndex, int temporalWindowSize, float h, int templateWindowSize, int searchWindowSize)


Simplify even more to make it very obvious. Compare these lines: 



 NativeMethods.photo_fastNlMeansDenoisingMulti( .... templateWindowSize, .... templateWindowSize, ....); public static extern void photo_fastNlMeansDenoisingMulti( .... int temporalWindowSize, .... int templateWindowSize, ....)


The analyzer draws attention to the fact that the developer used the templateWindowSize variable twice, although most likely, temporalWindowSize should be in the place of the first mention of this variable. It is also suspicious that the value of temporalWindowSize in the photo_fastNlMeansDenoisingMulti method is not used at all. Perhaps this was done deliberately, but in place of the developers it is worth taking a closer look at this code, is there a mistake crept in there?



Similar analyzer warnings:





The following error will be a bit similar to the previous one.



Warning PVS-Studio : V3066 Possible incorrect order of arguments passed to 'calib3d_Rodrigues_MatToVec' method: 'matrixM.CvPtr' and 'vectorM.CvPtr'. Cv2_calib3d.cs 86 



 public static void Rodrigues(double[,] matrix, out double[] vector, out double[,] jacobian) { .... using (var jacobianM = new Mat<double>()) { NativeMethods.calib3d_Rodrigues_MatToVec (matrixM.CvPtr, vectorM.CvPtr, jacobianM.CvPtr); .... } }


Let's look at the parameters calib3d_Rodrigues_MatToVec 



 public static extern void calib3d_Rodrigues_MatToVec( IntPtr vector, IntPtr matrix, IntPtr jacobian)


Perhaps when calling the calib3d_Rodrigues_MatToVec method, the arguments matrixM.CvPtr and vectorM.CvPtr were mixed up. Developers should take a closer look at this code. There is a possibility that an error crept into it that interferes with the correct calculations.



PVS-Studio Warning : V3063 A part of conditional expression is always false if it is evaluated: data == null. Mat.cs 3539 



 private void CheckArgumentsForConvert(....) { .... if (data == null) throw new ArgumentNullException(nameof(data)); MatType t = Type(); if (data == null || (data.Length * dataDimension) // <= (data.Length * dataDimension) % t.Channels != 0) .... }


The analyzer indicates that the second check data == null will never be true , because if in the first condition data is null , an exception will be thrown, and program execution will not reach the second check.







Figure 7






I understand that you are already tired, but very little is left.



PVS-Studio Warning : V3127 Two similar code fragments were found. Perhaps, this is a typo and 'window' variable should be used instead of 'src2' Cv2_imgproc.cs 1547 



 public static Point2d PhaseCorrelateRes(....) { if (src1 == null) throw new ArgumentNullException(nameof(src1)); if (src2 == null) throw new ArgumentNullException(nameof(src2)); if (window == null) throw new ArgumentNullException(nameof(src2)); // <= .... }


And then the analyzer found a typo. In this section of the code, the value of the variables is checked for null , and if this condition is met, an exception is thrown for each of the variables. However, the window variable is not so simple. If this variable is null , then an exception is also generated for it, but the text of this exception is spelled incorrectly. The variable window itself does not appear in the text of this exception; instead, src2 is indicated there . Apparently, the last condition should be like this: 



 if (window == null) throw new ArgumentNullException(nameof(window));


PVS-Studio Warning : V3142 Unreachable code detected. It is possible that an error is present. MatOfT.cs 873



Now, for a change, let's look at the case when the analyzer is actually completely right when reporting an unreachable code, but there is no error. This is the case when you can say that the analyzer generates a warning that is both correct and false at the same time. 



 public new Mat<TElem> SubMat(params Range[] ranges) { Mat result = base.SubMat(ranges); return Wrap(result); }


The analyzer claims that the return statement is unreachable here. To verify this, look at the body of the SubMat method. 



 public Mat SubMat(params Range[] ranges) { throw new NotImplementedException(); /* if (ranges == null) throw new ArgumentNullException(); ThrowIfDisposed(); CvSlice[] slices = new CvSlice[ranges.Length]; for (int i = 0; i < ranges.Length; i++) { slices[i] = ranges[i]; } IntPtr retPtr = NativeMethods.core_Mat_subMat1(ptr, ranges.Length, ranges); Mat retVal = new Mat(retPtr); return retVal;*/ }


As you can see, the function has not yet been added and always throws an exception. And the analyzer is right when reporting an unreachable code. But this can not be called a real mistake.



The following three errors found by the analyzer are of the same type, but they are so cool that I could not help writing them all out.



PVS-Studio warning : V3022 Expression 'String.IsNullOrEmpty ("winName")' is always false. Cv2_highgui.cs 46 



 public static void DestroyWindow(string winName) { if (String.IsNullOrEmpty("winName")) .... }


PVS-Studio warning : V3022 Expression 'string.IsNullOrEmpty ("fileName")' is always false. FrameSource.cs 37 



 public static FrameSource CreateFrameSource_Video(string fileName) { if (string.IsNullOrEmpty("fileName")) .... }


PVS-Studio warning : V3022 Expression 'string.IsNullOrEmpty ("fileName")' is always false. FrameSource.cs 53 



 public static FrameSource CreateFrameSource_Video_CUDA(string fileName) { if (string.IsNullOrEmpty("fileName")) .... }


Sometimes behind the warning of the V3022 analyzer (the expression is always true / false) there are really strange or funny things. In all three cases, the same situation is observed. The method code has a parameter of type string , the value of which must be checked. However, it is not the value of the variable that is checked, but the string literal with its name, i.e. the name is in vain quoted.







Figure 18






Apparently, the developer sealed once and using copy-paste multiplied this error by code.



Conclusion



OpenCvSharp developers have done an important and great job. And I, as a user of this library, am very grateful to them. Thanks.



However, being in the PVS-Studio team and looking at the code, I have to admit that the question of its quality has not been worked out enough. Most likely, static code analyzers are not used on a regular basis in this project. And many errors are corrected by more expensive methods (testing, according to user reviews, for example). And some generally remain to live for a long time in the code, and we just find them. This idea is presented in more detail in a short note on the philosophy of using the methodology of static analysis.



Since the project is open and located on GitHub, its developers have the opportunity to take advantage of the free licensing option PVS-Studio and start applying the analysis on a regular basis.



Thanks for attention. Download and test your projects using the trial version of PVS-Studio.











If you want to share this article with an English-speaking audience, then please use the link to the translation: Ekaterina Nikiforova. Checking the OpenCvSharp Wrapper for OpenCV with PVS-Studio .


More articles:


All Articles