1
0
mirror of https://github.com/opencv/opencv.git synced 2026-07-30 07:43:03 +04:00

Merge pull request #28185 from asmorkalov:as/static_analysys_fix

Fixed issues identified by PVS Studio #28185

Partially fixes https://github.com/opencv/opencv/issues/28167
Paper: https://pvs-studio.com/en/blog/posts/cpp/1321/

Closed items: N2, N4, N5, N6, N7, N8, N10, N11, N13, N14.

To be continued...

### Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

- [x] I agree to contribute to the project under Apache 2 License.
- [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
- [ ] The PR is proposed to the proper branch
- [ ] There is a reference to the original bug report and related work
- [ ] There is accuracy test, performance test and test data in opencv_extra repository, if applicable
      Patch to opencv_extra has the same branch name.
- [ ] The feature is well documented and sample code can be built with the project CMake
This commit is contained in:
Alexander Smorkalov
2025-12-18 11:32:19 +03:00
committed by GitHub
parent 0ca4117c27
commit 721bb7289d
13 changed files with 133 additions and 171 deletions
@@ -18,6 +18,7 @@
#include "../../precomp.hpp"
#include "apriltag_quad_thresh.hpp"
#include "unionfind.hpp"
//#define APRIL_DEBUG
#ifdef APRIL_DEBUG
@@ -93,7 +94,7 @@ void ptsort_(struct pt *pts, int sz)
// a merge sort with temp storage.
// Use stack storage if it's not too big.
cv::AutoBuffer<struct pt, 1024> _tmp_stack(sz);
AutoBuffer<struct pt, 1024> _tmp_stack(sz);
memcpy(_tmp_stack.data(), pts, sizeof(struct pt) * sz);
int asz = sz/2;
@@ -571,31 +572,6 @@ int quad_segment_agg(int sz, struct line_fit_pt *lfps, int indices[4]){
return 1;
}
#define DO_UNIONFIND(dx, dy) if (im.data[y*s + dy*s + x + dx] == v) unionfind_connect(uf, y*w + x, y*w + dy*w + x + dx);
static void do_unionfind_line(unionfind_t *uf, Mat &im, int w, int s, int y){
CV_Assert(y+1 < im.rows);
CV_Assert(!im.empty());
for (int x = 1; x < w - 1; x++) {
uint8_t v = im.data[y*s + x];
if (v == 127)
continue;
// (dx,dy) pairs for 8 connectivity:
// (REFERENCE) (1, 0)
// (-1, 1) (0, 1) (1, 1)
//
DO_UNIONFIND(1, 0);
DO_UNIONFIND(0, 1);
if (v == 255) {
DO_UNIONFIND(-1, 1);
DO_UNIONFIND(1, 1);
}
}
}
#undef DO_UNIONFIND
/**
* return 1 if the quad looks okay, 0 if it should be discarded
* quad
@@ -1309,11 +1285,11 @@ zarray_t *apriltag_quad_thresh(const DetectorParameters &parameters, const Mat &
////////////////////////////////////////////////////////
// step 2. find connected components.
unionfind_t *uf = unionfind_create(w * h);
UnionFind uf(w * h);
// TODO PARALLELIZE
for (int y = 0; y < h - 1; y++) {
do_unionfind_line(uf, thold, w, ts, y);
uf.do_line(thold, w, ts, y);
}
// XXX sizing??
@@ -1329,7 +1305,7 @@ zarray_t *apriltag_quad_thresh(const DetectorParameters &parameters, const Mat &
continue;
// XXX don't query this until we know we need it?
uint64_t rep0 = unionfind_get_representative(uf, y*w + x);
uint64_t rep0 = uf.get_representative(y*w + x);
// whenever we find two adjacent pixels such that one is
// white and the other black, we add the point half-way
@@ -1356,7 +1332,7 @@ zarray_t *apriltag_quad_thresh(const DetectorParameters &parameters, const Mat &
uint8_t v1 = thold.data[y*ts + dy*ts + x + dx]; \
\
if (v0 + v1 == 255) { \
uint64_t rep1 = unionfind_get_representative(uf, y*w + dy*w + x + dx); \
uint64_t rep1 = uf.get_representative(y*w + dy*w + x + dx); \
uint64_t clusterid; \
if (rep0 < rep1) \
clusterid = (rep1 << 32) + rep0; \
@@ -1405,9 +1381,9 @@ uint32_t *colors = (uint32_t*) calloc(w*h, sizeof(*colors));
for (int y = 0; y < h; y++) {
for (int x = 0; x < w; x++) {
uint32_t v = unionfind_get_representative(uf, y*w+x);
uint32_t v = uf.get_representative(y*w+x);
if (unionfind_get_set_size(uf, v) < parameters->aprilTagMinClusterPixels)
if (uf.get_set_size(v) < parameters->aprilTagMinClusterPixels)
continue;
uint32_t color = colors[v];
@@ -1533,8 +1509,6 @@ for (int i = 0; i < _zarray_size(quads); i++) {
imwrite("2.5 debug_lines.pnm", out);
#endif
unionfind_destroy(uf);
for (int i = 0; i < _zarray_size(clusters); i++) {
zarray_t *cluster;
_zarray_get(clusters, i, &cluster);
@@ -19,7 +19,6 @@
#ifndef _OPENCV_APRIL_QUAD_THRESH_HPP_
#define _OPENCV_APRIL_QUAD_THRESH_HPP_
#include "unionfind.hpp"
#include "zmaxheap.hpp"
#include "zarray.hpp"
+107 -107
View File
@@ -17,115 +17,115 @@
namespace cv {
namespace aruco {
typedef struct unionfind unionfind_t;
struct unionfind{
struct UnionFind {
UnionFind(uint32_t _maxid)
{
maxid = _maxid;
data.resize(maxid+1);
for (unsigned int i = 0; i <= maxid; i++) {
data[i].size = 1;
data[i].parent = i;
}
};
inline uint32_t get_representative(uint32_t id) {
uint32_t root = id;
// chase down the root
while (data[root].parent != root) {
root = data[root].parent;
}
// go back and collapse the tree.
//
// XXX: on some of our workloads that have very shallow trees
// (e.g. image segmentation), we are actually faster not doing
// this...
while (data[id].parent != root) {
uint32_t tmp = data[id].parent;
data[id].parent = root;
id = tmp;
}
return root;
}
inline uint32_t get_set_size(uint32_t id) {
uint32_t repid = get_representative(id);
return data[repid].size;
}
inline uint32_t connect(uint32_t aid, uint32_t bid) {
uint32_t aroot = get_representative(aid);
uint32_t broot = get_representative(bid);
if (aroot == broot)
return aroot;
// we don't perform "union by rank", but we perform a similar
// operation (but probably without the same asymptotic guarantee):
// We join trees based on the number of *elements* (as opposed to
// rank) contained within each tree. I.e., we use size as a proxy
// for rank. In my testing, it's often *faster* to use size than
// rank, perhaps because the rank of the tree isn't that critical
// if there are very few nodes in it.
uint32_t asize = data[aroot].size;
uint32_t bsize = data[broot].size;
// optimization idea: We could shortcut some or all of the tree
// that is grafted onto the other tree. Pro: those nodes were just
// read and so are probably in cache. Con: it might end up being
// wasted effort -- the tree might be grafted onto another tree in
// a moment!
if (asize > bsize) {
data[broot].parent = aroot;
data[aroot].size += bsize;
return aroot;
} else {
data[aroot].parent = broot;
data[broot].size += asize;
return broot;
}
}
#define DO_UNIONFIND(dx, dy) if (im.data[y*s + dy*s + x + dx] == v) connect(y*w + x, y*w + dy*w + x + dx);
void do_line(Mat &im, int w, int s, int y) {
CV_Assert(y+1 < im.rows);
CV_Assert(!im.empty());
for (int x = 1; x < w - 1; x++) {
uint8_t v = im.data[y*s + x];
if (v == 127)
continue;
// (dx,dy) pairs for 8 connectivity:
// (REFERENCE) (1, 0)
// (-1, 1) (0, 1) (1, 1)
//
DO_UNIONFIND(1, 0);
DO_UNIONFIND(0, 1);
if (v == 255) {
DO_UNIONFIND(-1, 1);
DO_UNIONFIND(1, 1);
}
}
}
#undef DO_UNIONFIND
struct ufrec {
// the parent of this node. If a node's parent is its own index,
// then it is a root.
uint32_t parent;
// for the root of a connected component, the number of components
// connected to it. For intermediate values, it's not meaningful.
uint32_t size;
};
uint32_t maxid;
struct ufrec *data;
std::vector<ufrec> data;
};
struct ufrec{
// the parent of this node. If a node's parent is its own index,
// then it is a root.
uint32_t parent;
// for the root of a connected component, the number of components
// connected to it. For intermediate values, it's not meaningful.
uint32_t size;
};
static inline unionfind_t *unionfind_create(uint32_t maxid){
unionfind_t *uf = (unionfind_t*) calloc(1, sizeof(unionfind_t));
uf->maxid = maxid;
uf->data = (struct ufrec*) malloc((maxid+1) * sizeof(struct ufrec));
for (unsigned int i = 0; i <= maxid; i++) {
uf->data[i].size = 1;
uf->data[i].parent = i;
}
return uf;
}
static inline void unionfind_destroy(unionfind_t *uf){
free(uf->data);
free(uf);
}
/*
static inline uint32_t unionfind_get_representative(unionfind_t *uf, uint32_t id)
{
// base case: a node is its own parent
if (uf->data[id].parent == id)
return id;
// otherwise, recurse
uint32_t root = unionfind_get_representative(uf, uf->data[id].parent);
// short circuit the path. [XXX This write prevents tail recursion]
uf->data[id].parent = root;
return root;
}
*/
// this one seems to be every-so-slightly faster than the recursive
// version above.
static inline uint32_t unionfind_get_representative(unionfind_t *uf, uint32_t id){
uint32_t root = id;
// chase down the root
while (uf->data[root].parent != root) {
root = uf->data[root].parent;
}
// go back and collapse the tree.
//
// XXX: on some of our workloads that have very shallow trees
// (e.g. image segmentation), we are actually faster not doing
// this...
while (uf->data[id].parent != root) {
uint32_t tmp = uf->data[id].parent;
uf->data[id].parent = root;
id = tmp;
}
return root;
}
static inline uint32_t unionfind_get_set_size(unionfind_t *uf, uint32_t id){
uint32_t repid = unionfind_get_representative(uf, id);
return uf->data[repid].size;
}
static inline uint32_t unionfind_connect(unionfind_t *uf, uint32_t aid, uint32_t bid){
uint32_t aroot = unionfind_get_representative(uf, aid);
uint32_t broot = unionfind_get_representative(uf, bid);
if (aroot == broot)
return aroot;
// we don't perform "union by rank", but we perform a similar
// operation (but probably without the same asymptotic guarantee):
// We join trees based on the number of *elements* (as opposed to
// rank) contained within each tree. I.e., we use size as a proxy
// for rank. In my testing, it's often *faster* to use size than
// rank, perhaps because the rank of the tree isn't that critical
// if there are very few nodes in it.
uint32_t asize = uf->data[aroot].size;
uint32_t bsize = uf->data[broot].size;
// optimization idea: We could shortcut some or all of the tree
// that is grafted onto the other tree. Pro: those nodes were just
// read and so are probably in cache. Con: it might end up being
// wasted effort -- the tree might be grafted onto another tree in
// a moment!
if (asize > bsize) {
uf->data[broot].parent = aroot;
uf->data[aroot].size += bsize;
return aroot;
} else {
uf->data[aroot].parent = broot;
uf->data[broot].size += asize;
return broot;
}
}
}}
#endif
@@ -84,7 +84,6 @@ void zmaxheap_destroy(zmaxheap_t *heap)
{
free(heap->values);
free(heap->data);
memset(heap, 0, sizeof(zmaxheap_t));
free(heap);
}
+1
View File
@@ -43,6 +43,7 @@
#ifndef __OPENCV_PRECOMP_H__
#define __OPENCV_PRECOMP_H__
#include "opencv2/core.hpp"
#include "opencv2/objdetect.hpp"
#include "opencv2/objdetect/barcode.hpp"
#include "opencv2/imgproc.hpp"