From 55691ece2c651468b2f21b371037d2827c359b3c Mon Sep 17 00:00:00 2001 From: Povilas Kanapickas Date: Sat, 23 May 2020 10:39:07 +0300 Subject: [PATCH] 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();