Merge branch 'xerox_mfp-729-read' into 'master'

xerox_mfp: Avoid a buffering stall, when reading image data

Closes #729

See merge request sane-project/backends!827

backend/xerox_mfp-tcp.c

@@ -1,6 +1,5 @@
 /*
- *	SANE backend for
+ *  SANE backend for Samsung SCX-4500W
- *		Samsung SCX-4500W
  *
  *  Network Scanners Support
  *  Copyright 2010 Alexander Kuznetsov <acca(at)cpan.org>
@@ -47,7 +46,8 @@
 #define RECV_TIMEOUT    1   /* seconds */
 extern int sanei_debug_xerox_mfp;
 
-int	tcp_dev_request(struct device *dev,
+int
+tcp_dev_request(struct device *dev,
                 SANE_Byte *cmd, size_t cmdlen,
                 SANE_Byte *resp, size_t *resplen)
 {
@@ -73,7 +73,9 @@ int	tcp_dev_request(struct device *dev,
         while (bytes_recv < *resplen && rc > 0) {
             rc = recv(dev->dn, resp+bytes_recv, *resplen-bytes_recv, 0);
 
-            if (rc > 0)	bytes_recv += rc;
+            if (rc > 0) {
+                bytes_recv += rc;
+            }
             else {
                 DBG(1, "%s: error %s, bytes requested: %i, bytes read: %i\n",
                     __func__, strerror(errno), (int)*resplen, (int)bytes_recv);
@@ -93,7 +95,8 @@ int	tcp_dev_request(struct device *dev,
     return SANE_STATUS_GOOD;
 }
 
-SANE_Status	tcp_dev_open(struct device *dev)
+SANE_Status
+tcp_dev_open(struct device *dev)
 {
     SANE_Status status;
     char *strhost;
@@ -107,10 +110,13 @@ SANE_Status	tcp_dev_open(struct device *dev)
     devname = dev->sane.name;
     DBG(3, "%s: open %s\n", __func__, devname);
 
-    if (strncmp(devname, "tcp", 3) != 0)	return SANE_STATUS_INVAL;
+    if (strncmp(devname, "tcp", 3) != 0)
+        return SANE_STATUS_INVAL;
+
     devname += 3;
     devname = sanei_config_skip_whitespace(devname);
-    if (!*devname)	return SANE_STATUS_INVAL;
+    if (!*devname)
+        return SANE_STATUS_INVAL;
 
     devname = sanei_config_get_string(devname, &strhost);
     devname = sanei_config_skip_whitespace(devname);
@@ -147,7 +153,8 @@ SANE_Status	tcp_dev_open(struct device *dev)
 void
 tcp_dev_close(struct device *dev)
 {
-    if (!dev)	return;
+    if (!dev)
+        return;
 
     DBG(3, "%s: closing dev %p\n", __func__, (void *)dev);
 
@@ -155,7 +162,8 @@ tcp_dev_close(struct device *dev)
     if (dev->scanning) {
         dev->cancel = 1;
         /* flush READ_IMAGE data */
-        if (dev->reading)	sane_read(dev, NULL, 1, NULL);
+        if (dev->reading)
+            sane_read(dev, NULL, 1, NULL);
         /* send cancel if not sent before */
         if (dev->state != SANE_STATUS_CANCELLED)
             ret_cancel(dev, 0);

backend/xerox_mfp.c

@@ -1414,15 +1414,69 @@ sane_read(SANE_Handle h, SANE_Byte *buf, SANE_Int maxlen, SANE_Int *lenp)
         dev->blocks++;
     }
 
+    /* Receive the on-device raw image data into a cyclic aka circular buffer.
+     *
+     * The circular buffer implementation is as follows:
+     *  - buffer::{data[DATASIZE], dataoff, datalen};
+     *  - data storage is preallocated to a fixed capacity: DATASIZE;
+     *  - dataoff is used to track the data start (for read);
+     *  - datalen is the stored data's length;
+     *  - DATATAIL() macro is to track the data end position (for write);
+     *  - Both start and end positions may be wrapped around at the DATASIZE,
+     *    (& DATAMASK);
+     *  - DATAROOM() macro is used to determine the currently available
+     *    length of the __contiguous__ storage after the data end (from DATATAIL)
+     *  - write is allowed only into the remaining capacity, overwriting of the
+     *    the buffered but not consumed data is not allowed;
+     *  - once data has been consumed, the dataoff and datalen must be adjusted
+     *    accordingly; dataoff may wrap around (& DATAMASK);
+     *  - when DATAROOM() returns 0, the data must be consumed before any more
+     *    data could be buffered.
+     *
+     *  NOTE: the DATAROOM() does not wrap around and yields the length
+     *  from DATATAIL() up to the DATASIZE position. So the returned length may
+     *  be shorter than the actual remaining capacity, that is there may be
+     *  more unoccupied space from the beginning of the buffer till dataoff.
+     *
+     *  This is because the buffer's storage pointer at the write position is passed
+     *  directly to the IO receive call, so it's expected to be contiguous and can't
+     *  wrap around. Thus in such cases this may result in extra IO receive call in
+     *  order to first fill the residual length till DATASIZE, and then continue
+     *  filling from the start of the buffer.
+     *
+     *  It's all manual and requires some discipline but it does the job.
+     */
     do {
         size_t datalen;
         int clrlen; /* cleared lines len */
         int olen; /* output len */
 
-        /* read as much data into the buffer */
+        /* set up to read as much data as room left in the buffer */
-        datalen = MIN(dev->blocklen, DATAROOM(dev) & USB_BLOCK_MASK);
+        datalen = DATAROOM(dev);
         while (datalen && dev->blocklen) {
             SANE_Byte *rbuf = dev->data + DATATAIL(dev);
+            /* USB 2.0 spec requires 512 byte packet max size for bulk transfers
+             * with high-speed endpoints. Smaller max sizes are allowed
+             * for slower endpoints (64, 32, 16, 8 for full-speed).
+             * Some Samsung MFP scanners require requests for at least the
+             * defined max size, when there are sufficient data available;
+             * otherwise successfully return 0 bytes.
+             * For example,
+             * - requesting 256 bytes with more than 256 bytes data available
+             *   will return 0 bytes;
+             * - requesting at least 512 will properly fulfill the request;
+             * - requesting 8 bytes with 16 bytes remaining will return 0 bytes;
+             *   yet requesting 16 bytes (or more) will correctly return the
+             *   remaining 16 bytes.
+             */
+            int usb_datalen = datalen & USB_BLOCK_MASK;
+
+            if (usb_datalen)
+                datalen = usb_datalen;
+            else if (strcmp(dev->io->ttype, "usb") == 0)
+                break;
+
+            datalen = MIN((size_t)dev->blocklen, datalen);
 
             DBG(9, "<> request len: %zu, [%d, %d; %d]\n",
                 datalen, dev->dataoff, DATATAIL(dev), dev->datalen);
@@ -1431,7 +1485,7 @@ sane_read(SANE_Handle h, SANE_Byte *buf, SANE_Int maxlen, SANE_Int *lenp)
                 SANE_STATUS_GOOD)
                 return status;
 
-            dev->datalen += datalen;
+            dev->datalen += datalen; /* of actually received data */
             dev->blocklen -= datalen;
 
             DBG(9, "<> got %zu, [%d, %d; %d]\n",
@@ -1440,7 +1494,7 @@ sane_read(SANE_Handle h, SANE_Byte *buf, SANE_Int maxlen, SANE_Int *lenp)
             if (dev->blocklen < 0)
                 return ret_cancel(dev, SANE_STATUS_IO_ERROR);
 
-            datalen = MIN(dev->blocklen, DATAROOM(dev) & USB_BLOCK_MASK);
+            datalen = DATAROOM(dev);
         }
 
         if (buf && lenp) { /* read mode */