[sane-devel] [PATCH] Add support to snapscan for Benq Scanwit 2720S film scanner

Paul Menzel paulepanter at users.sourceforge.net
Mon Feb 25 11:07:13 UTC 2013


Dear Andrew,


thank you for working on finishing the support.

Am Montag, den 25.02.2013, 10:00 +0000 schrieb Andrew Goodbody:

[…]

What is the current status?

My comments are inline.

> ---
>  backend/snapscan-options.c |  223 +++++++++++++++++++++++++++++---
>  backend/snapscan-scsi.c    |  309 ++++++++++++++++++++++++++++++++++----------
>  backend/snapscan-sources.c |   31 ++++-
>  backend/snapscan.c         |  133 ++++++++++++++-----
>  backend/snapscan.h         |   28 +++-
>  5 files changed, 597 insertions(+), 127 deletions(-)
> 
> diff --git a/backend/snapscan-options.c b/backend/snapscan-options.c
> index a7401bb..674c8d2 100644
> --- a/backend/snapscan-options.c
> +++ b/backend/snapscan-options.c
> @@ -1,9 +1,9 @@
>  /* sane - Scanner Access Now Easy.
>  
> -   Copyright (C) 1997, 1998, 2001 Franck Schnefra, Michel Roelofs,
> +   Copyright (C) 1997, 1998, 2001, 2013 Franck Schnefra, Michel Roelofs,
>     Emmanuel Blot, Mikko Tyolajarvi, David Mosberger-Tang, Wolfgang Goeller,
>     Petter Reinholdtsen, Gary Plewa, Sebastien Sable, Mikael Magnusson,
> -   Oliver Schwartz and Kevin Charter
> +   Andrew Goodbody, Oliver Schwartz and Kevin Charter
>  
>     This file is part of the SANE package.
>  
> @@ -67,11 +67,15 @@
>  static SANE_Int def_rgb_lpr = 4;
>  static SANE_Int def_gs_lpr = 12;
>  static SANE_Int def_bpp = 8;
> +static SANE_Int def_frame_no = 1;
>  
>  
>  /* predefined preview mode name */
>  static char md_auto[] = "Auto";
>  
> +/* predefined focus mode name */
> +static char md_manual[] = "Manual";
> +
>  /* predefined scan mode names */
>  static char md_colour[] = SANE_VALUE_SCAN_MODE_COLOR;
>  static char md_bilevelcolour[] = SANE_VALUE_SCAN_MODE_HALFTONE;
> @@ -103,6 +107,15 @@ static char lpr_desc[] = SANE_I18N(
>      "a scan; if it's set too high, X-based frontends may stop responding "
>      "to X events and your system could bog down.");
>  
> +static char frame_desc[] = SANE_I18N(
> +    "Frame number of media holder that should be scanned.");
> +
> +static char focus_mode_desc[] = SANE_I18N(
> +    "Use manual or automatice selection of focus point.");

1. automatic

> +
> +static char focus_desc[] = SANE_I18N(
> +    "Focus point for scanning.");
> +
>  /* ranges */
>  static const SANE_Range x_range_fb =
>  {
> @@ -174,6 +187,16 @@ static const SANE_Range y_range_tpo_2580 =
>      SANE_FIX (0.0), SANE_FIX (80.0), 0
>  };        /* mm */
>  
> +/* TPO range for the Scanwit 2720S */

TPO? Turn page over [1] ? Could you enlighten me please? It’s used for
the other devices too and search the Web did not help.

[1] http://en.wikipedia.org/wiki/TPO

Found it in `snapscan.h`.

        typedef enum
        {
            SRC_FLATBED = 0,    /* Flatbed (normal) */
            SRC_TPO,            /* Transparency unit */
            SRC_ADF
        } SnapScan_Source;

> +static const SANE_Range x_range_tpo_2720s =
> +{
> +    SANE_FIX (0.0), SANE_FIX (23.6), 0
> +};        /* mm */
> +static const SANE_Range y_range_tpo_2720s =
> +{
> +    SANE_FIX (0.0), SANE_FIX (35.7), 0
> +};        /* mm */
> +
>  /* TPO range for the Epson 3490 */
>  static const SANE_Range x_range_tpo_3490 =
>  {
> @@ -198,6 +221,14 @@ static const SANE_Range lpr_range =
>  {
>      1, 50, 1
>  };
> +static const SANE_Range frame_range =
> +{
> +    1, 6, 1
> +};
> +static const SANE_Range focus_range =
> +{
> +    0, 0x300, 6
> +};
>  
>  static const SANE_Range brightness_range =
>  {
> @@ -243,6 +274,8 @@ static void init_options (SnapScan_Scanner * ps)
>          {10, 50, 75, 100, 150, 200, 300, 400, 600, 800, 1600};
>      static SANE_Word resolutions_2400[] =
>          {10, 50, 75, 100, 150, 200, 300, 400, 600, 1200, 2400};
> +    static SANE_Word resolutions_2700[] =
> +        {4, 337, 675, 1350, 2700};
>      static SANE_Word resolutions_3200[] =
>          {15, 50, 150, 200, 240, 266, 300, 350, 360, 400, 600, 720, 800, 1200, 1600, 3200};
>      static SANE_String_Const names_all[] =
> @@ -253,6 +286,8 @@ static void init_options (SnapScan_Scanner * ps)
>          {md_auto, md_colour, md_bilevelcolour, md_greyscale, md_lineart, NULL};
>      static SANE_String_Const preview_names_basic[] =
>          {md_auto, md_colour, md_greyscale, md_lineart, NULL};
> +    static SANE_String_Const focus_modes[] =
> +        {md_auto, md_manual, NULL};
>      static SANE_Int bit_depth_list[4];
>      int bit_depths;
>      SANE_Option_Descriptor *po = ps->options;
> @@ -287,6 +322,10 @@ static void init_options (SnapScan_Scanner * ps)
>             y_range_tpo = y_range_tpo_2480;
>          }
>          break;
> +    case SCANWIT2720S:
> +        x_range_tpo = x_range_tpo_2720s;
> +        y_range_tpo = y_range_tpo_2720s;
> +        break;
>      case PERFECTION3490:
>          x_range_tpo = x_range_tpo_3490;
>          y_range_tpo = y_range_tpo_3490;
> @@ -318,6 +357,7 @@ static void init_options (SnapScan_Scanner * ps)
>      po[OPT_MODE_GROUP].cap = 0;
>      po[OPT_MODE_GROUP].constraint_type = SANE_CONSTRAINT_NONE;
>  
> +    ps->res = DEFAULT_RES;

Why move this from below?

>      po[OPT_SCANRES].name = SANE_NAME_SCAN_RESOLUTION;
>      po[OPT_SCANRES].title = SANE_TITLE_SCAN_RESOLUTION;
>      po[OPT_SCANRES].desc = SANE_DESC_SCAN_RESOLUTION;
> @@ -354,11 +394,17 @@ static void init_options (SnapScan_Scanner * ps)
>      case PERFECTION3490:
>          po[OPT_SCANRES].constraint.word_list = resolutions_3200;
>          break;
> +    case SCANWIT2720S:
> +        po[OPT_SCANRES].constraint.word_list = resolutions_2700;
> +        ps->val[OPT_SCANRES].w = 1350;
> +        ps->res = 1350;
> +        break;
>      default:
>          po[OPT_SCANRES].constraint.word_list = resolutions_600;
>          break;
>      }
> -    ps->res = DEFAULT_RES;
> +    DBG (DL_OPTION_TRACE,
> +        "sane_init_options resolution is %d\n", ps->res);
>  
>      po[OPT_PREVIEW].name = SANE_NAME_PREVIEW;
>      po[OPT_PREVIEW].title = SANE_TITLE_PREVIEW;
> @@ -487,8 +533,18 @@ static void init_options (SnapScan_Scanner * ps)
>          source_list[i] = 0;
>          po[OPT_SOURCE].size = max_string_size(source_list);
>          po[OPT_SOURCE].constraint.string_list = source_list;
> -        ps->source = SRC_FLATBED;
> -        ps->source_s = (SANE_Char *) strdup(src_flatbed);
> +        if (ps->pdev->model == SCANWIT2720S)
> +        {
> +            ps->source = SRC_TPO;
> +            ps->source_s = (SANE_Char *) strdup(src_tpo);
> +            ps->pdev->x_range.max = x_range_tpo.max;
> +            ps->pdev->y_range.max = y_range_tpo.max;
> +        }
> +        else
> +        {
> +            ps->source = SRC_FLATBED;
> +            ps->source_s = (SANE_Char *) strdup(src_flatbed);
> +        }
>      }
>  
>      po[OPT_GEOMETRY_GROUP].title = SANE_I18N("Geometry");
> @@ -581,12 +637,24 @@ static void init_options (SnapScan_Scanner * ps)
>      case PERFECTION3490:
>          bit_depth_list[++bit_depths] = 16;
>          break;
> +    case SCANWIT2720S:
> +        bit_depth_list[bit_depths] = 12;
> +        break;
>      default:
>          break;
>      }    
>      bit_depth_list[0] = bit_depths;
>      po[OPT_BIT_DEPTH].constraint.word_list = bit_depth_list;
> -    ps->val[OPT_BIT_DEPTH].w = def_bpp;
> +    if (ps->pdev->model == SCANWIT2720S)
> +    {
> +        ps->val[OPT_BIT_DEPTH].w = 12;
> +        ps->bpp_scan = 12;
> +    }
> +    else
> +    {
> +        ps->val[OPT_BIT_DEPTH].w = def_bpp;
> +        ps->bpp_scan = def_bpp;
> +    }

