genesys: Merge ImagePipelineNodeBuffered{CallableSource and GenesysUsb}

backend/genesys/device.cpp

@@ -114,9 +114,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

backend/genesys/device.h

@@ -350,7 +350,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/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,10 +81,28 @@ 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;
 }

backend/genesys/image_buffer.h

@@ -56,18 +56,30 @@ 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_;

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) :

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,36 +139,6 @@ 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 ImagePipelineNode
 {

backend/genesys/low.cpp

@@ -1009,9 +1009,16 @@ void build_image_pipeline(Genesys_Device* dev, const ScanSession& session)
     } else {
         auto read_bytes_left_after_deseg = session.output_line_bytes * session.output_line_count;
 
-        dev->pipeline.push_first_node<ImagePipelineNodeBufferedGenesysUsb>(
+        // historical code always aligned reads to 256 bytes. Need to check which actual devices
-                width, lines, format, read_bytes_left_after_deseg,
+        // need this, because anything with session.segment_count > 1 has used non-aligned reads
-                session.buffer_size_read, read_data_from_usb);
+        auto buffer_size = align_multiple_floor(session.buffer_size_read, 256);
+
+        auto node = std::unique_ptr<ImagePipelineNodeBufferedCallableSource>(
+            new ImagePipelineNodeBufferedCallableSource(width, lines, format,
+                                                        buffer_size, read_data_from_usb));
+        node->set_remaining_bytes(read_bytes_left_after_deseg);
+        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) +

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();