Skip to content
This repository was archived by the owner on Jul 22, 2018. It is now read-only.

Sync in changes from darktable. Closes #123. Fixes #152, #153, #155#157

Merged
klauspost merged 36 commits intoklauspost:developfrom
LebedevRI:darktable
Aug 29, 2016
Merged

Sync in changes from darktable. Closes #123. Fixes #152, #153, #155#157
klauspost merged 36 commits intoklauspost:developfrom
LebedevRI:darktable

Conversation

@LebedevRI
Copy link
Contributor

@LebedevRI LebedevRI commented Aug 23, 2016

Well, i lied.
But i'm doing this right now not because i want to, but rather because others are busy.

Notable changes:
A big amount of nikon NEF images resulting in wrong whitelevel was seen.
Further look into the problem showed:

  1. If nef is uncompressed, then all ok, whitelevel is always 16383.
  2. Else, 14-bit and 12-bit nefs have different white levels.
  3. In many cases we only had an entry with no mode, with either one of the whitelevels (either the 14-bit one, which should be ~15k, or 12-bit one, which should be ~4k)
  4. So both 14-bit and 12-bit images would receive the same whitelevel, which is wrong for one of them.

So i decided that is not ok, and changed NefDecoder to only accept explicit entries with mode="{bitness}bit-{compressed/uncompressed}".
And that did result in catching some cases.

