[{"id":18885,"web_url":"https://patchwork.libcamera.org/comment/18885/","msgid":"","date":"2021-08-18T00:27:13","subject":"Re: [libcamera-devel] [RFC PATCH 06/10] qcam: main_window: Use\n\toffset mapping FrameBuffer","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hiro,\n\nThank you for the patch.\n\nOn Mon, Aug 16, 2021 at 01:31:34PM +0900, Hirokazu Honda wrote:\n> FrameBuffer::Plane has offset info now. This uses the offset\n> in mapping FrameBuffer in MainWindow.\n> \n> Signed-off-by: Hirokazu Honda \n> ---\n> src/qcam/main_window.cpp | 15 ++++++++++-----\n> src/qcam/main_window.h | 1 +\n> 2 files changed, 11 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index 39d034de..f815d281 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -10,6 +10,7 @@\n> #include \n> #include \n> #include \n> +#include \n> \n> #include \n> #include \n> @@ -472,9 +473,11 @@ int MainWindow::startCapture()\n> \t\tfor (const std::unique_ptr &buffer : allocator_->buffers(stream)) {\n> \t\t\t/* Map memory buffers and cache the mappings. */\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\tsize_t length = lseek(plane.fd.fd(), 0, SEEK_END);\n> +\t\t\tvoid *memory = mmap(NULL, 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()] = { memory, length };\n> +\t\t\tplaneData_[buffer.get()] = { static_cast(memory) + plane.offset, plane.length };\n\nMaybe with line wrap ? Up to you.\n\n\t\t\tplaneData_[buffer.get()] = {\n\t\t\t\tstatic_cast(memory) + plane.offset,\n\t\t\t\tplane.length\n\t\t\t};\n\n> \n> \t\t\t/* Store buffers on the free list. */\n> \t\t\tfreeBuffers_[stream].enqueue(buffer.get());\n> @@ -541,6 +544,7 @@ error:\n> \t\tmunmap(buffer.memory, buffer.size);\n> \t}\n> \tmappedBuffers_.clear();\n> +\tplaneData_.clear();\n> \n> \tfreeBuffers_.clear();\n> \n> @@ -577,6 +581,7 @@ void MainWindow::stopCapture()\n> \t\tmunmap(buffer.memory, buffer.size);\n> \t}\n> \tmappedBuffers_.clear();\n> +\tplaneData_.clear();\n> \n> \trequests_.clear();\n> \tfreeQueue_.clear();\n> @@ -673,10 +678,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\tvoid *memory = planeData_[buffer].memory;\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 memory);\n> \t}\n> #endif\n> \n> @@ -753,7 +758,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, &planeData_[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 85d56ce4..d7ea994c 100644\n> --- a/src/qcam/main_window.h\n> +++ b/src/qcam/main_window.h\n> @@ -107,6 +107,7 @@ private:\n> \n> \tstd::unique_ptr config_;\n> \tstd::map mappedBuffers_;\n> +\tstd::map planeData_;\n\nCould you store the memory in a Span instead of a MappedBuffer\n? I've sent a patch to replace MappedBuffer with Span in qcam, so you'll\nlikely notice when rebasing.\n\nReviewed-by: Laurent Pinchart \n\n> \n> \t/* Capture state, buffers queue and statistics */\n> \tbool isCapturing_;","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 0AD06BD87C\n\tfor ;\n\tWed, 18 Aug 2021 00:27:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6E43E68894;\n\tWed, 18 Aug 2021 02:27:23 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E24876025D\n\tfor ;\n\tWed, 18 Aug 2021 02:27:21 +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 6792C499;\n\tWed, 18 Aug 2021 02:27:21 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"WtPnm7wv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629246441;\n\tbh=OJ3SvS1saDW152QrhuwPCm3h0jZKXujaehnhQ+5HBb0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WtPnm7wvfyzr55srl4QPe/j0iqb+8a8kI9lS0pIqYrMiErBOELfMC1xWKQXTziqms\n\tA5GUpCZcWoyIP0QzsbcMkSfI4EJynhSs0ZV1zd44Ce4JnkZz3bbaFHy40sRCfLXbwh\n\tkZQucu8D3tYRAU7X/Hs06a5QviFXcoJJ/+S/TGss=","Date":"Wed, 18 Aug 2021 03:27:13 +0300","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20210816043138.957984-1-hiroh@chromium.org>\n\t<20210816043138.957984-7-hiroh@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210816043138.957984-7-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [RFC PATCH 06/10] qcam: main_window: Use\n\toffset mapping FrameBuffer","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\" "}}]