PetteriAimonen / focus-stack

Fast and easy focus stacking
MIT License
249 stars 37 forks source link

task_merge::denoise_neighbors() #26

Closed chriscramer closed 1 year ago

chriscramer commented 1 year ago

I've been working through the algorithms used and comparing to the code. I think there may be a small issue in the denoise_neighbors() routine in task_merge.cc.

Lines 178 to 185 address the case where the center pixel is greater than or less than all of the surrounding pixels, but the pixels are not all the same. In line 181, there's a comment that we'll eliminate the outliner, then line 182 is:

int avg = (top + bottom + left + right) / 4;

Arguably, this should be mode rather than median, but that's something of a pain to implement and in theory, all of the values should be close together.

But even if they are close together, the avg from above is happing in integer space and will produce the wrong result. For example, if top = bottom = left = 3, and right = 2. Then the avg ends up as 2 and not 3 as one would expect.

I think the code should round, maybe something like:

int avg = (top + bottom + left + right) / 4.0 + 0.5;

thoughts?

PetteriAimonen commented 1 year ago

I think averaging is best in that case. The outlier is the center pixel that gets replaced by calculated value. Averaging from the sides approximates any gradient at that location.

But you are correct that the integer rounding causes a bias. I have fixed that now and clarified the comments.

PetteriAimonen commented 1 year ago

I guess arguably the first case of the if is redundant, as the second case has the same result for all-equal situation.

chriscramer commented 1 year ago

Makes sense - thanks for taking a look.