[feat] lens correction by antond-weta · Pull Request #255 · AcademySoftwareFoundation/rawtoaces

antond-weta · 2026-03-04T07:51:23Z

Description

Implements lens correction functionality

Tests

Checklist:

I have read the contribution guidelines.
I have updated the documentation, if applicable. (Check if there is no
need to update the documentation, for example if this is a bug fix that
doesn't change the API.)
I have ensured that the change is tested somewhere in the testsuite
(adding new test cases if necessary).
My code follows the prevailing code style of this project. If I haven't
already run clang-format before submitting, I definitely will look at the CI
test that runs clang-format and fix anything that it highlights as being
nonconforming.

Signed-off-by: Anton Dukhovnikov <antond@wetafx.co.nz>

codecov-commenter · 2026-03-04T07:56:18Z

Codecov Report

❌ Patch coverage is 92.22720% with 52 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.51%. Comparing base (4053a2c) to head (5ded3c4).

Files with missing lines	Patch %	Lines
src/rawtoaces_util/image_converter.cpp	80.32%	49 Missing ⚠️
src/rawtoaces_util/lens_correction.cpp	99.32%	2 Missing ⚠️
src/rawtoaces_util/exiftool.cpp	98.73%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #255      +/-   ##
==========================================
+ Coverage   90.22%   90.51%   +0.29%     
==========================================
  Files          15       17       +2     
  Lines        2680     3312     +632     
  Branches      405      498      +93     
==========================================
+ Hits         2418     2998     +580     
- Misses        262      314      +52

Files with missing lines	Coverage Δ
include/rawtoaces/image_converter.h	`100.00% <100.00%> (ø)`
src/rawtoaces/main.cpp	`100.00% <100.00%> (ø)`
src/rawtoaces_core/rawtoaces_core.cpp	`99.63% <100.00%> (ø)`
src/rawtoaces_util/cache_base.h	`100.00% <100.00%> (ø)`
src/rawtoaces_util/colour_transforms.cpp	`100.00% <ø> (ø)`
src/rawtoaces_util/lens_correction_cache.cpp	`100.00% <100.00%> (ø)`
src/rawtoaces_util/exiftool.cpp	`99.21% <98.73%> (-0.79%)`	⬇️
src/rawtoaces_util/lens_correction.cpp	`99.32% <99.32%> (ø)`
src/rawtoaces_util/image_converter.cpp	`77.44% <80.32%> (+0.61%)`	⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4053a2c...5ded3c4. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Anton Dukhovnikov <antond@wetafx.co.nz>

soswow · 2026-03-05T09:24:57Z

src/rawtoaces_util/exiftool.cpp

        exiftool_to_oiio = { { "Make", { "cameraMake", false } },
                             { "Model", { "cameraModel", false } },
                             { "LensID", { "lensModel", false } },
-                             { "LensModel", { "lensModel", false } },


Curious why we removing it?

Exiftool seems to be inconsistent with this field for some cameras. This makes it difficult to decide what to do when both LensID and LensModel are present and hold different value.

BTW I'm expecting this code to be tweaked a fair bit as this functionality gets more use. Collecting metadata from raw images is always a balancing act.

I've refactored that further to make it a bit more robust. The lensModel thingy is back!

soswow · 2026-03-05T09:28:06Z

src/rawtoaces_util/image_converter.cpp

+        fetch_lens_parameter(
+            lens_make,
+            src_spec,
+            "lenMake",


soswow · 2026-03-05T09:30:07Z

src/rawtoaces_util/lens_correction.cpp

+    auto cam = cameras[0];
+
+    const lfLens **lenses =
+        Database->FindLenses( cam, NULL, lens_model.c_str() );


should lens_make be used here instead of NULL?

const char* lens_make_ptr = lens_make.empty() ? nullptr : lens_make.c_str(); const lfLens** lenses = Database->FindLenses(cam, lens_make_ptr, lens_model.c_str());

soswow · 2026-03-05T09:44:04Z

src/rawtoaces_util/image_converter.cpp

+        {
+            if ( !apply_aberration_map(
+                     dst,
+                     src,


at each state of these three calls we give dst and src where is src is seem to be the same object that came in originally. The only reason it works if because our internal called uses same object for dst and src. But this is public API and hence we should handle the case where these two are not the same.

if ( &dst != &src ) dst.copy( src ); ... if ( !apply_vignette_map( dst, dst, ... if ( !apply_aberration_map( dst, dst, ... if ( !apply_distortion_map( dst, dst,

or some more robust way.

Also, it seems like you have tests void test_apply_lens_correction() where those two are not the same, so am I correct assuming the result would be only distortion map applied?

good catch! fixed.

not sure. I think I understand what LLM is saying, but I rather paste it as-is so nothing is lost in translation:

[image_converter.h (line 374)] documents apply_lens_correction(dst, src) as supporting distinct buffers, but the implementation starts with src_ptr = &src and only switches to dst after a prior stage succeeds [image_converter.cpp (line 1636)]. If the first enabled stage is distortion or aberration, the lower-level code depends on dst being already initialized: distortion takes its temp spec from dst_buffer.spec() [lens_correction.cpp (line 461)], and aberration actually warps from dst_buffer instead of src_buffer [lens_correction.cpp (line 600)], [lens_correction.cpp (line 612)]. Existing success-path tests are in-place for distortion/aberration [testLensCorrection.cpp (line 261)], [testLensCorrection.cpp (line 317)], while the only out-of-place success test is vignetting [testLensCorrection.cpp (line 466)].

Suggested fix: initialize a working buffer from src when dst != src, chain every enabled stage against the previous output, and add explicit out-of-place success tests for distortion-only, aberration-only, and combined correction.

soswow · 2026-03-05T09:44:41Z

src/rawtoaces_util/image_converter.cpp

+                last_error_message );
+        }
+
+        OIIO::ParamValueList options;


is it used?

soswow · 2026-03-05T09:46:13Z

src/bindings/py_util.cpp

        .value(
            "CropApplicationError",
            ImageConverter::Status::CropApplicationError )
        .value( "WriteError", ImageConverter::Status::WriteError )


we need to add

.value( "DatabaseNotFound", ImageConverter::Status::DatabaseNotFound )

soswow · 2026-03-05T09:47:43Z

src/docs/lens_correction.rst

+
+      rawtoaces                                    \
+      --lens-correction a                          \
+      --require-lens correction                    \


Suggested change

--require-lens correction \

--require-lens-correction \

soswow · 2026-03-05T09:48:03Z

src/docs/lens_correction.rst

+      --custom-lens-model "RF 24-105mm F4L IS USM" \
+      --custom-aperture 4.0                        \
+      --custom-focal-length 35.0                   \
+      --custom-focus_distance 240.0                \


Suggested change

--custom-focus_distance 240.0 \

--custom-focus-distance 240.0 \

soswow · 2026-03-05T09:49:44Z

tests/python/test_image_converter.py

+        converter.settings.custom_lens_make = "custom_lens_model"
+        assert converter.settings.custom_lens_make == "custom_lens_model"


Suggested change

converter.settings.custom_lens_make = "custom_lens_model"

assert converter.settings.custom_lens_make == "custom_lens_model"

converter.settings.custom_lens_model = "custom_lens_model"

assert converter.settings.custom_lens_model == "custom_lens_model"

Signed-off-by: Anton Dukhovnikov <antond@wetafx.co.nz>

soswow · 2026-03-10T11:58:17Z

Do we want maybe separate status LensCorrectionError for when correction failed?

soswow · 2026-03-10T12:03:07Z

src/rawtoaces_util/image_converter.cpp

+         ImageConverter::Settings::LensCorrectionType::None )
+    {
+        if ( settings.custom_lens_model.empty() )
+            keys_to_check.push_back( "lensModel" );


shuould we be doing same for lensMake?

soswow · 2026-03-10T12:03:54Z

src/rawtoaces_util/exiftool.cpp

-        { "aperture", "FNumber" },
-        { "focalLength", "FocalLength" }
+        { "cameraMake", "Make" },     { "cameraModel", "Model" },
+        { "lensModel", "LensModel" }, { "lensID", "LensID" },


do we need mapping for lensMake?

soswow · 2026-03-10T12:06:06Z

src/rawtoaces/main.cpp

        { "Missing the camera model name in the file metadata.",
          " You can provide a custom value using the --custom-camera-model "
          "parameter." },
+        { "Missing the lens model name in the file metadata.",


actual message is "Missing the <property> info in the file metadata."

soswow · 2026-03-10T12:07:22Z

src/bindings/py_util.cpp

+            "Distortion",
+            ImageConverter::Settings::LensCorrectionType::Distortion )
+        .value(
+            "Vignetting",


There is also None

enum class LensCorrectionType { None = 0, Aberration = 1 << 0, Distortion = 1 << 1, Vignetting = 1 << 2 };

[feat] lens correction

dc66724

Signed-off-by: Anton Dukhovnikov <antond@wetafx.co.nz>

fix documentation

2108012

Signed-off-by: Anton Dukhovnikov <antond@wetafx.co.nz>

soswow suggested changes Mar 5, 2026

View reviewed changes

address code review comments

5ded3c4

Signed-off-by: Anton Dukhovnikov <antond@wetafx.co.nz>

soswow reviewed Mar 10, 2026

View reviewed changes

	--custom-focus_distance 240.0 \
	--custom-focus-distance 240.0 \

		converter.settings.custom_lens_make = "custom_lens_model"
		assert converter.settings.custom_lens_make == "custom_lens_model"

Conversation

antond-weta commented Mar 4, 2026

Description

Tests

Checklist:

Uh oh!

codecov-commenter commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

soswow Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

soswow commented Mar 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Mar 4, 2026 •

edited

Loading

soswow Mar 10, 2026 •

edited

Loading