[{"id":19091,"web_url":"https://patchwork.libcamera.org/comment/19091/","msgid":"","date":"2021-08-26T11:52:49","subject":"Re: [libcamera-devel] [PATCH v3 1/3] android:\n\tgeneric_camera_buffer: Correct buffer mapping","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 Thu, Aug 26, 2021 at 05:00:19PM +0900, Hirokazu Honda wrote:\n> buffer_handle_t doesn't provide sufficient info to map a buffer\n> properly. cros::CameraBufferManager enables handling the buffer on\n> ChromeOS, but no way is provided for other platforms.\n> \n> Therefore, we put the assumption that planes are in the same buffer\n> and they are consecutive. This modifies the way of mapping in\n> generic_camera_buffer with the assumption.\n> \n> Signed-off-by: Hirokazu Honda \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/android/camera_buffer.h | 14 ++-\n> src/android/camera_stream.cpp | 4 +-\n> src/android/mm/cros_camera_buffer.cpp | 7 +-\n> src/android/mm/generic_camera_buffer.cpp | 103 ++++++++++++++++++-----\n> 4 files changed, 100 insertions(+), 28 deletions(-)\n> \n> diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h\n> index c4e3a9e7..87df2570 100644\n> --- a/src/android/camera_buffer.h\n> +++ b/src/android/camera_buffer.h\n> @@ -11,13 +11,17 @@\n> \n> #include \n> #include \n> +#include \n> +#include \n> \n> class CameraBuffer final : public libcamera::Extensible\n> {\n> \tLIBCAMERA_DECLARE_PRIVATE()\n> \n> public:\n> -\tCameraBuffer(buffer_handle_t camera3Buffer, int flags);\n> +\tCameraBuffer(buffer_handle_t camera3Buffer,\n> +\t\t libcamera::PixelFormat pixelFormat,\n> +\t\t const libcamera::Size &size, int flags);\n> \t~CameraBuffer();\n> \n> \tbool isValid() const;\n> @@ -31,8 +35,12 @@ public:\n> };\n> \n> #define PUBLIC_CAMERA_BUFFER_IMPLEMENTATION\t\t\t\t\\\n> -CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, int flags)\t\\\n> -\t: Extensible(std::make_unique(this, camera3Buffer, flags)) \\\n> +CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer,\t\t\\\n> +\t\t\t libcamera::PixelFormat pixelFormat,\t\t\\\n> +\t\t\t const libcamera::Size &size, int flags)\t\\\n> +\t: Extensible(std::make_unique(this, camera3Buffer,\t\\\n> +\t\t\t\t\t pixelFormat, size,\t\\\n> +\t\t\t\t\t flags))\t\t\t\\\n> {\t\t\t\t\t\t\t\t\t\\\n> }\t\t\t\t\t\t\t\t\t\\\n> CameraBuffer::~CameraBuffer()\t\t\t\t\t\t\\\n> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> index 61b44183..01909ec7 100644\n> --- a/src/android/camera_stream.cpp\n> +++ b/src/android/camera_stream.cpp\n> @@ -110,7 +110,9 @@ int CameraStream::process(const libcamera::FrameBuffer &source,\n> \t * \\todo Buffer mapping and processing should be moved to a\n> \t * separate thread.\n> \t */\n> -\tCameraBuffer dest(camera3Dest, PROT_READ | PROT_WRITE);\n> +\tconst StreamConfiguration &output = configuration();\n> +\tCameraBuffer dest(camera3Dest, formats::MJPEG, output.size,\n> +\t\t\t PROT_READ | PROT_WRITE);\n> \tif (!dest.isValid()) {\n> \t\tLOG(HAL, Error) << \"Failed to map android blob buffer\";\n> \t\treturn -EINVAL;\n> diff --git a/src/android/mm/cros_camera_buffer.cpp b/src/android/mm/cros_camera_buffer.cpp\n> index e8783ff8..50732637 100644\n> --- a/src/android/mm/cros_camera_buffer.cpp\n> +++ b/src/android/mm/cros_camera_buffer.cpp\n> @@ -20,8 +20,9 @@ class CameraBuffer::Private : public Extensible::Private\n> \tLIBCAMERA_DECLARE_PUBLIC(CameraBuffer)\n> \n> public:\n> -\tPrivate(CameraBuffer *cameraBuffer,\n> -\t\tbuffer_handle_t camera3Buffer, int flags);\n> +\tPrivate(CameraBuffer *cameraBuffer, buffer_handle_t camera3Buffer,\n> +\t\tlibcamera::PixelFormat pixelFormat, const libcamera::Size &size,\n> +\t\tint flags);\n> \t~Private();\n> \n> \tbool isValid() const { return valid_; }\n> @@ -46,6 +47,8 @@ private:\n> \n> CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer,\n> \t\t\t buffer_handle_t camera3Buffer,\n> +\t\t\t [[maybe_unused]] libcamera::PixelFormat pixelFormat,\n> +\t\t\t [[maybe_unused]] const libcamera::Size &size,\n> \t\t\t [[maybe_unused]] int flags)\n> \t: handle_(camera3Buffer), numPlanes_(0), valid_(false),\n> \t registered_(false)\n> diff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp\n> index b3af194c..22753490 100644\n> --- a/src/android/mm/generic_camera_buffer.cpp\n> +++ b/src/android/mm/generic_camera_buffer.cpp\n> @@ -12,6 +12,7 @@\n> \n> #include \n> \n> +#include \"libcamera/internal/formats.h\"\n> #include \"libcamera/internal/mapped_framebuffer.h\"\n> \n> using namespace libcamera;\n> @@ -24,8 +25,9 @@ class CameraBuffer::Private : public Extensible::Private,\n> \tLIBCAMERA_DECLARE_PUBLIC(CameraBuffer)\n> \n> public:\n> -\tPrivate(CameraBuffer *cameraBuffer,\n> -\t\tbuffer_handle_t camera3Buffer, int flags);\n> +\tPrivate(CameraBuffer *cameraBuffer, buffer_handle_t camera3Buffer,\n> +\t\tlibcamera::PixelFormat pixelFormat, const libcamera::Size &size,\n> +\t\tint flags);\n> \t~Private();\n> \n> \tunsigned int numPlanes() const;\n> @@ -33,35 +35,92 @@ public:\n> \tSpan plane(unsigned int plane);\n> \n> \tsize_t jpegBufferSize(size_t maxJpegBufferSize) const;\n> +\n> +private:\n> +\t/* \\todo Remove planes_ when it will be added to MappedBuffer */\n> +\tstd::vector> planes_;\n> };\n> \n> CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer,\n> -\t\t\t buffer_handle_t camera3Buffer, int flags)\n> +\t\t\t buffer_handle_t camera3Buffer,\n> +\t\t\t libcamera::PixelFormat pixelFormat,\n> +\t\t\t const libcamera::Size &size, int flags)\n> {\n> -\tmaps_.reserve(camera3Buffer->numFds);\n> \terror_ = 0;\n> \n> +\tconst auto &info = libcamera::PixelFormatInfo::info(pixelFormat);\n> +\tif (!info.isValid()) {\n> +\t\terror_ = -EINVAL;\n> +\t\tLOG(HAL, Error) << \"Invalid pixel format: \"\n> +\t\t\t\t<< pixelFormat.toString();\n> +\t\treturn;\n> +\t}\n> +\n> +\t/*\n> +\t * As Android doesn't offer an API to query buffer layouts, assume for\n> +\t * now that the buffer is backed by a single dmabuf, with planes being\n> +\t * stored contiguously.\n> +\t */\n> +\tint fd = -1;\n> \tfor (int i = 0; i < camera3Buffer->numFds; i++) {\n> -\t\tif (camera3Buffer->data[i] == -1)\n> +\t\tif (camera3Buffer->data[i] == -1 || camera3Buffer->data[i] == fd)\n> \t\t\tcontinue;\n> \n> -\t\toff_t length = lseek(camera3Buffer->data[i], 0, SEEK_END);\n> -\t\tif (length < 0) {\n> -\t\t\terror_ = -errno;\n> -\t\t\tLOG(HAL, Error) << \"Failed to query plane length\";\n> -\t\t\tbreak;\n> +\t\tif (fd != -1) {\n> +\t\t\terror_ = -EINVAL;\n> +\t\t\tLOG(HAL, Error) << \"Discontiguous planes are not supported\";\n> +\t\t\treturn;\n> \t\t}\n> \n> -\t\tvoid *address = mmap(nullptr, length, flags, MAP_SHARED,\n> -\t\t\t\t camera3Buffer->data[i], 0);\n> -\t\tif (address == MAP_FAILED) {\n> -\t\t\terror_ = -errno;\n> -\t\t\tLOG(HAL, Error) << \"Failed to mmap plane\";\n> -\t\t\tbreak;\n> -\t\t}\n> +\t\tfd = camera3Buffer->data[i];\n> +\t}\n> \n> -\t\tmaps_.emplace_back(static_cast(address),\n> -\t\t\t\t static_cast(length));\n> +\tif (fd == -1) {\n> +\t\terror_ = -EINVAL;\n> +\t\tLOG(HAL, Error) << \"No valid file descriptor\";\n> +\t\treturn;\n> +\t}\n> +\n> +\toff_t bufferLength = lseek(fd, 0, SEEK_END);\n> +\tif (bufferLength < 0) {\n> +\t\terror_ = -errno;\n> +\t\tLOG(HAL, Error) << \"Failed to get buffer length\";\n> +\t\treturn;\n> +\t}\n> +\n> +\tvoid *address = mmap(nullptr, bufferLength, flags, MAP_SHARED, fd, 0);\n> +\tif (address == MAP_FAILED) {\n> +\t\terror_ = -errno;\n> +\t\tLOG(HAL, Error) << \"Failed to mmap plane\";\n> +\t\treturn;\n> +\t}\n> +\tmaps_.emplace_back(static_cast(address), bufferLength);\n> +\n> +\tconst unsigned int numPlanes = info.numPlanes();\n> +\tplanes_.resize(numPlanes);\n> +\tunsigned int offset = 0;\n> +\tfor (unsigned int i = 0; i < numPlanes; ++i) {\n> +\t\t/*\n> +\t\t * \\todo Remove if this plane size computation function is\n> +\t\t * added to PixelFormatInfo.\n> +\t\t */\n> +\t\tconst unsigned int vertSubSample = info.planes[i].verticalSubSampling;\n> +\t\tconst unsigned int stride = info.stride(size.width, i, 1u);\n> +\t\tconst unsigned int planeSize =\n> +\t\t\tstride * ((size.height + vertSubSample - 1) / vertSubSample);\n> +\n> +\t\tplanes_[i] = libcamera::Span(\n> +\t\t\tstatic_cast(address) + offset, planeSize);\n> +\n> +\t\tif (bufferLength < offset + planeSize) {\n> +\t\t\terror_ = -EINVAL;\n> +\t\t\tLOG(HAL, Error) << \"Plane \" << i << \" is out of buffer\"\n> +\t\t\t\t\t<< \", buffer length=\" << bufferLength\n> +\t\t\t\t\t<< \", offset=\" << offset\n> +\t\t\t\t\t<< \", size=\" << planeSize;\n> +\t\t\treturn;\n> +\t\t}\n> +\t\toffset += planeSize;\n> \t}\n> }\n> \n> @@ -71,15 +130,15 @@ CameraBuffer::Private::~Private()\n> \n> unsigned int CameraBuffer::Private::numPlanes() const\n> {\n> -\treturn maps_.size();\n> +\treturn planes_.size();\n> }\n> \n> Span CameraBuffer::Private::plane(unsigned int plane)\n> {\n> -\tif (plane >= maps_.size())\n> +\tif (plane >= planes_.size())\n> \t\treturn {};\n> \n> -\treturn maps_[plane];\n> +\treturn planes_[plane];\n> }\n> \n> size_t CameraBuffer::Private::jpegBufferSize(size_t maxJpegBufferSize) const","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 AD1EDBD87C\n\tfor ;\n\tThu, 26 Aug 2021 11:53:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 08D1A6890E;\n\tThu, 26 Aug 2021 13:53:03 +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 19D66688A5\n\tfor ;\n\tThu, 26 Aug 2021 13:53:02 +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 85F55191F;\n\tThu, 26 Aug 2021 13:53: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=\"GJm0mYRx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629978781;\n\tbh=Q9yb9oh+YwCalWWspUZck2IZzIrIgSky4Qh4OxMS0cs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GJm0mYRxz9s+NczalCfulbhLoH2nCStepLR0LCV9sf/QwUBB4LqxjN16+/LzvT70e\n\tc3eb8ZzLiQ2RuSCoWLB3uXk4PhftJ2zj38WODROhEIFPnETqtZl0og7IpsF1WVtN46\n\twQ8kUcZPW3463VJy0JVHmKXrAsAE0ggmcN28qbts=","Date":"Thu, 26 Aug 2021 14:52:49 +0300","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20210826080021.3454520-1-hiroh@chromium.org>\n\t<20210826080021.3454520-2-hiroh@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210826080021.3454520-2-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [PATCH v3 1/3] android:\n\tgeneric_camera_buffer: Correct buffer mapping","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":19115,"web_url":"https://patchwork.libcamera.org/comment/19115/","msgid":"<20210826214314.5ife3h3bdifgmfq5@uno.localdomain>","date":"2021-08-26T21:43:14","subject":"Re: [libcamera-devel] [PATCH v3 1/3] android:\n\tgeneric_camera_buffer: Correct buffer mapping","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hiro,\n\nOn Thu, Aug 26, 2021 at 05:00:19PM +0900, Hirokazu Honda wrote:\n> buffer_handle_t doesn't provide sufficient info to map a buffer\n> properly. cros::CameraBufferManager enables handling the buffer on\n> ChromeOS, but no way is provided for other platforms.\n>\n> Therefore, we put the assumption that planes are in the same buffer\n> and they are consecutive. This modifies the way of mapping in\n> generic_camera_buffer with the assumption.\n>\n> Signed-off-by: Hirokazu Honda \n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> src/android/camera_buffer.h | 14 ++-\n> src/android/camera_stream.cpp | 4 +-\n> src/android/mm/cros_camera_buffer.cpp | 7 +-\n> src/android/mm/generic_camera_buffer.cpp | 103 ++++++++++++++++++-----\n> 4 files changed, 100 insertions(+), 28 deletions(-)\n>\n> diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h\n> index c4e3a9e7..87df2570 100644\n> --- a/src/android/camera_buffer.h\n> +++ b/src/android/camera_buffer.h\n> @@ -11,13 +11,17 @@\n>\n> #include \n> #include \n> +#include \n> +#include \n>\n> class CameraBuffer final : public libcamera::Extensible\n> {\n> \tLIBCAMERA_DECLARE_PRIVATE()\n>\n> public:\n> -\tCameraBuffer(buffer_handle_t camera3Buffer, int flags);\n> +\tCameraBuffer(buffer_handle_t camera3Buffer,\n> +\t\t libcamera::PixelFormat pixelFormat,\n> +\t\t const libcamera::Size &size, int flags);\n> \t~CameraBuffer();\n>\n> \tbool isValid() const;\n> @@ -31,8 +35,12 @@ public:\n> };\n>\n> #define PUBLIC_CAMERA_BUFFER_IMPLEMENTATION\t\t\t\t\\\n> -CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, int flags)\t\\\n> -\t: Extensible(std::make_unique(this, camera3Buffer, flags)) \\\n> +CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer,\t\t\\\n> +\t\t\t libcamera::PixelFormat pixelFormat,\t\t\\\n> +\t\t\t const libcamera::Size &size, int flags)\t\\\n> +\t: Extensible(std::make_unique(this, camera3Buffer,\t\\\n> +\t\t\t\t\t pixelFormat, size,\t\\\n> +\t\t\t\t\t flags))\t\t\t\\\n> {\t\t\t\t\t\t\t\t\t\\\n> }\t\t\t\t\t\t\t\t\t\\\n> CameraBuffer::~CameraBuffer()\t\t\t\t\t\t\\\n> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> index 61b44183..01909ec7 100644\n> --- a/src/android/camera_stream.cpp\n> +++ b/src/android/camera_stream.cpp\n> @@ -110,7 +110,9 @@ int CameraStream::process(const libcamera::FrameBuffer &source,\n> \t * \\todo Buffer mapping and processing should be moved to a\n> \t * separate thread.\n> \t */\n> -\tCameraBuffer dest(camera3Dest, PROT_READ | PROT_WRITE);\n> +\tconst StreamConfiguration &output = configuration();\n> +\tCameraBuffer dest(camera3Dest, formats::MJPEG, output.size,\n> +\t\t\t PROT_READ | PROT_WRITE);\n> \tif (!dest.isValid()) {\n> \t\tLOG(HAL, Error) << \"Failed to map android blob buffer\";\n> \t\treturn -EINVAL;\n> diff --git a/src/android/mm/cros_camera_buffer.cpp b/src/android/mm/cros_camera_buffer.cpp\n> index e8783ff8..50732637 100644\n> --- a/src/android/mm/cros_camera_buffer.cpp\n> +++ b/src/android/mm/cros_camera_buffer.cpp\n> @@ -20,8 +20,9 @@ class CameraBuffer::Private : public Extensible::Private\n> \tLIBCAMERA_DECLARE_PUBLIC(CameraBuffer)\n>\n> public:\n> -\tPrivate(CameraBuffer *cameraBuffer,\n> -\t\tbuffer_handle_t camera3Buffer, int flags);\n> +\tPrivate(CameraBuffer *cameraBuffer, buffer_handle_t camera3Buffer,\n> +\t\tlibcamera::PixelFormat pixelFormat, const libcamera::Size &size,\n> +\t\tint flags);\n> \t~Private();\n>\n> \tbool isValid() const { return valid_; }\n> @@ -46,6 +47,8 @@ private:\n>\n> CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer,\n> \t\t\t buffer_handle_t camera3Buffer,\n> +\t\t\t [[maybe_unused]] libcamera::PixelFormat pixelFormat,\n> +\t\t\t [[maybe_unused]] const libcamera::Size &size,\n> \t\t\t [[maybe_unused]] int flags)\n> \t: handle_(camera3Buffer), numPlanes_(0), valid_(false),\n> \t registered_(false)\n> diff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp\n> index b3af194c..22753490 100644\n> --- a/src/android/mm/generic_camera_buffer.cpp\n> +++ b/src/android/mm/generic_camera_buffer.cpp\n> @@ -12,6 +12,7 @@\n>\n> #include \n>\n> +#include \"libcamera/internal/formats.h\"\n> #include \"libcamera/internal/mapped_framebuffer.h\"\n>\n> using namespace libcamera;\n> @@ -24,8 +25,9 @@ class CameraBuffer::Private : public Extensible::Private,\n> \tLIBCAMERA_DECLARE_PUBLIC(CameraBuffer)\n>\n> public:\n> -\tPrivate(CameraBuffer *cameraBuffer,\n> -\t\tbuffer_handle_t camera3Buffer, int flags);\n> +\tPrivate(CameraBuffer *cameraBuffer, buffer_handle_t camera3Buffer,\n> +\t\tlibcamera::PixelFormat pixelFormat, const libcamera::Size &size,\n> +\t\tint flags);\n> \t~Private();\n>\n> \tunsigned int numPlanes() const;\n> @@ -33,35 +35,92 @@ public:\n> \tSpan plane(unsigned int plane);\n>\n> \tsize_t jpegBufferSize(size_t maxJpegBufferSize) const;\n> +\n> +private:\n> +\t/* \\todo Remove planes_ when it will be added to MappedBuffer */\n> +\tstd::vector> planes_;\n> };\n>\n> CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer,\n> -\t\t\t buffer_handle_t camera3Buffer, int flags)\n> +\t\t\t buffer_handle_t camera3Buffer,\n> +\t\t\t libcamera::PixelFormat pixelFormat,\n> +\t\t\t const libcamera::Size &size, int flags)\n> {\n> -\tmaps_.reserve(camera3Buffer->numFds);\n> \terror_ = 0;\n>\n> +\tconst auto &info = libcamera::PixelFormatInfo::info(pixelFormat);\n> +\tif (!info.isValid()) {\n> +\t\terror_ = -EINVAL;\n> +\t\tLOG(HAL, Error) << \"Invalid pixel format: \"\n> +\t\t\t\t<< pixelFormat.toString();\n> +\t\treturn;\n> +\t}\n> +\n> +\t/*\n> +\t * As Android doesn't offer an API to query buffer layouts, assume for\n> +\t * now that the buffer is backed by a single dmabuf, with planes being\n> +\t * stored contiguously.\n> +\t */\n> +\tint fd = -1;\n> \tfor (int i = 0; i < camera3Buffer->numFds; i++) {\n> -\t\tif (camera3Buffer->data[i] == -1)\n> +\t\tif (camera3Buffer->data[i] == -1 || camera3Buffer->data[i] == fd)\n> \t\t\tcontinue;\n>\n> -\t\toff_t length = lseek(camera3Buffer->data[i], 0, SEEK_END);\n> -\t\tif (length < 0) {\n> -\t\t\terror_ = -errno;\n> -\t\t\tLOG(HAL, Error) << \"Failed to query plane length\";\n> -\t\t\tbreak;\n> +\t\tif (fd != -1) {\n> +\t\t\terror_ = -EINVAL;\n> +\t\t\tLOG(HAL, Error) << \"Discontiguous planes are not supported\";\n> +\t\t\treturn;\n> \t\t}\n>\n> -\t\tvoid *address = mmap(nullptr, length, flags, MAP_SHARED,\n> -\t\t\t\t camera3Buffer->data[i], 0);\n> -\t\tif (address == MAP_FAILED) {\n> -\t\t\terror_ = -errno;\n> -\t\t\tLOG(HAL, Error) << \"Failed to mmap plane\";\n> -\t\t\tbreak;\n> -\t\t}\n> +\t\tfd = camera3Buffer->data[i];\n> +\t}\n>\n> -\t\tmaps_.emplace_back(static_cast(address),\n> -\t\t\t\t static_cast(length));\n> +\tif (fd == -1) {\n> +\t\terror_ = -EINVAL;\n> +\t\tLOG(HAL, Error) << \"No valid file descriptor\";\n> +\t\treturn;\n> +\t}\n> +\n> +\toff_t bufferLength = lseek(fd, 0, SEEK_END);\n> +\tif (bufferLength < 0) {\n> +\t\terror_ = -errno;\n> +\t\tLOG(HAL, Error) << \"Failed to get buffer length\";\n> +\t\treturn;\n> +\t}\n> +\n> +\tvoid *address = mmap(nullptr, bufferLength, flags, MAP_SHARED, fd, 0);\n> +\tif (address == MAP_FAILED) {\n> +\t\terror_ = -errno;\n> +\t\tLOG(HAL, Error) << \"Failed to mmap plane\";\n> +\t\treturn;\n> +\t}\n> +\tmaps_.emplace_back(static_cast(address), bufferLength);\n> +\n> +\tconst unsigned int numPlanes = info.numPlanes();\n> +\tplanes_.resize(numPlanes);\n> +\tunsigned int offset = 0;\n> +\tfor (unsigned int i = 0; i < numPlanes; ++i) {\n> +\t\t/*\n> +\t\t * \\todo Remove if this plane size computation function is\n> +\t\t * added to PixelFormatInfo.\n> +\t\t */\n> +\t\tconst unsigned int vertSubSample = info.planes[i].verticalSubSampling;\n> +\t\tconst unsigned int stride = info.stride(size.width, i, 1u);\n> +\t\tconst unsigned int planeSize =\n> +\t\t\tstride * ((size.height + vertSubSample - 1) / vertSubSample);\n> +\n> +\t\tplanes_[i] = libcamera::Span(\n> +\t\t\tstatic_cast(address) + offset, planeSize);\n> +\n> +\t\tif (bufferLength < offset + planeSize) {\n> +\t\t\terror_ = -EINVAL;\n> +\t\t\tLOG(HAL, Error) << \"Plane \" << i << \" is out of buffer\"\n> +\t\t\t\t\t<< \", buffer length=\" << bufferLength\n> +\t\t\t\t\t<< \", offset=\" << offset\n> +\t\t\t\t\t<< \", size=\" << planeSize;\n> +\t\t\treturn;\n> +\t\t}\n> +\t\toffset += planeSize;\n> \t}\n> }\n>\n> @@ -71,15 +130,15 @@ CameraBuffer::Private::~Private()\n>\n> unsigned int CameraBuffer::Private::numPlanes() const\n> {\n> -\treturn maps_.size();\n> +\treturn planes_.size();\n> }\n>\n> Span CameraBuffer::Private::plane(unsigned int plane)\n> {\n> -\tif (plane >= maps_.size())\n> +\tif (plane >= planes_.size())\n> \t\treturn {};\n>\n> -\treturn maps_[plane];\n> +\treturn planes_[plane];\n> }\n>\n> size_t CameraBuffer::Private::jpegBufferSize(size_t maxJpegBufferSize) const\n> --\n> 2.33.0.rc2.250.ged5fa647cd-goog\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 EEBFCBD87D\n\tfor ;\n\tThu, 26 Aug 2021 21:42:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6A1E66893B;\n\tThu, 26 Aug 2021 23:42:28 +0200 (CEST)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C203D68928\n\tfor ;\n\tThu, 26 Aug 2021 23:42:26 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id B7333FF802;\n\tThu, 26 Aug 2021 21:42:25 +0000 (UTC)"],"Date":"Thu, 26 Aug 2021 23:43:14 +0200","From":"Jacopo Mondi ","To":"Hirokazu Honda ","Message-ID":"<20210826214314.5ife3h3bdifgmfq5@uno.localdomain>","References":"<20210826080021.3454520-1-hiroh@chromium.org>\n\t<20210826080021.3454520-2-hiroh@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210826080021.3454520-2-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [PATCH v3 1/3] android:\n\tgeneric_camera_buffer: Correct buffer mapping","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\" "}}]