Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FUJIFILM X-A10 RAF support #4447

Closed
Arm-Ann opened this issue Mar 20, 2018 · 26 comments

Comments

@Arm-Ann
Copy link

commented Mar 20, 2018

There is a problem with opening Fujifilm X-A10 RAF files in RawTherapee 5.4-rc3 x64.
When I open the RAF file, only a portion of it is displayed in editor window.

Sample RAW files with in-camera JPG are available on https://filebin.net/hj8y4yeca9lbox3l/x-a10.zip
Screenshot of RawTherapee GUI with problem is available on https://filebin.net/hj8y4yeca9lbox3l/2018-03-20_112830.jpg

For example:
DSCF0907.raf opened as 32x312 image instead 4912x3278
DSCF0908.raf opened as 1658x72 image instead 4912x3278

OS: Windows 10 Pro
CPU: Intel Core i7-2600 3.4GHz
RAM: 8Gb

@Beep6581 Beep6581 changed the title Fujifilm X-A10 RAW support in 5.4-rc3 FUJIFILM X-A10 RAF support Mar 20, 2018

@Beep6581

This comment has been minimized.

Copy link
Owner

commented Mar 20, 2018

Additionally, I get a segfault when opening DSCF0907.RAF in
rtengine::RawImageSource::getAutoMatchedToneCurve
https://filebin.net/dt7bwrlt0s6ksoyj (same files as above but unzipped)
Stack backtrace: https://filebin.net/dt7bwrlt0s6ksoyj/log.txt

@Floessie

This comment has been minimized.

Copy link
Collaborator

commented Mar 20, 2018

@agriggio I guess, the algorithm is not supposed to work with a negative cx...

@agriggio

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2018

I'll take a look

@iliasg

This comment has been minimized.

Copy link
Collaborator

commented Mar 20, 2018

@agriggio
I had proposed a wrong correction for X-A10 in Dcraw.cc

if (!strncmp(model,"X-A10",5)) {

we need to define top_margin = 0, left_margin = 0
and maybe width = raw_width, height = raw_height
We also need to do the same for X-A3 and maybe X-A5

@agriggio

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2018

Thanks Ilias, I'm checking right now

@agriggio

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2018

This seems to fix the issues:

diff --git a/rtengine/dcraw.cc b/rtengine/dcraw.cc
--- a/rtengine/dcraw.cc
+++ b/rtengine/dcraw.cc
@@ -9256,9 +9256,11 @@
       flip = 6;
     } else if (load_raw != &CLASS packed_load_raw)
       maximum = (is_raw == 2 && shot_select) ? 0x2f00 : 0x3e00;
-    if (!strncmp(model,"X-A10",5)) {
+    if (!strncmp(model,"X-A10",5) || !strncmp(model, "X-A3", 4) || !strncmp(model, "X-A5", 4)) {
         raw_width = 4912;
         raw_height = 3278;
+        width = raw_width;
+        height = raw_height;
     }
     top_margin = (raw_height - height) >> 2 << 1;
     left_margin = (raw_width - width ) >> 2 << 1;

@Beep6581 Beep6581 added this to the v5.5 milestone Mar 20, 2018

@iliasg

This comment has been minimized.

Copy link
Collaborator

commented Mar 20, 2018

Alberto, X-A3 & X-A5 have more pixels ... raw_width = 6016, raw_height =4014

@agriggio

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2018

@iliasg thanks, I'll fix

@agriggio

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2018

Here's the revised patch:

diff --git a/rtengine/dcraw.cc b/rtengine/dcraw.cc
--- a/rtengine/dcraw.cc
+++ b/rtengine/dcraw.cc
@@ -9257,8 +9257,11 @@
     } else if (load_raw != &CLASS packed_load_raw)
       maximum = (is_raw == 2 && shot_select) ? 0x2f00 : 0x3e00;
     if (!strncmp(model,"X-A10",5)) {
-        raw_width = 4912;
-        raw_height = 3278;
+        width = raw_width = 4912;
+        height = raw_height = 3278;
+    } else if (!strncmp(model, "X-A3", 4) || !strncmp(model, "X-A5", 4)) {
+        width = raw_width = 6016;
+        height = raw_height = 4014;
     }
     top_margin = (raw_height - height) >> 2 << 1;
     left_margin = (raw_width - width ) >> 2 << 1;
@iliasg

This comment has been minimized.

Copy link
Collaborator

commented Mar 20, 2018

OK if this works (tests needed on both compressed and non-compressed RAFs .. which I am not able to do now) I guess it should be fine for 5.4

I just have to remind that the root of the problem is that Fuji changed the way the frame sizes are reported in exif
#3988 (comment)
Wrong size reported at the usual exif tags .. but the correct data exist at different tags ..
So an ideal solution would be to adapt on these changes ;) .. how has libraw dealed with this ?.

@Arm-Ann

