-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Several small improvements #3
base: master
Are you sure you want to change the base?
Conversation
hasepompase
commented
Jan 12, 2020
- Text file output. I use this data to plot the grid with Gnuplot.
- Var for grid width and height.
- Average value over the whole cell. The grid becomes smoother and the compensation works better.
- Determine the sensor model from the header of the RAW file and set the black level automatically.
- Lens shading table in binary format. This makes it easier to read in the data at runtime. I use this in combination with the c++ library raspicam.
@@ -208,6 +209,10 @@ int main(int argc, char *argv[]) | |||
height = hdr->height; | |||
single_channel_width = width/2; | |||
single_channel_height = height/2; | |||
grid_width = single_channel_width / 32 + (single_channel_width % 32 == 0 ? 0 : 1); | |||
grid_height = single_channel_height / 32 + (single_channel_height % 32 == 0 ? 0 : 1); | |||
printf("Grid size: %d x %d\n", grid_width, grid_height); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quicker to do (single_channel_width + 31) / 32
lens_shading_analyse.c
Outdated
printf("Sensor type: %s\n", model); | ||
if (black_level == 0) | ||
{ | ||
black_level = 16; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation
lens_shading_analyse.c
Outdated
gain = 32; //Clip at x1.0 | ||
fprintf(header, "%d,\n", gain ); | ||
fprintf(table, "%d %d %d %d\n", x, y, gain, i ); | ||
fprintf(table, "%d %d %d %d\n", x * 32 + 16, y * 32 + 16, gain, i ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels wrong.
The ISP interpolates the values at the centre of each grid cell, and interpolates between them. This is averaging over the entire cell, therefore is just a smoothing filter.
Enlarging the average to 5x5, or even up as far as 9x9 would be reasonable, but otherwise you're ignoring a large amount of the detail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not know which algorithm is used for the interpolation. I don't see a problem with this approach. From my point of view and for my applications the results are better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been thinking again. If you look at lens shading across the width (alternatively across the height), the brightness will be a function similar to figure a. The average value changes only slightly with the width of the viewed area. However, the brightness signal from the sensor is subject to noise (figure b). A wider area improves the average value. Only in case of peaks smaller areas are better (figure c). In this case, however, the peak would have to lie exactly in a calculation area.
Or am I misunderstanding something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Integrated a parameter for the cell size.
lens_shading_analyse.c
Outdated
} | ||
|
||
max_val *= 32; | ||
printf("Max_val is %.0f\n", max_val); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without a better description I can't say for certain what this is meant to be doing, but it's very ugly to run the same loop twice, once to find this max_val, and then again to compute the cell gains.
Allocate arrays for block_val and block_px (if it's really necessary) and compute once. Calculating the gains then becomes an iteration over those arrays instead of the whole image.
(Sorry, embedded programming always makes you think of more efficient code).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running the loop twice is of course not a high-performance solution. But it runs very fast on my Raspberry Pi 3B ;-)
Quick and dirty - I will fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double calculation loop removed
I was relatively happy with the first 5 commits you'd requested for merge (although black level is actually in the raw header and really ought to be pulled out from there), but I don't understand the 6th patch that you've just pushed. The commit text and/or comments in the code need significant expansion to describe what you're actually trying to do. I've also now noticed that these patches have a commit author of @motz when hasepompase is making the PR. |
Hasepompase and motz are the same person. I added the commit 423977f via the web interface. Sorry for the confusion. Eclipse sets my local user and on the command line my real name is used. Sorry, I will adjust my settings. I am working on a project with two lights mechanically connected to the camera. Despite several LEDs and diffuser, the illumination is not homogeneous and the brightest area is not in the center. Therefore I have to look for the brightest area in the photo. With the lens shading correction I get images with uniform brightness and contrast. |
This reverts commit b239de6.