For me it would be more clear, if you set def_bpp depending on the
device (in the beginning?) and just add `ps->bpp_scan = def_bpp;`. Why
is that needed. Bug fix?

>      

Whitespace errors. Check with `git show` (and set up colors for Git).

>      po[OPT_QUALITY_CAL].name = SANE_NAME_QUALITY_CAL;
>      po[OPT_QUALITY_CAL].title = SANE_TITLE_QUALITY_CAL;
> @@ -813,6 +881,45 @@ static void init_options (SnapScan_Scanner * ps)
>      po[OPT_THRESHOLD].constraint.range = &positive_percent_range;
>      ps->threshold = DEFAULT_THRESHOLD;
>  
> +    po[OPT_FRAME_NO].name = SANE_I18N("Frame");
> +    po[OPT_FRAME_NO].title = SANE_I18N("Frame to be scanned");
> +    po[OPT_FRAME_NO].desc = frame_desc;
> +    po[OPT_FRAME_NO].type = SANE_TYPE_INT;
> +    po[OPT_FRAME_NO].unit = SANE_UNIT_NONE;
> +    po[OPT_FRAME_NO].size = sizeof (SANE_Int);
> +    po[OPT_FRAME_NO].cap = SANE_CAP_SOFT_SELECT
> +                       | SANE_CAP_SOFT_DETECT
> +                       | SANE_CAP_INACTIVE;
> +    po[OPT_FRAME_NO].constraint_type = SANE_CONSTRAINT_RANGE;
> +    po[OPT_FRAME_NO].constraint.range = &frame_range;
> +    ps->frame_no = def_frame_no;
> +
> +    po[OPT_FOCUS_MODE].name = SANE_I18N("Focusmode");
> +    po[OPT_FOCUS_MODE].title = SANE_I18N("Auto or manual focus");
> +    po[OPT_FOCUS_MODE].desc = focus_mode_desc;
> +    po[OPT_FOCUS_MODE].type = SANE_TYPE_STRING;
> +    po[OPT_FOCUS_MODE].unit = SANE_UNIT_NONE;
> +    po[OPT_FOCUS_MODE].size = 16;
> +    po[OPT_FOCUS_MODE].cap = SANE_CAP_SOFT_SELECT
> +                       | SANE_CAP_SOFT_DETECT
> +                       | SANE_CAP_INACTIVE;
> +    po[OPT_FOCUS_MODE].constraint_type = SANE_CONSTRAINT_STRING_LIST;
> +    po[OPT_FOCUS_MODE].constraint.string_list = focus_modes;
> +    ps->focus_mode_s= md_auto;
> +    ps->focus_mode = MD_AUTO;
> +
> +    po[OPT_FOCUS_POINT].name = SANE_I18N("Focuspoint");

Focus_point? Not sure what name is used for though.

