[sane-devel] [PATCH] epkowa: 16-bit on Epson 4490

[Carl Troein]

Olaf Meeuwissen wrote:
> Carl Troein <carl.troein at ed.ac.uk> writes:
> 
>> Hi all,
> 
> Hi Carl,

Hi Olaf.

> Sorry for the late follow-up.

Ditto. :-)

>> My patch has been mentioned on this list before:
>> http://lists.alioth.debian.org/pipermail/sane-devel/2007-September/019874.html
>> but now I've split it into the actual 16-bit fix and some entirely
>> optional code cosmetics.
> 
> Thanks for the patches.  I'll take a look and see what, if, when and
> in what form things will make it into the epkowa backend.  Please note
> that from iscan's (somewhat myopic) point of view, 16-bit support is
> not required.  All scans, except bi-level, are done with 8-bit.  That
> means that any 16-bit support that makes it in will be pretty much
> untested.

There seems at least to be a few people who use scanimage or xsane
for scanning with the epkowa driver, and with any luck there'll
at least be someone to notice if at any point it stops working.

>> The reason that 16-bit scanning didn't work is that the 4490 is a 'D'
>> level scanner, which means that it doesn't do colour correction. Instead,
>> the driver does this (which means that 16-bit raw data from the scanner
>> is all the more important to reduce discretization) but the driver
>> assumes that the data is 8-bit RGB. So the first patch merely adds a
>> 16-bit version of color_correct and a check for the depth. The modified
>> code path is only reachable for D-level scanners and should change
>> nothing for the 8-bit case.
> 
> Hmm, I'd say all 16-bit scans should take that code path, irrespective
> of the scanner level (unless colour correction has been taken care of
> by the hardware already).

 From what I've seen when reading the source, it's only the D level
scanners that don't do it in hardware. Or at least that's how the driver
tests whether it should call color_correct. (Though I have no idea what's
going on in the non-free parts, nor have I been sober enough to figure
out how the different parts interact.)

>> The second and third patch are just minor cleanup that should have no
>> noticable effects except on code size. The first-middle-last cases of
>> the data download are rolled into a smaller (and IMHO more readable)
>> loop, and a redundant memset is removed.
> 
> I'm not sure about the unrolling of the loop.  Apart from code size
> (and readability) does it improve anything else?  Performance?

It seems I had partly forgotten what the patch does. :-/ It wasn't
first-middle-last that was unrolled in the existing code, but rather
the three channels (the existing sane_read has a block of code that's
repeated almost identically three times). The patch gathered the three
repeats into a loop, because I thought it looked better. But since
it doesn't actually fix anything or improve anything measurably, it
might be best to just ignore that patch for now.

Cheers,
//Carl

-- 
   Carl Troein - carl.troein at ed.ac.uk
   http://carl.troein.com/