Replace dcraw with faster raw decoder (libraw?)

[Beep6581]

Originally reported on Google Code with ID 594

This is an enhancement suggestion.

dcraw is not optimized for speed, which shows as rather slow loading times of RAW files
compared to the competition. When working with RawTherapee, I think the slow loading
times is disturbing so it would be valuable with an improvement.

libraw is based on dcraw and does (so far) not provide much speed improvement.

It seems like Klaus Post's RawSpeed library could be a good alternative though, currently
used in RawStudio and Darktable.

A quick test made on my workstation showed that a .CR2 (canon RAW) that took 1800 ms
to unpack (only unpacking measured, no post-processing) with dcraw took 580 ms with
RawSpeed. A .NEF (Nikon RAW) file that took 1370 ms to unpack with dcraw took 680 ms
with RawSpeed. That is 2 - 3 times faster, and the difference between 1,8 and 0,6 seconds
load time is certainly noticed by the user.

The drawback with RawSpeed is that it does not support all formats, so you need to
have a solution where you combine that and dcraw/libraw, which Darktable currently
does.

Reported by torger@ludd.ltu.se on 2011-03-23 15:04:12

[Beep6581]

Changed tagging

Reported by oduis@hotmail.com on 2011-03-27 22:03:28

• Labels added: Type-Enhancement
• Labels removed: Type-Defect

[Beep6581]

I think this is a great idea! RawStudio certainly loads raw files a lot faster than
RawTherapee.

Reported by borissimo86 on 2011-05-07 01:27:53

[Beep6581]

I try to clean our bug-tracker a bit: What's the status here?

Is it 'WontFix' or is somebody willing to implement this? I'll close it 'WontFix' in
a week if nobody complains.

Reported by rinni@gmx.net on 2013-01-28 22:42:34

[Beep6581]

Nothing done on this AFAIK. Dcraw is still slow, and it is still possible to integrate
rawspeed. However, in terms of a cleanup I think it is ok to set it to wontfix, we
can return to it later sometime when someone feels the urge to make performance improvements.

However, do we need to close these issues? Isn't keeping them at low priority enhancements
a good enough filter? I won't protest if you close it though :-)

Reported by torger@ludd.ltu.se on 2013-01-29 08:49:08

[Beep6581]

I'd prefer to keep low priority issues opened. I've updated the labels accordingly.

Reported by natureh.510 on 2013-01-29 15:01:46

• Status changed: Accepted
• Labels added: OpSys-All, Performance, Maintainability

[Beep6581]

May be a better solution would be Libraw. It already embeds Rawspeed and the user can
toggle between Rawspeed and Dcraw engine, useful for some formats that are not supported
by Rawspeed.

I think it would be a good thing two active and accessible (unlike Dave)open source
teams to join forces.  

Reported by iliasgiarimis on 2013-01-29 15:56:18

[Beep6581]

Yes, I think Libraw[1] would be the option to go as it's a library with stable API whereas
Rawspeed[2] is not a separate library and we would have similar issues as with dcraw
(besides the speed).

[1] http://www.libraw.org
[2] http://rawstudio.org/blog/?p=800

Reported by rinni@gmx.net on 2013-02-04 20:51:31

[Beep6581]

1. Would libraw offer any speed improvements vs dcraw?
2. If we use libraw, can RT still use its internal demosaicing?

Reported by michaelezra000 on 2013-02-04 21:12:15

[Beep6581]

3. With libraw, could we still use custom camera matrices, or the only way to use them
would be custom DCP profiles?

Reported by michaelezra000 on 2013-02-04 21:13:47

[Beep6581]

Issue 1847 has been merged into this issue.

Reported by entertheyoni on 2013-05-05 21:03:52

[Beep6581]

Reported by entertheyoni on 2013-05-05 21:04:27

• Status changed: New
• Labels removed: Maintainability

[Beep6581]

I don't know about the process to integrate new dcraw-versions ind RT (which is a necessity,
I think). But I can offer to take a look at dcraw to improve speed, if and only if
it makes sense regarding integration of new dcraw-versions.

