Skip to content

Commit

Permalink
fix(raw): fix channel layout (#4516)
Browse files Browse the repository at this point in the history
The function libraw_filter_to_str() converts the `imgdata.idata.filters`
field to human readable form, by mapping a few hardcoded values. I found
the issue by converting images with disabled demosaicing, which writes
the Bayer pattern into the ImageBuf metadata. The Panasonic GX9 didn't
have any pattern recognised; The Canon R5 was incorrectly recognised as
GMCY.

The `imgdata.idata.filters` actually contains a sequence of 2-bit
values, where each represents an index in a 4-character long string
`imgdata.idata.cdesc`. I have modified the code to extract the channels
from `imgdata.idata.cdesc` as expected.

I have also disabled the normalisation code, which I believe was
incorrect, and never executed due to the issue above.

The current tests pass, however, I'll start a discussion on the piece of
code I have concerns with.

---------

Signed-off-by: Anton Dukhovnikov <[email protected]>
  • Loading branch information
antond-weta authored Nov 6, 2024
1 parent 2f0caf7 commit de51ee6
Showing 1 changed file with 16 additions and 33 deletions.
49 changes: 16 additions & 33 deletions src/raw.imageio/rawinput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,28 +245,16 @@ OIIO_EXPORT const char* raw_input_extensions[]
OIIO_PLUGIN_EXPORTS_END

namespace {
const char*
libraw_filter_to_str(unsigned int filters)
std::string
libraw_filter_to_str(unsigned int filters, const char* cdesc)
{
// Convert the libraw filter pattern description
// into a slightly more human readable string
// LibRaw/internal/defines.h:166
switch (filters) {
// CYGM
case 0xe1e4e1e4: return "GMYC";
case 0x1b4e4b1e: return "CYGM";
case 0x1e4b4e1b: return "YCGM";
case 0xb4b4b4b4: return "GMCY";
case 0x1e4e1e4e: return "CYMG";

// RGB
case 0x16161616: return "BGRG";
case 0x61616161: return "GRGB";
case 0x49494949: return "GBGR";
case 0x94949494: return "RGBG";
default: break;
char result[5] = { 0, 0, 0, 0, 0 };
for (size_t i = 0; i < 4; i++) {
size_t index = filters & 3; // Grab the last 2 bits
result[i] = cdesc[index];
filters >>= 2;
}
return "";
return result;
}
} // namespace

Expand Down Expand Up @@ -490,22 +478,16 @@ RawInput::open_raw(bool unpack, const std::string& name,
if (config.get_int_attribute("raw:use_camera_wb", 1) == 1) {
auto& color = m_processor->imgdata.color;
auto& params = m_processor->imgdata.params;
auto& idata = m_processor->imgdata.idata;

auto is_rgbg_or_bgrg = [&](unsigned int filters) {
std::string filter(libraw_filter_to_str(filters));
return filter == "RGBG" || filter == "BGRG";
};
float norm[4] = { color.cam_mul[0], color.cam_mul[1], color.cam_mul[2],
color.cam_mul[3] };

if (is_rgbg_or_bgrg(idata.filters)) {
// normalize white balance around green
norm[0] /= norm[1];
norm[1] /= norm[1];
norm[2] /= norm[3] > 0 ? norm[3] : norm[1];
norm[3] /= norm[3] > 0 ? norm[3] : norm[1];
}
// // normalize white balance around green
// norm[0] /= norm[1];
// norm[2] /= norm[3] > 0 ? norm[3] : norm[1];
// norm[3] /= norm[3] > 0 ? norm[3] : norm[1];
// norm[1] /= norm[1];

params.user_mul[0] = norm[0];
params.user_mul[1] = norm[1];
params.user_mul[2] = norm[2];
Expand Down Expand Up @@ -676,7 +658,8 @@ RawInput::open_raw(bool unpack, const std::string& name,

// Put the details about the filter pattern into the metadata
std::string filter(
libraw_filter_to_str(m_processor->imgdata.idata.filters));
libraw_filter_to_str(m_processor->imgdata.idata.filters,
m_processor->imgdata.idata.cdesc));
if (filter.empty()) {
filter = "unknown";
}
Expand Down

0 comments on commit de51ee6

Please sign in to comment.