Merge branch 'genesys-simplify-buffers' into 'master'

genesys: Simplify read buffer management

See merge request sane-project/backends!466

backend/genesys/device.cpp

@@ -102,7 +102,6 @@ Genesys_Device::~Genesys_Device()
 
 void Genesys_Device::clear()
 {
-    read_buffer.clear();
     binarize_buffer.clear();
     local_buffer.clear();
 
@@ -114,9 +113,9 @@ void Genesys_Device::clear()
     dark_average_data.clear();
 }
 
-ImagePipelineNodeBytesSource& Genesys_Device::get_pipeline_source()
+ImagePipelineNodeBufferedCallableSource& Genesys_Device::get_pipeline_source()
 {
-    return static_cast<ImagePipelineNodeBytesSource&>(pipeline.front());
+    return static_cast<ImagePipelineNodeBufferedCallableSource&>(pipeline.front());
 }
 
 bool Genesys_Device::is_head_pos_known(ScanHeadId scan_head) const
@@ -258,7 +257,6 @@ std::ostream& operator<<(std::ostream& out, const Genesys_Device& dev)
         << "    read_active: " << dev.read_active << '\n'
         << "    parking: " << dev.parking << '\n'
         << "    document: " << dev.document << '\n'
-        << "    read_buffer.size(): " << dev.read_buffer.size() << '\n'
         << "    binarize_buffer.size(): " << dev.binarize_buffer.size() << '\n'
         << "    local_buffer.size(): " << dev.local_buffer.size() << '\n'
         << "    total_bytes_read: " << dev.total_bytes_read << '\n'

backend/genesys/device.h

@@ -318,8 +318,6 @@ struct Genesys_Device
     // for sheetfed scanner's, is TRUE when there is a document in the scanner
     bool document = false;
 
-    Genesys_Buffer read_buffer;
-
     // buffer for digital lineart from gray data
     Genesys_Buffer binarize_buffer;
     // local buffer for gray data during dynamix lineart
@@ -350,7 +348,7 @@ struct Genesys_Device
     // an buffer that allows reading from `pipeline` in chunks of any size
     ImageBuffer pipeline_buffer;
 
-    ImagePipelineNodeBytesSource& get_pipeline_source();
+    ImagePipelineNodeBufferedCallableSource& get_pipeline_source();
 
     std::unique_ptr<ScannerInterface> interface;
 

backend/genesys/fwd.h

@@ -74,7 +74,6 @@ class Image;
 
 // image_buffer.h
 class ImageBuffer;
-class ImageBufferGenesysUsb;
 
 // image_pipeline.h
 class ImagePipelineNode;

backend/genesys/genesys.cpp