This comment has been minimized.

Copy link
Author

commented Mar 20, 2018

I'm ready to test it with X-A10 files. But I need windows binnary. I can't build RT myself.

@agriggio

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2018

I quickly tested with a raw from the X-A3 (from dpreview) and it doesn't work :-(
But it works for the X-A10. So I'll push to dev only the X-A10 part

agriggio added a commit that referenced this issue Mar 20, 2018

@iliasg

This comment has been minimized.

Copy link
Collaborator

commented Mar 20, 2018

@agriggio Alberto strange !!
Just because I see X-A10 has raw crop entry in camconst.json while X-A3 not ..
can you try by inserting in camconst.json X-A3 item
"raw_crop": [ 0, 0, 6016, 4014 ], // full raw 6016x4014

@agriggio

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2018

Ilias, I will try, but I think the issue is deeper. I think the decoding is just wrong for the x-a3, I was just getting a frame with garbage...

@heckflosse

This comment has been minimized.

Copy link
Collaborator

commented Mar 20, 2018

@agriggio Alberto, maybe it's this

@iliasg

This comment has been minimized.

Copy link
Collaborator

commented Mar 20, 2018

@agriggio, was the frame size wrong also .. because without the X-A3 patch the size is something like 2000X50 (not always the same) and made of garbage .. if you had 6016x4014 the patch is correct.

@agriggio

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2018

@heckflosse I had a look but didn't see anything "obvious". I had a look at libraw 0.19 which supports those cameras, but I didn't notice anything there either. I am clearly missing something... :-)

@iliasg

This comment has been minimized.

Copy link
Collaborator

commented Mar 21, 2018

@agriggio Alberto, because we've seen such a garbage rendering when RT missinterpretes the raw bit depth, can you check if the X-A3's bitdepth = 14bits is correctly interpreted ?. X-A10 gives 12bit RAFs and works fine ..

@agriggio

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2018

@iliasg the problem was a bit deeper... we lack support for 14bit bayer raws from fuji. But the latest libraw has it, so I was able to port it easily. Here's a patch that seems to work:

diff --git a/rtengine/dcraw.cc b/rtengine/dcraw.cc
--- a/rtengine/dcraw.cc
+++ b/rtengine/dcraw.cc
@@ -9264,6 +9264,9 @@
     if (!strncmp(model,"X-A10",5)) {
         width = raw_width = 4912;
         height = raw_height = 3278;
+    } else if (!strncmp(model, "X-A3", 4) || !strncmp(model, "X-A5", 4)) {
+        width = raw_width = 6016;
+        height = raw_height = 4014;
     }
     top_margin = (raw_height - height) >> 2 << 1;
     left_margin = (raw_width - width ) >> 2 << 1;
diff --git a/rtengine/dcraw.h b/rtengine/dcraw.h
--- a/rtengine/dcraw.h
+++ b/rtengine/dcraw.h
@@ -290,6 +290,7 @@
 void fuji_compressed_load_raw();
 void fuji_decode_loop(const struct fuji_compressed_params* common_info, int count, INT64* raw_block_offsets, unsigned *block_sizes);
 void parse_fuji_compressed_header();
+void fuji_14bit_load_raw();    
 void pentax_load_raw();
 void nikon_load_raw();
 int nikon_is_compressed();
diff --git a/rtengine/fujicompressed.cc b/rtengine/fujicompressed.cc
--- a/rtengine/fujicompressed.cc
+++ b/rtengine/fujicompressed.cc
@@ -1034,3 +1034,75 @@
     data_offset += 16;
     load_raw = &CLASS fuji_compressed_load_raw;
 }
