[{"id":18892,"web_url":"https://patchwork.libcamera.org/comment/18892/","msgid":"<20210818030859.GH1733965@pyrite.rasen.tech>","date":"2021-08-18T03:08:59","subject":"Re: [libcamera-devel] [PATCH] qcam: Replace MappedBuffer with\n\tSpan","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Laurent,\n\nOn Wed, Aug 18, 2021 at 03:22:08AM +0300, Laurent Pinchart wrote:\n> The MappedBuffer structure is a custom container that binds a data\n> pointer with a length. This is exactly what Span is. Use it instead.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Paul Elder \n\n> ---\n> src/qcam/main_window.cpp | 17 +++++++++--------\n> src/qcam/main_window.h | 2 +-\n> src/qcam/viewfinder.h | 9 +++------\n> src/qcam/viewfinder_gl.cpp | 5 +++--\n> src/qcam/viewfinder_gl.h | 2 +-\n> src/qcam/viewfinder_qt.cpp | 5 +++--\n> src/qcam/viewfinder_qt.h | 2 +-\n> 7 files changed, 21 insertions(+), 21 deletions(-)\n> \n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index 39d034de6bb2..1adaae60d83b 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -474,7 +474,8 @@ int MainWindow::startCapture()\n> \t\t\tconst FrameBuffer::Plane &plane = buffer->planes().front();\n> \t\t\tvoid *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,\n> \t\t\t\t\t plane.fd.fd(), 0);\n> -\t\t\tmappedBuffers_[buffer.get()] = { memory, plane.length };\n> +\t\t\tmappedBuffers_[buffer.get()] = { static_cast(memory),\n> +\t\t\t\t\t\t\t plane.length };\n> \n> \t\t\t/* Store buffers on the free list. */\n> \t\t\tfreeBuffers_[stream].enqueue(buffer.get());\n> @@ -537,8 +538,8 @@ error:\n> \trequests_.clear();\n> \n> \tfor (auto &iter : mappedBuffers_) {\n> -\t\tconst MappedBuffer &buffer = iter.second;\n> -\t\tmunmap(buffer.memory, buffer.size);\n> +\t\tconst Span &buffer = iter.second;\n> +\t\tmunmap(buffer.data(), buffer.size());\n> \t}\n> \tmappedBuffers_.clear();\n> \n> @@ -573,8 +574,8 @@ void MainWindow::stopCapture()\n> \tcamera_->requestCompleted.disconnect(this, &MainWindow::requestComplete);\n> \n> \tfor (auto &iter : mappedBuffers_) {\n> -\t\tconst MappedBuffer &buffer = iter.second;\n> -\t\tmunmap(buffer.memory, buffer.size);\n> +\t\tconst Span &buffer = iter.second;\n> +\t\tmunmap(buffer.data(), buffer.size());\n> \t}\n> \tmappedBuffers_.clear();\n> \n> @@ -673,10 +674,10 @@ void MainWindow::processRaw(FrameBuffer *buffer,\n> \t\t\t\t\t\t\t\"DNG Files (*.dng)\");\n> \n> \tif (!filename.isEmpty()) {\n> -\t\tconst MappedBuffer &mapped = mappedBuffers_[buffer];\n> +\t\tconst Span &mapped = mappedBuffers_[buffer];\n> \t\tDNGWriter::write(filename.toStdString().c_str(), camera_.get(),\n> \t\t\t\t rawStream_->configuration(), metadata, buffer,\n> -\t\t\t\t mapped.memory);\n> +\t\t\t\t mapped.data());\n> \t}\n> #endif\n> \n> @@ -753,7 +754,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)\n> \t\t<< \"fps:\" << Qt::fixed << qSetRealNumberPrecision(2) << fps;\n> \n> \t/* Render the frame on the viewfinder. */\n> -\tviewfinder_->render(buffer, &mappedBuffers_[buffer]);\n> +\tviewfinder_->render(buffer, mappedBuffers_[buffer]);\n> }\n> \n> void MainWindow::queueRequest(FrameBuffer *buffer)\n> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h\n> index 85d56ce49abe..6788de8ddde9 100644\n> --- a/src/qcam/main_window.h\n> +++ b/src/qcam/main_window.h\n> @@ -106,7 +106,7 @@ private:\n> \tFrameBufferAllocator *allocator_;\n> \n> \tstd::unique_ptr config_;\n> -\tstd::map mappedBuffers_;\n> +\tstd::map> mappedBuffers_;\n> \n> \t/* Capture state, buffers queue and statistics */\n> \tbool isCapturing_;\n> diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h\n> index 46747c227d0c..42d40f1f33f0 100644\n> --- a/src/qcam/viewfinder.h\n> +++ b/src/qcam/viewfinder.h\n> @@ -11,14 +11,11 @@\n> #include \n> #include \n> \n> +#include \n> +\n> #include \n> #include \n> \n> -struct MappedBuffer {\n> -\tvoid *memory;\n> -\tsize_t size;\n> -};\n> -\n> class ViewFinder\n> {\n> public:\n> @@ -27,7 +24,7 @@ public:\n> \tvirtual const QList &nativeFormats() const = 0;\n> \n> \tvirtual int setFormat(const libcamera::PixelFormat &format, const QSize &size) = 0;\n> -\tvirtual void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) = 0;\n> +\tvirtual void render(libcamera::FrameBuffer *buffer, libcamera::Span mem) = 0;\n> \tvirtual void stop() = 0;\n> \n> \tvirtual QImage getCurrentImage() = 0;\n> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n> index add87db88207..40226601f9fd 100644\n> --- a/src/qcam/viewfinder_gl.cpp\n> +++ b/src/qcam/viewfinder_gl.cpp\n> @@ -110,7 +110,8 @@ QImage ViewFinderGL::getCurrentImage()\n> \treturn grabFramebuffer();\n> }\n> \n> -void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> +void ViewFinderGL::render(libcamera::FrameBuffer *buffer,\n> +\t\t\t libcamera::Span mem)\n> {\n> \tif (buffer->planes().size() != 1) {\n> \t\tqWarning() << \"Multi-planar buffers are not supported\";\n> @@ -120,7 +121,7 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> \tif (buffer_)\n> \t\trenderComplete(buffer_);\n> \n> -\tdata_ = static_cast(map->memory);\n> +\tdata_ = mem.data();\n> \t/*\n> \t * \\todo Get the stride from the buffer instead of computing it naively\n> \t */\n> diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h\n> index 4a0f8ca58f29..3334549e0be4 100644\n> --- a/src/qcam/viewfinder_gl.h\n> +++ b/src/qcam/viewfinder_gl.h\n> @@ -39,7 +39,7 @@ public:\n> \tconst QList &nativeFormats() const override;\n> \n> \tint setFormat(const libcamera::PixelFormat &format, const QSize &size) override;\n> -\tvoid render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override;\n> +\tvoid render(libcamera::FrameBuffer *buffer, libcamera::Span mem) override;\n> \tvoid stop() override;\n> \n> \tQImage getCurrentImage() override;\n> diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp\n> index e436714c6bdb..efa1d412584b 100644\n> --- a/src/qcam/viewfinder_qt.cpp\n> +++ b/src/qcam/viewfinder_qt.cpp\n> @@ -78,14 +78,15 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format,\n> \treturn 0;\n> }\n> \n> -void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> +void ViewFinderQt::render(libcamera::FrameBuffer *buffer,\n> +\t\t\t libcamera::Span mem)\n> {\n> \tif (buffer->planes().size() != 1) {\n> \t\tqWarning() << \"Multi-planar buffers are not supported\";\n> \t\treturn;\n> \t}\n> \n> -\tunsigned char *memory = static_cast(map->memory);\n> +\tunsigned char *memory = mem.data();\n> \tsize_t size = buffer->metadata().planes[0].bytesused;\n> \n> \t{\n> diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h\n> index 501c72a70dec..1a569b9cee6e 100644\n> --- a/src/qcam/viewfinder_qt.h\n> +++ b/src/qcam/viewfinder_qt.h\n> @@ -32,7 +32,7 @@ public:\n> \tconst QList &nativeFormats() const override;\n> \n> \tint setFormat(const libcamera::PixelFormat &format, const QSize &size) override;\n> -\tvoid render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override;\n> +\tvoid render(libcamera::FrameBuffer *buffer, libcamera::Span mem) override;\n> \tvoid stop() override;\n> \n> \tQImage getCurrentImage() override;\n> -- \n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8EAF9BD87D\n\tfor ;\n\tWed, 18 Aug 2021 03:09:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D7BB368894;\n\tWed, 18 Aug 2021 05:09:07 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0A6E46025D\n\tfor ;\n\tWed, 18 Aug 2021 05:09:07 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6CC5E466;\n\tWed, 18 Aug 2021 05:09:05 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"lU3SEOWY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629256146;\n\tbh=12FoZyErO8YzwDWMNNKTFub8pgbCRiynP7OENakmJ10=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=lU3SEOWYFANJOq8phpMgz1sJ7aFGumrJ3Hz+MfT+pQG7WiKAkZayKAgs74vpBqDqP\n\tn1hqhe3e7+RumggEKaUmr8lWr8BG5Qe8xgjg120mmCFZjTxgmvEZDDe1L9EFQmG4pN\n\tnoiIsl7GIpF47mwGydL0Mh/lIcoOMu4rG9COarLs=","Date":"Wed, 18 Aug 2021 12:08:59 +0900","From":"paul.elder@ideasonboard.com","To":"Laurent Pinchart ","Message-ID":"<20210818030859.GH1733965@pyrite.rasen.tech>","References":"<20210818002208.13631-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210818002208.13631-1-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] qcam: Replace MappedBuffer with\n\tSpan","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18894,"web_url":"https://patchwork.libcamera.org/comment/18894/","msgid":"","date":"2021-08-18T05:29:36","subject":"Re: [libcamera-devel] [PATCH] qcam: Replace MappedBuffer with\n\tSpan","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Laurent,\n\nThank you for the patch.\n\nOn 18/08/2021 02:22, Laurent Pinchart wrote:\n> The MappedBuffer structure is a custom container that binds a data\n> pointer with a length. This is exactly what Span is. Use it instead.\n> \n\nCould you explain why it is better with a Span than with a buffer and\nlength ? As far as I can tell, it is only a matter of how it is declared\n:-) ?\n\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/qcam/main_window.cpp | 17 +++++++++--------\n> src/qcam/main_window.h | 2 +-\n> src/qcam/viewfinder.h | 9 +++------\n> src/qcam/viewfinder_gl.cpp | 5 +++--\n> src/qcam/viewfinder_gl.h | 2 +-\n> src/qcam/viewfinder_qt.cpp | 5 +++--\n> src/qcam/viewfinder_qt.h | 2 +-\n> 7 files changed, 21 insertions(+), 21 deletions(-)\n> \n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index 39d034de6bb2..1adaae60d83b 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -474,7 +474,8 @@ int MainWindow::startCapture()\n> \t\t\tconst FrameBuffer::Plane &plane = buffer->planes().front();\n> \t\t\tvoid *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,\n> \t\t\t\t\t plane.fd.fd(), 0);\n> -\t\t\tmappedBuffers_[buffer.get()] = { memory, plane.length };\n> +\t\t\tmappedBuffers_[buffer.get()] = { static_cast(memory),\n> +\t\t\t\t\t\t\t plane.length };\n> \n> \t\t\t/* Store buffers on the free list. */\n> \t\t\tfreeBuffers_[stream].enqueue(buffer.get());\n> @@ -537,8 +538,8 @@ error:\n> \trequests_.clear();\n> \n> \tfor (auto &iter : mappedBuffers_) {\n> -\t\tconst MappedBuffer &buffer = iter.second;\n> -\t\tmunmap(buffer.memory, buffer.size);\n> +\t\tconst Span &buffer = iter.second;\n> +\t\tmunmap(buffer.data(), buffer.size());\n> \t}\n> \tmappedBuffers_.clear();\n> \n> @@ -573,8 +574,8 @@ void MainWindow::stopCapture()\n> \tcamera_->requestCompleted.disconnect(this, &MainWindow::requestComplete);\n> \n> \tfor (auto &iter : mappedBuffers_) {\n> -\t\tconst MappedBuffer &buffer = iter.second;\n> -\t\tmunmap(buffer.memory, buffer.size);\n> +\t\tconst Span &buffer = iter.second;\n> +\t\tmunmap(buffer.data(), buffer.size());\n> \t}\n> \tmappedBuffers_.clear();\n> \n> @@ -673,10 +674,10 @@ void MainWindow::processRaw(FrameBuffer *buffer,\n> \t\t\t\t\t\t\t\"DNG Files (*.dng)\");\n> \n> \tif (!filename.isEmpty()) {\n> -\t\tconst MappedBuffer &mapped = mappedBuffers_[buffer];\n> +\t\tconst Span &mapped = mappedBuffers_[buffer];\n> \t\tDNGWriter::write(filename.toStdString().c_str(), camera_.get(),\n> \t\t\t\t rawStream_->configuration(), metadata, buffer,\n> -\t\t\t\t mapped.memory);\n> +\t\t\t\t mapped.data());\n> \t}\n> #endif\n> \n> @@ -753,7 +754,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)\n> \t\t<< \"fps:\" << Qt::fixed << qSetRealNumberPrecision(2) << fps;\n> \n> \t/* Render the frame on the viewfinder. */\n> -\tviewfinder_->render(buffer, &mappedBuffers_[buffer]);\n> +\tviewfinder_->render(buffer, mappedBuffers_[buffer]);\n> }\n> \n> void MainWindow::queueRequest(FrameBuffer *buffer)\n> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h\n> index 85d56ce49abe..6788de8ddde9 100644\n> --- a/src/qcam/main_window.h\n> +++ b/src/qcam/main_window.h\n> @@ -106,7 +106,7 @@ private:\n> \tFrameBufferAllocator *allocator_;\n> \n> \tstd::unique_ptr config_;\n> -\tstd::map mappedBuffers_;\n> +\tstd::map> mappedBuffers_;\n> \n> \t/* Capture state, buffers queue and statistics */\n> \tbool isCapturing_;\n> diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h\n> index 46747c227d0c..42d40f1f33f0 100644\n> --- a/src/qcam/viewfinder.h\n> +++ b/src/qcam/viewfinder.h\n> @@ -11,14 +11,11 @@\n> #include \n> #include \n> \n> +#include \n> +\n> #include \n> #include \n> \n> -struct MappedBuffer {\n> -\tvoid *memory;\n> -\tsize_t size;\n> -};\n> -\n> class ViewFinder\n> {\n> public:\n> @@ -27,7 +24,7 @@ public:\n> \tvirtual const QList &nativeFormats() const = 0;\n> \n> \tvirtual int setFormat(const libcamera::PixelFormat &format, const QSize &size) = 0;\n> -\tvirtual void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) = 0;\n> +\tvirtual void render(libcamera::FrameBuffer *buffer, libcamera::Span mem) = 0;\n> \tvirtual void stop() = 0;\n> \n> \tvirtual QImage getCurrentImage() = 0;\n> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n> index add87db88207..40226601f9fd 100644\n> --- a/src/qcam/viewfinder_gl.cpp\n> +++ b/src/qcam/viewfinder_gl.cpp\n> @@ -110,7 +110,8 @@ QImage ViewFinderGL::getCurrentImage()\n> \treturn grabFramebuffer();\n> }\n> \n> -void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> +void ViewFinderGL::render(libcamera::FrameBuffer *buffer,\n> +\t\t\t libcamera::Span mem)\n> {\n> \tif (buffer->planes().size() != 1) {\n> \t\tqWarning() << \"Multi-planar buffers are not supported\";\n> @@ -120,7 +121,7 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> \tif (buffer_)\n> \t\trenderComplete(buffer_);\n> \n> -\tdata_ = static_cast(map->memory);\n> +\tdata_ = mem.data();\n> \t/*\n> \t * \\todo Get the stride from the buffer instead of computing it naively\n> \t */\n> diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h\n> index 4a0f8ca58f29..3334549e0be4 100644\n> --- a/src/qcam/viewfinder_gl.h\n> +++ b/src/qcam/viewfinder_gl.h\n> @@ -39,7 +39,7 @@ public:\n> \tconst QList &nativeFormats() const override;\n> \n> \tint setFormat(const libcamera::PixelFormat &format, const QSize &size) override;\n> -\tvoid render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override;\n> +\tvoid render(libcamera::FrameBuffer *buffer, libcamera::Span mem) override;\n> \tvoid stop() override;\n> \n> \tQImage getCurrentImage() override;\n> diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp\n> index e436714c6bdb..efa1d412584b 100644\n> --- a/src/qcam/viewfinder_qt.cpp\n> +++ b/src/qcam/viewfinder_qt.cpp\n> @@ -78,14 +78,15 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format,\n> \treturn 0;\n> }\n> \n> -void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> +void ViewFinderQt::render(libcamera::FrameBuffer *buffer,\n> +\t\t\t libcamera::Span mem)\n> {\n> \tif (buffer->planes().size() != 1) {\n> \t\tqWarning() << \"Multi-planar buffers are not supported\";\n> \t\treturn;\n> \t}\n> \n> -\tunsigned char *memory = static_cast(map->memory);\n> +\tunsigned char *memory = mem.data();\n> \tsize_t size = buffer->metadata().planes[0].bytesused;\n> \n> \t{\n> diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h\n> index 501c72a70dec..1a569b9cee6e 100644\n> --- a/src/qcam/viewfinder_qt.h\n> +++ b/src/qcam/viewfinder_qt.h\n> @@ -32,7 +32,7 @@ public:\n> \tconst QList &nativeFormats() const override;\n> \n> \tint setFormat(const libcamera::PixelFormat &format, const QSize &size) override;\n> -\tvoid render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override;\n> +\tvoid render(libcamera::FrameBuffer *buffer, libcamera::Span mem) override;\n> \tvoid stop() override;\n> \n> \tQImage getCurrentImage() override;\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 72E9ABD87C\n\tfor ;\n\tWed, 18 Aug 2021 05:29:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CD60468894;\n\tWed, 18 Aug 2021 07:29:40 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D4C1D60505\n\tfor ;\n\tWed, 18 Aug 2021 07:29:38 +0200 (CEST)","from tatooine.ideasonboard.com (unknown\n\t[IPv6:2a01:e0a:169:7140:946e:2bbb:370e:41e4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 85EBC466;\n\tWed, 18 Aug 2021 07:29:38 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"PqAYcQPV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629264578;\n\tbh=pmSvi84ENyDGZzYS2GCeliBX6wR30zT8dCBpnxlB2aA=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=PqAYcQPVLHhB4omS/b54+UewKhWSmDP662CguU81wzkSJ1rpUIE8VDXdzEPRTPx/e\n\t+wA00DkePwX2l0EYvQZVZGjhnktdN/H9WnGKY/o0A+u9crJyWCKWoM5kgN2+jBZ7cV\n\t2zx/WuB/rNiK3puvHMbRRU+LhMKlYnTfM59a51UY=","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210818002208.13631-1-laurent.pinchart@ideasonboard.com>","From":"Jean-Michel Hautbois ","Message-ID":"","Date":"Wed, 18 Aug 2021 07:29:36 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.11.0","MIME-Version":"1.0","In-Reply-To":"<20210818002208.13631-1-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH] qcam: Replace MappedBuffer with\n\tSpan","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18898,"web_url":"https://patchwork.libcamera.org/comment/18898/","msgid":"","date":"2021-08-18T08:48:54","subject":"Re: [libcamera-devel] [PATCH] qcam: Replace MappedBuffer with\n\tSpan","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jean-Michel,\n\nOn Wed, Aug 18, 2021 at 07:29:36AM +0200, Jean-Michel Hautbois wrote:\n> On 18/08/2021 02:22, Laurent Pinchart wrote:\n> > The MappedBuffer structure is a custom container that binds a data\n> > pointer with a length. This is exactly what Span is. Use it instead.\n> \n> Could you explain why it is better with a Span than with a buffer and\n> length ? As far as I can tell, it is only a matter of how it is declared\n> :-) ?\n\nBecause a span *is* a buffer pointer and length. It's a standard class\nthat groups both fields, so it's better than a ad-hoc class that does\nthe same.\n\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/qcam/main_window.cpp | 17 +++++++++--------\n> > src/qcam/main_window.h | 2 +-\n> > src/qcam/viewfinder.h | 9 +++------\n> > src/qcam/viewfinder_gl.cpp | 5 +++--\n> > src/qcam/viewfinder_gl.h | 2 +-\n> > src/qcam/viewfinder_qt.cpp | 5 +++--\n> > src/qcam/viewfinder_qt.h | 2 +-\n> > 7 files changed, 21 insertions(+), 21 deletions(-)\n> > \n> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> > index 39d034de6bb2..1adaae60d83b 100644\n> > --- a/src/qcam/main_window.cpp\n> > +++ b/src/qcam/main_window.cpp\n> > @@ -474,7 +474,8 @@ int MainWindow::startCapture()\n> > \t\t\tconst FrameBuffer::Plane &plane = buffer->planes().front();\n> > \t\t\tvoid *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,\n> > \t\t\t\t\t plane.fd.fd(), 0);\n> > -\t\t\tmappedBuffers_[buffer.get()] = { memory, plane.length };\n> > +\t\t\tmappedBuffers_[buffer.get()] = { static_cast(memory),\n> > +\t\t\t\t\t\t\t plane.length };\n> > \n> > \t\t\t/* Store buffers on the free list. */\n> > \t\t\tfreeBuffers_[stream].enqueue(buffer.get());\n> > @@ -537,8 +538,8 @@ error:\n> > \trequests_.clear();\n> > \n> > \tfor (auto &iter : mappedBuffers_) {\n> > -\t\tconst MappedBuffer &buffer = iter.second;\n> > -\t\tmunmap(buffer.memory, buffer.size);\n> > +\t\tconst Span &buffer = iter.second;\n> > +\t\tmunmap(buffer.data(), buffer.size());\n> > \t}\n> > \tmappedBuffers_.clear();\n> > \n> > @@ -573,8 +574,8 @@ void MainWindow::stopCapture()\n> > \tcamera_->requestCompleted.disconnect(this, &MainWindow::requestComplete);\n> > \n> > \tfor (auto &iter : mappedBuffers_) {\n> > -\t\tconst MappedBuffer &buffer = iter.second;\n> > -\t\tmunmap(buffer.memory, buffer.size);\n> > +\t\tconst Span &buffer = iter.second;\n> > +\t\tmunmap(buffer.data(), buffer.size());\n> > \t}\n> > \tmappedBuffers_.clear();\n> > \n> > @@ -673,10 +674,10 @@ void MainWindow::processRaw(FrameBuffer *buffer,\n> > \t\t\t\t\t\t\t\"DNG Files (*.dng)\");\n> > \n> > \tif (!filename.isEmpty()) {\n> > -\t\tconst MappedBuffer &mapped = mappedBuffers_[buffer];\n> > +\t\tconst Span &mapped = mappedBuffers_[buffer];\n> > \t\tDNGWriter::write(filename.toStdString().c_str(), camera_.get(),\n> > \t\t\t\t rawStream_->configuration(), metadata, buffer,\n> > -\t\t\t\t mapped.memory);\n> > +\t\t\t\t mapped.data());\n> > \t}\n> > #endif\n> > \n> > @@ -753,7 +754,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)\n> > \t\t<< \"fps:\" << Qt::fixed << qSetRealNumberPrecision(2) << fps;\n> > \n> > \t/* Render the frame on the viewfinder. */\n> > -\tviewfinder_->render(buffer, &mappedBuffers_[buffer]);\n> > +\tviewfinder_->render(buffer, mappedBuffers_[buffer]);\n> > }\n> > \n> > void MainWindow::queueRequest(FrameBuffer *buffer)\n> > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h\n> > index 85d56ce49abe..6788de8ddde9 100644\n> > --- a/src/qcam/main_window.h\n> > +++ b/src/qcam/main_window.h\n> > @@ -106,7 +106,7 @@ private:\n> > \tFrameBufferAllocator *allocator_;\n> > \n> > \tstd::unique_ptr config_;\n> > -\tstd::map mappedBuffers_;\n> > +\tstd::map> mappedBuffers_;\n> > \n> > \t/* Capture state, buffers queue and statistics */\n> > \tbool isCapturing_;\n> > diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h\n> > index 46747c227d0c..42d40f1f33f0 100644\n> > --- a/src/qcam/viewfinder.h\n> > +++ b/src/qcam/viewfinder.h\n> > @@ -11,14 +11,11 @@\n> > #include \n> > #include \n> > \n> > +#include \n> > +\n> > #include \n> > #include \n> > \n> > -struct MappedBuffer {\n> > -\tvoid *memory;\n> > -\tsize_t size;\n> > -};\n> > -\n> > class ViewFinder\n> > {\n> > public:\n> > @@ -27,7 +24,7 @@ public:\n> > \tvirtual const QList &nativeFormats() const = 0;\n> > \n> > \tvirtual int setFormat(const libcamera::PixelFormat &format, const QSize &size) = 0;\n> > -\tvirtual void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) = 0;\n> > +\tvirtual void render(libcamera::FrameBuffer *buffer, libcamera::Span mem) = 0;\n> > \tvirtual void stop() = 0;\n> > \n> > \tvirtual QImage getCurrentImage() = 0;\n> > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n> > index add87db88207..40226601f9fd 100644\n> > --- a/src/qcam/viewfinder_gl.cpp\n> > +++ b/src/qcam/viewfinder_gl.cpp\n> > @@ -110,7 +110,8 @@ QImage ViewFinderGL::getCurrentImage()\n> > \treturn grabFramebuffer();\n> > }\n> > \n> > -void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> > +void ViewFinderGL::render(libcamera::FrameBuffer *buffer,\n> > +\t\t\t libcamera::Span mem)\n> > {\n> > \tif (buffer->planes().size() != 1) {\n> > \t\tqWarning() << \"Multi-planar buffers are not supported\";\n> > @@ -120,7 +121,7 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> > \tif (buffer_)\n> > \t\trenderComplete(buffer_);\n> > \n> > -\tdata_ = static_cast(map->memory);\n> > +\tdata_ = mem.data();\n> > \t/*\n> > \t * \\todo Get the stride from the buffer instead of computing it naively\n> > \t */\n> > diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h\n> > index 4a0f8ca58f29..3334549e0be4 100644\n> > --- a/src/qcam/viewfinder_gl.h\n> > +++ b/src/qcam/viewfinder_gl.h\n> > @@ -39,7 +39,7 @@ public:\n> > \tconst QList &nativeFormats() const override;\n> > \n> > \tint setFormat(const libcamera::PixelFormat &format, const QSize &size) override;\n> > -\tvoid render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override;\n> > +\tvoid render(libcamera::FrameBuffer *buffer, libcamera::Span mem) override;\n> > \tvoid stop() override;\n> > \n> > \tQImage getCurrentImage() override;\n> > diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp\n> > index e436714c6bdb..efa1d412584b 100644\n> > --- a/src/qcam/viewfinder_qt.cpp\n> > +++ b/src/qcam/viewfinder_qt.cpp\n> > @@ -78,14 +78,15 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format,\n> > \treturn 0;\n> > }\n> > \n> > -void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> > +void ViewFinderQt::render(libcamera::FrameBuffer *buffer,\n> > +\t\t\t libcamera::Span mem)\n> > {\n> > \tif (buffer->planes().size() != 1) {\n> > \t\tqWarning() << \"Multi-planar buffers are not supported\";\n> > \t\treturn;\n> > \t}\n> > \n> > -\tunsigned char *memory = static_cast(map->memory);\n> > +\tunsigned char *memory = mem.data();\n> > \tsize_t size = buffer->metadata().planes[0].bytesused;\n> > \n> > \t{\n> > diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h\n> > index 501c72a70dec..1a569b9cee6e 100644\n> > --- a/src/qcam/viewfinder_qt.h\n> > +++ b/src/qcam/viewfinder_qt.h\n> > @@ -32,7 +32,7 @@ public:\n> > \tconst QList &nativeFormats() const override;\n> > \n> > \tint setFormat(const libcamera::PixelFormat &format, const QSize &size) override;\n> > -\tvoid render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override;\n> > +\tvoid render(libcamera::FrameBuffer *buffer, libcamera::Span mem) override;\n> > \tvoid stop() override;\n> > \n> > \tQImage getCurrentImage() override;","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 66993BD87D\n\tfor ;\n\tWed, 18 Aug 2021 08:49:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B3190688A3;\n\tWed, 18 Aug 2021 10:49:02 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DE8A66025E\n\tfor ;\n\tWed, 18 Aug 2021 10:49:01 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5836B466;\n\tWed, 18 Aug 2021 10:49:01 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"QG0VkRkH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629276541;\n\tbh=xSVijLozEk3hC7xEbnLj/RH84o5nprigYjceTMrw3MI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=QG0VkRkHlo8LvlWrKgtcbr+Xdql0t/RWx3wRzyPKwmtHGV4WAlZVUpTzATwqGACFz\n\tSnD/a7KYpLJPMSm/F1J0CJtft70QtD86mhsHGhlE7FQ8a0ugu+sC6QxIXmEGubWQoB\n\thDPlw83C9bInaKDO4Y1l66bFRl0F/qfg7QvDvh9Q=","Date":"Wed, 18 Aug 2021 11:48:54 +0300","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20210818002208.13631-1-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] qcam: Replace MappedBuffer with\n\tSpan","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18900,"web_url":"https://patchwork.libcamera.org/comment/18900/","msgid":"<04c60554-fa5b-a1e7-3c8e-736ff9d0fc87@ideasonboard.com>","date":"2021-08-18T09:02:40","subject":"Re: [libcamera-devel] [PATCH] qcam: Replace MappedBuffer with\n\tSpan","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 18/08/2021 01:22, Laurent Pinchart wrote:\n> The MappedBuffer structure is a custom container that binds a data\n> pointer with a length. This is exactly what Span is. Use it instead.\n\nIt also potentially conflicts (thought different namespaces, and code\ntree so it's fine) with the one in mapped_framebuffer.h - so I'm happy\nto see this change.\n\nOf course I see potential to use MappedFrameBuffer in the future, if it\never gets exposed as an API (either through libcamera, or a helpers\nlibrary) to avoid duplication of support for mapping buffers, and allow\nsupport for multiplanar formats... but that's all futureware so:\n\nReviewed-by: Kieran Bingham \n\n\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> src/qcam/main_window.cpp | 17 +++++++++--------\n> src/qcam/main_window.h | 2 +-\n> src/qcam/viewfinder.h | 9 +++------\n> src/qcam/viewfinder_gl.cpp | 5 +++--\n> src/qcam/viewfinder_gl.h | 2 +-\n> src/qcam/viewfinder_qt.cpp | 5 +++--\n> src/qcam/viewfinder_qt.h | 2 +-\n> 7 files changed, 21 insertions(+), 21 deletions(-)\n> \n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index 39d034de6bb2..1adaae60d83b 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -474,7 +474,8 @@ int MainWindow::startCapture()\n> \t\t\tconst FrameBuffer::Plane &plane = buffer->planes().front();\n> \t\t\tvoid *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,\n> \t\t\t\t\t plane.fd.fd(), 0);\n> -\t\t\tmappedBuffers_[buffer.get()] = { memory, plane.length };\n> +\t\t\tmappedBuffers_[buffer.get()] = { static_cast(memory),\n> +\t\t\t\t\t\t\t plane.length };\n> \n> \t\t\t/* Store buffers on the free list. */\n> \t\t\tfreeBuffers_[stream].enqueue(buffer.get());\n> @@ -537,8 +538,8 @@ error:\n> \trequests_.clear();\n> \n> \tfor (auto &iter : mappedBuffers_) {\n> -\t\tconst MappedBuffer &buffer = iter.second;\n> -\t\tmunmap(buffer.memory, buffer.size);\n> +\t\tconst Span &buffer = iter.second;\n> +\t\tmunmap(buffer.data(), buffer.size());\n> \t}\n> \tmappedBuffers_.clear();\n> \n> @@ -573,8 +574,8 @@ void MainWindow::stopCapture()\n> \tcamera_->requestCompleted.disconnect(this, &MainWindow::requestComplete);\n> \n> \tfor (auto &iter : mappedBuffers_) {\n> -\t\tconst MappedBuffer &buffer = iter.second;\n> -\t\tmunmap(buffer.memory, buffer.size);\n> +\t\tconst Span &buffer = iter.second;\n> +\t\tmunmap(buffer.data(), buffer.size());\n> \t}\n> \tmappedBuffers_.clear();\n> \n> @@ -673,10 +674,10 @@ void MainWindow::processRaw(FrameBuffer *buffer,\n> \t\t\t\t\t\t\t\"DNG Files (*.dng)\");\n> \n> \tif (!filename.isEmpty()) {\n> -\t\tconst MappedBuffer &mapped = mappedBuffers_[buffer];\n> +\t\tconst Span &mapped = mappedBuffers_[buffer];\n> \t\tDNGWriter::write(filename.toStdString().c_str(), camera_.get(),\n> \t\t\t\t rawStream_->configuration(), metadata, buffer,\n> -\t\t\t\t mapped.memory);\n> +\t\t\t\t mapped.data());\n> \t}\n> #endif\n> \n> @@ -753,7 +754,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)\n> \t\t<< \"fps:\" << Qt::fixed << qSetRealNumberPrecision(2) << fps;\n> \n> \t/* Render the frame on the viewfinder. */\n> -\tviewfinder_->render(buffer, &mappedBuffers_[buffer]);\n> +\tviewfinder_->render(buffer, mappedBuffers_[buffer]);\n> }\n> \n> void MainWindow::queueRequest(FrameBuffer *buffer)\n> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h\n> index 85d56ce49abe..6788de8ddde9 100644\n> --- a/src/qcam/main_window.h\n> +++ b/src/qcam/main_window.h\n> @@ -106,7 +106,7 @@ private:\n> \tFrameBufferAllocator *allocator_;\n> \n> \tstd::unique_ptr config_;\n> -\tstd::map mappedBuffers_;\n> +\tstd::map> mappedBuffers_;\n> \n> \t/* Capture state, buffers queue and statistics */\n> \tbool isCapturing_;\n> diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h\n> index 46747c227d0c..42d40f1f33f0 100644\n> --- a/src/qcam/viewfinder.h\n> +++ b/src/qcam/viewfinder.h\n> @@ -11,14 +11,11 @@\n> #include \n> #include \n> \n> +#include \n> +\n> #include \n> #include \n> \n> -struct MappedBuffer {\n> -\tvoid *memory;\n> -\tsize_t size;\n> -};\n> -\n> class ViewFinder\n> {\n> public:\n> @@ -27,7 +24,7 @@ public:\n> \tvirtual const QList &nativeFormats() const = 0;\n> \n> \tvirtual int setFormat(const libcamera::PixelFormat &format, const QSize &size) = 0;\n> -\tvirtual void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) = 0;\n> +\tvirtual void render(libcamera::FrameBuffer *buffer, libcamera::Span mem) = 0;\n> \tvirtual void stop() = 0;\n> \n> \tvirtual QImage getCurrentImage() = 0;\n> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n> index add87db88207..40226601f9fd 100644\n> --- a/src/qcam/viewfinder_gl.cpp\n> +++ b/src/qcam/viewfinder_gl.cpp\n> @@ -110,7 +110,8 @@ QImage ViewFinderGL::getCurrentImage()\n> \treturn grabFramebuffer();\n> }\n> \n> -void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> +void ViewFinderGL::render(libcamera::FrameBuffer *buffer,\n> +\t\t\t libcamera::Span mem)\n> {\n> \tif (buffer->planes().size() != 1) {\n> \t\tqWarning() << \"Multi-planar buffers are not supported\";\n> @@ -120,7 +121,7 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> \tif (buffer_)\n> \t\trenderComplete(buffer_);\n> \n> -\tdata_ = static_cast(map->memory);\n> +\tdata_ = mem.data();\n> \t/*\n> \t * \\todo Get the stride from the buffer instead of computing it naively\n> \t */\n> diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h\n> index 4a0f8ca58f29..3334549e0be4 100644\n> --- a/src/qcam/viewfinder_gl.h\n> +++ b/src/qcam/viewfinder_gl.h\n> @@ -39,7 +39,7 @@ public:\n> \tconst QList &nativeFormats() const override;\n> \n> \tint setFormat(const libcamera::PixelFormat &format, const QSize &size) override;\n> -\tvoid render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override;\n> +\tvoid render(libcamera::FrameBuffer *buffer, libcamera::Span mem) override;\n> \tvoid stop() override;\n> \n> \tQImage getCurrentImage() override;\n> diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp\n> index e436714c6bdb..efa1d412584b 100644\n> --- a/src/qcam/viewfinder_qt.cpp\n> +++ b/src/qcam/viewfinder_qt.cpp\n> @@ -78,14 +78,15 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format,\n> \treturn 0;\n> }\n> \n> -void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> +void ViewFinderQt::render(libcamera::FrameBuffer *buffer,\n> +\t\t\t libcamera::Span mem)\n> {\n> \tif (buffer->planes().size() != 1) {\n> \t\tqWarning() << \"Multi-planar buffers are not supported\";\n> \t\treturn;\n> \t}\n> \n> -\tunsigned char *memory = static_cast(map->memory);\n> +\tunsigned char *memory = mem.data();\n> \tsize_t size = buffer->metadata().planes[0].bytesused;\n> \n> \t{\n> diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h\n> index 501c72a70dec..1a569b9cee6e 100644\n> --- a/src/qcam/viewfinder_qt.h\n> +++ b/src/qcam/viewfinder_qt.h\n> @@ -32,7 +32,7 @@ public:\n> \tconst QList &nativeFormats() const override;\n> \n> \tint setFormat(const libcamera::PixelFormat &format, const QSize &size) override;\n> -\tvoid render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override;\n> +\tvoid render(libcamera::FrameBuffer *buffer, libcamera::Span mem) override;\n> \tvoid stop() override;\n> \n> \tQImage getCurrentImage() override;\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id CC46ABD87D\n\tfor ;\n\tWed, 18 Aug 2021 09:02:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3707768895;\n\tWed, 18 Aug 2021 11:02:45 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 06D4E6025E\n\tfor ;\n\tWed, 18 Aug 2021 11:02:44 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 76A35466;\n\tWed, 18 Aug 2021 11:02:43 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"mEvZCMFa\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629277363;\n\tbh=A7cK3S3cMD0jzHqpGldvXb/E3fF4sgtJos02pW/A1mQ=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=mEvZCMFaOfaSyfwdic1MKqPgoUPYqKqRkFAIKeBULzEKs4LdiY0PP4Vm5k1/xElBd\n\t9gfOz0q13sP6NpJn4FiMlCsgpOZmQbZW3gP7jRQ4jy2ID1BTOaB0Lu56H6kxu5X3C3\n\tHx2806b1Kg6ave0URlIwaRg3M1zbrB8bviUwPicw=","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210818002208.13631-1-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham ","Message-ID":"<04c60554-fa5b-a1e7-3c8e-736ff9d0fc87@ideasonboard.com>","Date":"Wed, 18 Aug 2021 10:02:40 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.11.0","MIME-Version":"1.0","In-Reply-To":"<20210818002208.13631-1-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH] qcam: Replace MappedBuffer with\n\tSpan","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18901,"web_url":"https://patchwork.libcamera.org/comment/18901/","msgid":"","date":"2021-08-18T09:08:33","subject":"Re: [libcamera-devel] [PATCH] qcam: Replace MappedBuffer with\n\tSpan","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Wed, Aug 18, 2021 at 10:02:40AM +0100, Kieran Bingham wrote:\n> On 18/08/2021 01:22, Laurent Pinchart wrote:\n> > The MappedBuffer structure is a custom container that binds a data\n> > pointer with a length. This is exactly what Span is. Use it instead.\n> \n> It also potentially conflicts (thought different namespaces, and code\n> tree so it's fine) with the one in mapped_framebuffer.h - so I'm happy\n> to see this change.\n> \n> Of course I see potential to use MappedFrameBuffer in the future, if it\n> ever gets exposed as an API (either through libcamera, or a helpers\n> library) to avoid duplication of support for mapping buffers, and allow\n> support for multiplanar formats... but that's all futureware so:\n\nI have an increasingly strong feeling it will happen one way or another\n:-)\n\n> Reviewed-by: Kieran Bingham \n> \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/qcam/main_window.cpp | 17 +++++++++--------\n> > src/qcam/main_window.h | 2 +-\n> > src/qcam/viewfinder.h | 9 +++------\n> > src/qcam/viewfinder_gl.cpp | 5 +++--\n> > src/qcam/viewfinder_gl.h | 2 +-\n> > src/qcam/viewfinder_qt.cpp | 5 +++--\n> > src/qcam/viewfinder_qt.h | 2 +-\n> > 7 files changed, 21 insertions(+), 21 deletions(-)\n> > \n> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> > index 39d034de6bb2..1adaae60d83b 100644\n> > --- a/src/qcam/main_window.cpp\n> > +++ b/src/qcam/main_window.cpp\n> > @@ -474,7 +474,8 @@ int MainWindow::startCapture()\n> > \t\t\tconst FrameBuffer::Plane &plane = buffer->planes().front();\n> > \t\t\tvoid *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,\n> > \t\t\t\t\t plane.fd.fd(), 0);\n> > -\t\t\tmappedBuffers_[buffer.get()] = { memory, plane.length };\n> > +\t\t\tmappedBuffers_[buffer.get()] = { static_cast(memory),\n> > +\t\t\t\t\t\t\t plane.length };\n> > \n> > \t\t\t/* Store buffers on the free list. */\n> > \t\t\tfreeBuffers_[stream].enqueue(buffer.get());\n> > @@ -537,8 +538,8 @@ error:\n> > \trequests_.clear();\n> > \n> > \tfor (auto &iter : mappedBuffers_) {\n> > -\t\tconst MappedBuffer &buffer = iter.second;\n> > -\t\tmunmap(buffer.memory, buffer.size);\n> > +\t\tconst Span &buffer = iter.second;\n> > +\t\tmunmap(buffer.data(), buffer.size());\n> > \t}\n> > \tmappedBuffers_.clear();\n> > \n> > @@ -573,8 +574,8 @@ void MainWindow::stopCapture()\n> > \tcamera_->requestCompleted.disconnect(this, &MainWindow::requestComplete);\n> > \n> > \tfor (auto &iter : mappedBuffers_) {\n> > -\t\tconst MappedBuffer &buffer = iter.second;\n> > -\t\tmunmap(buffer.memory, buffer.size);\n> > +\t\tconst Span &buffer = iter.second;\n> > +\t\tmunmap(buffer.data(), buffer.size());\n> > \t}\n> > \tmappedBuffers_.clear();\n> > \n> > @@ -673,10 +674,10 @@ void MainWindow::processRaw(FrameBuffer *buffer,\n> > \t\t\t\t\t\t\t\"DNG Files (*.dng)\");\n> > \n> > \tif (!filename.isEmpty()) {\n> > -\t\tconst MappedBuffer &mapped = mappedBuffers_[buffer];\n> > +\t\tconst Span &mapped = mappedBuffers_[buffer];\n> > \t\tDNGWriter::write(filename.toStdString().c_str(), camera_.get(),\n> > \t\t\t\t rawStream_->configuration(), metadata, buffer,\n> > -\t\t\t\t mapped.memory);\n> > +\t\t\t\t mapped.data());\n> > \t}\n> > #endif\n> > \n> > @@ -753,7 +754,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)\n> > \t\t<< \"fps:\" << Qt::fixed << qSetRealNumberPrecision(2) << fps;\n> > \n> > \t/* Render the frame on the viewfinder. */\n> > -\tviewfinder_->render(buffer, &mappedBuffers_[buffer]);\n> > +\tviewfinder_->render(buffer, mappedBuffers_[buffer]);\n> > }\n> > \n> > void MainWindow::queueRequest(FrameBuffer *buffer)\n> > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h\n> > index 85d56ce49abe..6788de8ddde9 100644\n> > --- a/src/qcam/main_window.h\n> > +++ b/src/qcam/main_window.h\n> > @@ -106,7 +106,7 @@ private:\n> > \tFrameBufferAllocator *allocator_;\n> > \n> > \tstd::unique_ptr config_;\n> > -\tstd::map mappedBuffers_;\n> > +\tstd::map> mappedBuffers_;\n> > \n> > \t/* Capture state, buffers queue and statistics */\n> > \tbool isCapturing_;\n> > diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h\n> > index 46747c227d0c..42d40f1f33f0 100644\n> > --- a/src/qcam/viewfinder.h\n> > +++ b/src/qcam/viewfinder.h\n> > @@ -11,14 +11,11 @@\n> > #include \n> > #include \n> > \n> > +#include \n> > +\n> > #include \n> > #include \n> > \n> > -struct MappedBuffer {\n> > -\tvoid *memory;\n> > -\tsize_t size;\n> > -};\n> > -\n> > class ViewFinder\n> > {\n> > public:\n> > @@ -27,7 +24,7 @@ public:\n> > \tvirtual const QList &nativeFormats() const = 0;\n> > \n> > \tvirtual int setFormat(const libcamera::PixelFormat &format, const QSize &size) = 0;\n> > -\tvirtual void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) = 0;\n> > +\tvirtual void render(libcamera::FrameBuffer *buffer, libcamera::Span mem) = 0;\n> > \tvirtual void stop() = 0;\n> > \n> > \tvirtual QImage getCurrentImage() = 0;\n> > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n> > index add87db88207..40226601f9fd 100644\n> > --- a/src/qcam/viewfinder_gl.cpp\n> > +++ b/src/qcam/viewfinder_gl.cpp\n> > @@ -110,7 +110,8 @@ QImage ViewFinderGL::getCurrentImage()\n> > \treturn grabFramebuffer();\n> > }\n> > \n> > -void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> > +void ViewFinderGL::render(libcamera::FrameBuffer *buffer,\n> > +\t\t\t libcamera::Span mem)\n> > {\n> > \tif (buffer->planes().size() != 1) {\n> > \t\tqWarning() << \"Multi-planar buffers are not supported\";\n> > @@ -120,7 +121,7 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> > \tif (buffer_)\n> > \t\trenderComplete(buffer_);\n> > \n> > -\tdata_ = static_cast(map->memory);\n> > +\tdata_ = mem.data();\n> > \t/*\n> > \t * \\todo Get the stride from the buffer instead of computing it naively\n> > \t */\n> > diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h\n> > index 4a0f8ca58f29..3334549e0be4 100644\n> > --- a/src/qcam/viewfinder_gl.h\n> > +++ b/src/qcam/viewfinder_gl.h\n> > @@ -39,7 +39,7 @@ public:\n> > \tconst QList &nativeFormats() const override;\n> > \n> > \tint setFormat(const libcamera::PixelFormat &format, const QSize &size) override;\n> > -\tvoid render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override;\n> > +\tvoid render(libcamera::FrameBuffer *buffer, libcamera::Span mem) override;\n> > \tvoid stop() override;\n> > \n> > \tQImage getCurrentImage() override;\n> > diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp\n> > index e436714c6bdb..efa1d412584b 100644\n> > --- a/src/qcam/viewfinder_qt.cpp\n> > +++ b/src/qcam/viewfinder_qt.cpp\n> > @@ -78,14 +78,15 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format,\n> > \treturn 0;\n> > }\n> > \n> > -void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> > +void ViewFinderQt::render(libcamera::FrameBuffer *buffer,\n> > +\t\t\t libcamera::Span mem)\n> > {\n> > \tif (buffer->planes().size() != 1) {\n> > \t\tqWarning() << \"Multi-planar buffers are not supported\";\n> > \t\treturn;\n> > \t}\n> > \n> > -\tunsigned char *memory = static_cast(map->memory);\n> > +\tunsigned char *memory = mem.data();\n> > \tsize_t size = buffer->metadata().planes[0].bytesused;\n> > \n> > \t{\n> > diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h\n> > index 501c72a70dec..1a569b9cee6e 100644\n> > --- a/src/qcam/viewfinder_qt.h\n> > +++ b/src/qcam/viewfinder_qt.h\n> > @@ -32,7 +32,7 @@ public:\n> > \tconst QList &nativeFormats() const override;\n> > \n> > \tint setFormat(const libcamera::PixelFormat &format, const QSize &size) override;\n> > -\tvoid render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override;\n> > +\tvoid render(libcamera::FrameBuffer *buffer, libcamera::Span mem) override;\n> > \tvoid stop() override;\n> > \n> > \tQImage getCurrentImage() override;","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E5B8ABD87D\n\tfor ;\n\tWed, 18 Aug 2021 09:08:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6B9FE68895;\n\tWed, 18 Aug 2021 11:08:42 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3252B6025E\n\tfor ;\n\tWed, 18 Aug 2021 11:08:41 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9B869466;\n\tWed, 18 Aug 2021 11:08:40 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Pmgmcrou\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629277720;\n\tbh=8C9boH64jGF+zcFI8VwMYIJ9Gvi2MfgRXaqCfpV24H0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=PmgmcroudXQXuytTSbfqt8a7vsqxaoI58JK8hHON/pK7S4KS+efn9U+q52CyBEdcC\n\tMPWeC19uCvQtUsbyO8CkaMVAuE3sOBW0WG7Vw0WguQsJz7+d5RPxUMhovKhmxLKoEs\n\tfd5cU5mcEJcy50Vbq2JwYbwAQQxglILaI9iNGBoY=","Date":"Wed, 18 Aug 2021 12:08:33 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210818002208.13631-1-laurent.pinchart@ideasonboard.com>\n\t<04c60554-fa5b-a1e7-3c8e-736ff9d0fc87@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<04c60554-fa5b-a1e7-3c8e-736ff9d0fc87@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] qcam: Replace MappedBuffer with\n\tSpan","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18968,"web_url":"https://patchwork.libcamera.org/comment/18968/","msgid":"","date":"2021-08-20T08:49:40","subject":"Re: [libcamera-devel] [PATCH] qcam: Replace MappedBuffer with\n\tSpan","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Laurent, thank you for the patch.\n\nOn Wed, Aug 18, 2021 at 6:08 PM Laurent Pinchart\n wrote:\n>\n> Hi Kieran,\n>\n> On Wed, Aug 18, 2021 at 10:02:40AM +0100, Kieran Bingham wrote:\n> > On 18/08/2021 01:22, Laurent Pinchart wrote:\n> > > The MappedBuffer structure is a custom container that binds a data\n> > > pointer with a length. This is exactly what Span is. Use it instead.\n> >\n> > It also potentially conflicts (thought different namespaces, and code\n> > tree so it's fine) with the one in mapped_framebuffer.h - so I'm happy\n> > to see this change.\n> >\n> > Of course I see potential to use MappedFrameBuffer in the future, if it\n> > ever gets exposed as an API (either through libcamera, or a helpers\n> > library) to avoid duplication of support for mapping buffers, and allow\n> > support for multiplanar formats... but that's all futureware so:\n>\n> I have an increasingly strong feeling it will happen one way or another\n> :-)\n>\n> > Reviewed-by: Kieran Bingham \n> >\n> > > Signed-off-by: Laurent Pinchart \n\nReviewed-by: Hirokazu Honda \n> > > ---\n> > > src/qcam/main_window.cpp | 17 +++++++++--------\n> > > src/qcam/main_window.h | 2 +-\n> > > src/qcam/viewfinder.h | 9 +++------\n> > > src/qcam/viewfinder_gl.cpp | 5 +++--\n> > > src/qcam/viewfinder_gl.h | 2 +-\n> > > src/qcam/viewfinder_qt.cpp | 5 +++--\n> > > src/qcam/viewfinder_qt.h | 2 +-\n> > > 7 files changed, 21 insertions(+), 21 deletions(-)\n> > >\n> > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> > > index 39d034de6bb2..1adaae60d83b 100644\n> > > --- a/src/qcam/main_window.cpp\n> > > +++ b/src/qcam/main_window.cpp\n> > > @@ -474,7 +474,8 @@ int MainWindow::startCapture()\n> > > const FrameBuffer::Plane &plane = buffer->planes().front();\n> > > void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,\n> > > plane.fd.fd(), 0);\n> > > - mappedBuffers_[buffer.get()] = { memory, plane.length };\n> > > + mappedBuffers_[buffer.get()] = { static_cast(memory),\n> > > + plane.length };\n> > >\n> > > /* Store buffers on the free list. */\n> > > freeBuffers_[stream].enqueue(buffer.get());\n> > > @@ -537,8 +538,8 @@ error:\n> > > requests_.clear();\n> > >\n> > > for (auto &iter : mappedBuffers_) {\n> > > - const MappedBuffer &buffer = iter.second;\n> > > - munmap(buffer.memory, buffer.size);\n> > > + const Span &buffer = iter.second;\n> > > + munmap(buffer.data(), buffer.size());\n> > > }\n> > > mappedBuffers_.clear();\n> > >\n> > > @@ -573,8 +574,8 @@ void MainWindow::stopCapture()\n> > > camera_->requestCompleted.disconnect(this, &MainWindow::requestComplete);\n> > >\n> > > for (auto &iter : mappedBuffers_) {\n> > > - const MappedBuffer &buffer = iter.second;\n> > > - munmap(buffer.memory, buffer.size);\n> > > + const Span &buffer = iter.second;\n> > > + munmap(buffer.data(), buffer.size());\n> > > }\n> > > mappedBuffers_.clear();\n> > >\n> > > @@ -673,10 +674,10 @@ void MainWindow::processRaw(FrameBuffer *buffer,\n> > > \"DNG Files (*.dng)\");\n> > >\n> > > if (!filename.isEmpty()) {\n> > > - const MappedBuffer &mapped = mappedBuffers_[buffer];\n> > > + const Span &mapped = mappedBuffers_[buffer];\n> > > DNGWriter::write(filename.toStdString().c_str(), camera_.get(),\n> > > rawStream_->configuration(), metadata, buffer,\n> > > - mapped.memory);\n> > > + mapped.data());\n> > > }\n> > > #endif\n> > >\n> > > @@ -753,7 +754,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)\n> > > << \"fps:\" << Qt::fixed << qSetRealNumberPrecision(2) << fps;\n> > >\n> > > /* Render the frame on the viewfinder. */\n> > > - viewfinder_->render(buffer, &mappedBuffers_[buffer]);\n> > > + viewfinder_->render(buffer, mappedBuffers_[buffer]);\n> > > }\n> > >\n> > > void MainWindow::queueRequest(FrameBuffer *buffer)\n> > > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h\n> > > index 85d56ce49abe..6788de8ddde9 100644\n> > > --- a/src/qcam/main_window.h\n> > > +++ b/src/qcam/main_window.h\n> > > @@ -106,7 +106,7 @@ private:\n> > > FrameBufferAllocator *allocator_;\n> > >\n> > > std::unique_ptr config_;\n> > > - std::map mappedBuffers_;\n> > > + std::map> mappedBuffers_;\n> > >\n> > > /* Capture state, buffers queue and statistics */\n> > > bool isCapturing_;\n> > > diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h\n> > > index 46747c227d0c..42d40f1f33f0 100644\n> > > --- a/src/qcam/viewfinder.h\n> > > +++ b/src/qcam/viewfinder.h\n> > > @@ -11,14 +11,11 @@\n> > > #include \n> > > #include \n> > >\n> > > +#include \n> > > +\n> > > #include \n> > > #include \n> > >\n> > > -struct MappedBuffer {\n> > > - void *memory;\n> > > - size_t size;\n> > > -};\n> > > -\n> > > class ViewFinder\n> > > {\n> > > public:\n> > > @@ -27,7 +24,7 @@ public:\n> > > virtual const QList &nativeFormats() const = 0;\n> > >\n> > > virtual int setFormat(const libcamera::PixelFormat &format, const QSize &size) = 0;\n> > > - virtual void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) = 0;\n> > > + virtual void render(libcamera::FrameBuffer *buffer, libcamera::Span mem) = 0;\n> > > virtual void stop() = 0;\n> > >\n> > > virtual QImage getCurrentImage() = 0;\n> > > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n> > > index add87db88207..40226601f9fd 100644\n> > > --- a/src/qcam/viewfinder_gl.cpp\n> > > +++ b/src/qcam/viewfinder_gl.cpp\n> > > @@ -110,7 +110,8 @@ QImage ViewFinderGL::getCurrentImage()\n> > > return grabFramebuffer();\n> > > }\n> > >\n> > > -void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> > > +void ViewFinderGL::render(libcamera::FrameBuffer *buffer,\n> > > + libcamera::Span mem)\n> > > {\n> > > if (buffer->planes().size() != 1) {\n> > > qWarning() << \"Multi-planar buffers are not supported\";\n> > > @@ -120,7 +121,7 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> > > if (buffer_)\n> > > renderComplete(buffer_);\n> > >\n> > > - data_ = static_cast(map->memory);\n> > > + data_ = mem.data();\n> > > /*\n> > > * \\todo Get the stride from the buffer instead of computing it naively\n> > > */\n> > > diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h\n> > > index 4a0f8ca58f29..3334549e0be4 100644\n> > > --- a/src/qcam/viewfinder_gl.h\n> > > +++ b/src/qcam/viewfinder_gl.h\n> > > @@ -39,7 +39,7 @@ public:\n> > > const QList &nativeFormats() const override;\n> > >\n> > > int setFormat(const libcamera::PixelFormat &format, const QSize &size) override;\n> > > - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override;\n> > > + void render(libcamera::FrameBuffer *buffer, libcamera::Span mem) override;\n> > > void stop() override;\n> > >\n> > > QImage getCurrentImage() override;\n> > > diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp\n> > > index e436714c6bdb..efa1d412584b 100644\n> > > --- a/src/qcam/viewfinder_qt.cpp\n> > > +++ b/src/qcam/viewfinder_qt.cpp\n> > > @@ -78,14 +78,15 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format,\n> > > return 0;\n> > > }\n> > >\n> > > -void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> > > +void ViewFinderQt::render(libcamera::FrameBuffer *buffer,\n> > > + libcamera::Span mem)\n> > > {\n> > > if (buffer->planes().size() != 1) {\n> > > qWarning() << \"Multi-planar buffers are not supported\";\n> > > return;\n> > > }\n> > >\n> > > - unsigned char *memory = static_cast(map->memory);\n> > > + unsigned char *memory = mem.data();\n> > > size_t size = buffer->metadata().planes[0].bytesused;\n> > >\n> > > {\n> > > diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h\n> > > index 501c72a70dec..1a569b9cee6e 100644\n> > > --- a/src/qcam/viewfinder_qt.h\n> > > +++ b/src/qcam/viewfinder_qt.h\n> > > @@ -32,7 +32,7 @@ public:\n> > > const QList &nativeFormats() const override;\n> > >\n> > > int setFormat(const libcamera::PixelFormat &format, const QSize &size) override;\n> > > - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override;\n> > > + void render(libcamera::FrameBuffer *buffer, libcamera::Span mem) override;\n> > > void stop() override;\n> > >\n> > > QImage getCurrentImage() override;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C1FFFBD87D\n\tfor ;\n\tFri, 20 Aug 2021 08:49:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2B0BE68895;\n\tFri, 20 Aug 2021 10:49:53 +0200 (CEST)","from mail-ed1-x52f.google.com (mail-ed1-x52f.google.com\n\t[IPv6:2a00:1450:4864:20::52f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 00DFF68890\n\tfor ;\n\tFri, 20 Aug 2021 10:49:51 +0200 (CEST)","by mail-ed1-x52f.google.com with SMTP id s3so658250edd.11\n\tfor ;\n\tFri, 20 Aug 2021 01:49:51 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"S+hNgW9w\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=ND7Zzn6f6kcJt+6pUgg74CJS/fp5ixiDV2+UiwdZgpM=;\n\tb=S+hNgW9wQIRmh2xSYjL9RDp6/ENTDnOfFvsIKm3+79YHnE5uTS6/1QY9dlBJLFHVpf\n\t7GoaUG2UUgQG1ea/GzF4r6M9l9zkkYDioxY5zH3UPcFc4YjI/x5C0wrC6jc1sbAWnuRc\n\t2Zb1H5TN58wDMzPdtuHgsqWSkyOTzpvqlv1/I=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=ND7Zzn6f6kcJt+6pUgg74CJS/fp5ixiDV2+UiwdZgpM=;\n\tb=C/X/PDH7jfop6GrTYMbQQFo3ZOGTY7q7ZbigbzbCi7hk2V9OmBpJqPJhuvIHfo7ZL8\n\tSUb3fXHrHf3Epd4BezMdTC/hS2nq5CwpG45mVssDRb5JDCx6KgzerC68avjlxLxqSeu+\n\t+3WqrM4bcfIB+lh9THHP/j5f4VutiUylMYbsXLSgvFW7GKIYiYVtFTjRqFaztaOkJUGh\n\tb11hDHVTgmEszDO8Xt7N+cmFxx7Y9c/PLEM0u9PXUYPIkC4+MmPV6BcPSO3PsVr2jY1m\n\tzy8PIAeCaxndW2Y5aD5PebrAaZXPrYG0Zx/2i15dqcRFIZ9tWd27F/c1Ujgu2hngA7Hu\n\tR60Q==","X-Gm-Message-State":"AOAM531wu86rHVnLN2uTwoN9OYVH7KCoa8yidRQvvrbVQK3QIi9snZP+\n\teRstYMujWfztAQUVphRKfvzODf4uieyxmw3Fg0Dgrw==","X-Google-Smtp-Source":"ABdhPJyrwRMZ74RgduxvXECSAirLvZQVve67wJz7o6ldUjllF5TnPflcke3y4VJnMt34BITjfFaZlpMTSuWx2Qm9NIM=","X-Received":"by 2002:a50:ef11:: with SMTP id\n\tm17mr20855053eds.233.1629449391590; \n\tFri, 20 Aug 2021 01:49:51 -0700 (PDT)","MIME-Version":"1.0","References":"<20210818002208.13631-1-laurent.pinchart@ideasonboard.com>\n\t<04c60554-fa5b-a1e7-3c8e-736ff9d0fc87@ideasonboard.com>\n\t","In-Reply-To":"","From":"Hirokazu Honda ","Date":"Fri, 20 Aug 2021 17:49:40 +0900","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] qcam: Replace MappedBuffer with\n\tSpan","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]