feat: IBA:demosaic add X-Trans demosaicing #4579
Conversation
antond-weta
force-pushed
the
iba_demosaic_xtrans
branch
7 times, most recently
from
7748ff7 to
694a124
Compare
Signed-off-by: Anton Dukhovnikov <[email protected]>
antond-weta
force-pushed
the
iba_demosaic_xtrans
branch
from
694a124 to
3f9ffce
Compare
| /// The following algorithms are supported for X-Trans-pattern images: | ||
| /// - `linear` - simple linear demosaicing. Fast, but can produce artefacts along sharp edges. | ||
| /// | ||
| /// - "layout" : string (default: "RGGB" for Bayer, "GRBGBR BGGRGG RGGBGG GBRGRB RGGBGG BGGRGG" for X-Trans) |
There was a problem hiding this comment.
I'm not sure the text alignment is correct here. Can you double check this whole surrounding area? Also might be good to look at the artifacts saved by the "docs CI" to be sure that the resulting documentation definitely look as you imagine it should.
There was a problem hiding this comment.
Yes, I had checked the artefacts, those looked fine.
I think I've spent a few hours fixing that bit, as I can't build the docs locally. The reason that alignment looks weird is because there are nested bullet point lists, and the 'linear' thingy is a single entry list (made it that way to be consistent with other things) belonging to the previous parameter ('algorithm').
There was a problem hiding this comment.
OK, thanks, I was just making sure since it was a 2-level list and I wasn't sure from looking these diffs whether everything was aligned as you intended. If they looked ok when viewing the artifacts, then all is fine.
| for (size_t y = 0; y < pattern_size; y++) { | ||
| for (size_t x = 0; x < pattern_size; x++) { | ||
| int c = channel_at_offset(x + x_offset, y + y_offset); | ||
| assert(c < 4); |
There was a problem hiding this comment.
Let's use OIIO_ASSERT (or OIIO_DASSERT if you are sure you want it to do nothing at all for non-debug builds).
|
Larry, with the added unit tests covering all permutations, would it be worth simplifying the oiiotool-based tests to only check that every parameter gets passed to the algo? |
Signed-off-by: Anton Dukhovnikov <[email protected]>
I guess it's not critical to test it all twice. Either way is fine, I guess -- either have thorough C++ unit tests and a perfunctory oiiotool-based test to ensure that the params are all passed, OR if the oiiotool tests can exercise all corners of the algorithm, then the pure C++ tests are actually redundant. Whichever you think is better is fine with me. |
|
N.B.: The Windows CI failure is unrelated to this patch and has already been fixed in main. |
lgritz
merged commit dc409d8
into
AcademySoftwareFoundation:main
lgritz
changed the title
…#4579) Implements X-Trans pattern demosaicing using a simple bilinear algorithm. Adds the Xtrans pattern layout to the ImageBuf attributes in the raw reader plugin. I have implemented unit tests in imagebufalgo_test.cpp which cover all permutations of the Bayer and X-Trans layouts, as the X-Trans pattern is a bit too complicated to generate using oiiotool. --------- Signed-off-by: Anton Dukhovnikov <[email protected]>
…#4579) Implements X-Trans pattern demosaicing using a simple bilinear algorithm. Adds the Xtrans pattern layout to the ImageBuf attributes in the raw reader plugin. I have implemented unit tests in imagebufalgo_test.cpp which cover all permutations of the Bayer and X-Trans layouts, as the X-Trans pattern is a bit too complicated to generate using oiiotool. --------- Signed-off-by: Anton Dukhovnikov <[email protected]>
Description
Implements X-Trans pattern demosaicing using a simple bilinear algorithm. Adds the Xtrans pattern layout to the ImageBuf attributes in the raw reader plugin.
Tests
I have implemented unittests in imagebufalgo_test.cpp which cover all permutations of the Bayer and X-Trans layouts, as the X-Trans pattern is a bit too complicated to generate using oiiotool.
Checklist:
need to update the documentation, for example if this is a bug fix that
doesn't change the API.)
(adding new test cases if necessary).
corresponding Python bindings (and if altering ImageBufAlgo functions, also
exposed the new functionality as oiiotool options).
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.