Added merging in the frequency domain, improved alignment of tiles.

[chris-rank]

Automatic selection of merging approach

• merging in frequency domain if Bayer pattern detected
• merging in spatial domain (current approach) in all other cases

Merging in frequency domain

• merging in frequency domain leads to slightly better details at comparable noise reduction, especially in high-contrast structures
• bursts with bracketed exposures are supported resulting in more efficient denoising (i.e. less images are needed for comparable results)
• blending of 4 executions of pipeline with raised cosine window
• motion map that locally controls strength of temporal denoising in merging equation
• slightly improved preservation of details through additional term in merging equation and a deconvolution filter at the end of the pipeline
• improved noise model that takes estimate of read noise into account
• sub-pixel alignment in the frequency domain
• specifically tailored fast Fourier Transform, which works with float4 vectors (for Bayer pattern) and exploits the symmetry of the real input signal when translated to the frequency domain for faster processing speeds

Merging in spatial domain (-> current approach)

• replacement of box filter for blurring with binomial filter, which is the better choice for image blurring
• bug fix in this filter that removes a color shift in the image (box filter was shifted due to wrong selection of indices)

Improvement of alignment (both merging approaches)

• application of slight binomial blurring before generation of gaussian pyramid
• check of three candidate motion vectors after upsampling to correct potential issues at transitions (as proposed in the original publication)
• padding of original image that size of original image is a multiple of all downsampling levels and nearest neighbor upsampling does not skip rows or columns
• massive improvement of computation time for search distance 2

Other improvements

• improved compatibility of output DNG with inclusion of maker notes (e.g. required for lens sharpness in DxO Photolab)
• added noise reduction strength in filename, e.g. "x_merged_nr14.dng", that users can fine-tune the robustness parameters for their individual image without manually renaming the image
• hot pixel correction if Bayer pattern is detected
• for underexposed bursts, optional tone mapping can be applied to reduce quantization errors in the shadows
• special case for noise reduction = 25 (max value): simple temporal averaging, which may be interesting for tripod-based star trails and simulation of ND filters
• removed scaling of input images to value range of 0 to 1, which improves fidelity as the textures are stored as float16
• progress bar shows percentage instead of file names
• direction of robustness slider was reverted and renamed into noise reduction
• improved documentation and code structure

References used for inspiration