> +    po[OPT_FOCUS_POINT].title = SANE_I18N("Focus point");
> +    po[OPT_FOCUS_POINT].desc = focus_desc;
> +    po[OPT_FOCUS_POINT].type = SANE_TYPE_INT;
> +    po[OPT_FOCUS_POINT].unit = SANE_UNIT_NONE;
> +    po[OPT_FOCUS_POINT].size = sizeof (SANE_Int);
> +    po[OPT_FOCUS_POINT].cap = SANE_CAP_SOFT_SELECT
> +                       | SANE_CAP_SOFT_DETECT
> +                       | SANE_CAP_INACTIVE;
> +    po[OPT_FOCUS_POINT].constraint_type = SANE_CONSTRAINT_RANGE;
> +    po[OPT_FOCUS_POINT].constraint.range = &focus_range;
> +
>      po[OPT_ADVANCED_GROUP].title = SANE_I18N("Advanced");
>      po[OPT_ADVANCED_GROUP].desc = "";
>      po[OPT_ADVANCED_GROUP].type = SANE_TYPE_GROUP;
> @@ -942,7 +1049,16 @@ static void control_options(SnapScan_Scanner *pss)
>          default:
>              break;
>          }
> -    }        
> +    }
> +    if (pss->pdev->model == SCANWIT2720S)
> +    {
> +        pss->options[OPT_FRAME_NO].cap &= ~SANE_CAP_INACTIVE;
> +        pss->options[OPT_FOCUS_MODE].cap &= ~SANE_CAP_INACTIVE;
> +        if (pss->focus_mode == MD_MANUAL)
> +        {
> +            pss->options[OPT_FOCUS_POINT].cap &= ~SANE_CAP_INACTIVE;
> +        }
> +    }
>  }
>  
>  SANE_Status sane_control_option (SANE_Handle h,
> @@ -1080,7 +1196,16 @@ SANE_Status sane_control_option (SANE_Handle h,
>              break;
>          case OPT_BIT_DEPTH:
>              *(SANE_Int *) v = pss->val[OPT_BIT_DEPTH].w;
> -            break;        
> +            break;
> +        case OPT_FRAME_NO:
> +            *(SANE_Int *) v = pss->frame_no;
> +            break;
> +        case OPT_FOCUS_MODE:
> +            strcpy ((SANE_String) v, pss->focus_mode_s);
> +            break;
> +        case OPT_FOCUS_POINT:
> +            *(SANE_Int *) v = pss->focus;
> +            break;
>          default:
>              DBG (DL_MAJOR_ERROR,
>                   "%s: invalid option number %ld\n",
> @@ -1495,6 +1620,31 @@ SANE_Status sane_control_option (SANE_Handle h,
>              if (i)
>                  *i |= SANE_INFO_RELOAD_PARAMS;
>              break;
> +        case OPT_FRAME_NO:
> +            pss->frame_no = *(SANE_Int *) v;
> +            break;
> +        case OPT_FOCUS_MODE:
> +            {
> +                char *s = (SANE_String) v;
> +                if (strcmp (s, md_manual) == 0)
> +                {
> +                    pss->focus_mode_s = md_manual;
> +                    pss->focus_mode = MD_MANUAL;
> +                    pss->options[OPT_FOCUS_POINT].cap &= ~SANE_CAP_INACTIVE;
> +                }
> +                else
> +                {
> +                    pss->focus_mode_s = md_auto;
> +                    pss->focus_mode = MD_AUTO;
> +                    pss->options[OPT_FOCUS_POINT].cap |= SANE_CAP_INACTIVE;
> +                }
> +                if (i)
> +                    *i = SANE_INFO_RELOAD_OPTIONS | SANE_INFO_RELOAD_PARAMS;
> +                break;
> +            }
> +        case OPT_FOCUS_POINT:
> +            pss->focus = *(SANE_Int *) v;
> +            break;
>          default:
>              DBG (DL_MAJOR_ERROR,
>                   "%s: invalid option number %ld\n",
> @@ -1526,7 +1676,14 @@ SANE_Status sane_control_option (SANE_Handle h,
>          switch (n)
>          {
>          case OPT_SCANRES:
> -            pss->res = 300;
> +            if (pss->pdev->model == SCANWIT2720S)
> +            {
> +                pss->res = 1350;

So as the 2720S is a film scanner, it gets a higher default(?)
resolution?

> +            }
> +            else
> +            {
> +                pss->res = 300;
> +            }
>              if (i)
>                  *i |= SANE_INFO_RELOAD_PARAMS;
>              break;
> @@ -1558,13 +1715,26 @@ SANE_Status sane_control_option (SANE_Handle h,
>              pss->preview_mode = MD_GREYSCALE;
>              break;
>          case OPT_SOURCE:
> -            pss->source = SRC_FLATBED;
> -            pss->pdev->x_range.max = x_range_fb.max;
> -            pss->pdev->y_range.max = y_range_fb.max;
> -            pss->predef_window = pdw_none;
> -            if (pss->source_s)
> -                free (pss->source_s);
> -            pss->source_s = (SANE_Char *) strdup(src_flatbed);
> +            if (pss->pdev->model == SCANWIT2720S)
> +            {
> +                pss->source = SRC_TPO;
> +                pss->pdev->x_range.max = x_range_tpo.max;
> +                pss->pdev->y_range.max = y_range_tpo.max;
> +                pss->predef_window = pdw_none;
> +                if (pss->source_s)
> +                    free (pss->source_s);
> +                pss->source_s = (SANE_Char *) strdup(src_tpo);
> +            }
> +            else
> +            {
> +                pss->source = SRC_FLATBED;
> +                pss->pdev->x_range.max = x_range_fb.max;
> +                pss->pdev->y_range.max = y_range_fb.max;
> +                pss->predef_window = pdw_none;
> +                if (pss->source_s)
> +                    free (pss->source_s);
> +                pss->source_s = (SANE_Char *) strdup(src_flatbed);
> +            }
>              if (i)
>                  *i |= SANE_INFO_RELOAD_PARAMS | SANE_INFO_RELOAD_OPTIONS;
>              break;
> @@ -1598,7 +1768,26 @@ SANE_Status sane_control_option (SANE_Handle h,
>              pss->gs_lpr = def_gs_lpr;
>              break;
>          case OPT_BIT_DEPTH:
> -            pss->val[OPT_BIT_DEPTH].w = def_bpp;
> +            if (pss->pdev->model == SCANWIT2720S)
> +            {
> +                pss->val[OPT_BIT_DEPTH].w = 12;
> +            }
> +            else
> +            {
> +                pss->val[OPT_BIT_DEPTH].w = def_bpp;
> +            }
> +            break;
> +        case OPT_FRAME_NO:
> +            pss->frame_no = def_frame_no;
> +            break;
> +        case OPT_FOCUS_MODE:
> +            pss->focus_mode_s = md_auto;
> +            pss->focus_mode = MD_AUTO;
> +            if (i)
> +                *i = SANE_INFO_RELOAD_OPTIONS;
> +            break;
> +        case OPT_FOCUS_POINT:
> +            pss->focus = 0x13e;

What does 0x13e imply?

>              break;
>          default:
>              DBG (DL_MAJOR_ERROR,
> diff --git a/backend/snapscan-scsi.c b/backend/snapscan-scsi.c
> index 7482215..3b0c697 100644
> --- a/backend/snapscan-scsi.c
> +++ b/backend/snapscan-scsi.c
> @@ -1,9 +1,9 @@
>  /* sane - Scanner Access Now Easy.
>  
> -   Copyright (C) 1997, 1998, 2001 Franck Schnefra, Michel Roelofs,
> +   Copyright (C) 1997, 1998, 2001, 2002, 2013 Franck Schnefra, Michel Roelofs,
>     Emmanuel Blot, Mikko Tyolajarvi, David Mosberger-Tang, Wolfgang Goeller,
>     Petter Reinholdtsen, Gary Plewa, Sebastien Sable, Mikael Magnusson,
> -   Oliver Schwartz and Kevin Charter
> +   Max Ushakov, Andrew Goodbody, Oliver Schwartz and Kevin Charter
>  
>     This file is part of the SANE package.
>  
> @@ -289,6 +289,7 @@ static SANE_Status snapscan_cmd(SnapScan_Bus bus, int fd, const void *src,
>  #define RESERVE_UNIT           0x16
>  #define RELEASE_UNIT           0x17
>  #define SEND_DIAGNOSTIC        0x1D
> +#define OBJECT_POSITION        0x31
>  #define GET_DATA_BUFFER_STATUS 0x34
>  
>  #define SCAN_LEN 6
> @@ -561,6 +562,7 @@ static SANE_Status inquiry (SnapScan_Scanner *pss)
>          pss->bpp = 14;
>          break;
>      case STYLUS_CX1500:
> +    case SCANWIT2720S:
>          pss->bpp = 12;
>          break;
>      default:
> @@ -674,6 +676,10 @@ static void release_unit (SnapScan_Scanner *pss)
>  #define DTCQ_GAMMA_RED14 0x96
>  #define DTCQ_GAMMA_GREEN14 0x97
>  #define DTCQ_GAMMA_BLUE14 0x98
> +#define DTCQ_GAMMA_GRAY12_16BIT 0xa0
> +#define DTCQ_GAMMA_RED12_16BIT 0xa1
> +#define DTCQ_GAMMA_GREEN12_16BIT 0xa2
> +#define DTCQ_GAMMA_BLUE12_16BIT 0xa3
>  #define DTCQ_GAMMA_GRAY14_16BIT 0xa5 /* ? */
>  #define DTCQ_GAMMA_RED14_16BIT 0xa6
>  #define DTCQ_GAMMA_GREEN14_16BIT 0xa7
> @@ -734,6 +740,12 @@ static SANE_Status send (SnapScan_Scanner *pss, u_char dtc, u_char dtcq)
>          case DTCQ_GAMMA_BLUE12:
>              tl = 4096;
>              break;
> +        case DTCQ_GAMMA_GRAY12_16BIT:    /* 12-bit tables with 16 bit data */
> +        case DTCQ_GAMMA_RED12_16BIT:
> +        case DTCQ_GAMMA_GREEN12_16BIT:
> +        case DTCQ_GAMMA_BLUE12_16BIT:
> +            tl = 8192;
> +            break;
>          case DTCQ_GAMMA_GRAY14:    /* 14-bit tables */
>          case DTCQ_GAMMA_RED14:
>          case DTCQ_GAMMA_GREEN14:
> @@ -832,13 +844,10 @@ static SANE_Status send_calibration_5150(SnapScan_Scanner *pss)
>  #define SET_WINDOW_P_BLUE_UNDER_COLOR      45
>  #define SET_WINDOW_P_GREEN_UNDER_COLOR     44
>  
> -static SANE_Status set_window (SnapScan_Scanner *pss)
> +static SANE_Status prepare_set_window (SnapScan_Scanner *pss)
>  {
> -    static const char *me = "set_window";
> -    SANE_Status status;
> -    unsigned char source;
> +    static const char *me = "prepare_set_window";
>      u_char *pc;
> -    int pos_factor;
>  
>      DBG (DL_CALL_TRACE, "%s\n", me);
>      zero_buf (pss->cmd, MAX_SCSI_CMD_LEN);
> @@ -860,62 +869,11 @@ static SANE_Status set_window (SnapScan_Scanner *pss)
>      u_short_to_u_charp (pss->res, pc + SET_WINDOW_P_YRES);
>      DBG (DL_CALL_TRACE, "%s Resolution: %d\n", me, pss->res);
>  
> -    switch (pss->pdev->model)
> -    {
> -        case PRISA5000:
> -        case PRISA5000E:
> -        case PRISA5150:
> -            pos_factor = (pss->res > 600) ?  1200 : 600;
> -            break;
> -        case PERFECTION1270:
> -        case PERFECTION1670:
> -            pos_factor = (pss->res > 800) ?  1600 : 800;
> -            break;
> -        case PERFECTION2480:
> -            pos_factor = (pss->res > 1200) ?  2400 : 1200;
> -            break;
> -        case PERFECTION3490:
> -            pos_factor = (pss->res > 1600) ?  3200 : 1600;
> -            break;
> -        default:
> -            pos_factor = pss->actual_res;
> -            break;
> -    }
> -    /* it's an ugly sound if the scanner drives against the rear
> -       wall, and with changing max values we better be sure */
> -    check_range(&(pss->brx), pss->pdev->x_range);
> -    check_range(&(pss->bry), pss->pdev->y_range);
> -    {
> -        int tlxp =
> -            (int) (pos_factor*IN_PER_MM*SANE_UNFIX(pss->tlx));
> -        int tlyp =
> -            (int) (pos_factor*IN_PER_MM*SANE_UNFIX(pss->tly));
> -        int brxp =
> -            (int) (pos_factor*IN_PER_MM*SANE_UNFIX(pss->brx));
> -        int bryp =
> -            (int) (pos_factor*IN_PER_MM*SANE_UNFIX(pss->bry));
> -
> -        /* Check for brx > tlx and bry > tly */
> -        if (brxp <= tlxp) {
> -            tlxp = MAX (0, brxp - 75);
> -        }
> -        if (bryp <= tlyp) {
> -            tlyp = MAX (0, bryp - 75);
> -        }
> -
> -        u_int_to_u_char4p (tlxp, pc + SET_WINDOW_P_TLX);
> -        u_int_to_u_char4p (tlyp, pc + SET_WINDOW_P_TLY);
> -        u_int_to_u_char4p (MAX (((unsigned) (brxp - tlxp)), 75),
> -                           pc + SET_WINDOW_P_WIDTH);
> -        u_int_to_u_char4p (MAX (((unsigned) (bryp - tlyp)), 75),
> -                           pc + SET_WINDOW_P_LENGTH);
> -        DBG (DL_INFO, "%s Width:  %d\n", me, brxp-tlxp);
> -        DBG (DL_INFO, "%s Length: %d\n", me, bryp-tlyp);
> -    }
>      pc[SET_WINDOW_P_BRIGHTNESS] = 128;
>      pc[SET_WINDOW_P_THRESHOLD] =
>          (u_char) (255.0*(pss->threshold / 100.0));
>      pc[SET_WINDOW_P_CONTRAST] = 128;
> +
>      {
>          SnapScan_Mode mode = pss->mode;
>          pss->bpp_scan = pss->val[OPT_BIT_DEPTH].w;
> @@ -923,7 +881,8 @@ static SANE_Status set_window (SnapScan_Scanner *pss)
>          if (pss->preview)
>          {
>              mode = pss->preview_mode;
> -            pss->bpp_scan = 8;
> +            if (pss->pdev->model != SCANWIT2720S)
> +                pss->bpp_scan = 8;

Should there be an else branch?

>          }
>              
>          DBG (DL_MINOR_INFO, "%s Mode: %d\n", me, mode);
> @@ -982,6 +941,87 @@ static SANE_Status set_window (SnapScan_Scanner *pss)
>              pc[SET_WINDOW_P_GAMMA_NO] = 0x01;   /* downloaded gamma table */
>          }
>      }
> +
> +    pc[SET_WINDOW_P_RED_UNDER_COLOR] = 0xff;    /* defaults */
> +    pc[SET_WINDOW_P_BLUE_UNDER_COLOR] = 0xff;
> +    pc[SET_WINDOW_P_GREEN_UNDER_COLOR] = 0xff;
> +
> +    return SANE_STATUS_GOOD;
> +}
> +
> +static SANE_Status set_window (SnapScan_Scanner *pss)
> +{
> +    static const char *me = "set_window";
> +    SANE_Status status;
> +    unsigned char source;
> +    u_char *pc;
> +    int pos_factor;
> +
> +    DBG (DL_CALL_TRACE, "%s\n", me);
> +    status = prepare_set_window(pss);
> +    CHECK_STATUS (status, me, "prepare_set_window");
> +
> +    pc = pss->cmd;
> +
> +    /* header; we support only one window */
> +    pc += SET_WINDOW_LEN;
> +
> +    /* the sole window descriptor */
> +    pc += SET_WINDOW_HEADER_LEN;
> +
> +    switch (pss->pdev->model)
> +    {
> +        case PRISA5000:
> +        case PRISA5000E:
> +        case PRISA5150:
> +            pos_factor = (pss->res > 600) ?  1200 : 600;
> +            break;
> +        case PERFECTION1270:
> +        case PERFECTION1670:
> +            pos_factor = (pss->res > 800) ?  1600 : 800;
> +            break;
> +        case PERFECTION2480:
> +            pos_factor = (pss->res > 1200) ?  2400 : 1200;
> +            break;
> +        case PERFECTION3490:
> +            pos_factor = (pss->res > 1600) ?  3200 : 1600;
> +            break;
> +        default:
> +            pos_factor = pss->actual_res;
> +            break;
> +    }
> +    /* it's an ugly sound if the scanner drives against the rear
> +       wall, and with changing max values we better be sure */
> +    check_range(&(pss->brx), pss->pdev->x_range);
> +    check_range(&(pss->bry), pss->pdev->y_range);
> +    {
> +        int tlxp =
> +            (int) (pos_factor*IN_PER_MM*SANE_UNFIX(pss->tlx));
> +        int tlyp =
> +            (int) (pos_factor*IN_PER_MM*SANE_UNFIX(pss->tly));
> +        int brxp =
> +            (int) (pos_factor*IN_PER_MM*SANE_UNFIX(pss->brx));
> +        int bryp =
> +            (int) (pos_factor*IN_PER_MM*SANE_UNFIX(pss->bry));
> +
> +        /* Check for brx > tlx and bry > tly */
> +        if (brxp <= tlxp) {
> +            tlxp = MAX (0, brxp - 75);
> +        }
> +        if (bryp <= tlyp) {
> +            tlyp = MAX (0, bryp - 75);
> +        }
> +
> +        u_int_to_u_char4p (tlxp, pc + SET_WINDOW_P_TLX);
> +        u_int_to_u_char4p (tlyp, pc + SET_WINDOW_P_TLY);
> +        u_int_to_u_char4p (MAX (((unsigned) (brxp - tlxp)), 75),
> +                           pc + SET_WINDOW_P_WIDTH);
> +        u_int_to_u_char4p (MAX (((unsigned) (bryp - tlyp)), 75),
> +                           pc + SET_WINDOW_P_LENGTH);
> +        DBG (DL_INFO, "%s Width:  %d\n", me, brxp-tlxp);
> +        DBG (DL_INFO, "%s Length: %d\n", me, bryp-tlyp);
> +    }
> +
>      source = 0x0;
>      if (pss->preview) {
>          source |= 0x80; /* no high quality in preview */
> @@ -1003,9 +1043,6 @@ static SANE_Status set_window (SnapScan_Scanner *pss)
>      }
>      pc[SET_WINDOW_P_OPERATION_MODE] = source;
>      DBG (DL_MINOR_INFO, "%s: operation mode set to 0x%02x\n", me, (int) source);
> -    pc[SET_WINDOW_P_RED_UNDER_COLOR] = 0xff;    /* defaults */
> -    pc[SET_WINDOW_P_BLUE_UNDER_COLOR] = 0xff;
> -    pc[SET_WINDOW_P_GREEN_UNDER_COLOR] = 0xff;
>  
>      do {
>          status = snapscan_cmd (pss->pdev->bus, pss->fd, pss->cmd,
> @@ -1020,6 +1057,91 @@ static SANE_Status set_window (SnapScan_Scanner *pss)
>      return status;
>  }
>  
> +static SANE_Status set_window_autofocus (SnapScan_Scanner *copy)
> +{
> +    static char me[] = "set_window_autofocus";
> +    SANE_Status status;
> +
> +    DBG (DL_CALL_TRACE, "%s(%p)\n", me, (void*)copy);
> +
> +    copy->res = copy->actual_res;
> +    status = prepare_set_window (copy);
> +    CHECK_STATUS (status, me, "prepare_set_window");

• set_window_autofocus
• Could __func__ or something like this be used?

> +
> +    u_int_to_u_char4p (1700, copy->cmd + SET_WINDOW_DESC + SET_WINDOW_P_TLY);
> +    /* fill in width & height */
> +    u_int_to_u_char4p (2550, copy->cmd + SET_WINDOW_DESC + SET_WINDOW_P_WIDTH);
> +    u_int_to_u_char4p (128, copy->cmd + SET_WINDOW_DESC + SET_WINDOW_P_LENGTH);
> +
> +    copy->cmd[SET_WINDOW_DESC + SET_WINDOW_P_BITS_PER_PIX] = 12;
> +    copy->cmd[SET_WINDOW_DESC + SET_WINDOW_P_OPERATION_MODE] = 0x49; /* focusing mode */
> +    return snapscan_cmd (copy->pdev->bus, copy->fd, copy->cmd,
> +                SET_WINDOW_TOTAL_LEN, NULL, NULL);
> +}
> +
> +#define SET_FRAME_LEN  10
> +
> +static SANE_Status set_frame (SnapScan_Scanner *pss, SANE_Byte frame_no)
> +{
> +    static const char *me = "set_frame";
> +    SANE_Status status;
> +
> +    DBG (DL_CALL_TRACE, "%s\n", me);
> +    DBG (DL_VERBOSE, "%s setting frame to %d\n", me, frame_no);
> +    zero_buf (pss->cmd, MAX_SCSI_CMD_LEN);
> +    pss->cmd[0] = OBJECT_POSITION;
> +    pss->cmd[1] = 2;          /* Absolute position used here to select the frame */
> +    pss->cmd[4] = frame_no;
> +
> +    status = snapscan_cmd (pss->pdev->bus, pss->fd, pss->cmd, SET_FRAME_LEN, NULL, NULL);
> +    CHECK_STATUS (status, me, "snapscan_cmd");
> +
> +    return status;
> +}
> +
> +static SANE_Status set_focus (SnapScan_Scanner *pss, SANE_Int focus)
> +{
> +    static const char *me = "set_focus";
> +    SANE_Status status;
> +
> +    DBG (DL_CALL_TRACE, "%s(%d)\n", me, focus);
> +    zero_buf (pss->cmd, MAX_SCSI_CMD_LEN);
> +    pss->cmd[0] = OBJECT_POSITION;
> +    pss->cmd[1] = 4;          /* Rotate object but here it sets the focus point */
> +    pss->cmd[3] = (focus >> 8) & 0xFF;
> +    pss->cmd[4] = focus & 0xFF;
> +    status = snapscan_cmd (pss->pdev->bus, pss->fd, pss->cmd, SET_FRAME_LEN, NULL, NULL);
> +    CHECK_STATUS (status, me, "snapscan_cmd");
> +    return status;
> +}
> +
> +static SANE_Int get_8 (u_char *p)
> +{
> +    SANE_Int b;
> +    b = p[0] | (p[1] << 8);
> +    return b;
> +}
> +
> +

Only one empty line?

> +static double get_val (u_char *p, SANE_Int len, SANE_Int x)
> +{
> +       return get_8 (p + ((x + len) << 1)) / 255.0;
> +}
> +
> +static double func (u_char *p, int len)

Maybe a more descriptive name can be chosen here?

> +{
> +       double v, m, s;
> +       SANE_Int i;
> +
> +       s = 0;
> +       for (i = 0; i < len-1; i++) {
> +               v = get_val (p, len, i);
> +               m = get_val (p, len, i+1);
> +               s += fabs (v - m);
> +       }
> +       return s;
> +}
> +
>  static SANE_Status scan (SnapScan_Scanner *pss)
>  {
>      static const char *me = "scan";
> @@ -1061,6 +1183,60 @@ static SANE_Status scsi_read (SnapScan_Scanner *pss, u_char read_type)
>      return status;
>  }
>  
> +static SANE_Status get_focus (SnapScan_Scanner *pss)
> +{
> +    static const char *me = "get_focus";
> +    SANE_Int f, max_f;

More descriptive names? What does f mean? focus?

> +    double sum, max;
> +    SANE_Status status;
> +    SnapScan_Scanner copy;
> +
> +    copy = *pss;

Why not pass *pss around?

> +
> +    DBG (DL_CALL_TRACE, "%s\n", me);
> +    reserve_unit(&copy);
> +
> +    status = set_window_autofocus (&copy);
> +    CHECK_STATUS (status, me, "set_window_autofocus");
> +
> +    status = inquiry (&copy);
> +    CHECK_STATUS (status, me, "inquiry");
> +
> +    status = scan (&copy);
> +    CHECK_STATUS (status, me, "scan");
> +
> +    status = set_frame (&copy, copy.frame_no);
> +    CHECK_STATUS (status, me, "set_frame2");

No 2 at end.

> +
> +    DBG (DL_VERBOSE, "%s: Expected number of bytes for each read %d\n", me, (int)copy.bytes_per_line);
> +    DBG (DL_VERBOSE, "%s: Expected number of pixels per line %d\n", me, (int)copy.pixels_per_line);

What is done in the following loop?

> +    max_f = -1;
> +    max = -1;
> +    for (f = 0; f <= 0x300; f += 6) {
> +        status = set_focus (&copy, f);
> +        CHECK_STATUS (status, me, "set_focus");
> +
> +        copy.expected_read_bytes = copy.bytes_per_line;
> +        status = scsi_read (&copy, READ_IMAGE);
> +        CHECK_STATUS (status, me, "scsi_read");
> +
> +        sum = func (copy.buf, copy.pixels_per_line);
> +
> +        if (sum > max) {
> +            max = sum;
> +            max_f = f;
> +        }
> +    }
> +    pss->focus = max_f;
> +    DBG (DL_VERBOSE, "%s: Focus point found to be at 0x%x\n", me, max_f);
> +    release_unit (&copy);
> +
> +    wait_scanner_ready (&copy);
> +    CHECK_STATUS (status, me, "wait_scanner_ready");
> +
> +    return status;
> +}
> +
>  /*
>  static SANE_Status request_sense (SnapScan_Scanner *pss)
>  {
> @@ -1100,6 +1276,8 @@ static SANE_Status send_diagnostic (SnapScan_Scanner *pss)
>          ||
>       pss->pdev->model == SNAPSCAN1236
>          ||
> +     pss->pdev->model == SCANWIT2720S
> +        ||
>          pss->pdev->model == ARCUS1200)
>      {
>          return SANE_STATUS_GOOD;
> @@ -1378,6 +1556,8 @@ static SANE_Status calibrate (SnapScan_Scanner *pss)
>      {
>         return send_calibration_5150(pss);
>      }
> +
> +    DBG (DL_CALL_TRACE, "%s\n", me);
>        
>      if (line_length) {
>          int num_lines = pss->phys_buf_sz / line_length;
> @@ -1425,7 +1605,6 @@ static SANE_Status download_firmware(SnapScan_Scanner * pss)
>      SANE_Status status = SANE_STATUS_GOOD;
>      char cModelName[8], cModel[255];
>      unsigned char bModelNo;
> -    int readByte;
>  
>      bModelNo =*(pss->buf + INQUIRY_HWMI);
>      zero_buf((unsigned char *)cModel, 255);
> @@ -1512,7 +1691,7 @@ static SANE_Status download_firmware(SnapScan_Scanner * pss)
>              pCDB = (unsigned char *)malloc(bufLength + cdbLength);
>              pFwBuf = pCDB + cdbLength;
>              zero_buf (pCDB, cdbLength);
> -            readByte = fread(pFwBuf,1,bufLength,fd);
> +            (void)fread(pFwBuf,1,bufLength,fd);

Why?

>  
>              *pCDB = SEND;
>              *(pCDB + 2) = 0x87;
> diff --git a/backend/snapscan-sources.c b/backend/snapscan-sources.c
> index bd15e8e..d91060e 100644
> --- a/backend/snapscan-sources.c
> +++ b/backend/snapscan-sources.c
> @@ -1,9 +1,9 @@
>  /* sane - Scanner Access Now Easy.
>  
> -   Copyright (C) 1997, 1998 Franck Schnefra, Michel Roelofs,
> +   Copyright (C) 1997, 1998, 2002, 2013 Franck Schnefra, Michel Roelofs,
>     Emmanuel Blot, Mikko Tyolajarvi, David Mosberger-Tang, Wolfgang Goeller,
> -   Petter Reinholdtsen, Gary Plewa, Sebastien Sable, Oliver Schwartz
> -   and Kevin Charter
> +   Petter Reinholdtsen, Gary Plewa, Sebastien Sable, Max Ushakov,
> +   Andrew Goodbody, Oliver Schwartz and Kevin Charter
>  
>     This file is part of the SANE package.
>  
> @@ -932,6 +932,13 @@ typedef struct
>      SANE_Int round_read;
>  } RGBRouter;
>  
> +static void put_int16r (int n, u_char *p)
> +{
> +       p[0] = (n & 0x00ff);
> +       p[1] = (n & 0xff00) >> 8;
> +}
> +
> +
>  static SANE_Int RGBRouter_remaining (Source *pself)
>  {
>      RGBRouter *ps = (RGBRouter *) pself;
> @@ -951,7 +958,7 @@ static SANE_Status RGBRouter_get (Source *pself,
>      SANE_Status status = SANE_STATUS_GOOD;
>      SANE_Int remaining = *plen;
>      SANE_Byte *s;
> -    SANE_Int i;
> +    SANE_Int i, t;
>      SANE_Int r, g, b;
>      SANE_Int run_req;
>      SANE_Int org_len = *plen;
> @@ -998,6 +1005,22 @@ static SANE_Status RGBRouter_get (Source *pself,
>                      *s++ = ps->cbuf[g++];
>                      *s++ = ps->cbuf[b++];
>                  }
> +                else if (pself->pss->pdev->model == SCANWIT2720S)
> +                {
> +                    t = (((ps->cbuf[r+1] << 8) | ps->cbuf[r]) & 0xfff) << 4;
> +                    put_int16r (t, s);
> +                    s += 2;
> +                    r += 2;
> +                    t = (((ps->cbuf[g+1] << 8) | ps->cbuf[g]) & 0xfff) << 4;
> +                    put_int16r (t, s);
> +                    s += 2;
> +                    g += 2;
> +                    t = (((ps->cbuf[b+1] << 8) | ps->cbuf[b]) & 0xfff) << 4;
> +                    put_int16r (t, s);
> +                    s += 2;
> +                    b += 2;
> +                    i++;
> +                }
>                  else
>                  {
>                      *s++ = ps->cbuf[r++];
> diff --git a/backend/snapscan.c b/backend/snapscan.c
> index 284ad08..7885dee 100644
> --- a/backend/snapscan.c
> +++ b/backend/snapscan.c
> @@ -1,9 +1,10 @@
>  /* sane - Scanner Access Now Easy.
>  
> -   Copyright (C) 1997-2005 Franck Schnefra, Michel Roelofs, Emmanuel Blot,
> -   Mikko Tyolajarvi, David Mosberger-Tang, Wolfgang Goeller, Simon Munton,
> -   Petter Reinholdtsen, Gary Plewa, Sebastien Sable, Mikael Magnusson,
> -   Oliver Schwartz and Kevin Charter
> +   Copyright (C) 1997-2005, 2013 Franck Schnefra, Michel Roelofs,
> +   Emmanuel Blot, Mikko Tyolajarvi, David Mosberger-Tang, Wolfgang Goeller,
> +   Simon Munton, Petter Reinholdtsen, Gary Plewa, Sebastien Sable,
> +   Mikael Magnusson, Max Ushakov, Andrew Goodbody, Oliver Schwartz
> +   and Kevin Charter
>  
>     This file is part of the SANE package.
>  
> @@ -133,6 +134,10 @@ if ((s) != SANE_STATUS_GOOD) { DBG(DL_MAJOR_ERROR, "%s: %s command failed: %s\n"
>  #define MM_PER_IN 25.4                /* # millimetres per inch */
>  #define IN_PER_MM 0.03937        /* # inches per millimetre  */
>  
> +#define GAMMA_8BIT     0
> +#define GAMMA_16BIT    1
> +#define GAMMA_12_16BIT 2
> +
>  #ifndef SANE_I18N
>  #define SANE_I18N(text) text
>  #endif
> @@ -147,7 +152,7 @@ static SANE_Char password[SANE_MAX_PASSWORD_LEN];
>  /* function prototypes */
>  
>  static void gamma_n (double gamma, int brightness, int contrast,
> -                     u_char *buf, int length, int gamma_16bit);
> +                     u_char *buf, int length, int gamma_mode);
>  static void gamma_to_sane (int length, u_char *in, SANE_Int *out);
>  
>  static size_t max_string_size(SANE_String_Const strings[]);
> @@ -181,11 +186,16 @@ static inline int calibration_line_length(SnapScan_Scanner *pss)
>      case PERFECTION2480:
>      case PERFECTION3490:
>          pos_factor = pss->actual_res / 2;
> +        pixel_length = pos_factor * 8.5;
> +        break;
> +    case SCANWIT2720S:
> +        pixel_length = 2550;

Can this be calculated for the 2720S too?

> +        break;
>      default:
>          pos_factor = pss->actual_res;
> +        pixel_length = pos_factor * 8.5;
>          break;
>      }
> -    pixel_length = pos_factor * 8.5;
>  
>      if(is_colour_mode(actual_mode(pss))) {
>          return 3 * pixel_length;
> @@ -285,7 +295,7 @@ static size_t max_string_size (SANE_String_Const strings[])
>  
>  /* gamma table computation */
>  static void gamma_n (double gamma, int brightness, int contrast,
> -                      u_char *buf, int bpp, int gamma_16bit)
> +                      u_char *buf, int bpp, int gamma_mode)

Ok, just renaming.

>  {
>      int i;
>      double i_gamma = 1.0/gamma;
> @@ -295,27 +305,37 @@ static void gamma_n (double gamma, int brightness, int contrast,
>  
>      for (i = 0;  i < length;  i++)
>      {
> +        int x;
>          double val = (i - mid) * (1.0 + contrast / 100.0)
>              + (1.0 + brightness / 100.0) * mid;
>          val = LIMIT(val, 0, max);
> -        if (gamma_16bit)
> +        switch (gamma_mode)
>          {
> -            int x = LIMIT(65535*pow ((double) val/max, i_gamma) + 0.5, 0, 65535);
> +        case GAMMA_16BIT:
> +            x = LIMIT(65535*pow ((double) val/max, i_gamma) + 0.5, 0, 65535);
>  
>              buf[2*i] = (u_char) x;
>              buf[2*i + 1] = (u_char) (x >> 8);
> -        }
> -        else
> +            break;
> +        case GAMMA_12_16BIT:
> +            buf[2*i] = (u_char) i;
> +            buf[2*i + 1] = (u_char) (i >> 8);
> +            break;
> +        case GAMMA_8BIT:
>              buf[i] =
>                  (u_char) LIMIT(255*pow ((double) val/max, i_gamma) + 0.5, 0, 255);
> +            break;
> +        default:
> +            break;
> +        }
>      }
>  }
>  
> -static void gamma_from_sane (int length, SANE_Int *in, u_char *out, int gamma_16bit)
> +static void gamma_from_sane (int length, SANE_Int *in, u_char *out, int gamma_mode)
>  {
>      int i;
>      for (i = 0; i < length; i++)
> -        if (gamma_16bit)
> +        if (gamma_mode != GAMMA_8BIT)

What bug does this fix?

>          {
>              out[2*i] = (u_char) LIMIT(in[i], 0, 65535);
>              out[2*i + 1] = (u_char) (LIMIT(in[i], 0, 65535) >> 8);
> @@ -468,7 +488,14 @@ static SANE_Status snapscani_init_device_structure(
>          (*pd)->dev.vendor = strdup (vendor);
>      }
>      (*pd)->dev.model = strdup (model);
> -    (*pd)->dev.type = strdup (SNAPSCAN_TYPE);
> +    if (model_num == SCANWIT2720S)
> +    {
> +        (*pd)->dev.type = strdup (SNAPSCAN_FS_TYPE);
> +    }
> +    else
> +    {
> +        (*pd)->dev.type = strdup (SNAPSCAN_TYPE);
> +    }
>      (*pd)->bus = bus_type;
>      (*pd)->model = model_num;
>  
> @@ -1111,6 +1138,8 @@ SANE_Status sane_get_parameters (SANE_Handle h,
>      p->format = (is_colour_mode(mode)) ? SANE_FRAME_RGB : SANE_FRAME_GRAY;
>      if (mode == MD_LINEART)
>          p->depth = 1;
> +    else if (pss->pdev->model == SCANWIT2720S)
> +        p->depth = 16;
>      else if (pss->preview)
>          p->depth = 8;
>      else 
> @@ -1333,7 +1362,7 @@ static SANE_Status download_gamma_tables (SnapScan_Scanner *pss)
>      int dtcq_gamma_red;
>      int dtcq_gamma_green;
>      int dtcq_gamma_blue;
> -    int gamma_16bit = 0;
> +    int gamma_mode = GAMMA_8BIT;
>  
>      DBG (DL_CALL_TRACE, "%s\n", me);
>      switch (mode)
> @@ -1367,10 +1396,22 @@ static SANE_Status download_gamma_tables (SnapScan_Scanner *pss)
>          dtcq_gamma_blue = DTCQ_GAMMA_BLUE10;
>          break;
>      case 12:
> -        dtcq_gamma_gray = DTCQ_GAMMA_GRAY12;
> -        dtcq_gamma_red = DTCQ_GAMMA_RED12;
> -        dtcq_gamma_green = DTCQ_GAMMA_GREEN12;
> -        dtcq_gamma_blue = DTCQ_GAMMA_BLUE12;
> +        if (pss->pdev->model == SCANWIT2720S)
> +        {
> +            DBG (DL_DATA_TRACE, "%s: Sending 16bit gamma table for %d bpp\n", me, pss->bpp);

Also add that DBG output for non-2720S devices?

> +            dtcq_gamma_gray = DTCQ_GAMMA_GRAY12_16BIT;
> +            dtcq_gamma_red = DTCQ_GAMMA_RED12_16BIT;
> +            dtcq_gamma_green = DTCQ_GAMMA_GREEN12_16BIT;
> +            dtcq_gamma_blue = DTCQ_GAMMA_BLUE12_16BIT;
> +            gamma_mode = GAMMA_12_16BIT;
> +        }
> +        else
> +        {
> +            dtcq_gamma_gray = DTCQ_GAMMA_GRAY12;
> +            dtcq_gamma_red = DTCQ_GAMMA_RED12;
> +            dtcq_gamma_green = DTCQ_GAMMA_GREEN12;
> +            dtcq_gamma_blue = DTCQ_GAMMA_BLUE12;
> +        }
>          break;
>      case 14:
>          if (pss->bpp_scan == 16)
> @@ -1379,7 +1420,7 @@ static SANE_Status download_gamma_tables (SnapScan_Scanner *pss)
>              dtcq_gamma_red = DTCQ_GAMMA_RED14_16BIT;
>              dtcq_gamma_green = DTCQ_GAMMA_GREEN14_16BIT;
>              dtcq_gamma_blue = DTCQ_GAMMA_BLUE14_16BIT;
> -            gamma_16bit = 1;
> +            gamma_mode = GAMMA_16BIT;
>          }
>          else
>          {
> @@ -1405,34 +1446,34 @@ static SANE_Status download_gamma_tables (SnapScan_Scanner *pss)
>              {
>                  /* Use greyscale gamma for all rgb channels */
>                  gamma_from_sane (pss->gamma_length, pss->gamma_table_gs,
> -                                 pss->buf + SEND_LENGTH, gamma_16bit);
> +                                 pss->buf + SEND_LENGTH, gamma_mode);
>                  status = send_gamma_table(pss, DTC_GAMMA, dtcq_gamma_red);
>                  CHECK_STATUS (status, me, "send");
>  
>                  gamma_from_sane (pss->gamma_length, pss->gamma_table_gs,
> -                                 pss->buf + SEND_LENGTH, gamma_16bit);
> +                                 pss->buf + SEND_LENGTH, gamma_mode);
>                  status = send_gamma_table(pss, DTC_GAMMA, dtcq_gamma_green);
>                  CHECK_STATUS (status, me, "send");
>  
>                  gamma_from_sane (pss->gamma_length, pss->gamma_table_gs,
> -                                 pss->buf + SEND_LENGTH, gamma_16bit);
> +                                 pss->buf + SEND_LENGTH, gamma_mode);
>                  status = send_gamma_table(pss, DTC_GAMMA, dtcq_gamma_blue);
>                  CHECK_STATUS (status, me, "send");
>              }
>              else
>              {
>                  gamma_from_sane (pss->gamma_length, pss->gamma_table_r,
> -                                 pss->buf + SEND_LENGTH, gamma_16bit);
> +                                 pss->buf + SEND_LENGTH, gamma_mode);
>                  status = send_gamma_table(pss, DTC_GAMMA, dtcq_gamma_red);
>                  CHECK_STATUS (status, me, "send");
>  
>                  gamma_from_sane (pss->gamma_length, pss->gamma_table_g,
> -                                 pss->buf + SEND_LENGTH, gamma_16bit);
> +                                 pss->buf + SEND_LENGTH, gamma_mode);
>                  status = send_gamma_table(pss, DTC_GAMMA, dtcq_gamma_green);
>                  CHECK_STATUS (status, me, "send");
>  
>                  gamma_from_sane (pss->gamma_length, pss->gamma_table_b,
> -                                 pss->buf + SEND_LENGTH, gamma_16bit);
> +                                 pss->buf + SEND_LENGTH, gamma_mode);
>                  status = send_gamma_table(pss, DTC_GAMMA, dtcq_gamma_blue);
>                  CHECK_STATUS (status, me, "send");
>              }
> @@ -1443,34 +1484,34 @@ static SANE_Status download_gamma_tables (SnapScan_Scanner *pss)
>              {
>                  /* Use greyscale gamma for all rgb channels */
>                  gamma_n (gamma_gs, pss->bright, pss->contrast,
> -                         pss->buf + SEND_LENGTH, pss->bpp, gamma_16bit);
> +                         pss->buf + SEND_LENGTH, pss->bpp, gamma_mode);
>                  status = send_gamma_table(pss, DTC_GAMMA, dtcq_gamma_red);
>                  CHECK_STATUS (status, me, "send");
>  
>                  gamma_n (gamma_gs, pss->bright, pss->contrast,
> -                         pss->buf + SEND_LENGTH, pss->bpp, gamma_16bit);
> +                         pss->buf + SEND_LENGTH, pss->bpp, gamma_mode);
>                  status = send_gamma_table(pss, DTC_GAMMA, dtcq_gamma_green);
>                  CHECK_STATUS (status, me, "send");
>  
>                  gamma_n (gamma_gs, pss->bright, pss->contrast,
> -                         pss->buf + SEND_LENGTH, pss->bpp, gamma_16bit);
> +                         pss->buf + SEND_LENGTH, pss->bpp, gamma_mode);
>                  status = send_gamma_table(pss, DTC_GAMMA, dtcq_gamma_blue);
>                  CHECK_STATUS (status, me, "send");
>              }
>              else
>              {
>                  gamma_n (gamma_r, pss->bright, pss->contrast,
> -                         pss->buf + SEND_LENGTH, pss->bpp, gamma_16bit);
> +                         pss->buf + SEND_LENGTH, pss->bpp, gamma_mode);
>                  status = send_gamma_table(pss, DTC_GAMMA, dtcq_gamma_red);
>                  CHECK_STATUS (status, me, "send");
>  
>                  gamma_n (gamma_g, pss->bright, pss->contrast,
> -                         pss->buf + SEND_LENGTH, pss->bpp, gamma_16bit);
> +                         pss->buf + SEND_LENGTH, pss->bpp, gamma_mode);
>                  status = send_gamma_table(pss, DTC_GAMMA, dtcq_gamma_green);
>                  CHECK_STATUS (status, me, "send");
>  
>                  gamma_n (gamma_b, pss->bright, pss->contrast,
> -                         pss->buf + SEND_LENGTH, pss->bpp, gamma_16bit);
> +                         pss->buf + SEND_LENGTH, pss->bpp, gamma_mode);
>                  status = send_gamma_table(pss, DTC_GAMMA, dtcq_gamma_blue);
>                  CHECK_STATUS (status, me, "send");
>              }
> @@ -1481,14 +1522,14 @@ static SANE_Status download_gamma_tables (SnapScan_Scanner *pss)
>          if(pss->val[OPT_CUSTOM_GAMMA].b)
>          {
>              gamma_from_sane (pss->gamma_length, pss->gamma_table_gs,
> -                             pss->buf + SEND_LENGTH, gamma_16bit);
> +                             pss->buf + SEND_LENGTH, gamma_mode);
>              status = send_gamma_table(pss, DTC_GAMMA, dtcq_gamma_gray);
>              CHECK_STATUS (status, me, "send");
>          }
>          else
>          {
>              gamma_n (gamma_gs, pss->bright, pss->contrast,
> -                     pss->buf + SEND_LENGTH, pss->bpp, gamma_16bit);
> +                     pss->buf + SEND_LENGTH, pss->bpp, gamma_mode);
>              status = send_gamma_table(pss, DTC_GAMMA, dtcq_gamma_gray);
>              CHECK_STATUS (status, me, "send");
>          }
> @@ -1631,8 +1672,26 @@ SANE_Status sane_start (SANE_Handle h)
>  
>      pss->state = ST_SCAN_INIT;
>  
> +    if ((pss->pdev->model == SCANWIT2720S) && (pss->focus_mode == MD_AUTO))
> +    {
> +        status = get_focus(pss);
> +        CHECK_STATUS (status, me, "get_focus");
> +    }
> +
>      reserve_unit(pss);
>  
> +    if (pss->pdev->model == SCANWIT2720S)
> +    {
> +        status = set_frame(pss, 0);
> +        CHECK_STATUS (status, me, "set_frame1");

Ah that is why the 2 from earlier. Maybe make that more clear though.
First call or something like this.

> +    }
> +
> +    if (pss->pdev->model == SCANWIT2720S)
> +    {
> +        status = set_focus(pss, pss->focus);
> +        CHECK_STATUS (status, me, "set_focus");
> +    }
> +