@klauspost this sync up all the darktable changes to rawspeed, and brings our bundled rs in line with the upstream (not counting #101)

hanatos and others added 30 commits August 23, 2016 13:26
Added basic support for Canon 80D
overflow_before_widen: Potentially overflowing expression
    _count << RawSpeed::datashifts[_type]
with type RawSpeed::uint32 (32 bits, unsigned)
is evaluated using 32-bit arithmetic,
and then used in a context that expects an expression
of type uint64 (64 bits, unsigned).

Coverity CID 1357657
All iso levels have the same black/white levels...
Reported by Jack Massey on IRC, with a sample.
The sample looked very dark, but had purple highlights.

We should probably just stop loading NEF's unless that specific
mode is listed in cameras.xml, else we are quite likely to
be providing broken images. Especially since that is what we do
for the cameras that are entirely not listed in cameras.xml
This is basically the same problem as with Rw2Decoder.

It looks like all the nikon cameras has different modes -
14-bit compressed, 12-bit compressed,
14-bit uncompressed, 12-bit uncompressed.

And the problem here is that 14-bit and 12-bit have different white levels.

Since NefDecoder::checkSupportInternal() just silently fallbacks
to the generic entry, if present (with mode=""),
this has result in silently-garbled images.

Let's not do that.
For all the nikon cameras, if only the entry for 14-bit mode
are present, this *will* finally break support for 12-bit modes.
(as in, rawspeed will stop silently producing subtly broken images)

Same for 12-bit modes.

This will probably break following cameras:
(these have white="0", so i can't guess what they are)
        <Camera make="NIKON CORPORATION" model="NIKON 1 J1" decoder_version="4">
        <Camera make="NIKON CORPORATION" model="NIKON 1 J2" decoder_version="4">
        <Camera make="NIKON CORPORATION" model="NIKON 1 S1" decoder_version="4">
        <Camera make="NIKON CORPORATION" model="NIKON 1 V1" decoder_version="4">
(these are apparently actually 16bit, but report as (?)-bit)
        <Camera make="NIKON" model="COOLPIX P330" decoder_version="5">
        <Camera make="NIKON" model="COOLPIX P7700" decoder_version="5">
        <Camera make="NIKON" model="COOLPIX P7800" decoder_version="5">

The basic idea for this commit:
$ grep -ri "<camera" cameras.xml | grep -i nikon | grep -iv "mode=" | sort
For each camera in that list ^ look for the presence of same entry
but with modes, and if any - add missing modes.

If the white level is 14-bit integer - the entry is for mode="14bit-*"
And it is safe to assume that the entry for 12bit is missing

If the white level is 12-bit integer - the entry is for mode="12bit-*"

In the end, there should be no <Camera> entries for nikon cameras with
missing / empty mode tag.
Was caught thanks to last few commits...
Was caught thanks to last few commits...
Was caught thanks to last few commits...
This is a blind commit, without samples.
This is a blind commit, without samples.
For all nikon cameras with 14-bit entries and no 12-bit entries,
add 12-bit entries with obviously-wrong white level,
and  supported="no".

This way the users will not recieve garbled images, and once
they provide a sample, the support can be easily added by
setting appropriate whitelevel and removing supported="no"
If you duplicate any camera entry, rawspeed detects it:
RawSpeed:CameraMetaData: Duplicate entry found for camera: <name>, Skipping!

It deletes the camera, but the caller then just continues
to operate with freed pointer.

=================================================================
==32315==ERROR: AddressSanitizer: heap-use-after-free on address 0x614000058328 at pc 0x7fa0d8ba31c5 bp 0x7fa0c0f52ae0 sp 0x7fa0c0f52ad8
READ of size 8 at 0x614000058328 thread T1
     0 0x7fa0d8ba31c4 in RawSpeed::CameraMetaData::CameraMetaData(char const*) /home/lebedevri/darktable/src/external/rawspeed/RawSpeed/CameraMetaData.cpp:41
     1 0x7fa0d89a9d4b in dt_rawspeed_load_meta /home/lebedevri/darktable/src/common/imageio_rawspeed.cc:64
     2 0x7fa0d89aa3d1 in dt_rawspeed_lookup_makermodel /home/lebedevri/darktable/src/common/imageio_rawspeed.cc:76
     3 0x7fa0d898cf48 in dt_image_refresh_makermodel /home/lebedevri/darktable/src/common/image.c:1026
     4 0x7fa0d899641e in dt_image_cache_allocate /home/lebedevri/darktable/src/common/image_cache.c:127
     5 0x7fa0d892c8b9 in dt_cache_get_with_caller /home/lebedevri/darktable/src/common/cache.c:206
     6 0x7fa0d89974dc in dt_image_cache_get /home/lebedevri/darktable/src/common/image_cache.c:169
     7 0x7fa0d89c2957 in dt_mipmap_cache_get_with_caller /home/lebedevri/darktable/src/common/mipmap_cache.c:685
     8 0x7fa0d8a0d74d in dt_image_load_job_run /home/lebedevri/darktable/src/control/jobs/image_jobs.c:35
     9 0x7fa0d8a015db in dt_control_run_job /home/lebedevri/darktable/src/control/jobs.c:306
     10 0x7fa0d8a015db in dt_control_work /home/lebedevri/darktable/src/control/jobs.c:540
     11 0x7fa0d4487463 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x7463)
     12 0x7fa0d09bf30c in clone (/lib/x86_64-linux-gnu/libc.so.6+0xe730c)

0x614000058328 is located 232 bytes inside of 432-byte region [0x614000058240,0x6140000583f0)
freed by thread T1 here:
     0 0x7fa0d8e8b2d0 in operator delete(void*) (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc32d0)
     1 0x7fa0d8ba1aad in RawSpeed::CameraMetaData::addCamera(RawSpeed::Camera*) /home/lebedevri/darktable/src/external/rawspeed/RawSpeed/CameraMetaData.cpp:87
     2 0x7fa0d8ba2f2e in RawSpeed::CameraMetaData::CameraMetaData(char const*) /home/lebedevri/darktable/src/external/rawspeed/RawSpeed/CameraMetaData.cpp:42
     3 0x7fa0d89a9d4b in dt_rawspeed_load_meta /home/lebedevri/darktable/src/common/imageio_rawspeed.cc:64
     4 0x7fa0d89aa3d1 in dt_rawspeed_lookup_makermodel /home/lebedevri/darktable/src/common/imageio_rawspeed.cc:76
     5 0x7fa0d898cf48 in dt_image_refresh_makermodel /home/lebedevri/darktable/src/common/image.c:1026
     6 0x7fa0d899641e in dt_image_cache_allocate /home/lebedevri/darktable/src/common/image_cache.c:127
     7 0x7fa0d892c8b9 in dt_cache_get_with_caller /home/lebedevri/darktable/src/common/cache.c:206
     8 0x7fa0d89974dc in dt_image_cache_get /home/lebedevri/darktable/src/common/image_cache.c:169
     9 0x7fa0d89c2957 in dt_mipmap_cache_get_with_caller /home/lebedevri/darktable/src/common/mipmap_cache.c:685
     10 0x7fa0d8a0d74d in dt_image_load_job_run /home/lebedevri/darktable/src/control/jobs/image_jobs.c:35
     11 0x7fa0d8a015db in dt_control_run_job /home/lebedevri/darktable/src/control/jobs.c:306
     12 0x7fa0d8a015db in dt_control_work /home/lebedevri/darktable/src/control/jobs.c:540
     13 0x7fa0d4487463 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x7463)