@@ -4113,7 +4113,6 @@ static void genesys_read_ordered_data(Genesys_Device* dev, SANE_Byte* destinatio
 {
     DBG_HELPER(dbg);
     size_t bytes = 0;
-  uint8_t *work_buffer_src;
 
     if (!dev->read_active) {
       *len = 0;
@@ -4148,28 +4147,12 @@ static void genesys_read_ordered_data(Genesys_Device* dev, SANE_Byte* destinatio
             dev->cmd_set->detect_document_end(dev);
         }
 
-        std::size_t size = dev->read_buffer.size() - dev->read_buffer.avail();
-
-        dev->pipeline_buffer.get_data(size, dev->read_buffer.get_write_pos(size));
-        dev->read_buffer.produce(size);
-
-        bytes = std::min(dev->read_buffer.avail(), *len);
-
-        work_buffer_src = dev->read_buffer.get_read_pos();
-
-        std::memcpy(destination, work_buffer_src, bytes);
-        *len = bytes;
-
-        /* avoid signaling some extra data because we have treated a full block
-        * on the last block */
         if (dev->total_bytes_read + *len > dev->total_bytes_to_read) {
             *len = dev->total_bytes_to_read - dev->total_bytes_read;
         }
 
-        /* count bytes sent to frontend */
+        dev->pipeline_buffer.get_data(*len, destination);
         dev->total_bytes_read += *len;
-
-        dev->read_buffer.consume(bytes);
     }
 
   /* end scan if all needed data have been read */

backend/genesys/gl124.cpp

@@ -776,9 +776,6 @@ void CommandSetGl124::init_regs_for_scan_session(Genesys_Device* dev, const Gene
 
   /*** prepares data reordering ***/
 
-    dev->read_buffer.clear();
-    dev->read_buffer.alloc(session.buffer_size_read);
-
     dev->read_active = true;
 
     dev->session = session;

backend/genesys/gl646.cpp

@@ -840,9 +840,6 @@ void CommandSetGl646::init_regs_for_scan_session(Genesys_Device* dev, const Gene
     // setup analog frontend
     gl646_set_fe(dev, sensor, AFE_SET, session.output_resolution);
 
-    dev->read_buffer.clear();
-    dev->read_buffer.alloc(session.buffer_size_read);
-
     build_image_pipeline(dev, session);
 
     dev->read_active = true;

backend/genesys/gl841.cpp

@@ -1056,9 +1056,6 @@ dummy \ scanned lines
                                    session.params.flags);
   }
 
-    dev->read_buffer.clear();
-    dev->read_buffer.alloc(session.buffer_size_read);
-
     build_image_pipeline(dev, session);
 
     dev->read_active = true;

backend/genesys/gl842.cpp

@@ -521,9 +521,6 @@ void CommandSetGl842::init_regs_for_scan_session(Genesys_Device* dev, const Gene
                                session.optical_line_count, dummy, session.params.starty,
                                session.params.flags);
 
-    dev->read_buffer.clear();
-    dev->read_buffer.alloc(session.buffer_size_read);
-
     build_image_pipeline(dev, session);
 
     dev->read_active = true;

backend/genesys/gl843.cpp

@@ -1054,9 +1054,6 @@ void CommandSetGl843::init_regs_for_scan_session(Genesys_Device* dev, const Gene
                                session.optical_line_count, dummy, session.params.starty,
                                session.params.flags);
 
-    dev->read_buffer.clear();
-    dev->read_buffer.alloc(session.buffer_size_read);
-
     build_image_pipeline(dev, session);
 
     dev->read_active = true;

backend/genesys/gl846.cpp

@@ -693,9 +693,6 @@ void CommandSetGl846::init_regs_for_scan_session(Genesys_Device* dev, const Gene
 
   /*** prepares data reordering ***/
 
-    dev->read_buffer.clear();
-    dev->read_buffer.alloc(session.buffer_size_read);
-
     dev->read_active = true;
 
     dev->session = session;

backend/genesys/gl847.cpp

@@ -569,9 +569,6 @@ void CommandSetGl847::init_regs_for_scan_session(Genesys_Device* dev, const Gene
                                session.optical_line_count, dummy, session.params.starty,
                                session.params.flags);
 
-    dev->read_buffer.clear();
-    dev->read_buffer.alloc(session.buffer_size_read);
-
     dev->read_active = true;
 
     dev->session = session;

backend/genesys/image_buffer.cpp

@@ -45,13 +45,13 @@
 
 #include "image_buffer.h"
 #include "image.h"
+#include "utilities.h"
 
 namespace genesys {
 
 ImageBuffer::ImageBuffer(std::size_t size, ProducerCallback producer) :
     producer_{producer},
-    size_{size},
+    size_{size}
-    buffer_offset_{size}
 {
     buffer_.resize(size_);
 }
@@ -81,85 +81,30 @@ bool ImageBuffer::get_data(std::size_t size, std::uint8_t* out_data)
     bool got_data = true;
     do {
         buffer_offset_ = 0;
-        got_data &= producer_(size_, buffer_.data());
+
+        std::size_t size_to_read = size_;
+        if (remaining_size_ != BUFFER_SIZE_UNSET) {
+            size_to_read = std::min<std::uint64_t>(size_to_read, remaining_size_);
+            remaining_size_ -= size_to_read;
+        }
+
+        std::size_t aligned_size_to_read = size_to_read;
+        if (remaining_size_ == 0 && last_read_multiple_ != BUFFER_SIZE_UNSET) {
+            aligned_size_to_read = align_multiple_ceil(size_to_read, last_read_multiple_);
+        }
+
+        got_data &= producer_(aligned_size_to_read, buffer_.data());
+        curr_size_ = size_to_read;
 
         copy_buffer();
-    } while(out_data < out_data_end && got_data);
+
+        if (remaining_size_ == 0 && out_data < out_data_end) {
+            got_data = false;
+        }
+
+    } while (out_data < out_data_end && got_data);
 
     return got_data;
 }
 
-ImageBufferGenesysUsb::ImageBufferGenesysUsb(std::size_t total_size,
-                                             std::size_t buffer_size,
-                                             ProducerCallback producer) :
-    remaining_size_{total_size},
-    buffer_size_{buffer_size},
-    producer_{producer}
-{}
-
-bool ImageBufferGenesysUsb::get_data(std::size_t size, std::uint8_t* out_data)
-{
-    const std::uint8_t* out_data_end = out_data + size;
-
-    auto copy_buffer = [&]()
-    {
-        std::size_t bytes_copy = std::min<std::size_t>(out_data_end - out_data, available());
-        std::memcpy(out_data, buffer_.data() + buffer_offset_, bytes_copy);
-        out_data += bytes_copy;
-        buffer_offset_ += bytes_copy;
-    };
-
-    // first, read remaining data from buffer
-    if (available() > 0) {
-        copy_buffer();
-    }
-
-    if (out_data == out_data_end) {
-        return true;
-    }
-
-    // now the buffer is empty and there's more data to be read
-    do {
-        if (remaining_size_ == 0)
-            return false;
-
-        auto bytes_to_read = get_read_size();
-        buffer_offset_ = 0;
-        buffer_end_ = bytes_to_read;
-        buffer_.resize(bytes_to_read);
-
-        producer_(bytes_to_read, buffer_.data());
-
-        if (remaining_size_ < bytes_to_read) {
-            remaining_size_ = 0;
-        } else {
-            remaining_size_ -= bytes_to_read;
-        }
-
-        copy_buffer();
-    } while(out_data < out_data_end);
-    return true;
-}
-
-std::size_t ImageBufferGenesysUsb::get_read_size()
-{
-    std::size_t size = buffer_size_;
-
-    // never read an odd number. exception: last read
-    // the chip internal counter does not count half words.
-    size &= ~1;
-
-    // Some setups need the reads to be multiples of 256 bytes
-    size &= ~0xff;
-
-    if (remaining_size_ < size) {
-        size = remaining_size_;
-        /*round up to a multiple of 256 bytes */
-        size += (size & 0xff) ? 0x100 : 0x00;
-        size &= ~0xff;
-    }
-
-    return size;
-}
-
 } // namespace genesys

backend/genesys/image_buffer.h

@@ -56,57 +56,35 @@ class ImageBuffer
 {
 public:
     using ProducerCallback = std::function<bool(std::size_t size, std::uint8_t* out_data)>;
+    static constexpr std::uint64_t BUFFER_SIZE_UNSET = std::numeric_limits<std::uint64_t>::max();
 
     ImageBuffer() {}
     ImageBuffer(std::size_t size, ProducerCallback producer);
 
-    std::size_t size() const { return size_; }
+    std::size_t available() const { return curr_size_ - buffer_offset_; }
-    std::size_t available() const { return size_ - buffer_offset_; }
+
+    // allows adjusting the amount of data left so that we don't do a full size read from the
+    // producer on the last iteration. Set to BUFFER_SIZE_UNSET to ignore buffer size.
+    std::uint64_t remaining_size() const { return remaining_size_; }
+    void set_remaining_size(std::uint64_t bytes) { remaining_size_ = bytes; }
+
+    // May be used to force the last read to be rounded up of a certain number of bytes
+    void set_last_read_multiple(std::uint64_t bytes) { last_read_multiple_ = bytes; }
 
     bool get_data(std::size_t size, std::uint8_t* out_data);
 
 private:
     ProducerCallback producer_;
     std::size_t size_ = 0;
+    std::size_t curr_size_ = 0;
+
+    std::uint64_t remaining_size_ = BUFFER_SIZE_UNSET;
+    std::uint64_t last_read_multiple_ = BUFFER_SIZE_UNSET;
 
     std::size_t buffer_offset_ = 0;
     std::vector<std::uint8_t> buffer_;
 };
 
-// This class is similar to ImageBuffer, but preserves historical peculiarities of buffer handling
-// in the backend to preserve exact behavior
-class ImageBufferGenesysUsb
-{
-public:
-    using ProducerCallback = std::function<void(std::size_t size, std::uint8_t* out_data)>;
-
-    ImageBufferGenesysUsb() {}
-    ImageBufferGenesysUsb(std::size_t total_size, std::size_t buffer_size,
-                          ProducerCallback producer);
-
-    std::size_t remaining_size() const { return remaining_size_; }
-
-    void set_remaining_size(std::size_t bytes) { remaining_size_ = bytes; }
-
-    std::size_t available() const { return buffer_end_ - buffer_offset_; }
-
-    bool get_data(std::size_t size, std::uint8_t* out_data);
-
-private:
-
-    std::size_t get_read_size();
-
-    std::size_t remaining_size_ = 0;
-
-    std::size_t buffer_size_ = 0;
-
-    std::size_t buffer_offset_ = 0;
-    std::size_t buffer_end_ = 0;
-    std::vector<std::uint8_t> buffer_;
-
-    ProducerCallback producer_;
-};
-
 } // namespace genesys
 
 #endif // BACKEND_GENESYS_IMAGE_BUFFER_H

