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

Reply via email to