previously allocated by thread T1 here:
     0 0x7fa0d8e8acd0 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc2cd0)
     1 0x7fa0d8ba2f11 in RawSpeed::CameraMetaData::CameraMetaData(char const*) /home/lebedevri/darktable/src/external/rawspeed/RawSpeed/CameraMetaData.cpp:41
     2 0x7fa0d89a9d4b in dt_rawspeed_load_meta /home/lebedevri/darktable/src/common/imageio_rawspeed.cc:64
     3 0x7fa0d89aa3d1 in dt_rawspeed_lookup_makermodel /home/lebedevri/darktable/src/common/imageio_rawspeed.cc:76
     4 0x7fa0d898cf48 in dt_image_refresh_makermodel /home/lebedevri/darktable/src/common/image.c:1026
     5 0x7fa0d899641e in dt_image_cache_allocate /home/lebedevri/darktable/src/common/image_cache.c:127
     6 0x7fa0d892c8b9 in dt_cache_get_with_caller /home/lebedevri/darktable/src/common/cache.c:206
     7 0x7fa0d89974dc in dt_image_cache_get /home/lebedevri/darktable/src/common/image_cache.c:169
     8 0x7fa0d89c2957 in dt_mipmap_cache_get_with_caller /home/lebedevri/darktable/src/common/mipmap_cache.c:685
     9 0x7fa0d8a0d74d in dt_image_load_job_run /home/lebedevri/darktable/src/control/jobs/image_jobs.c:35
     10 0x7fa0d8a015db in dt_control_run_job /home/lebedevri/darktable/src/control/jobs.c:306
     11 0x7fa0d8a015db in dt_control_work /home/lebedevri/darktable/src/control/jobs.c:540
     12 0x7fa0d4487463 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x7463)

Thread T1 created by T0 here:
     0 0x7fa0d8df9039 in pthread_create (/usr/lib/x86_64-linux-gnu/libasan.so.3+0x31039)
     1 0x7fa0d8a01dcc in dt_control_jobs_init /home/lebedevri/darktable/src/control/jobs.c:596
     2 0x7fa0d89f7b6d in dt_control_init /home/lebedevri/darktable/src/control/control.c:119
     3 0x7fa0d894d65c in dt_init /home/lebedevri/darktable/src/common/darktable.c:932
     4 0x400c6f in main /home/lebedevri/darktable/src/main.c:24
     5 0x7fa0d08f872f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2072f)

SUMMARY: AddressSanitizer: heap-use-after-free /home/lebedevri/darktable/src/external/rawspeed/RawSpeed/CameraMetaData.cpp:41 in RawSpeed::CameraMetaData::CameraMetaData(char const*)
Was erroneously set to that value in 53af22a7d8b255f386bf1c012c2202d3324d3c75

Reported by Pascal Obry.
Was added in 53af22a7d8b255f386bf1c012c2202d3324d3c75.
But there is no sample to verify it...
The raw file is just plain sensor dump.
Has no metadata at all.
Not even TIFF.
@klauspost
Copy link
Owner

Thanks - it seems to be backwards compatible from what you describe. I will look through the code and merge.

@klauspost klauspost merged commit b8287ff into klauspost:develop Aug 29, 2016
@LebedevRI LebedevRI deleted the darktable branch August 29, 2016 14:18
@LebedevRI
Copy link
Contributor Author

@klauspost thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants