One last commit, which drops in Frank Markesteijn's demosaic algorithm for x-trans sensors, as taken from dcraw. This needs much more work: it's just a wrapper for dcraw and does the processing via integers (not floats), plus it uses a local RGB->Lab conversion with a hard-coded camera matrix (rather than dt's colorspaces API). And it integrates awkwardly with the existing module parameters & GUI. That said, it seems to work and produce good results (albeit very slowly).
As mentioned, I have to slow down on this for a couple weeks, but wanted to put out there everything which I had. The commits before this one on the xtrans2 branch are more polished. Best, Dan On Tue, Feb 11, 2014, at 02:26 PM, Dan Torop wrote: > Belated greetings! > > Things which now seem to work for x-trans images: > > - loading (via LibRaw 0.16.0) > - bilinear demosaicing and quick downsampling > - all raw-dependent modules except cacorrect (that is, highlight, > temperature, hotpixels, and rawdenoise) > - recognizing cameras with these sensors (and limited basecurve support) > > To make this good: > > - a good demosaicing algorithm, probably a float adaptation of "Frank > Markesteijn's algorithm for Fuji X-Trans sensors" from dcraw, maybe VNG > (which is much faster, but noticeably worse) > - some code optimization (OpenMP certainly, maybe SSE, perhaps OpenCL) > - find if there are any more Bayer-specific paths to work around (though > have already pored through the code for these) > - more testing/bugfixes > - possibly enable cacorrect module -- if its algorithm works with > non-Bayer sensor patterns > - using RawSpeed to load images when that code is ready > - misc. tuning (things are good enough now, but could generalize > histogram_step_raw and demosaic & temperature tile alignments, for > example) > > I may be short of time until late February, but am very excited to do > the rest. Meantime, the code seems surprisingly workable (even with only > bilinear demosaic) for viewing/processing images. > > As this becomes more usable, would it be helpful to consolidate these > changes into a few big commits, rather than a chain of small revisions? > (I will be rebasing this to the current upstream master regardless.) > > Revised code is in the xtrans2 branch at > https://github.com/dtorop/darktable. > > It's been exciting to see some of the workings of the dt code! And to > realize the wisdom of the dt developers in not rashly embarking on > support for new sensors... > > Dan > > > On Wed, Feb 5, 2014, at 04:26 AM, johannes hanika wrote: > > heya, > > > > (put dt-dev back into cc, might be interesting to others, too) > > > > > > On Wed, Feb 5, 2014 at 10:28 AM, Dan Torop <d...@pnym.net> wrote: > > > > > >> __ > > > > > >> Hi Johannes, > >> > >> Things are coming along with this. Loading/viewing x-trans images > >> generally works. The temperature module also now handles them. > > > > nice :) > > > >> > >> > >> Still many other things to do for a first pass, including: > >> > >> - Update a few more raw-dependent modules (but not chromatic > >> aberration...). I'd be more than content with highlight reconstruction and > >> hot pixel. Disable the rest of these for x-trans files. > > > > sounds reasonable for a start. there was also some work on a Lab-space > > defringing module that might serve as a workaround for C/A for fuji. > > > >> > >> > >> > - Make OpenCL processing for x-trans raw files fall back to CPU. (I > don't even have a working OpenCL set-up with which to test code.) > > > > fair enough. not crucial the first time around. > > > > >> > >> - May need to specially handle sensor matrix for flipped/rotated images. > > > > hm? i think orientation/crop+rotate modules will come later on in the pipe. > > > >> > >> > >> - Work around any other Bayer-specific things in main code paths. > > > > yeah, should be limited to modules before demosaic, it's not all that much > > special bayer code, but very crucial code.. > > > > >> > >> > >> - Alter demosaic UI for x-trans as many options will be disabled. > >> > >> - Make images look right in lighttable panel (something's wrong at the > >> moment). > >> > >> - Implement a fancier demosaic algorithm for x-trans. Currently it's just > >> linear interpolation. > >> > >> - Check for regressions. > >> > >> - Make sure code style matches dt project. > >> > >> - Put in support for an x-trans sensor camera. > >> > >> Would it make sense to add a flag to dt_image_flags_t to identify if an > >> image is non-Bayer? Right now I'm checking if dt_image_t has "filters" set > >> to 9, which is sort of arbitrary. I am storing the x-trans sensor pattern > >> in dt_image_t, it's just a 6x6 byte array so should be manageable. (I > >> don't understand the whole mipmap/caching system, by the way, so I hope > >> I'm not digging myself into a hole there.) > >> > > > > the sensor pattern stuff is only relevant once you loaded the raw, so i > > don't see how these flags need caching. you'll reload them with the raw in > > case it is dropped in between. setting filters to 9 is fine imo. changing > > the size of the image struct isn't critical, the only cache that ends up on > > disk is for pixel data. > > > > > > cheers, > > jo > > > > > >> > >> > >> > Latest code is in my fork on github... With luck I'm not screwing things > up too much, and the code could be usable. It is in no way optimized. > >> > >> Best, > >> Dan > >> > >> > >> > >> > >> On Tue, Feb 4, 2014, at 03:08 AM, johannes hanika wrote: > >>> heya, > >>> > >>> nice, all that sounds like an impressive amount of work already.. > >>> > >>> some comments interleaved below: > >>> > >>> > >>> On Tue, Feb 4, 2014 at 2:38 AM, Dan Torop <d...@pnym.net> wrote: > >>> > >>>> Hi Developers, > >>>> > > >>>> > I've been roughing out a code path to read the new fujifilm "x-trans" > >>>> > sensors, mindful of > >>>> > >>>> http://www.darktable.org/2012/12/released-1-1-1/comment-page-1/#comment-8531. > >>>> > At this point I've dealt with the first step on that list only. > >>>> > >>>> > Notes so far: > >>>> > >>>> > - LibRaw < 0.15 as shipped with darktable hangs in packed_load_raw() > >>>> > when reading x-trans files (at least those from fuji x100s). Newer > >>>> > LibRaw versions don't have this problem. But LibRaw >= 0.15.0 loses the > >>>> > "document mode" by which dt_imageio_open_raw() loads grayscale > >>>> > uninterpolated data (hence > >>>> > >>>> http://sourceforge.net/p/darktable/mailman/message/31719896/). I > >>>> > switched to LibRaw 0.16.0, and use rawdata.raw_image[] rather than > >>>> > libraw_dcraw_make_mem_image() in document mode to get equivalent data. > >>>> > This works for x-trans, but perhaps breaks/alters behavior with other > >>>> > cameras. > >>> > >>> very good, we meant to do that some time ago but never got to it. > >>> definitely interested in that part :) > >>> > >>> > >>>> > > > >>>> > - The dt_image_t struct needs a 6x6 lookup for x-trans filters. There > >>>> > also needs to be a way to detect when to use this -- I'm using an ugly > >>>> > hack for now. > >>> > >>> right. i think we just use the uint32_t filters for bayer. so either > >>> store it in image_t and wire the image_cache to also write it to db in > >>> case the image is evicted from cache or just always reload from the raw > >>> file directly. arguably we don't need it when the raw isn't loaded. so i > >>> think we should be fine caching stuff there, 6x6 doesn't sound like an > >>> excessive amount of memory. > >>> > >>> > > >>>> > > >>>> > - I made a naive proof-of-concept demosaic function which just produces > >>>> > grayscale from x-trans sensors. A sane route may be to adapt dcraw >= > >>>> > 9.19's xtrans_interpolate() code to handle float data. > >>> > >>> okay. > >>>> > > > >>>> > - In develop/imageop.c, there should be x-trans-specific quick clip/zoom > >>>> > routines. I sketched out a bad one (which doesn't get the color right). > >>>> > This code will presumably not make a half-size image but a 1/3-size one > >>>> > (or maybe even 1/6), due to the sensor. For both this and demosaicing, > >>>> > I can't match DT developers' image processing skills, much less their > >>>> > fluency with SIMD and OpenMP. I don't think I'd touch OpenCL. > >>>> > >>>> > >>>> > To do would be: > >>>> > >>>> > - Fix things already broken by work above. > >>> > >>> :) > >>>> > >>>> > - Deal with all the other modules which touch raw data > >>> > >>> that's going to be a major pain, we could just disable those for the time > >>> being (no chromatic aberration correction for x-trans..) > >>>> > >>>> > - Add per-camera support > >>>> > >>>> > >>>> > Questions for DT developers: > >>>> > >>>> > - Do you have interest in such an x-trans sensor patch? > >>> > >>> not personally, i don't have such a camera. but i think it would be great > >>> for the workflow of those who do. there's a slight issue about support, > >>> we should make sure there are enough interested devs to handle user > >>> requests specific to this format. especially since it sounds like the > >>> initial support for x-trans will be a lot worse than for bayer patterns. > >>> i would think it's still a good first step. > >>> > >>>> > >>>> > - A potential patch could be pretty invasive -- not just requiring the > >>>> > LibRaw upgrade > >>> > >>> that's great, we should have done that earlier i guess. > >>> > >>>> > but also touching at least one data structure > >>>> > (dt_image_t) > >>> > >>> no worries. > >>> > >>>> and requiring alternate code paths in a lot of key > >>>> > > operations. > >>> > >>> yeah, that's where support is going to be the issue. > >>> > >>>> > Do you feel OK about that and/or do you have thoughts on how > >>>> > to mitigate this? For example, I noticed the AMaZE demosaic algorithm > >>>> > ended up in a separate file. I don't think the x-trans code would be > >>>> > that lengthy, but it's also not localized. > >>> > >>> it'll be local to every module. as far as i can see the only place where > >>> you need to alter the core is in imageop.c for the scaling functions? if > >>> the rest stays in src/iop/ i'm fine with it. > >>> > > >>>> > >>>> > - Are there exemplary non-x-trans raw files read via LibRaw? I should > >>>> > verify that they still work as before. > >>> > >>> if you build with cmake -DDONT_USE_RAWSPEED=on all raw images will be > >>> loaded through libraw. well, i think we have a blacklist of extensions > >>> now, but in principle that should trigger the libraw codepath. > >>> > > >>>> > >>>> > - Advice would be great. > >>> > >>> i can help out if you have specific questions about the code base. > >>>> > >>>> > >>>> > So far I'm keeping everything in a local branch, but am happy to make a > >>>> > fork via github. I'm not sure if this is worth pursuing -- or if I'm the > >>>> > one to do so. I'm also happy to talk on #darktable IRC. > >>> > >>> i think pushing it somewhere public would definitely increase visibility > >>> and might be helpful. > >>> > >>> cheers, > >>> jo > >>> > >>>> > >>>> > Thanks, > >>>> > Dan > >>> > > > > ------------------------------------------------------------------------------ > Android apps run on BlackBerry 10 > Introducing the new BlackBerry 10.2.1 Runtime for Android apps. > Now with support for Jelly Bean, Bluetooth, Mapview and more. > Get your Android app in front of a whole new audience. Start now. > http://pubads.g.doubleclick.net/gampad/clk?id=124407151&iu=/4140/ostg.clktrk > _______________________________________________ > darktable-devel mailing list > darktable-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/darktable-devel ------------------------------------------------------------------------------ Android apps run on BlackBerry 10 Introducing the new BlackBerry 10.2.1 Runtime for Android apps. Now with support for Jelly Bean, Bluetooth, Mapview and more. Get your Android app in front of a whole new audience. Start now. http://pubads.g.doubleclick.net/gampad/clk?id=124407151&iu=/4140/ostg.clktrk _______________________________________________ darktable-devel mailing list darktable-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/darktable-devel