backend/genesys/image_pipeline.cpp

@@ -53,15 +53,6 @@ namespace genesys {
 
 ImagePipelineNode::~ImagePipelineNode() {}
 
-std::size_t ImagePipelineNodeBytesSource::consume_remaining_bytes(std::size_t bytes)
-{
-    if (bytes > remaining_bytes_) {
-        bytes = remaining_bytes_;
-    }
-    remaining_bytes_ -= bytes;
-    return bytes;
-}
-
 bool ImagePipelineNodeCallableSource::get_next_row_data(std::uint8_t* out_data)
 {
     bool got_data = producer_(get_row_bytes(), out_data);
@@ -78,7 +69,7 @@ ImagePipelineNodeBufferedCallableSource::ImagePipelineNodeBufferedCallableSource
     format_{format},
     buffer_{input_batch_size, producer}
 {
-    set_remaining_bytes(height_ * get_row_bytes());
+    buffer_.set_remaining_size(height_ * get_row_bytes());
 }
 
 bool ImagePipelineNodeBufferedCallableSource::get_next_row_data(std::uint8_t* out_data)
@@ -92,13 +83,7 @@ bool ImagePipelineNodeBufferedCallableSource::get_next_row_data(std::uint8_t* ou
 
     bool got_data = true;
 
-    auto row_bytes = get_row_bytes();
+    got_data &= buffer_.get_data(get_row_bytes(), out_data);
-    auto bytes_to_ask = consume_remaining_bytes(row_bytes);
-    if (bytes_to_ask < row_bytes) {
-        got_data = false;
-    }
-
-    got_data &= buffer_.get_data(bytes_to_ask, out_data);
     curr_row_++;
     if (!got_data) {
         eof_ = true;
@@ -106,37 +91,6 @@ bool ImagePipelineNodeBufferedCallableSource::get_next_row_data(std::uint8_t* ou
     return got_data;
 }
 
-
-ImagePipelineNodeBufferedGenesysUsb::ImagePipelineNodeBufferedGenesysUsb(
-        std::size_t width, std::size_t height, PixelFormat format, std::size_t total_size,
-        std::size_t buffer_size, ProducerCallback producer) :
-    width_{width},
-    height_{height},
-    format_{format},
-    buffer_{total_size, buffer_size, producer}
-{
-    set_remaining_bytes(total_size);
-}
-
-bool ImagePipelineNodeBufferedGenesysUsb::get_next_row_data(std::uint8_t* out_data)
-{
-    if (remaining_bytes() != buffer_.remaining_size() + buffer_.available()) {
-        buffer_.set_remaining_size(remaining_bytes() - buffer_.available());
-    }
-    bool got_data = true;
-
-    std::size_t row_bytes = get_row_bytes();
-    std::size_t ask_bytes = consume_remaining_bytes(row_bytes);
-    if (ask_bytes < row_bytes) {
-        got_data = false;
-    }
-    got_data &= buffer_.get_data(ask_bytes, out_data);
-    if (!got_data) {
-        eof_ = true;
-    }
-    return got_data;
-}
-
 ImagePipelineNodeArraySource::ImagePipelineNodeArraySource(std::size_t width, std::size_t height,
                                                            PixelFormat format,
                                                            std::vector<std::uint8_t> data) :
@@ -151,7 +105,6 @@ ImagePipelineNodeArraySource::ImagePipelineNodeArraySource(std::size_t width, st
         throw SaneException("The given array is too small (%zu bytes). Need at least %zu",
                             data_.size(), size);
     }
-    set_remaining_bytes(size);
 }
 
 bool ImagePipelineNodeArraySource::get_next_row_data(std::uint8_t* out_data)
@@ -161,21 +114,11 @@ bool ImagePipelineNodeArraySource::get_next_row_data(std::uint8_t* out_data)
         return false;
     }
 
-    bool got_data = true;
-
     auto row_bytes = get_row_bytes();
-    auto bytes_to_ask = consume_remaining_bytes(row_bytes);
+    std::memcpy(out_data, data_.data() + row_bytes * next_row_, row_bytes);
-    if (bytes_to_ask < row_bytes) {
-        got_data = false;
-    }
-
-    std::memcpy(out_data, data_.data() + get_row_bytes() * next_row_, bytes_to_ask);
     next_row_++;
 
-    if (!got_data) {
+    return true;
-        eof_ = true;
-    }
-    return got_data;
 }
 
 

backend/genesys/image_pipeline.h

@@ -75,18 +75,6 @@ public:
     virtual bool get_next_row_data(std::uint8_t* out_data) = 0;
 };
 