Ingo

Reported by heckflosse@i-weyrich.de on 2013-05-05 23:32:01

[Beep6581]

I think it could be worthwhile to make optimizations for the "big three", ie Nikon .NEF,
Canon .CR2 and Sony .ARW, and stay low on the others and let dcraw original code do
the decoding. Then we get both wide support and high performance for most common models.
We could either bring in from rawspeed or write own code.

Writing own code might not be that bad actually, because it should be only the "load_raw"
function that need to be replaced for a significant speed improvement (the rest should
be fairly negligible), and the dcraw modifications would then be one-liners, easy to
maintain.

For example to significantly speed up CR2 loading you would need to make a drop-in
replacement of the lossless_jpeg_load_raw() function in dcraw.cc. What we would do
is to place those optimised functions in an own .cc file and then just call them from
dcraw.cc with one line, in this case the first line in lossless_jpeg_load_raw() and
then just return.

Reported by torger@ludd.ltu.se on 2013-10-22 12:46:44

[Beep6581]

I'd vote for the "own methods" or the "replaced by rawspeed for all" solution. Managing
2 raw decoder would be a coding overhead IMHO.

Reported by natureh.510 on 2013-10-23 09:07:01

[Beep6581]

I have not looked deeply into this recently, but I'm quite sure that we can't really
replace dcraw if we want wide camera support. Librawspeed don't support as many cameras
as dcraw does (I think).

There are possibilities to further isolate/encapsulate the dcraw code and make easier
to maintain.

It could be the case that we'd want dcraw (for wide support, odd formats), adobe DNG
sdk (for best DNG support) and librawspeed (to speed up some formats) all in parallel
:-). I think it can be done. It will be some overhead but I think supporting many formats
is a quite valuable feature, although optimization effort should probably only be put
on the most popular.

I could possibly make some contributions there in the future, it's a space I'm confident
working in.

Reported by torger@ludd.ltu.se on 2013-10-23 09:49:37

[Beep6581]

I did some tries to speed up loading of a NEF, but got only a speedup of about 10% without
changing the algorithm, mainly by making two versions of unsigned CLASS getbithuff_t::operator()
(int nbits, ushort *huff), one with two arguments and another one with only one argument.
This way I could eliminate some branches. If there is some interest in this speedup,
I can make a patch. For further reduction of processing time we could try to implement
this one: http://u.cs.biu.ac.il/~wiseman/dcc.pdf

On the other side, the time to decode a D800 NEF on my machine is only 800 ms. Maybe
I should continue my work on optimizing Amaze first...

Ingo

Reported by heckflosse@i-weyrich.de on 2013-10-23 20:07:06

[Beep6581]