+
+
+namespace {
+
+inline void unpack7bytesto4x16(unsigned char *src, unsigned short *dest)
+{
+  dest[0] = (src[0] << 6) | (src[1] >> 2);
+  dest[1] = ((src[1] & 0x3) << 12) | (src[2] << 4) | (src[3] >> 4);
+  dest[2] = (src[3] & 0xf) << 10 | (src[4] << 2) | (src[5] >> 6);
+  dest[3] = ((src[5] & 0x3f) << 8) | src[6];
+}
+
+inline void unpack28bytesto16x16ns(unsigned char *src, unsigned short *dest)
+{
+  dest[0] = (src[3] << 6) | (src[2] >> 2);
+  dest[1] = ((src[2] & 0x3) << 12) | (src[1] << 4) | (src[0] >> 4);
+  dest[2] = (src[0] & 0xf) << 10 | (src[7] << 2) | (src[6] >> 6);
+  dest[3] = ((src[6] & 0x3f) << 8) | src[5];
+  dest[4] = (src[4] << 6) | (src[11] >> 2);
+  dest[5] = ((src[11] & 0x3) << 12) | (src[10] << 4) | (src[9] >> 4);
+  dest[6] = (src[9] & 0xf) << 10 | (src[8] << 2) | (src[15] >> 6);
+  dest[7] = ((src[15] & 0x3f) << 8) | src[14];
+  dest[8] = (src[13] << 6) | (src[12] >> 2);
+  dest[9] = ((src[12] & 0x3) << 12) | (src[19] << 4) | (src[18] >> 4);
+  dest[10] = (src[18] & 0xf) << 10 | (src[17] << 2) | (src[16] >> 6);
+  dest[11] = ((src[16] & 0x3f) << 8) | src[23];
+  dest[12] = (src[22] << 6) | (src[21] >> 2);
+  dest[13] = ((src[21] & 0x3) << 12) | (src[20] << 4) | (src[27] >> 4);
+  dest[14] = (src[27] & 0xf) << 10 | (src[26] << 2) | (src[25] >> 6);
+  dest[15] = ((src[25] & 0x3f) << 8) | src[24];
+}
+
+#define swab32(x)                                                                                                      \
+  ((unsigned int)((((unsigned int)(x) & (unsigned int)0x000000ffUL) << 24) |                                           \
+                  (((unsigned int)(x) & (unsigned int)0x0000ff00UL) << 8) |                                            \
+                  (((unsigned int)(x) & (unsigned int)0x00ff0000UL) >> 8) |                                            \
+                  (((unsigned int)(x) & (unsigned int)0xff000000UL) >> 24)))
+
+inline void swab32arr(unsigned *arr, unsigned len)
+{
+  for (unsigned i = 0; i < len; i++)
+    arr[i] = swab32(arr[i]);
+}
+#undef swab32
+
+
+} // namespace
+
+void CLASS fuji_14bit_load_raw()
+{
+  const unsigned linelen = raw_width * 7 / 4;
+  const unsigned pitch = raw_width;
+  unsigned char *buf = (unsigned char *)malloc(linelen);
+  merror(buf, "fuji_14bit_load_raw()");
+
+  for (int row = 0; row < raw_height; row++)
+  {
+    unsigned bytesread = fread(buf, 1, linelen, ifp);
+    unsigned short *dest = &raw_image[pitch * row];
+    if (bytesread % 28)
+    {
+      swab32arr((unsigned *)buf, bytesread / 4);
+      for (int sp = 0, dp = 0; dp < pitch - 3 && sp < linelen - 6 && sp < bytesread - 6; sp += 7, dp += 4)
+        unpack7bytesto4x16(buf + sp, dest + dp);
+    }
+    else
+      for (int sp = 0, dp = 0; dp < pitch - 15 && sp < linelen - 27 && sp < bytesread - 27; sp += 28, dp += 16)
+        unpack28bytesto16x16ns(buf + sp, dest + dp);
+  }
+  free(buf);
+}
+
diff --git a/rtengine/rawimage.cc b/rtengine/rawimage.cc
--- a/rtengine/rawimage.cc
+++ b/rtengine/rawimage.cc
@@ -454,8 +454,12 @@
     }
 
     if(!strcmp(make,"Fujifilm") && raw_height * raw_width * 2u != raw_size) {
-        parse_fuji_compressed_header();
-	}
+        if (raw_width * raw_height * 7 / 4 == raw_size) {
+            load_raw = &RawImage::fuji_14bit_load_raw;
+        } else {
+            parse_fuji_compressed_header();
+        }
+    }
 
     if (flip == 5) {
         this->rotate_deg = 270;

Note you also need to change the white level in camconst.json from 4050 to 16100 (copied from other Fuji models, not sure if it is accurate)

@iliasg

This comment has been minimized.

Copy link
Collaborator

commented Mar 21, 2018

Many thanks :)

I think WL=16100 is safe for Fujis

Regarding 14bit Bayer Fujis .. RT already supports Fuji GFX-50s (both compressed and non_compressed) which is Bayer 14bit ;) .. so I wonder what changed for X-A3 .. the "compressed header" ? ..

@heckflosse

This comment has been minimized.

Copy link
Collaborator

commented Mar 21, 2018

@iliasg isn't GFX-50s uncompressed 14bit info in 16bit data?

@agriggio

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2018

@heckflosse yes, that's also my understanding

@agriggio

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2018

@iliasg @heckflosse I just checked again. It is true that we already supported GFX-50s compressed raws. However, the compression format seems to be totally different than that used for the X-A3 :-/

@iliasg

This comment has been minimized.

Copy link
Collaborator

commented Mar 21, 2018

@heckflosse Ingo yes .. GFX-50s gives 16bit data with 2 high bits zeroed .. I had forgotten this ..

@agriggio

This comment has been minimized.

Copy link
Contributor

commented Mar 22, 2018

can we close?

@heckflosse

This comment has been minimized.

Copy link
Collaborator

commented Mar 22, 2018

@agriggio Yes

@agriggio agriggio closed this Mar 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.