Amazing, yes you are completely right, that's the reason it was always failing. In gcc, I added
LOCAL_CFLAGS += -fsigned-char
to the make file and it works perfectly and decodes like a charm! As I read, the default of a char is not really defined in c and depends on the architecture, so this error makes perfectly sens. But may there would be a way to explicitly init is as a signed char directly in code so that these things will not happen to others?
so after quite a lot of coffee, I found some kind of translation method to translate the stacktrace to real code lines. Here's the result:
********** Crash dump: **********
signal 11 (SIGSEGV), code 2 (SEGV_ACCERR), fault addr 0x7246192bb8
Routine fuji_decode_sample_odd(fuji_compressed_block*, fuji_compressed_params const*, unsigned short*, int, int_pair*) at libraw_fuji_compressed.cpp:490
Routine LibRaw::xtrans_decode_block(fuji_compressed_block*, fuji_compressed_params const*, int) at libraw_fuji_compressed.cpp:727
Routine LibRaw::fuji_decode_strip(fuji_compressed_params const*, int, long long, unsigned int) at libraw_fuji_compressed.cpp:919
Routine LibRaw::fuji_decode_loop(fuji_compressed_params const*, int, long long*, unsigned int*) at libraw_fuji_compressed.cpp:999
Routine LibRaw::fuji_compressed_load_raw() at libraw_fuji_compressed.cpp:983
Routine LibRaw::unpack() at libraw_cxx.cpp:2811 (discriminator 3)
Routine libraw_unpack at libraw_c_api.cpp:130
Routine Java_com_tssystems_Libraw_open at libraw.c:51
For me it seems like the access of the grads array via the gradient variable runs further than the array, but I'm not sure. What do you think? Does any output of the variables may help?
Just a note: The value of the gradient variable before it fails is 2272 which seems way higher than all others (they are around 250). Maybe there's something going wrong here?
// Edit: The really strange thing is that the Ra... Rg values change each time I run it and the crash finally ocurse. For instance, now I get
Ra 16383 Rb 0 Rc 0 Rd 0 Rg 0
or
Ra 0 Rb 0 Rc 0 Rd 0 Rg 849
before it will crash. Seems there is something really going wrong with the values?
For some other pretty strange reason it almost seems like something is running parallel, because I can't see the full var dump of the q_table in onces. But this could also be a transfering issue of the log in android since the buffer speed is somewhat limited? I don't have openmp enabled.
libraw_raw2image does NOT subtracts black level from image[] array, most likely this is the source of the problem (image looks dull and with magenta cast, right?)
My experience is very mixed with debugging c code on Android. However I’ll try to find a way to generate a dump or something similar to find out where it fails and will report back later.
Thank you very much for your fast response, looking at the data structure documentation for imgdata.idata.filters, I see this:
unsigned filters;
Bit mask describing the order of color pixels in the matrix (0 for full-color images). 32 bits of this field describe 16 pixels (8 rows with two pixels in each, from left to right and from top to bottom). Each two bits have values 0 to 3, which correspond to four possible colors. Convenient work with this field is ensured by the COLOR(row,column) function, which returns the number of the active color for a given pixel.
values less than 1000 are reserved as special cases:
1 - Leaf Catchlight with 16x16 bayer matrix;
9 - Fuji X-Trans (6x6 matrix)
3..8 and 10..999 - are unused.
Some points are not very clear to me:
-32 bits for 16 pixels, 2bits per pixels to code color, but what is the mapping ? (0->red, 1->green, 2->blue)
-What does this 1-->999 code means ?
Current libraw snapshot can run user-specified demosaic routine, just set interpolate_bayer pointer to point to your function.
Look into other demosaic functions code (e.g. lin_interpolate()) to see how to loop through all pixels.
To get bayer pattern, use imgdata.idata.filters variable, or COLOR(r,c) call.
I'm also a little irritated. I basically changed nothing and used the latest git (just from today). The only thing I changed for Android is to manually implement the "swab" function, but the implementation I use should be the same as the original from c. I could also see no calls on this function when the xtrans decoding is going on.
C++ standard defines arrays as dense arrays, no spacing or extra rows padding.
Also, because xtrans/_abs arrays are defined in LibRaw class definition, and it is possible that we compile two source files with different -O.. switches, array layout should not changed from -O0 (no warning) to -O2 (warning issued).
So, I think this is gcc problem: it should either show this warning with any -O.. setting, or do not show it with -O2
Anyway, this loop was already changed to to 6-wide loops in master branch with commit message 'to make gcc happy' :)
That is a very common optimization and is not likely to be the issue. The problem is usually found in the expression itself, commonly either the right-side array element is undefined or the result overflows the variable.
On the 6th iteration of that loop the result of this expression is undefined:
Can you point me to a matrix that "libraw" will use to convert DNG file? (to XYZ) The "cam_xyz" contain all zeros, there should be some matrix being used.
gcc preprocessor does not count blank and comment lines, and I gave the wrong code section. sorry.
I pulled the rest of the warnings from the build log:
/tmp/portage/portage/media-libs/libraw-0.18.5/work/LibRaw-0.18.5/src/libraw_cxx.cpp: In member function ‘virtual int LibRaw::open_datastream(LibRaw_abstract_datastream*)’:
Yes, default char type (signed/unsigned) is undefined (although it is signed in all our test/production environments).
We'll change char definition to signed char (or, maybe, to int8_t, will see)
Thank you for reporting the bug.
Amazing, yes you are completely right, that's the reason it was always failing. In gcc, I added
LOCAL_CFLAGS += -fsigned-char
to the make file and it works perfectly and decodes like a charm! As I read, the default of a char is not really defined in c and depends on the architecture, so this error makes perfectly sens. But may there would be a way to explicitly init is as a signed char directly in code so that these things will not happen to others?
Thanks again!
it looks like I know the answer:
q_table is defined as:
char *q_table; /* quantization table */
So, signed char, from -127 to 128.
It is initialized with ..-4...+4 values at lines 110-130.
Is there any chance that your char type is by default unsigned char?
Hi Alex,
so after quite a lot of coffee, I found some kind of translation method to translate the stacktrace to real code lines. Here's the result:
For me it seems like the access of the grads array via the gradient variable runs further than the array, but I'm not sure. What do you think? Does any output of the variables may help?
Just a note: The value of the gradient variable before it fails is 2272 which seems way higher than all others (they are around 250). Maybe there's something going wrong here?
Thanks
Torsten
Hi,
here are some dumps:
// Edit: The really strange thing is that the Ra... Rg values change each time I run it and the crash finally ocurse. For instance, now I get
before it will crash. Seems there is something really going wrong with the values?
For some other pretty strange reason it almost seems like something is running parallel, because I can't see the full var dump of the q_table in onces. But this could also be a transfering issue of the log in android since the buffer speed is somewhat limited? I don't have openmp enabled.
Both 2272 and 250 are too high values for gradient variable b/c table size is only 41.
gradient is calculated by:
q_table contains values from -4 to +4, so maximum gradient value is 40.
Could you please try to dump q_table values? And, also, values used to calc gradient: Rb - Rf, Rc - Rb
Dear Sir:
libraw_raw2image does NOT subtracts black level from image[] array, most likely this is the source of the problem (image looks dull and with magenta cast, right?)
no_auto_scale completely disables LibRaw::scale_colors() step
This is not intended for common image processing using dcraw_process, this option is special-purpose
Hi Alex,
My experience is very mixed with debugging c code on Android. However I’ll try to find a way to generate a dump or something similar to find out where it fails and will report back later.
Thanks
Torsten
We always welcome raw samples.
To send us files, you may use any file sharing service (WeTransfer, Dropbox, Google Drive, etc) and send link to it to info@libraw.org.
Please use simple public links (available for anyone, who knows the URL), but not 'Share for account' feature
0...999 range is reserved for future custom layouts, like Fuji 6x6.
32 bits describes 16 pixels, 4x4 block. For all today cameras it is repeated 2x2 blocks.
00 - red, 01 - green, 10 - blue, 11 - green2 channels
(just use LibRaw::COLOR(row,col) call to get layout)
Thank you very much for your fast response, looking at the data structure documentation for imgdata.idata.filters, I see this:
unsigned filters;
Bit mask describing the order of color pixels in the matrix (0 for full-color images). 32 bits of this field describe 16 pixels (8 rows with two pixels in each, from left to right and from top to bottom). Each two bits have values 0 to 3, which correspond to four possible colors. Convenient work with this field is ensured by the COLOR(row,column) function, which returns the number of the active color for a given pixel.
values less than 1000 are reserved as special cases:
1 - Leaf Catchlight with 16x16 bayer matrix;
9 - Fuji X-Trans (6x6 matrix)
3..8 and 10..999 - are unused.
Some points are not very clear to me:
-32 bits for 16 pixels, 2bits per pixels to code color, but what is the mapping ? (0->red, 1->green, 2->blue)
-What does this 1-->999 code means ?
I will take a look at the source code tomorrow
Current libraw snapshot can run user-specified demosaic routine, just set interpolate_bayer pointer to point to your function.
Look into other demosaic functions code (e.g. lin_interpolate()) to see how to loop through all pixels.
To get bayer pattern, use imgdata.idata.filters variable, or COLOR(r,c) call.
Is there any way to get exact code point that fails with exception? maybe coredump or so?
Hi Alex,
yes, it's the line, I put it behind the bracket:
{__android_log_print(ANDROID_LOG_INFO,"libraw","decode_block %d %d %d",g_even_pos,g_odd_pos,line_width); ...This is the output I see:
I'm also a little irritated. I basically changed nothing and used the latest git (just from today). The only thing I changed for Android is to manually implement the "swab" function, but the implementation I use should be the same as the original from c. I could also see no calls on this function when the xtrans decoding is going on.
Regards
Torsten
Run the file through valgrind (under FreeBSD), do not see any fuji-decoder problems (buffer overrun, etc), so no idea.
BTW, what exact version do you use (in my working version line 712 is { bracket after while operator)?
1) If use_camera_matrix value suggests to use camera (raw metadata) color, than this color data is copied over rgb_cam array at open_file() stage.
2) If camera matrix is not available, embedded color data (from adobe_coeff() large array) is used.
Camera color matrix is parsed into imgdata.color.cmatrix
Sorry, wrong. If use_camera_matrix is set, so camera matrix should be used, camera color matrix is copied to rgb_cam
C++ standard defines arrays as dense arrays, no spacing or extra rows padding.
Also, because xtrans/_abs arrays are defined in LibRaw class definition, and it is possible that we compile two source files with different -O.. switches, array layout should not changed from -O0 (no warning) to -O2 (warning issued).
So, I think this is gcc problem: it should either show this warning with any -O.. setting, or do not show it with -O2
Anyway, this loop was already changed to to 6-wide loops in master branch with commit message 'to make gcc happy' :)
You need to apply white balance (usually, before demosaic stage)
That is a very common optimization and is not likely to be the issue. The problem is usually found in the expression itself, commonly either the right-side array element is undefined or the result overflows the variable.
On the 6th iteration of that loop the result of this expression is undefined:
imgdata.idata.xtrans
Hi Alex,
Thanks for the quick reply!
Can you point me to a matrix that "libraw" will use to convert DNG file? (to XYZ) The "cam_xyz" contain all zeros, there should be some matrix being used.
Regards,
Mio
what is wrong with
loop if both xtrans and xtrans_abs are [6][6] arrays, so xtrans[0][35] is equal to xtrans[5][5]?
"libraw" does not average/weight DNG color data
cmatrix is camera color data, it used according to use_camera_matrix value.
gcc preprocessor does not count blank and comment lines, and I gave the wrong code section. sorry.
I pulled the rest of the warnings from the build log:
/tmp/portage/portage/media-libs/libraw-0.18.5/work/LibRaw-0.18.5/src/libraw_cxx.cpp: In member function ‘virtual int LibRaw::open_datastream(LibRaw_abstract_datastream*)’:
/tmp/portage/portage/media-libs/libraw-0.18.5/work/LibRaw-0.18.5/src/libraw_cxx.cpp:1784:64: warning: iteration 6 invokes undefined behavior [-Waggressive-loop-optimizations]
imgdata.idata.xtrans[0]
= imgdata.idata.xtrans_abs[0][c]; ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^ /tmp/portage/portage/media-libs/libraw-0.18.5/work/LibRaw-0.18.5/src/libraw_cxx.cpp:1783:22: note: within this loop for(int c = 0; c < 36; c++) ~~^~~~ So it appears the actual code section begins at line 1999: // XTrans Compressed? if (!imgdata.idata.dng_version && !strcasecmp(imgdata.idata.make, "Fujifilm") && (load_raw == &LibRaw::unpacked_load_raw)) { if (imgdata.sizes.raw_width * imgdata.sizes.raw_height * 2 != libraw_internal_data.unpacker_data.data_size) { if (imgdata.sizes.raw_width * imgdata.sizes.raw_height * 7 / 4 == libraw_internal_data.unpacker_data.data_size) load_raw = &LibRaw::fuji_14bit_load_raw; else parse_fuji_compressed_header(); } if (imgdata.idata.filters == 9) { // Adjust top/left margins for X-Trans int newtm = imgdata.sizes.top_margin % 6 ? (imgdata.sizes.top_margin / 6 + 1) * 6 : imgdata.sizes.top_margin; int newlm = imgdata.sizes.left_margin % 6 ? (imgdata.sizes.left_margin / 6 + 1) * 6 : imgdata.sizes.left_margin; if (newtm != imgdata.sizes.top_margin || newlm != imgdata.sizes.left_margin) { imgdata.sizes.height -= (newtm - imgdata.sizes.top_margin); imgdata.sizes.top_margin = newtm; imgdata.sizes.width -= (newlm - imgdata.sizes.left_margin); imgdata.sizes.left_margin = newlm; for (int c1 = 0; c1 < 6; c1++) for (int c2 = 0; c2 < 6; c2++) imgdata.idata.xtrans[c1][c2] = imgdata.idata.xtrans_abs[c1][c2]; } } } gcc optimizes the nested 6x6 loop to: for(int c = 0; c < 36; c++) on the 6th iteration of that loop the result of imgdata.idata.xtrans[c] = imgdata.idata.xtrans_abs[c]; is undefined.Pages