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
>
>
>
>
------------------------------------------------------------------------------
Managing the Performance of Cloud-Based Applications
Take advantage of what the Cloud has to offer - Avoid Common Pitfalls.
Read the Whitepaper.
http://pubads.g.doubleclick.net/gampad/clk?id=121051231&iu=/4140/ostg.clktrk
_______________________________________________
darktable-devel mailing list
darktable-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/darktable-devel