-class ImagePipelineNodeBytesSource : public ImagePipelineNode
-{
-public:
-    std::size_t remaining_bytes() const { return remaining_bytes_; }
-    void set_remaining_bytes(std::size_t bytes) { remaining_bytes_ = bytes; }
-
-    std::size_t consume_remaining_bytes(std::size_t bytes);
-
-private:
-    std::size_t remaining_bytes_ = 0;
-};
-
 // A pipeline node that produces data from a callable
 class ImagePipelineNodeCallableSource : public ImagePipelineNode
 {
@@ -118,7 +106,7 @@ private:
 };
 
 // A pipeline node that produces data from a callable requesting fixed-size chunks.
-class ImagePipelineNodeBufferedCallableSource : public ImagePipelineNodeBytesSource
+class ImagePipelineNodeBufferedCallableSource : public ImagePipelineNode
 {
 public:
     using ProducerCallback = std::function<bool(std::size_t size, std::uint8_t* out_data)>;
@@ -135,8 +123,9 @@ public:
 
     bool get_next_row_data(std::uint8_t* out_data) override;
 
-    std::size_t buffer_size() const { return buffer_.size(); }
+    std::size_t remaining_bytes() const { return buffer_.remaining_size(); }
-    std::size_t buffer_available() const { return buffer_.available(); }
+    void set_remaining_bytes(std::size_t bytes) { buffer_.set_remaining_size(bytes); }
+    void set_last_read_multiple(std::size_t bytes) { buffer_.set_last_read_multiple(bytes); }
 
 private:
     ProducerCallback producer_;
@@ -150,38 +139,8 @@ private:
     ImageBuffer buffer_;
 };
 
