Merge branch 's-png' into s-bmpqoi

esphome/components/online_image/online_image.cpp

@@ -16,7 +16,7 @@ static const char *const IF_MODIFIED_SINCE_HEADER_NAME = "if-modified-since";
 
 namespace esphome::online_image {
 
-// Time one loop pass may spend decoding a JPEG as it downloads
+// Time one loop pass may spend reading and decoding a JPEG as it downloads
 static constexpr uint32_t STREAM_PASS_BUDGET_MS = 20;
 // Shortest time between two on_download_progress calls
 static constexpr uint32_t PROGRESS_INTERVAL_MS = 100;
@@ -297,8 +297,7 @@ void OnlineImage::loop_stream_() {
     this->source_.compact();
 
   if (this->downloader_->get_bytes_read() < total) {
-    const size_t available = std::min(this->download_buffer_.free_capacity(), this->download_buffer_initial_size_);
-    if (available == 0) {
+    if (this->download_buffer_.free_capacity() == 0) {
       // compact() already ran, so a full buffer means the decoder read nothing in the last pass
       if (now - this->last_data_time_ >= this->parent_->get_timeout()) {
         ESP_LOGE(TAG, "Image does not fit in the %zu byte download buffer; increase buffer_size",
@@ -307,12 +306,18 @@ void OnlineImage::loop_stream_() {
         return;
       }
     } else {
-      const int len = this->downloader_->read(this->download_buffer_.append(), available);
-      if (len > 0) {
+      int len;
+      do {
+        len = this->downloader_->read(this->download_buffer_.append(),
+                                      std::min(this->download_buffer_.free_capacity(), STREAM_READ_CHUNK));
+        if (len <= 0)
+          break;
         this->last_data_time_ = now;
         this->download_buffer_.write(len);
         this->report_progress_(false);
-      } else if (len < 0) {
+      } while (this->downloader_->get_bytes_read() < total && this->download_buffer_.free_capacity() != 0 &&
+               millis() - now < STREAM_PASS_BUDGET_MS);
+      if (len < 0) {
         ESP_LOGE(TAG, "Error downloading image: %d", len);
         this->fail_download_();
         return;

esphome/components/online_image/online_image.h

@@ -16,6 +16,10 @@ using t_http_codes = enum {
   HTTP_CODE_NOT_FOUND = 404,
 };
 
+/// Largest read() while a JPEG decodes as it downloads. A read() blocks until it has every byte it asked for, so
+/// this bounds how long one loop pass waits on a slow link.
+static constexpr size_t STREAM_READ_CHUNK = 1460;
+
 /**
  * @brief Download an image from a given URL, and decode it using the specified decoder.
  * The image will then be stored in a buffer, so that it can be re-displayed without the

tests/components/online_image/test_download_end.cpp

@@ -28,6 +28,7 @@ class FakeContainer : public http_request::HttpContainer {
   }
 
   int read(uint8_t *buf, size_t max_len) override {
+    this->largest_read = std::max(this->largest_read, max_len);
     if (this->read_error_)
       return -1;
     size_t len = std::min(max_len, this->available_ - this->bytes_read_);
@@ -53,6 +54,9 @@ class FakeContainer : public http_request::HttpContainer {
     this->response_headers_.push_back({name, value});
   }
 
+  /// The most bytes any one read() asked for
+  size_t largest_read{0};
+
  protected:
   std::vector<uint8_t> body_;
   size_t available_;
@@ -644,6 +648,25 @@ TEST_F(OnlineImageDownloadEnd, JpegDecodesThroughASmallBuffer) {
   EXPECT_TRUE(this->http_.sent_header("if-none-match"));
 }
 
+// A read() waits for every byte it asks for, so a large buffer is filled a little at a time rather than in one
+// read that blocks the loop until the whole buffer has arrived
+TEST_F(OnlineImageDownloadEnd, JpegReadsInSmallPiecesWithALargeBuffer) {
+  auto image = this->make_image_(runtime_image::JPEG, 65536);
+  auto container =
+      std::make_shared<FakeContainer>(std::vector<uint8_t>(JPEG_48X40, JPEG_48X40 + sizeof(JPEG_48X40)), 0, false);
+  this->start_(*image, container);
+
+  for (int i = 0; i < 100 && this->finished_ + this->errors_ == 0; i++) {
+    container->deliver(sizeof(JPEG_48X40));
+    image->loop();
+  }
+
+  ASSERT_EQ(this->finished_, 1);
+  EXPECT_EQ(this->errors_, 0);
+  EXPECT_LE(container->largest_read, STREAM_READ_CHUNK);
+  EXPECT_EQ(image_pixels(*image), reference_jpeg_pixels());
+}
+
 // A slow link: the decoder waits part way through the image for the rest to arrive
 TEST_F(OnlineImageDownloadEnd, JpegWaitsForASlowDownload) {
   auto image = this->make_image_(runtime_image::JPEG, 1024);