It is not just a question of speed.  The process of copying and pasting
dcraw code (and that's what it is, essentially) is also fraught with
security risk.  If a buffer overflow is found in dcraw it will take a
long time for RT to:

1. notice
2. determine if our version is vulnerable
3. fix
4. release

This is one of the main reasons to use shared libraries.  I have always
looked with irritation and disbelief at the blurb in dcraw source where
Dave C. says copying & pasting is his *preferred* way.

Reported by nobrowser on 2014-07-23 04:10:12

[Beep6581]

FYI, I was prompted to post the above after reading this:

https://lwn.net/Articles/604217/

Reported by nobrowser on 2014-07-23 04:12:29

[Beep6581]

RawTherapee is not a networking daemon running on an internet server, and it's not run
with root privileges. We need not to worry from dcraw buffer overflows other than it's
a bit ugly to crash when reading a broken file.

Concerning buffer overflows I can inform you that RT's code in general is cluttered
with those type of "security" risks, much of the code is not very defensively written,
from historical reasons and depending on various coders' coding style. However, as
said RawTherapee is not the type of software that needs to be secure.

I have written networking daemons in my professional career, and when writing that
type of software you look very differently on this issues of course. However, I have
actually never needed a raw decoding library for the networking daemon...

My current view on RT's dcraw usage is that it's a indeed a bit ugly from a "code beauty"
perspective, but it works quite well and we support new cameras quicker than the alternative
methods.

Dcraw can be wrapped in a way that requires less maintenance than we have today in
RT (I've done it in a commercial project), but it's not too bad as it is, so I think
we can live with the current way yet for some time. Not ideal, but I rather put my
limited time elsewhere for the time being.

One need to keep in mind that RT is a very advanced raw decoder which have functionality
that stretches into the raw decoding internals, which means that we need to modify
the code there. It also goes very much the other way around, the way RT's raw processing
internals is arranged is very much related to how dcraw has arranged the data, which
means changing a lib with an entirely different API is not so easy.

LibRaw which libifies dcraw is a nice effort, but I don't think it's mature enough
for RT at this point (and I'm not sure it's API fits RT's design), and is also a bit
slow to update, they're still on 9.20 while RT is on 9.22 (latest). We also have the
camconst.json stuff and some own hacks which means RT is sometimes first with supporting
a new camera.

Reported by torger@ludd.ltu.se on 2014-08-26 20:18:13

[Beep6581]

I'm afraid I have to disagree with a lot of what you say.

A crash is *not* the only risk.  Suppose I have some private photos in the same directory
as the doctored one.  RT has opened them in the file browser.  What is to prevent the
exploit from emailing them to the cracker?

Also, "defensive" is not at all the same as "secure".  I am *against* "defensive" coding.

Reported by nobrowser on 2014-08-26 21:25:06

[Beep6581]

It's theoretically possible yes. I'm just not particularly worried for malicious raw
files targeted at a specific RT build for a specific platform. LibRaw is the current
best candidate for dcraw replacement, but it's too early to still.

Reported by torger@ludd.ltu.se on 2014-08-26 22:09:24

[Beep6581]

Reported by entertheyoni on 2014-11-01 12:40:58

• Status changed: Accepted

[Beep6581]

If we were to use rawspeed we would need to include it in RT's sources because it can only be used bundled, it cannot be built as a shared lib. The master branch of rawspeed is currently very old, the development branch is active and doubled the commit count since the original rawspeed author Klaus transferred rawspeed to @LebedevRI. We could include the files from the develop branch, or to be a little safer we could use the rawspeed version used by darktable since it would have been already tested.

https://sonarcloud.io/dashboard?id=rawspeed%3Adevelop
https://github.com/darktable-org/darktable/tree/master/src/external

[LebedevRI]

the original rawspeed author Klaus transferred rawspeed to @LebedevRI.

Just to clarify, not to me, to @darktable-org, i'm just acting as a maintainer

[agriggio]

any estimate on the required effort? @heckflosse Ingo, do you have any idea? what would be the user visible impact? (thinking of pixelshift and camconst.json in particular)

[heckflosse]

@agriggio Currently we can not replace dcraw by rawspeed for a lot of reasons:

1. rawspeed does not support multiframe raw files which are needed for pixelshift
2. rawspeed does not support compressed fuji files
3. switching from camconst.json to the rawspeed mechanism is not easy

But: We could try to use rawspeed additionally to dcraw for decoding files which currently are not supported by dcraw.

[agriggio]

@heckflosse thanks for confirming what was my understanding as well. I think having rawspeed as an optional component would be interesting indeed.

[LebedevRI]

2. rawspeed does not support compressed fuji files

Actually, perhaps someone can help me here.
I need some raw file which triggers this branch:

RawTherapee/rtengine/fujicompressed.cc

Lines 76 to 80 in 6e55f6b

	} else if (info->q_point[4] == 0xFFF) {
	info->total_values = 4096;
	info->raw_bits = 12;
	info->max_bits = 48;
	info->maxDiff = 64;

I would post it on disscuss, but i suppose more people here know how to actually check it (e.g. put derror(); in that branch, and see if any raw stops to load?)

[LebedevRI]

2. rawspeed does not support compressed fuji files

Done.

[heckflosse]

@LebedevRI I really like that you solved this issue in RawSpeed! But to avoid confusion, as your fix is not related to rt, I will close this issue now.
That does not mean, I won't pick up this again. It's just to avoid confusion of rt users.

Ingo