[{"id":18883,"web_url":"https://patchwork.libcamera.org/comment/18883/","msgid":"","date":"2021-08-17T23:55:12","subject":"Re: [libcamera-devel] [RFC PATCH 04/10] cam: file_sink: Use offset\n\tin 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:32PM +0900, Hirokazu Honda wrote:\n> This fixes the way of mapping FrameBuffer in FrameSink by\n> using offset.\n> \n> Signed-off-by: Hirokazu Honda \n> ---\n> src/cam/file_sink.cpp | 5 ++++-\n> src/cam/file_sink.h | 1 +\n> 2 files changed, 5 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/cam/file_sink.cpp b/src/cam/file_sink.cpp\n> index 2d30694a..92b2d53d 100644\n> --- a/src/cam/file_sink.cpp\n> +++ b/src/cam/file_sink.cpp\n> @@ -51,12 +51,15 @@ int FileSink::configure(const libcamera::CameraConfiguration &config)\n> \n> void FileSink::mapBuffer(FrameBuffer *buffer)\n> {\n> +\t/* \\todo use MappedFrameBuffer. */\n> \tfor (const FrameBuffer::Plane &plane : buffer->planes()) {\n> \t\tvoid *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,\n> \t\t\t\t plane.fd.fd(), 0);\n> \n> \t\tmappedBuffers_[plane.fd.fd()] =\n> \t\t\tstd::make_pair(memory, plane.length);\n> +\t\tplaneData_[plane.fd.fd()] =\n> +\t\t\tstatic_cast(memory) + plane.offset;\n\nI don't think this will work. With an NV12 buffer, for instance,\nplane.fd.fd() will be the same for both planes, so you'll overwrite\nplaneData_ of the first plane with the second plane. It's fine in this\npatch, as we don't create FrameBuffer instances with multiple planes,\nbut it will break as soon as we do. Should we instead store the\naddresses in a std::map, void *> (with\nthe unsigned int being the plane index) ? Of maybe just a\nstd::map ?\n\n> \t}\n> }\n> \n> @@ -102,7 +105,7 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer)\n> \t\tconst FrameBuffer::Plane &plane = buffer->planes()[i];\n> \t\tconst FrameMetadata::Plane &meta = buffer->metadata().planes[i];\n> \n> -\t\tvoid *data = mappedBuffers_[plane.fd.fd()].first;\n> +\t\tvoid *data = planeData_[plane.fd.fd()];\n> \t\tunsigned int length = std::min(meta.bytesused, plane.length);\n> \n> \t\tif (meta.bytesused > plane.length)\n> diff --git a/src/cam/file_sink.h b/src/cam/file_sink.h\n> index c3eb230a..d236d6d8 100644\n> --- a/src/cam/file_sink.h\n> +++ b/src/cam/file_sink.h\n> @@ -33,6 +33,7 @@ private:\n> \tstd::map streamNames_;\n> \tstd::string pattern_;\n> \tstd::map> mappedBuffers_;\n> +\tstd::map planeData_;\n> };\n> \n> #endif /* __CAM_FILE_SINK_H__ */","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 6A3BABD87C\n\tfor ;\n\tTue, 17 Aug 2021 23:55:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EE5356025D;\n\tWed, 18 Aug 2021 01:55:21 +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 83B1B6025D\n\tfor ;\n\tWed, 18 Aug 2021 01:55:20 +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 F27B5499;\n\tWed, 18 Aug 2021 01:55:19 +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=\"SI3J5Dkh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629244520;\n\tbh=sH4q4XomQoLvUidoiEed9CgJG+ux2hM2uM3V86gAqAE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=SI3J5Dkh93lOb4pwKwvHAvT0mYfZM3ay1iWv4Fy5zAx1MWWycvoM9LM2kEKM7Bdi8\n\tI5ir8WGWs1FlGfGxX0NBlTgEO43QpNsF/JaQzlXUzRihGhFch5mZz+1WKBbLJZFe2l\n\tVTIqu5p5bx5Pb0wQeSmYdsbHAlpVfYUtyjlM/RQQ=","Date":"Wed, 18 Aug 2021 02:55:12 +0300","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20210816043138.957984-1-hiroh@chromium.org>\n\t<20210816043138.957984-5-hiroh@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210816043138.957984-5-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [RFC PATCH 04/10] cam: file_sink: Use offset\n\tin 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\" "}}]