• Hasinoff et al. 2016: Burst photography for high dynamic range and low-light imaging on mobile cameras (https://graphics.stanford.edu/papers/hdrp/hasinoff-hdrplus-sigasia16.pdf)
• Monod et al. 2021: An analysis and implementation of the HDR+ burst denoising method (https://www.ipol.im/pub/art/2021/336/)
• Liba et al. 2019: Handheld mobile photography in very low light (https://graphics.stanford.edu/papers/night-sight-sigasia19/night-sight-sigasia19.pdf)
• Delbracio et al. 2015: Burst deblurring: Removing camera shake through fourier burst accumulation (https://openaccess.thecvf.com/content_cvpr_2015/papers/Delbracio_Burst_Deblurring_Removing_2015_CVPR_paper.pdf)

[Alex-Vasile]

I'm excited to get my hands on this, especially since it work since there's a lot in this PR, but I worry that there's too much in here and it'll make Martin's job of reviewing difficult.

When you finish pushing changes, may I recommend you split this into a few separate PRs, each focused on only one specific thing (i.e. frequency merge, DNG output changes, general improvements, and spatial merging improvements).

[chris-rank]

I fully understand that merging code and resolving conflicts after substantial changes is difficult. For me the question is if it is really necessary to decouple all changes and pull the code for all new features separately or if Martin's current code can just be replaced by the new code from my branch. While there are many changes under the hood, it is more like a restructuring and extension of the code (i.e. Martin's code is still contained besides a few corrections). As the changes in the code are substantial, I would not expect that each new line of code can be checked for correctness. I would rather suggest to use a test-based approach here by defining a set of test bursts and test the code with these examples (e.g. Bayer DNGs/RAWs from different camera manufacturers, mobile phones and Fuji X-Trans files). I have performed substantial testing with >20 test bursts, but my set of bursts may not be complete. I know that in the ideal case, each change should be checked in separately, but my code development during the last months was not structured very well. I am sorry for that! @martin-marek It would be great to hear your opinion on the pull request. @Alex-Vasile If you are interested in testing the new features, you can just clone/download my repository and build it in Xcode.

[Alex-Vasile]

For me the question is if it is really necessary to decouple all changes and pull the code for all new features separately or if Martin's current code can just be replaced by the new code from my branch.

Personally, I would say yes. And I am not even talking about correctness yet, which I believe lends itself to being checked on a function by function basis for "algorithmic" code like this.

I argue that it should be split up into single purpose PRs for understandability. As it currently stands, this PR has over 3,000 lines of code changed. That makes it hard to conceptually understand why a particular line has been changed, or what impact it will have, or even how it's used.

I know that in the ideal case, each change should be checked in separately, but my code development during the last months was not structured very well. I am sorry for that!

That's totally fine, and we are all grateful for the work you've done to implement the frequency domain merging specifically, and the other changes more generally. The issue is still that all of the code needs to be checked over and understood by someone before being merged in.

If you are interested in testing the new features, you can just clone/download my repository and build it in Xcode.

And I will actually clone to check it out since I would like to start testing it for non-bayer systems.

[chris-rank]

The difficulty I see with decoupling the changes is that there will be many inter-connections in the code. While some new features may be decoupled easily, others required changes at several different locations in the code. I did not only add functions, but had to re-structure also the "main" function of the app. I may go back in my history of commits and start a pull request for a much earlier version with potentially less changes and then start pull requests for several intermediate versions. However, this would increase the effort for Martin even more as he also would need to check obsolete lines of code of the intermediate versions. @Alex-Vasile It is great that you can test the code for non-Bayer systems as I do not have access to non-Bayer RAW files. However, please note that in the current implementation, this switches back to Martin's merging approach (with a few corrections and improvements). The reason is that the standard Fourier transform assumes an equidistant grid of pixels and the Fuji X-Trans pattern (by far most common non-Bayer pattern for digital cameras) does not provide that.

[Alex-Vasile]

The difficulty I see with decoupling the changes is that there will be many inter-connections in the code. While some new features may be decoupled easily, others required changes at several different locations in the code. I did not only add functions, but had to re-structure also the "main" function of the app. I may go back in my history of commits and start a pull request for a much earlier version with potentially less changes and then start pull requests for several intermediate versions. However, this would increase the effort for Martin even more as he also would need to check obsolete lines of code of the intermediate versions.

Yeah, fair enough. I guess rather than spend the effort on that, could you:

• Make sure that your comment at the top here is up to date with all changes that you've made (and link it to any issues that they close)
• Add some description to the functions, I assume the frequency code is based off of Handheld Mobile Photography in Very Low Light, knowing which functions correspond to which equations, and which aspects of the paper, you implemented would be really helpful. I have no idea where to start at with reading it at the moment.

Oh, and the merge conflict should be very easy to deal with, I believe you should be able to pull from main and merge in as the conflict is from this tiny PR: a9e94cbcc9eac59e6aaf8e2745d21abc79bbd9ad

@Alex-Vasile It is great that you can test the code for non-Bayer systems as I do not have access to non-Bayer RAW files. However, please note that in the current implementation, this switches back to Martin's merging approach (with a few corrections and improvements). The reason is that the standard Fourier transform assumes an equidistant grid of pixels and the Fuji X-Trans pattern (by far most common non-Bayer pattern for digital cameras) does not provide that.

I cloned and built and immediately ran directly into #2. (My non-bayer camera is a Foveon sensor). I'll go find some raws from DPReview and see what I can get out of them.

One point of feedback. You changed it so that it saves some the robustness parameter in the image name. I think it would be useful to take it a step further and save the full setting in the image. There's currently no way to explore the possibility space interactively with the app, so I have to save an test a few different permutations. The NR value in the name is helpful for this, but it would also be helpful to save the tile size and search distance.

[martin-marek]

Hi @chris-rank and @Alex-Vasile, thank you for all your input! At this point, Chris has made so many changes that it would take a lot of time for me to understand them all – unfortunately, I don't have that time right now. But Burst Photo is an open-source project, so I want everyone to be able to contribute (as long as it improves the overall user experience / doesn't create too much technical debt).

Perhaps we could use Chris' suggested approach to simply test his build with a large sample of images and analyze the output? As long as there are no significant downsides (ie it is not too much slower), I'll be happy to merge. Or perhaps we can make new features optional in the settings. Or maybe @Alex-Vasile, would you like to review any of Chris' changes?

Lastly, regarding tile size and search distance. In my experience, both make little different compared to the "robustness" parameter. This might be even more true after Chris' update. Do you also have similar experience? If so, it might be worth considering removing these settings altogether, so that users will be forced to focus on the "robustness" parameter.

[Alex-Vasile]

Hi @chris-rank and @Alex-Vasile, thank you for all your input! At this point, Chris has made so many changes that it would take a lot of time for me to understand them all – unfortunately, I don't have that time right now. But Burst Photo is an open-source project, so I want everyone to be able to contribute (as long as it improves the overall user experience / doesn't create too much technical debt).

Perhaps we could use Chris' suggested approach to simply test his build with a large sample of images and analyze the output? As long as there are no significant downsides (ie it is not too much slower), I'll be happy to merge. Or perhaps we can make new features optional in the settings. Or maybe @Alex-Vasile, would you like to review any of Chris' changes?

I would caution against merging in without review (I can take a look through it) because of the cost of merging in a change this big that is not understood. Whatever it'll cost in time to review (and document and etc.) now when Chris is around it'll cost several multiples of that later down the round, especially if it happen if/when Chris is no longer contributing to the project.

Lastly, regarding tile size and search distance. In my experience, both make little different compared to the "robustness" parameter. This might be even more true after Chris' update. Do you also have similar experience? If so, it might be worth considering removing these settings altogether, so that users will be forced to focus on the "robustness" parameter.

I will have to sit down and test that explicitly under a few different scenarios. The only real conclusions I've been able to make so far is (that for a bayer sensor and handheld shots) when comparing the previous and new method at the default values for all 3 settings (tile size, search distance, and robustness) are:

• The new methods results in higher resolution (or less blurriness which is how it's perceived in the final image) than the old method in areas of high frequency detail (hairs on a cat's face in the plane of focus)
• The new method results in noticeably lower noise reduction on blurred out backgrounds in the same shot as above. (The same may be happening in the rest of the image, but it's not as noticeable since the noise may be lost in the high frequency information).

(I don't have results on hand for these, but I can post them later.)

[chris-rank]

• I agree with Alex that some code checks and knowledge transfer (in addition to testing) is probably the better way to go. Similar to Martin, I have to reduce my coding activities in the near future, but I am happy to contribute to discussions and answer questions. @Alex-Vasile I would offer to meet virtually to give you a brief overview of the code structure and changes. I could imagine that this would make things easier for you.
• For documentation, I will try to add comments and some explanation to the code and update the new features in the pull request.
• In addition, I will add the uppercase feature from PR: #12 to my code.
• For the settings, I would not remove anything although the robustness / noise reduction parameter is by far the most important one. I would recommend to highlight the parameter in the UI (see my screenshot as a suggestion). The other two parameters may be relevant in extreme cases (extreme noise => larger tile size, very large motion amplitude, e.g car from left to right => larger search distance). I my opinion, adding the other two parameters to the filename may increase complexity for the user and move focus away from the robustness / noise reduction parameter.
• It is correct that the calculation time of the frequency-based merging is much longer (approx. 5-6x longer on my MacBook Air M1). The main reason is that the whole pipeline is repeated 4 times with slight shifts of the image, which explains most of the increase. Still I think calculation times should be acceptable for most use cases (and I am sure that performance can be improved by experienced Metal programmers). On my 7 core M1 GPU, I achieve approx. 22-23 MPix per second.
• It is correct that the default noise reduction differs between the old and new app. The reason for change was that the range of noise reduction strength depends on many parameters (e.g. sensor size and quality, ISO value, day/night scene, underexposure, user taste, spatial noise reduction applied in post?, ...) and the original range was too limited in my opinion. Especially when applying spatial noise reduction in post, you may keep some noise in exchange of better detail preservation.

[Alex-Vasile]

• I agree with Alex that some code checks and knowledge transfer (in addition to testing) is probably the better way to go. Similar to Martin, I have to reduce my coding activities in the near future, but I am happy to contribute to discussions and answer questions. @Alex-Vasile I would offer to meet virtually to give you a brief overview of the code structure and changes. I could imagine that this would make things easier for you.
• For documentation, I will try to add comments and some explanation to the code and update the new features in the pull request.

I'd really appreciate that. I think that would be real useful, thought I think it'll be best used after you get a chance to add the comments. In the meanwhile, I'll look around and see what I can pick up myself.

• It is correct that the calculation time of the frequency-based merging is much longer (approx. 5-6x longer on my MacBook Air M1). The main reason is that the whole pipeline is repeated 4 times with slight shifts of the image, which explains most of the increase. Still I think calculation times should be acceptable for most use cases (and I am sure that performance can be improved by experienced Metal programmers). On my 7 core M1 GPU, I achieve approx. 22-23 MPix per second.

Given the nature of the app that's probably a good throughput for now. But it may be worth adding a setting to switch between the two approaches.

[chris-rank]

@Alex-Vasile @martin-marek Within the last days, I added detailed documentation to my code and cleaned and slightly refined the code. In my opinion, the quality of the algorithm is as good as it can get with the frequency-based approach. Only with more sophisticated approaches from literature (e.g. with neural networks and/or super-resolution), further improvement may be possible. While the performance is slower than in the original version, it has reached approx. 32 MPix per second on my 7 core Apple M1 GPU, which should be sufficiently fast in my opinion. Still I am sure that further optimization is possible. From now on, I will reduce my programming efforts and hope that the current version is worth for a future update of the Burst Photo App. Please feel free to drop me a message whenever you have questions or need support/help. I will follow the discussion on Github.

[Alex-Vasile]

@Alex-Vasile @martin-marek

Within the last days, I added detailed documentation to my code and cleaned and slightly refined the code. In my opinion, the quality of the algorithm is as good as it can get with the frequency-based approach. Only with more sophisticated approaches from literature (e.g. with neural networks and/or super-resolution), further improvement may be possible. While the performance is slower than in the original version, it has reached approx. 32 MPix per second on my 7 core Apple M1 GPU, which should be sufficiently fast in my opinion. Still I am sure that further optimization is possible.

From now on, I will reduce my programming efforts and hope that the current version is worth for a future update of the Burst Photo App. Please feel free to drop me a message whenever you have questions or need support/help. I will follow the discussion on Github.

Thanks for the work! I'll work my way through it as I get a chance over the next little while.

[Alex-Vasile]

@chris-rank: https://matrix.to/#/@alex-vasile-5c9a6a9bd73408ce4fbbe52a:gitter.im 1-1 chat room for gitter.

[Alex-Vasile]

@chris-rank. Could you please have the exposure bracketing functionality as a separate PR.

Make a branch off your main and open that as a separate PR. This is only getting more out of hand.