Hot answers tagged

22

Vectorization with NumPy When read with cv2.imread or skimage.io.imread or scipy.misc.imread, you would already have the image data as a NumPy array. Now, NumPy supports various vectorization capabilities, which we can use to speed up things quite a bit. I. Crop to remove all black rows and columns across entire image To solve our case, one method would ...


20

As you've discovered, looping over individual pixels in Python is very slow. You need to organize your computation so that it uses a series of NumPy (or SciPy, or Scikit-Image, or OpenCV) operations on the whole image. In this case, you could use numpy.argwhere to find the bounding box of the non-black regions: # Mask of non-black pixels (assuming image ...


13

Use conditional indexing: RED, GREEN, BLUE = (2, 1, 0) # Your numbers empty_img = numpy.zeros((height, width, 3), numpy.uint8) reds = img[:, :, RED] greens = img[:, :, GREEN] blues = img[:, :, BLUE] empty_img[(greens < 35) | (reds <= greens >= blues)][BLUE] = 255 Edit: empty_img[(greens < 35) | ((reds <= greens) & (blues <= greens)...


12

There's a simpler way to create the empty image using numpy.zeros_like: empty_img = numpy.zeros_like(img) As Austin Hastings correctly pointed out, the trick is to use vectorized operations provided by numpy: RED, GREEN, BLUE = (2, 1, 0) reds = img[:, :, RED] greens = img[:, :, GREEN] blues = img[:, :, BLUE] mask = (greens < 35) | (reds > greens) |...


11

Bug if (intersects) { isSingle = false; break; } This looks like a bug. The variable isSingle is already false. Setting it to false here does nothing. Code Review: You use std::vector but don't include its header. std::vector <Rect> overlaps; You should fix that. References and Const correctness. This ...


10

You should read and follow PEP8. This means: Put all imports at the top of the file. Put a space around assignment operators. Place a space after most commas. Use descriptive names. This is as it makes your code much easier to read. We put spaces around words in English to increase readability, we do it in programming languages to do the same. You should: ...


10

Your header asks for optimization, but your question asks for any ideas for improving the code. I'll answer the latter. If you need to optimize in terms of execution speed or memory use, I recommend using a profiler to measure what the critical path is. Overall design Your program uses a very C-like structure. Using an object-oriented approach can usually ...


9

I think the trick is trying to vectorise this as much as possible: By the look of it, the code is trying to threshold at 0 and count pixels under 255. We can change the first part of the loop to: counter = np.sum(image_in < 255) # Sums work on binary values counter2 = np.sum(np.bitwise_and(image_in < 255, image_in2 != 0)) And the second to: # ...


8

Just for correctness and not for performance reasons: You nowhere mark any key as being clustered. As a result, keys may be clustered multiple times. In case all keys shall be clustered maximum one time, one may do the following modifications to the code above: Add a following first line clustered[i] = true; behind // add P to cluster c clusters[c]....


8

As you indicated in the question, you need to vectorize the for loop: it = product(xrange(n_divs), xrange(n_divs), xrange(cellx), xrange(celly)) for m, n, i, j in it: # grad value grad = magnit[m * cellx + i, n * celly + j][0] # normalized grad value norm_grad = grad / img_area # Orientation Angle angle = angles[m*cellx + i, n*celly+...


8

Fix the includes Don't include headers you're not using. I had to remove a bunch of headers that aren't available here, simply to get your code to compile. I then had to include the math header for the use of fabs in getColorAccuracy() - which I then changed to std::abs, so <cmath>. I then had: #include <opencv2/core/core.hpp> #include <...


8