Unify to just one if condition?

>      /* set up the window and fetch the resulting scanner parameters */
>      status = set_window(pss);
>      CHECK_STATUS (status, me, "set_window");
> @@ -1698,6 +1757,12 @@ SANE_Status sane_start (SANE_Handle h)
>          return status;
>      }
>  
> +    if (pss->pdev->model == SCANWIT2720S)
> +    {
> +        status = set_frame(pss, pss->frame_no);
> +        CHECK_STATUS (status, me, "set_frame");
> +    }
> +
>      if (pss->source == SRC_ADF)
>      {
>          /* Wait for scanner ready again (e.g. until paper is loaded from an ADF) */
> diff --git a/backend/snapscan.h b/backend/snapscan.h
> index 7913aa2..0ab8020 100644
> --- a/backend/snapscan.h
> +++ b/backend/snapscan.h
> @@ -1,9 +1,9 @@
>  /* sane - Scanner Access Now Easy.
>  
> -   Copyright (C) 1997, 1998, 1999, 2001  Franck Schnefra, Michel Roelofs,
> -   Emmanuel Blot, Mikko Tyolajarvi, David Mosberger-Tang, Wolfgang Goeller,
> -   Petter Reinholdtsen, Gary Plewa, Sebastien Sable, Mikael Magnusson,
> -   Oliver Schwartz and Kevin Charter
> +   Copyright (C) 1997, 1998, 1999, 2001, 2002, 2013  Franck Schnefra,
> +   Michel Roelofs, Emmanuel Blot, Mikko Tyolajarvi, David Mosberger-Tang,
> +   Wolfgang Goeller, Petter Reinholdtsen, Gary Plewa, Sebastien Sable,
> +   Mikael Magnusson, Andrew Goodbody, Oliver Schwartz and Kevin Charter
>  
>     This file is part of the SANE package.
>   
> @@ -60,6 +60,7 @@
>  
>  #define DEFAULT_DEVICE "/dev/scanner" /* Check this if config is missing */
>  #define SNAPSCAN_TYPE      "flatbed scanner"
> +#define SNAPSCAN_FS_TYPE   "film scanner"
>  #define TMP_FILE_PREFIX "/var/tmp/snapscan"
>  #define SNAPSCAN_CONFIG_FILE "snapscan.conf"
>  #define FIRMWARE_KW "firmware"
> @@ -107,7 +108,8 @@ typedef enum
>      PERFECTION2480,     /* Epson Perfection 2480 - 2400 DPI */
>      PERFECTION3490,     /* Epson Perfection 3490 - 3200 DPI */
>      STYLUS_CX1500,      /* Epson Stylus CX 1500 - 600 DPI */
> -    ARCUS1200          /* Agfa Arcus 1200 - 1200 DPI (rebadged Acer?) */
> +    ARCUS1200,          /* Agfa Arcus 1200 - 1200 DPI (rebadged Acer?) */
> +    SCANWIT2720S        /* Benq Scanwit 2720S film scanner 2700 DPI */
>  } SnapScan_Model;
>  
>  struct SnapScan_Driver_desc {
> @@ -146,7 +148,8 @@ static struct SnapScan_Driver_desc drivers[] =
>      {PERFECTION1670, "Perfection 1670"},
>      {PERFECTION2480, "Perfection 2480"},
>      {PERFECTION3490, "Perfection 3490"},
> -    {STYLUS_CX1500,  "Stylus CX 1500"}
> +    {STYLUS_CX1500,  "Stylus CX 1500"},
> +    {SCANWIT2720S,   "Benq Scanwit 2720S"}

BenQ ScanWit 2720S

>  };
>  
>  #define known_drivers ((int) (sizeof(drivers)/sizeof(drivers[0])))
> @@ -200,7 +203,8 @@ static struct SnapScan_Model_desc scanners[] =
>      {"EPSON Scanner1",      PERFECTION2480}, /* dummy entry to detect scanner */
>      {"EPSON Scanner2",      PERFECTION3490}, /* dummy entry to detect scanner */
>      {"EPSON MFP00",            STYLUS_CX1500},
> -    {"ARCUS 1200",          ARCUS1200}
> +    {"ARCUS 1200",          ARCUS1200},
> +    {"FilmScanner____1",    SCANWIT2720S}
>  };
>  
>  #define known_scanners ((int) (sizeof(scanners)/sizeof(scanners[0])))
> @@ -271,6 +275,9 @@ typedef enum
>      OPT_PREVIEW_MODE,      /* preview mode */
>      OPT_HIGHQUALITY,       /* scan quality (fast / high) */
>      OPT_SOURCE,            /* scan source (flatbed / TPO) */
> +    OPT_FRAME_NO,          /* frame number for film scanner */
> +    OPT_FOCUS_MODE,        /* manual or auto focus for film scanner */
> +    OPT_FOCUS_POINT,       /* focus point for film scanner */
>      OPT_GEOMETRY_GROUP,    /* geometry group */
>      OPT_TLX,               /* top left x */
>      OPT_TLY,               /* top left y */
> @@ -338,6 +345,9 @@ typedef struct snapscan_device
>  }
>  SnapScan_Device;
>  
> +#define MD_AUTO                0
> +#define MD_MANUAL      1
> +
>  #define MAX_SCSI_CMD_LEN 256    /* not that large */
>  #define DEFAULT_SCANNER_BUF_SZ 1024*63
>  
> @@ -418,6 +428,10 @@ struct snapscan_scanner
>      SANE_Bool firmware_loaded;   /* true if firmware was downloaded */
>      SANE_Word usb_vendor;        /* USB vendor id */
>      SANE_Word usb_product;       /* USB product id */
> +    SANE_Byte frame_no;          /* frame number for film scanner */
> +    SANE_Int focus_mode;         /* focus mode value */
> +    SANE_String focus_mode_s;    /* focus mode string */
> +    SANE_Word focus;             /* focus point */
>  };
>  
>  #endif
> -- 
> 1.7.10.4

The patch is pretty big. Review would be easier if you factored out the
bug fixes and maybe add the new options like the frame option in a
separate patch and then lastly the ScanWit 2720S.

Oliver might disagree though.


Thanks,

Paul


[1] http://git-scm.com/book/en/Customizing-Git-Git-Configuration
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.alioth.debian.org/pipermail/sane-devel/attachments/20130225/c0927ef2/attachment-0001.pgp>


More information about the sane-devel mailing list