From afa798d71a440032dcab309468c7e6dcb0b5b852 Mon Sep 17 00:00:00 2001 From: Povilas Kanapickas Date: Sat, 23 May 2020 10:39:04 +0300 Subject: [PATCH 1/9] genesys: Simplify ImagePipelineNodeArraySource --- backend/genesys/image_pipeline.cpp | 15 ++------------- backend/genesys/image_pipeline.h | 2 +- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/backend/genesys/image_pipeline.cpp b/backend/genesys/image_pipeline.cpp index 98ce25254..40ceddcac 100644 --- a/backend/genesys/image_pipeline.cpp +++ b/backend/genesys/image_pipeline.cpp @@ -151,7 +151,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 +160,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); - if (bytes_to_ask < row_bytes) { - got_data = false; - } - - std::memcpy(out_data, data_.data() + get_row_bytes() * next_row_, bytes_to_ask); + std::memcpy(out_data, data_.data() + row_bytes * next_row_, row_bytes); next_row_++; - if (!got_data) { - eof_ = true; - } - return got_data; + return true; } diff --git a/backend/genesys/image_pipeline.h b/backend/genesys/image_pipeline.h index 0097d3e3c..f85c17574 100644 --- a/backend/genesys/image_pipeline.h +++ b/backend/genesys/image_pipeline.h @@ -181,7 +181,7 @@ private: }; // 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, From fe323f19cb9de8f02b7f8482cf9d1e6d437ed535 Mon Sep 17 00:00:00 2001 From: Povilas Kanapickas Date: Sat, 23 May 2020 10:39:05 +0300 Subject: [PATCH 2/9] genesys: Add a way to push constructed nodes to pipeline --- backend/genesys/image_pipeline.h | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/backend/genesys/image_pipeline.h b/backend/genesys/image_pipeline.h index f85c17574..4a1bdc7af 100644 --- a/backend/genesys/image_pipeline.h +++ b/backend/genesys/image_pipeline.h @@ -585,19 +585,31 @@ public: template void push_first_node(Args&&... args) + { + push_first_node(std::unique_ptr(new Node(std::forward(args)...))); + } + + template + void push_first_node(std::unique_ptr&& node) { if (!nodes_.empty()) { throw SaneException("Trying to append first node when there are existing nodes"); } - nodes_.emplace_back(std::unique_ptr(new Node(std::forward(args)...))); + nodes_.emplace_back(std::move(node)); } template void push_node(Args&&... args) { ensure_node_exists(); - nodes_.emplace_back(std::unique_ptr(new Node(*nodes_.back(), - std::forward(args)...))); + push_node(std::unique_ptr(new Node(*nodes_.back(), std::forward(args)...))); + } + + template + void push_node(std::unique_ptr&& node) + { + ensure_node_exists(); + nodes_.emplace_back(std::move(node)); } bool get_next_row_data(std::uint8_t* out_data) From 8981e583e20559c7e5dccbbeea7fbae114f826a8 Mon Sep 17 00:00:00 2001 From: Povilas Kanapickas Date: Sat, 23 May 2020 10:39:06 +0300 Subject: [PATCH 3/9] genesys: Move math utilities to utilities.h --- backend/genesys/low.h | 45 ------------------------------------- backend/genesys/utilities.h | 45 +++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 45 deletions(-) diff --git a/backend/genesys/low.h b/backend/genesys/low.h index 396a6d870..e7d5d650c 100644 --- a/backend/genesys/low.h +++ b/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 -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 -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 */ /*---------------------------------------------------------------------------*/ diff --git a/backend/genesys/utilities.h b/backend/genesys/utilities.h index 2f4ceb73e..fdab7701e 100644 --- a/backend/genesys/utilities.h +++ b/backend/genesys/utilities.h @@ -76,6 +76,51 @@ inline double fixed_to_double(SANE_Word v) return static_cast(v) / (1 << SANE_FIXED_SCALE_SHIFT); } +template +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 +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 void compute_array_percentile_approx(T* result, const T* data, std::size_t line_count, std::size_t elements_per_line, From 55691ece2c651468b2f21b371037d2827c359b3c Mon Sep 17 00:00:00 2001 From: Povilas Kanapickas Date: Sat, 23 May 2020 10:39:07 +0300 Subject: [PATCH 4/9] genesys: Merge ImagePipelineNodeBuffered{CallableSource and GenesysUsb} --- backend/genesys/device.cpp | 4 +- backend/genesys/device.h | 2 +- backend/genesys/image_buffer.cpp | 26 +++- backend/genesys/image_buffer.h | 16 ++- backend/genesys/image_pipeline.cpp | 50 +------ backend/genesys/image_pipeline.h | 49 +------ backend/genesys/low.cpp | 13 +- .../backend/genesys/tests_image_pipeline.cpp | 128 +++++++++++++++--- 8 files changed, 163 insertions(+), 125 deletions(-) diff --git a/backend/genesys/device.cpp b/backend/genesys/device.cpp index 878f93c14..d3290c7f0 100644 --- a/backend/genesys/device.cpp +++ b/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(pipeline.front()); + return static_cast(pipeline.front()); } bool Genesys_Device::is_head_pos_known(ScanHeadId scan_head) const diff --git a/backend/genesys/device.h b/backend/genesys/device.h index d3cd663db..32d5e8b7d 100644 --- a/backend/genesys/device.h +++ b/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 interface; diff --git a/backend/genesys/image_buffer.cpp b/backend/genesys/image_buffer.cpp index fe4d75517..986f6b27c 100644 --- a/backend/genesys/image_buffer.cpp +++ b/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}, - buffer_offset_{size} + size_{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(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; } diff --git a/backend/genesys/image_buffer.h b/backend/genesys/image_buffer.h index 0b3c4f844..e4db039e0 100644 --- a/backend/genesys/image_buffer.h +++ b/backend/genesys/image_buffer.h @@ -56,18 +56,30 @@ class ImageBuffer { public: using ProducerCallback = std::function; + static constexpr std::uint64_t BUFFER_SIZE_UNSET = std::numeric_limits::max(); ImageBuffer() {} ImageBuffer(std::size_t size, ProducerCallback producer); - std::size_t size() const { return size_; } - std::size_t available() const { return size_ - buffer_offset_; } + std::size_t available() const { return curr_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 buffer_; diff --git a/backend/genesys/image_pipeline.cpp b/backend/genesys/image_pipeline.cpp index 40ceddcac..8d67be99e 100644 --- a/backend/genesys/image_pipeline.cpp +++ b/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(); - 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); + got_data &= buffer_.get_data(get_row_bytes(), 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 data) : diff --git a/backend/genesys/image_pipeline.h b/backend/genesys/image_pipeline.h index 4a1bdc7af..cd849d844 100644 --- a/backend/genesys/image_pipeline.h +++ b/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; @@ -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 buffer_available() const { return buffer_.available(); } + std::size_t remaining_bytes() const { return buffer_.remaining_size(); } + 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; - - 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 { diff --git a/backend/genesys/low.cpp b/backend/genesys/low.cpp index 56fcfd95a..5a5797a78 100644 --- a/backend/genesys/low.cpp +++ b/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( - width, lines, format, read_bytes_left_after_deseg, - session.buffer_size_read, read_data_from_usb); + // historical code always aligned reads to 256 bytes. Need to check which actual devices + // need this, because anything with session.segment_count > 1 has used non-aligned reads + auto buffer_size = align_multiple_floor(session.buffer_size_read, 256); + + auto node = std::unique_ptr( + 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("gl_pipeline_" + std::to_string(s_pipeline_index) + diff --git a/testsuite/backend/genesys/tests_image_pipeline.cpp b/testsuite/backend/genesys/tests_image_pipeline.cpp index d13178060..7eed9e657 100644 --- a/testsuite/backend/genesys/tests_image_pipeline.cpp +++ b/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 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 dummy; - dummy.resize(1086780); + dummy.resize(1000); - ASSERT_TRUE(buffer.get_data(453120, dummy.data())); - ASSERT_TRUE(buffer.get_data(453120, dummy.data())); - ASSERT_TRUE(buffer.get_data(180550, dummy.data())); + ASSERT_TRUE(buffer.get_data(1000, dummy.data())); + ASSERT_TRUE(buffer.get_data(1000, dummy.data())); + ASSERT_TRUE(buffer.get_data(500, dummy.data())); std::vector 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 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 dummy; - dummy.resize(1086780); + dummy.resize(700); - ASSERT_TRUE(buffer.get_data(453120, dummy.data())); - ASSERT_TRUE(buffer.get_data(453120, dummy.data())); - buffer.set_remaining_size(10000); - ASSERT_FALSE(buffer.get_data(56640, dummy.data())); + ASSERT_TRUE(buffer.get_data(600, dummy.data())); + ASSERT_TRUE(buffer.get_data(600, dummy.data())); + ASSERT_TRUE(buffer.get_data(600, dummy.data())); + ASSERT_TRUE(buffer.get_data(700, dummy.data())); std::vector expected = { - // note that the sizes are rounded-up to 256 bytes - 453120, 453120, 10240 + 1000, 1000, 500 + }; + ASSERT_EQ(requests, expected); +} + +void test_image_buffer_larger_reads() +{ + std::vector 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 dummy; + dummy.resize(2500); + + ASSERT_TRUE(buffer.get_data(2500, dummy.data())); + + std::vector expected = { + 1000, 1000, 500 + }; + ASSERT_EQ(requests, expected); +} + +void test_image_buffer_uncapped_remaining_bytes() +{ + std::vector 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 dummy; + dummy.resize(3000); + + ASSERT_TRUE(buffer.get_data(3000, dummy.data())); + ASSERT_FALSE(buffer.get_data(3000, dummy.data())); + + std::vector expected = { + 1000, 1000, 1000, 1000 + }; + ASSERT_EQ(requests, expected); +} + +void test_image_buffer_capped_remaining_bytes() +{ + std::vector 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 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 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_genesys_usb_capped_remaining_bytes(); + test_image_buffer_exact_reads(); + 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(); From 9873fdf92270b1e8a6cf688c8366583e350f6e1a Mon Sep 17 00:00:00 2001 From: Povilas Kanapickas Date: Sat, 23 May 2020 10:39:08 +0300 Subject: [PATCH 5/9] genesys: Don't read too much data when segment count is more than one --- backend/genesys/low.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/genesys/low.cpp b/backend/genesys/low.cpp index 5a5797a78..4d4cb9d85 100644 --- a/backend/genesys/low.cpp +++ b/backend/genesys/low.cpp @@ -987,7 +987,7 @@ void build_image_pipeline(Genesys_Device* dev, const ScanSession& session) if (session.segment_count > 1) { // BUG: we're reading one line too much dev->pipeline.push_first_node( - width, lines + 1, format, + width, lines, format, get_usb_buffer_read_size(dev->model->asic_type, session), read_data_from_usb); if (dbg_log_image_data()) { From f5af63326381a64aafe6e888fef564ae22b513ee Mon Sep 17 00:00:00 2001 From: Povilas Kanapickas Date: Sat, 23 May 2020 10:39:09 +0300 Subject: [PATCH 6/9] genesys: Simplify data buffering in main image pipeline --- backend/genesys/low.cpp | 77 +++++++++-------------------------------- 1 file changed, 17 insertions(+), 60 deletions(-) diff --git a/backend/genesys/low.cpp b/backend/genesys/low.cpp index 4d4cb9d85..4439b2999 100644 --- a/backend/genesys/low.cpp +++ b/backend/genesys/low.cpp @@ -931,33 +931,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 +953,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 - // This allows to be sure that the changes to the image pipeline have not introduced - // regressions. + auto buffer_size = session.buffer_size_read; + + // 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( + 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("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( - width, lines, format, - get_usb_buffer_read_size(dev->model->asic_type, session), read_data_from_usb); - - if (dbg_log_image_data()) { - dev->pipeline.push_node("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(output_width, dev->segment_order, session.conseq_pixel_dist, @@ -1006,27 +982,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; - - // historical code always aligned reads to 256 bytes. Need to check which actual devices - // need this, because anything with session.segment_count > 1 has used non-aligned reads - auto buffer_size = align_multiple_floor(session.buffer_size_read, 256); - - auto node = std::unique_ptr( - 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("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)) { From d172b9cc4d0f8d07c0075b3a0b29a2e97ebe6975 Mon Sep 17 00:00:00 2001 From: Povilas Kanapickas Date: Sat, 23 May 2020 10:39:10 +0300 Subject: [PATCH 7/9] genesys: Remove read buffer after image processing --- backend/genesys/device.cpp | 2 -- backend/genesys/device.h | 2 -- backend/genesys/genesys.cpp | 19 +------------------ backend/genesys/gl124.cpp | 3 --- backend/genesys/gl646.cpp | 3 --- backend/genesys/gl841.cpp | 3 --- backend/genesys/gl842.cpp | 3 --- backend/genesys/gl843.cpp | 3 --- backend/genesys/gl846.cpp | 3 --- backend/genesys/gl847.cpp | 3 --- 10 files changed, 1 insertion(+), 43 deletions(-) diff --git a/backend/genesys/device.cpp b/backend/genesys/device.cpp index d3290c7f0..bcef646b2 100644 --- a/backend/genesys/device.cpp +++ b/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(); @@ -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' diff --git a/backend/genesys/device.h b/backend/genesys/device.h index 32d5e8b7d..72d6806f4 100644 --- a/backend/genesys/device.h +++ b/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 diff --git a/backend/genesys/genesys.cpp b/backend/genesys/genesys.cpp index d3fd87eab..93af28c50 100644 --- a/backend/genesys/genesys.cpp +++ b/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 */ diff --git a/backend/genesys/gl124.cpp b/backend/genesys/gl124.cpp index 2ba0321d3..3a7e4faa1 100644 --- a/backend/genesys/gl124.cpp +++ b/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; diff --git a/backend/genesys/gl646.cpp b/backend/genesys/gl646.cpp index eda6d1fbe..9cd9cbe95 100644 --- a/backend/genesys/gl646.cpp +++ b/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; diff --git a/backend/genesys/gl841.cpp b/backend/genesys/gl841.cpp index 7422a3737..dd851100f 100644 --- a/backend/genesys/gl841.cpp +++ b/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; diff --git a/backend/genesys/gl842.cpp b/backend/genesys/gl842.cpp index 439b9750e..469025a87 100644 --- a/backend/genesys/gl842.cpp +++ b/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; diff --git a/backend/genesys/gl843.cpp b/backend/genesys/gl843.cpp index 5a0c50a65..de57a65de 100644 --- a/backend/genesys/gl843.cpp +++ b/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; diff --git a/backend/genesys/gl846.cpp b/backend/genesys/gl846.cpp index 0b6616d6a..7dc00ef04 100644 --- a/backend/genesys/gl846.cpp +++ b/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; diff --git a/backend/genesys/gl847.cpp b/backend/genesys/gl847.cpp index 9c2e2fec2..9ac8c97a7 100644 --- a/backend/genesys/gl847.cpp +++ b/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; From ec5af182398c94d01741cd9d8d1ce0109903cfe1 Mon Sep 17 00:00:00 2001 From: Povilas Kanapickas Date: Sat, 23 May 2020 10:39:11 +0300 Subject: [PATCH 8/9] genesys: Remove no longer used ImageBufferGenesysUsb --- backend/genesys/fwd.h | 1 - backend/genesys/image_buffer.cpp | 73 -------------------------------- backend/genesys/image_buffer.h | 34 --------------- 3 files changed, 108 deletions(-) diff --git a/backend/genesys/fwd.h b/backend/genesys/fwd.h index ee45a44d4..9f3a89c66 100644 --- a/backend/genesys/fwd.h +++ b/backend/genesys/fwd.h @@ -74,7 +74,6 @@ class Image; // image_buffer.h class ImageBuffer; -class ImageBufferGenesysUsb; // image_pipeline.h class ImagePipelineNode; diff --git a/backend/genesys/image_buffer.cpp b/backend/genesys/image_buffer.cpp index 986f6b27c..c4f8019d3 100644 --- a/backend/genesys/image_buffer.cpp +++ b/backend/genesys/image_buffer.cpp @@ -107,77 +107,4 @@ bool ImageBuffer::get_data(std::size_t size, std::uint8_t* out_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(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 diff --git a/backend/genesys/image_buffer.h b/backend/genesys/image_buffer.h index e4db039e0..1910244cd 100644 --- a/backend/genesys/image_buffer.h +++ b/backend/genesys/image_buffer.h @@ -85,40 +85,6 @@ private: std::vector 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; - - 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 buffer_; - - ProducerCallback producer_; -}; - } // namespace genesys #endif // BACKEND_GENESYS_IMAGE_BUFFER_H From 28e6ad89b8ca150db2f8a159be8c83b2288d466d Mon Sep 17 00:00:00 2001 From: Povilas Kanapickas Date: Sat, 23 May 2020 10:39:12 +0300 Subject: [PATCH 9/9] genesys: Simplify read buffer size calculation --- backend/genesys/low.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/backend/genesys/low.cpp b/backend/genesys/low.cpp index 4439b2999..43f6ebe3e 100644 --- a/backend/genesys/low.cpp +++ b/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);