-class ImagePipelineNodeBufferedGenesysUsb : public ImagePipelineNodeBytesSource
-{
-public:
-    using ProducerCallback = std::function<void(std::size_t size, std::uint8_t* out_data)>;
-
-    ImagePipelineNodeBufferedGenesysUsb(std::size_t width, std::size_t height,
-                                        PixelFormat format, std::size_t total_size,
-                                        std::size_t buffer_size, ProducerCallback producer);
-
-    std::size_t get_width() const override { return width_; }
-    std::size_t get_height() const override { return height_; }
-    PixelFormat get_format() const override { return format_; }
-
-    bool eof() const override { return eof_; }
-
-    bool get_next_row_data(std::uint8_t* out_data) override;
-
-    std::size_t buffer_available() const { return buffer_.available(); }
-
-private:
-    ProducerCallback producer_;
-    std::size_t width_ = 0;
-    std::size_t height_ = 0;
-    PixelFormat format_ = PixelFormat::UNKNOWN;
-
-    bool eof_ = false;
-
-    ImageBufferGenesysUsb buffer_;
-};
-
 // A pipeline node that produces data from the given array.
-class ImagePipelineNodeArraySource : public ImagePipelineNodeBytesSource
+class ImagePipelineNodeArraySource : public ImagePipelineNode
 {
 public:
     ImagePipelineNodeArraySource(std::size_t width, std::size_t height, PixelFormat format,
@@ -585,19 +544,31 @@ public:
 
     template<class Node, class... Args>
     void push_first_node(Args&&... args)
+    {
+        push_first_node(std::unique_ptr<Node>(new Node(std::forward<Args>(args)...)));
+    }
+
+    template<class Node>
+    void push_first_node(std::unique_ptr<Node>&& node)
     {
         if (!nodes_.empty()) {
             throw SaneException("Trying to append first node when there are existing nodes");
         }
-        nodes_.emplace_back(std::unique_ptr<Node>(new Node(std::forward<Args>(args)...)));
+        nodes_.emplace_back(std::move(node));
     }
 
     template<class Node, class... Args>
     void push_node(Args&&... args)
     {
         ensure_node_exists();
-        nodes_.emplace_back(std::unique_ptr<Node>(new Node(*nodes_.back(),
+        push_node(std::unique_ptr<Node>(new Node(*nodes_.back(), std::forward<Args>(args)...)));
-                                                           std::forward<Args>(args)...)));
+    }
+
+    template<class Node, class... Args>
+    void push_node(std::unique_ptr<Node>&& node)
+    {
+        ensure_node_exists();
+        nodes_.emplace_back(std::move(node));
     }
 
     bool get_next_row_data(std::uint8_t* out_data)

backend/genesys/low.cpp

@@ -661,11 +661,6 @@ static unsigned align_int_up(unsigned num, unsigned alignment)
     return num;
 }
 
-std::size_t compute_session_buffer_sizes(const ScanSession& s)
-{
-    return s.output_line_bytes * (32 + s.max_color_shift_lines + s.num_staggered_lines);
-}
-
 void compute_session_pipeline(const Genesys_Device* dev, ScanSession& s)
 {
     auto channels = s.params.channels;
@@ -902,7 +897,7 @@ void compute_session(const Genesys_Device* dev, ScanSession& s, const Genesys_Se
     s.output_total_bytes_raw = s.output_line_bytes_raw * s.output_line_count;
     s.output_total_bytes = s.output_line_bytes * s.output_line_count;
 
-    s.buffer_size_read = compute_session_buffer_sizes(s);
+    s.buffer_size_read = s.output_line_bytes_raw * 64;
     compute_session_pipeline(dev, s);
     compute_session_pixel_offsets(dev, s, sensor);
 
@@ -931,33 +926,6 @@ void compute_session(const Genesys_Device* dev, ScanSession& s, const Genesys_Se
     debug_dump(DBG_info, s);
 }
 
-static std::size_t get_usb_buffer_read_size(AsicType asic, const ScanSession& session)
-{
-    switch (asic) {
-        case AsicType::GL646:
-            // buffer not used on this chip set
-            return 1;
-
-        case AsicType::GL124:
-            // BUG: we shouldn't multiply by channels here nor adjuct by resolution factor
-            return session.output_line_bytes_raw * session.optical_resolution / session.full_resolution
-                    * session.params.channels;
-
-        case AsicType::GL845:
-        case AsicType::GL846:
-        case AsicType::GL847:
-            // BUG: we shouldn't multiply by channels here
-            return session.output_line_bytes_raw * session.params.channels;
-
-        case AsicType::GL842:
-        case AsicType::GL843:
-            return session.output_line_bytes_raw * 2;
-
-        default:
-            throw SaneException("Unknown asic type");
-    }
-}
-
 void build_image_pipeline(Genesys_Device* dev, const ScanSession& session)
 {
     static unsigned s_pipeline_index = 0;
@@ -980,22 +948,25 @@ void build_image_pipeline(Genesys_Device* dev, const ScanSession& session)
 
     dev->pipeline.clear();
 
-    // FIXME: here we are complicating things for the time being to preserve the existing behaviour
+    auto buffer_size = session.buffer_size_read;
-    // This allows to be sure that the changes to the image pipeline have not introduced
+
-    // regressions.
+    // At least GL841 requires reads to be aligned to 2 bytes and will fail on some devices on
+    // certain circumstances.
+    buffer_size = align_multiple_ceil(buffer_size, 2);
+
+    auto node = std::unique_ptr<ImagePipelineNodeBufferedCallableSource>(
+                new ImagePipelineNodeBufferedCallableSource(
+                    width, lines, format, buffer_size, read_data_from_usb));
+    node->set_last_read_multiple(2);
+    dev->pipeline.push_first_node(std::move(node));
+
+    if (dbg_log_image_data()) {
+        dev->pipeline.push_node<ImagePipelineNodeDebug>("gl_pipeline_" +
+                                                        std::to_string(s_pipeline_index) +
+                                                        "_0_from_usb.tiff");
+    }
 
     if (session.segment_count > 1) {
-        // BUG: we're reading one line too much
-        dev->pipeline.push_first_node<ImagePipelineNodeBufferedCallableSource>(
-                width, lines + 1, format,
-                get_usb_buffer_read_size(dev->model->asic_type, session), read_data_from_usb);
-
-        if (dbg_log_image_data()) {
-            dev->pipeline.push_node<ImagePipelineNodeDebug>("gl_pipeline_" +
-                                                            std::to_string(s_pipeline_index) +
-                                                            "_0_from_usb.tiff");
-        }
-
         auto output_width = session.output_segment_pixel_group_count * session.segment_count;
         dev->pipeline.push_node<ImagePipelineNodeDesegment>(output_width, dev->segment_order,
                                                             session.conseq_pixel_dist,
@@ -1006,20 +977,8 @@ void build_image_pipeline(Genesys_Device* dev, const ScanSession& session)
                                                             std::to_string(s_pipeline_index) +
                                                             "_1_after_desegment.tiff");
         }
-    } else {
-        auto read_bytes_left_after_deseg = session.output_line_bytes * session.output_line_count;
-
-        dev->pipeline.push_first_node<ImagePipelineNodeBufferedGenesysUsb>(
-                width, lines, format, read_bytes_left_after_deseg,
-                session.buffer_size_read, read_data_from_usb);
-        if (dbg_log_image_data()) {
-            dev->pipeline.push_node<ImagePipelineNodeDebug>("gl_pipeline_" +
-                                                            std::to_string(s_pipeline_index) +
-                                                            "_0_from_usb.tiff");
-        }
     }
 
-
     if (depth == 16) {
         unsigned num_swaps = 0;
         if (has_flag(dev->model->flags, ModelFlag::SWAP_16BIT_DATA)) {

backend/genesys/low.h

@@ -423,51 +423,6 @@ void build_image_pipeline(Genesys_Device* dev, const ScanSession& session);
 std::uint8_t compute_frontend_gain(float value, float target_value,
                                    FrontendType frontend_type);
 
-template<class T>
-inline T abs_diff(T a, T b)
-{
-    if (a < b) {
-        return b - a;
-    } else {
-        return a - b;
-    }
-}
-
-inline uint64_t align_multiple_floor(uint64_t x, uint64_t multiple)
-{
-    if (multiple == 0) {
-        return x;
-    }
-    return (x / multiple) * multiple;
-}
-
-inline uint64_t align_multiple_ceil(uint64_t x, uint64_t multiple)
-{
-    if (multiple == 0) {
-        return x;
-    }
-    return ((x + multiple - 1) / multiple) * multiple;
-}
-
-inline uint64_t multiply_by_depth_ceil(uint64_t pixels, uint64_t depth)
-{
-    if (depth == 1) {
-        return (pixels / 8) + ((pixels % 8) ? 1 : 0);
-    } else {
-        return pixels * (depth / 8);
-    }
-}
-
-template<class T>
-inline T clamp(const T& value, const T& lo, const T& hi)
-{
-    if (value < lo)
-        return lo;
-    if (value > hi)
-        return hi;
-    return value;
-}
-
 /*---------------------------------------------------------------------------*/
 /*                ASIC specific functions declarations                       */
 /*---------------------------------------------------------------------------*/

backend/genesys/utilities.h

@@ -76,6 +76,51 @@ inline double fixed_to_double(SANE_Word v)
     return static_cast<double>(v) / (1 << SANE_FIXED_SCALE_SHIFT);
 }
 
+template<class T>
+inline T abs_diff(T a, T b)
+{
+    if (a < b) {
+        return b - a;
+    } else {
+        return a - b;
+    }
+}
+
+inline std::uint64_t align_multiple_floor(std::uint64_t x, std::uint64_t multiple)
+{
+    if (multiple == 0) {
+        return x;
+    }
+    return (x / multiple) * multiple;
+}
+
+inline std::uint64_t align_multiple_ceil(std::uint64_t x, std::uint64_t multiple)
+{
+    if (multiple == 0) {
+        return x;
+    }
+    return ((x + multiple - 1) / multiple) * multiple;
+}
+
+inline std::uint64_t multiply_by_depth_ceil(std::uint64_t pixels, std::uint64_t depth)
+{
+    if (depth == 1) {
+        return (pixels / 8) + ((pixels % 8) ? 1 : 0);
+    } else {
+        return pixels * (depth / 8);
+    }
+}
+
+template<class T>
+inline T clamp(const T& value, const T& lo, const T& hi)
+{
+    if (value < lo)
+        return lo;
+    if (value > hi)
+        return hi;
+    return value;
+}
+
 template<class T>
 void compute_array_percentile_approx(T* result, const T* data,
                                      std::size_t line_count, std::size_t elements_per_line,

testsuite/backend/genesys/tests_image_pipeline.cpp

@@ -32,54 +32,139 @@
 
 namespace genesys {
 
-void test_image_buffer_genesys_usb()
+
+void test_image_buffer_exact_reads()
 {
     std::vector<std::size_t> requests;
 
-    auto on_read_usb = [&](std::size_t x, std::uint8_t* data)
+    auto on_read = [&](std::size_t x, std::uint8_t* data)
     {
         (void) data;
         requests.push_back(x);
+        return true;
     };
 
-    ImageBufferGenesysUsb buffer{1086780, 453120, on_read_usb};
+    ImageBuffer buffer{1000, on_read};
+    buffer.set_remaining_size(2500);
 
     std::vector<std::uint8_t> dummy;
-    dummy.resize(1086780);
+    dummy.resize(1000);
 
-    ASSERT_TRUE(buffer.get_data(453120, dummy.data()));
+    ASSERT_TRUE(buffer.get_data(1000, dummy.data()));
-    ASSERT_TRUE(buffer.get_data(453120, dummy.data()));
+    ASSERT_TRUE(buffer.get_data(1000, dummy.data()));
-    ASSERT_TRUE(buffer.get_data(180550, dummy.data()));
+    ASSERT_TRUE(buffer.get_data(500, dummy.data()));
 
     std::vector<std::size_t> expected = {
-        453120, 453120, 180736
+        1000, 1000, 500
     };
     ASSERT_EQ(requests, expected);
 }
 
-void test_image_buffer_genesys_usb_capped_remaining_bytes()
+void test_image_buffer_smaller_reads()
 {
     std::vector<std::size_t> requests;
 
-    auto on_read_usb = [&](std::size_t x, std::uint8_t* data)
+    auto on_read = [&](std::size_t x, std::uint8_t* data)
     {
         (void) data;
         requests.push_back(x);
+        return true;
     };
 
-    ImageBufferGenesysUsb buffer{1086780, 453120, on_read_usb};
+    ImageBuffer buffer{1000, on_read};
+    buffer.set_remaining_size(2500);
 
     std::vector<std::uint8_t> dummy;
-    dummy.resize(1086780);
+    dummy.resize(700);
 
-    ASSERT_TRUE(buffer.get_data(453120, dummy.data()));
+    ASSERT_TRUE(buffer.get_data(600, dummy.data()));
-    ASSERT_TRUE(buffer.get_data(453120, dummy.data()));
+    ASSERT_TRUE(buffer.get_data(600, dummy.data()));
-    buffer.set_remaining_size(10000);
+    ASSERT_TRUE(buffer.get_data(600, dummy.data()));
-    ASSERT_FALSE(buffer.get_data(56640, dummy.data()));
+    ASSERT_TRUE(buffer.get_data(700, dummy.data()));
 
     std::vector<std::size_t> expected = {
-        // note that the sizes are rounded-up to 256 bytes
+        1000, 1000, 500
-        453120, 453120, 10240
+    };
+    ASSERT_EQ(requests, expected);
+}
+
+void test_image_buffer_larger_reads()
+{
+    std::vector<std::size_t> requests;
+
+    auto on_read = [&](std::size_t x, std::uint8_t* data)
+    {
+        (void) data;
+        requests.push_back(x);
+        return true;
+    };
+
+    ImageBuffer buffer{1000, on_read};
+    buffer.set_remaining_size(2500);
+
+    std::vector<std::uint8_t> dummy;
+    dummy.resize(2500);
+
+    ASSERT_TRUE(buffer.get_data(2500, dummy.data()));
+
+    std::vector<std::size_t> expected = {
+        1000, 1000, 500
+    };
+    ASSERT_EQ(requests, expected);
+}
+
+void test_image_buffer_uncapped_remaining_bytes()
+{
+    std::vector<std::size_t> requests;
+    unsigned request_count = 0;
+    auto on_read = [&](std::size_t x, std::uint8_t* data)
+    {
+        (void) data;
+        requests.push_back(x);
+        request_count++;
+        return request_count < 4;
+    };
+
+    ImageBuffer buffer{1000, on_read};
+
+    std::vector<std::uint8_t> dummy;
+    dummy.resize(3000);
+
+    ASSERT_TRUE(buffer.get_data(3000, dummy.data()));
+    ASSERT_FALSE(buffer.get_data(3000, dummy.data()));
+
+    std::vector<std::size_t> expected = {
+        1000, 1000, 1000, 1000
+    };
+    ASSERT_EQ(requests, expected);
+}
+
+void test_image_buffer_capped_remaining_bytes()
+{
+    std::vector<std::size_t> requests;
+
+    auto on_read = [&](std::size_t x, std::uint8_t* data)
+    {
+        (void) data;
+        requests.push_back(x);
+        return true;
+    };
+
+    ImageBuffer buffer{1000, on_read};
+    buffer.set_remaining_size(10000);
+    buffer.set_last_read_multiple(16);
+
+    std::vector<std::uint8_t> dummy;
+    dummy.resize(2000);
+
+    ASSERT_TRUE(buffer.get_data(2000, dummy.data()));
+    ASSERT_TRUE(buffer.get_data(2000, dummy.data()));
+    buffer.set_remaining_size(100);
+    ASSERT_FALSE(buffer.get_data(200, dummy.data()));
+
+    std::vector<std::size_t> expected = {
+        // note that the sizes are rounded-up to 16 bytes
+        1000, 1000, 1000, 1000, 112
     };
     ASSERT_EQ(requests, expected);
 }
@@ -840,8 +925,11 @@ void test_node_calibrate_16bit()
 
 void test_image_pipeline()
 {
-    test_image_buffer_genesys_usb();
+    test_image_buffer_exact_reads();
-    test_image_buffer_genesys_usb_capped_remaining_bytes();
+    test_image_buffer_smaller_reads();
+    test_image_buffer_larger_reads();
+    test_image_buffer_uncapped_remaining_bytes();
+    test_image_buffer_capped_remaining_bytes();
     test_node_buffered_callable_source();
     test_node_format_convert();
     test_node_desegment_1_line();