In terms of making more robust, a fairly obvious improvement would be to ensure that argv[1] exists before using it (that's what argc is for). If there's no argument, or the file can't be read, emit a useful error message to std::cerr and return EXIT_FAILURE - negative return values from main() can vary by platform. Why do we declare CannyThreshold with ...


8

First, get rid of all the unneeded whitespace. Use consistent amount between functions (Python's official style-guide, PEP8, recommends two). PEP8 also recommends using spaces in lists, after the commas, and lower_case for all variables and functions (your T in threshold_slow violates this). Don't use magic numbers in your code. Give them readable names and ...


7

I don't have cv2 installed (yet) so I'm going blind here, but I have a few comments. First, the problem you described can be solved very easily with itertools.product: import itertools def get_set_intersections(chars="+-", base=2): numbers = range(1, base+1) for signs in itertools.product(chars, repeat=base): yield "".join("{}{}".format(...


7

Just looking over the two functions, I would probably say the first function is a better practice. It's shorter (meaning it's more maintainable), it uses fewer variables (which can cause less confusion), and does the same thing as the second function. Overall the advantage is that the first function is just shorter. In terms of speed/efficiency (if I had ...


6

Its pretty good: Don't do this: using namespace cv; using namespace std; Its a bad habit that one day will get you into a lot of trouble. The reason the namespace names are so short is so that adding std:: or cv:: before identifiers is not overburdensome. Read more about the issues: Why is “using namespace std” considered bad practice? I suppose this is ...


6

Prefer <cstdio> to <stdio.h> and <cmath> to <math.h> in C++ code. You will probably never need the C compatibility headers unless you're writing headers that must also be included in C code. The compatibility headers are less useful in C++ code, because they declare everything in the global namespace rather than neatly in std. ...


5

Algorithm You will benefit greatly from using OpenCV's built-in functionality rather than performing this operation yourself. OpenCV supports masked operations, which will only apply to pixels where the mask is nonzero. cv::Mat global_average(const cv::Mat& input, const cv::Mat& levels) { assert(levels.channels() == 1); cv::Mat output(input....


5

Correctness You have a subtle bug in your code. When down- and upsampling your image with pyrDown()and pyrUp(), you compute your image size with integer division. This will lose a pixel if your image dimensions are odd. You can fix by storing the full image size in a variable: const auto fullSize = image.size(); and using that as the dstsize argument for ...


5

Architectural Ideas Let's start at the Architecture of your Application and the data transfer. There's basically two places where we can optimize the performance of your application. I'm ignoring latency for now, since that is mostly determined by the network and the performance of the image processing. This means if we can improve the speed of image ...


4

Intro I'm not an expert "well-versed in the art", so this is mostly a style review of the C++ code rather than providing the performance-improving hints that you're really looking for; I hope someone else will step up with those insights for you! Headers and namespaces We don't seem to be using <stdio.h> or <iostream>, but do require <...


4

The two-dimensional Gaussian function can be obtained by composing two one-dimensional Gaussians. I changed your code slightly so that it would compile (and not optimize away the unused kernel): #include <iostream> int main() { int rows = 20000, cols = 20000; const auto kernel = getGaussianKernel(rows, cols, 50, 50 ); std::cout << ...


4

Some notes only: Instead of (_, im) = webcam.read() is more common not to use parentheses: _, im = webcam.read() (and similarly in other places of your code). Too many blank lines. In the PEP 8 - Style Guide for Python Code repeatedly occurs the word "sparingly" for the Blank Lines usage. (Correcting English only): Instead of print("Webcam is open? "...


4

A quick, partial review (just one function) [maybe I'll have more time later to figure out what your code does]. auto calculateSD(const cv::Mat& src, auto rows, auto cols) { double sum{0}; double sq_sum{0}; #pragma omp for for(auto j=0;j<rows;j++) for(auto i=0;i<cols;i++) { sum += src.at<float>(j,...


4

import os, codecs, datetime, glob E401 multiple imports on one line Recommend you run $ flake8 and heed its advice, as PEP-8 asks for just one import per line. Use isort to organize them. Each of your functions has lovely comments; thank you. Recommend you turn the one-sentence comments into docstrings. The add_subtitles() function is maybe slightly long,...


4

I've read this question and the associated code a couple of times because I really wanted to review it. First of all I'd say that there's not much to review because it's really well written (and it's a complex subject). Usage of functools.partial This may be opiniated, but I don't think you should be using partial here. I think you used it to make the ...


4

Avoid using namespace Especially for large namespaces such as std, bringing so many identifiers into scope is dangerous. Prefer to import just the identifiers you need, and/or qualify at use; as it is, I get a compilation failure: 195834.cpp: In function ‘int main(int, char**)’: 195834.cpp:81:5: error: reference to ‘end’ is ambiguous end = ...


3

One of your problems is that you are passing by parameter to the ClientWrite's threads a reference to a struct that you're replacing in the 'for loop', so all the threads will have the same struct (cf.client, cf.outbuf and cf.outlen) so you're sending all the data to the last cf.client. You should replace this code: struct clientPayload cp = { (MJPGWriter*)...


3

If you want to zero a Matx, you can take advantage of its val public data member, which is a good old C array. Then you can rely on the standard library algorithm std::fill to set every element of the matrix to 0; the algorithm generally does static dispatch at compile time to call std::memset whenever possible. Using it should allow you to always be safe ...


3

Is it true that the complexity of the code is O(NlogN), where N is the total number of pixels (512x512 by example, not 512). You main loop in bwlabelis: for i in range(M - 1): for j in range(N - 1): if (im[i][j] == im[i][j+1]): qf.union(i * N + j, i * N + j + 1) if (im[i + 1][j] == im[i][j]): qf.union(i * N + j, (...


Only top voted, non community-wiki answers of a minimum length are eligible