From patchwork Mon Aug 9 13:29:29 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 13260 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 72BF5C3240 for ; Mon, 9 Aug 2021 13:29:38 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 10E2A6888F; Mon, 9 Aug 2021 15:29:37 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="MyxhvDJl"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 364F6687EB for ; Mon, 9 Aug 2021 15:29:34 +0200 (CEST) Received: from Monstersaurus.local (cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id E4038466; Mon, 9 Aug 2021 15:29:33 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1628515774; bh=iUclvwjcSIfmMFqvje7WD4iCW7ht+/NxdX9uVklz3uA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=MyxhvDJluQJpd7zIhuTEAxp62z7ouySlNu+2fug/ZhR/xLuf+Gj32ZXqyQQGgvtg0 N14hNdB5HNF6bh6L4KPvZF1NXcoVARN2SeDcvCqi4KLha19cU7p2to1QRG/AgnqckU tJfkBK5cslun6LFx5VRZOCXJWA3AlBye1qU9ACPQ= From: Kieran Bingham To: libcamera devel Date: Mon, 9 Aug 2021 14:29:29 +0100 Message-Id: <20210809132929.1824114-4-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210809132929.1824114-1-kieran.bingham@ideasonboard.com> References: <20210809132929.1824114-1-kieran.bingham@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 3/3] libcamera: MappedFrameBuffer: Use typed Flags X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Remove the need for callers to reference PROT_READ/PROT_WRITE directly from by instead exposing the Read/Write mapping options as flags from the MappedFrameBuffer class itself. While here, introduce the header which is required for the uint8_t as part of the Plane. Signed-off-by: Kieran Bingham Reviewed-by: Laurent Pinchart Reviewed-by: Hirokazu Honda --- v2 - Use fully scoped enum class - Swap MapMode and MapModes - s/mmap_flags/mmapFlags/ - Remove and fix documentation regarding the modes - add LIBCAMERA_FLAGS_ENABLE_OPERATORS() v3: - Remove no longer necessary sys/mman.h inclusions throughout - fix documentation - rename MapMode as MapFlag and MapModes as MapFlags --- .../libcamera/internal/mapped_framebuffer.h | 15 ++++++-- src/android/jpeg/encoder_libjpeg.cpp | 3 +- src/android/jpeg/thumbnailer.cpp | 2 +- src/android/yuv/post_processor_yuv.cpp | 2 +- src/ipa/ipu3/ipu3.cpp | 3 +- src/ipa/raspberrypi/raspberrypi.cpp | 3 +- src/libcamera/framebuffer.cpp | 1 - src/libcamera/mapped_framebuffer.cpp | 34 ++++++++++++++++--- test/mapped-buffer.cpp | 6 ++-- 9 files changed, 51 insertions(+), 18 deletions(-) diff --git a/include/libcamera/internal/mapped_framebuffer.h b/include/libcamera/internal/mapped_framebuffer.h index 41e587364260..68c8a43c35dc 100644 --- a/include/libcamera/internal/mapped_framebuffer.h +++ b/include/libcamera/internal/mapped_framebuffer.h @@ -7,10 +7,11 @@ #ifndef __LIBCAMERA_INTERNAL_MAPPED_FRAMEBUFFER_H__ #define __LIBCAMERA_INTERNAL_MAPPED_FRAMEBUFFER_H__ -#include +#include #include #include +#include #include #include @@ -44,9 +45,19 @@ private: class MappedFrameBuffer : public MappedBuffer { public: - MappedFrameBuffer(const FrameBuffer *buffer, int flags); + enum class MapFlag { + Read = 0 << 1, + Write = 1 << 1, + ReadWrite = Read | Write, + }; + + using MapFlags = Flags; + + MappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags); }; +LIBCAMERA_FLAGS_ENABLE_OPERATORS(MappedFrameBuffer::MapFlag) + } /* namespace libcamera */ #endif /* __LIBCAMERA_INTERNAL_MAPPED_FRAMEBUFFER_H__ */ diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp index 372018d2207f..a7a636011045 100644 --- a/src/android/jpeg/encoder_libjpeg.cpp +++ b/src/android/jpeg/encoder_libjpeg.cpp @@ -12,7 +12,6 @@ #include #include #include -#include #include #include @@ -182,7 +181,7 @@ void EncoderLibJpeg::compressNV(Span frame) int EncoderLibJpeg::encode(const FrameBuffer &source, Span dest, Span exifData, unsigned int quality) { - MappedFrameBuffer frame(&source, PROT_READ); + MappedFrameBuffer frame(&source, MappedFrameBuffer::MapFlag::Read); if (!frame.isValid()) { LOG(JPEG, Error) << "Failed to map FrameBuffer : " << strerror(frame.error()); diff --git a/src/android/jpeg/thumbnailer.cpp b/src/android/jpeg/thumbnailer.cpp index 535e2cece914..79d83926af5c 100644 --- a/src/android/jpeg/thumbnailer.cpp +++ b/src/android/jpeg/thumbnailer.cpp @@ -41,7 +41,7 @@ void Thumbnailer::createThumbnail(const FrameBuffer &source, const Size &targetSize, std::vector *destination) { - MappedFrameBuffer frame(&source, PROT_READ); + MappedFrameBuffer frame(&source, MappedFrameBuffer::MapFlag::Read); if (!frame.isValid()) { LOG(Thumbnailer, Error) << "Failed to map FrameBuffer : " diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp index 509d4244d614..3e793a580377 100644 --- a/src/android/yuv/post_processor_yuv.cpp +++ b/src/android/yuv/post_processor_yuv.cpp @@ -57,7 +57,7 @@ int PostProcessorYuv::process(const FrameBuffer &source, if (!isValidBuffers(source, *destination)) return -EINVAL; - const MappedFrameBuffer sourceMapped(&source, PROT_READ); + const MappedFrameBuffer sourceMapped(&source, MappedFrameBuffer::MapFlag::Read); if (!sourceMapped.isValid()) { LOG(YUV, Error) << "Failed to mmap camera frame buffer"; return -EINVAL; diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index 2647bf2f3b96..10093acdad30 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -6,7 +6,6 @@ */ #include -#include #include #include @@ -211,7 +210,7 @@ void IPAIPU3::mapBuffers(const std::vector &buffers) for (const IPABuffer &buffer : buffers) { const FrameBuffer fb(buffer.planes); buffers_.emplace(buffer.id, - MappedFrameBuffer(&fb, PROT_READ | PROT_WRITE)); + MappedFrameBuffer(&fb, MappedFrameBuffer::MapFlag::ReadWrite)); } } diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index 76f67dd4567a..592e2fe41617 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -411,7 +411,8 @@ void IPARPi::mapBuffers(const std::vector &buffers) { for (const IPABuffer &buffer : buffers) { const FrameBuffer fb(buffer.planes); - buffers_.emplace(buffer.id, MappedFrameBuffer(&fb, PROT_READ | PROT_WRITE)); + buffers_.emplace(buffer.id, + MappedFrameBuffer(&fb, MappedFrameBuffer::MapFlag::ReadWrite)); } } diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp index 3d98affb20f9..2223e24321ca 100644 --- a/src/libcamera/framebuffer.cpp +++ b/src/libcamera/framebuffer.cpp @@ -10,7 +10,6 @@ #include #include -#include #include #include diff --git a/src/libcamera/mapped_framebuffer.cpp b/src/libcamera/mapped_framebuffer.cpp index 0e30fc542154..5a70c18147f2 100644 --- a/src/libcamera/mapped_framebuffer.cpp +++ b/src/libcamera/mapped_framebuffer.cpp @@ -142,21 +142,45 @@ MappedBuffer::~MappedBuffer() * \brief Map a FrameBuffer using the MappedBuffer interface */ +/** + * \enum MappedFrameBuffer::MapFlag + * \brief Specify the mapping mode for the FrameBuffer + * \var MappedFrameBuffer::Read + * \brief Create a read-only mapping + * \var MappedFrameBuffer::Write + * \brief Create a write-only mapping + * \var MappedFrameBuffer::ReadWrite + * \brief Create a mapping that can be both read and written + */ + +/** + * \typedef MappedFrameBuffer::MapFlags + * \brief A bitwise combination of MappedFrameBuffer::MapFlag values + */ + /** * \brief Map all planes of a FrameBuffer * \param[in] buffer FrameBuffer to be mapped * \param[in] flags Protection flags to apply to map * - * Construct an object to map a frame buffer for CPU access. - * The flags are passed directly to mmap and should be either PROT_READ, - * PROT_WRITE, or a bitwise-or combination of both. + * Construct an object to map a frame buffer for CPU access. The mapping can be + * made as Read only, Write only or support Read and Write operations by setting + * the MapFlag flags accordingly. */ -MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, int flags) +MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags) { maps_.reserve(buffer->planes().size()); + int mmapFlags = 0; + + if (flags & MapFlag::Read) + mmapFlags |= PROT_READ; + + if (flags & MapFlag::Write) + mmapFlags |= PROT_WRITE; + for (const FrameBuffer::Plane &plane : buffer->planes()) { - void *address = mmap(nullptr, plane.length, flags, + void *address = mmap(nullptr, plane.length, mmapFlags, MAP_SHARED, plane.fd.fd(), 0); if (address == MAP_FAILED) { error_ = -errno; diff --git a/test/mapped-buffer.cpp b/test/mapped-buffer.cpp index a3d1511b74ce..97571945cbca 100644 --- a/test/mapped-buffer.cpp +++ b/test/mapped-buffer.cpp @@ -71,7 +71,7 @@ protected: const std::unique_ptr &buffer = allocator_->buffers(stream_).front(); std::vector maps; - MappedFrameBuffer map(buffer.get(), PROT_READ); + MappedFrameBuffer map(buffer.get(), MappedFrameBuffer::MapFlag::Read); if (!map.isValid()) { cout << "Failed to successfully map buffer" << endl; return TestFail; @@ -90,13 +90,13 @@ protected: } /* Test for multiple successful maps on the same buffer. */ - MappedFrameBuffer write_map(buffer.get(), PROT_WRITE); + MappedFrameBuffer write_map(buffer.get(), MappedFrameBuffer::MapFlag::Write); if (!write_map.isValid()) { cout << "Failed to map write buffer" << endl; return TestFail; } - MappedFrameBuffer rw_map(buffer.get(), PROT_READ | PROT_WRITE); + MappedFrameBuffer rw_map(buffer.get(), MappedFrameBuffer::MapFlag::ReadWrite); if (!rw_map.isValid()) { cout << "Failed to map RW buffer" << endl; return TestFail;