{"id":13432,"url":"https://patchwork.libcamera.org/api/patches/13432/?format=json","web_url":"https://patchwork.libcamera.org/patch/13432/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20210823124321.980847-2-hiroh@chromium.org>","date":"2021-08-23T12:43:19","name":"[libcamera-devel,RFC,1/3] android: generic_camera_buffer: Correct buffer mapping","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"0eb360f4378e08b865be91b018627b4604bab8db","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/?format=json","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/13432/mbox/","series":[{"id":2384,"url":"https://patchwork.libcamera.org/api/series/2384/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2384","date":"2021-08-23T12:43:18","name":"Improve CameraBuffer implementation","version":1,"mbox":"https://patchwork.libcamera.org/series/2384/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/13432/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/13432/checks/","tags":{},"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 30895C3241\n\tfor ;\n\tMon, 23 Aug 2021 12:43:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3E2A5688AE;\n\tMon, 23 Aug 2021 14:43:34 +0200 (CEST)","from mail-pj1-x1030.google.com (mail-pj1-x1030.google.com\n\t[IPv6:2607:f8b0:4864:20::1030])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A634C6025B\n\tfor ;\n\tMon, 23 Aug 2021 14:43:31 +0200 (CEST)","by mail-pj1-x1030.google.com with SMTP id\n\tot2-20020a17090b3b4200b0019127f8ed87so324328pjb.1\n\tfor ;\n\tMon, 23 Aug 2021 05:43:31 -0700 (PDT)","from hiroh2.tok.corp.google.com\n\t([2401:fa00:8f:203:184e:5d20:8fcb:dfcd])\n\tby smtp.gmail.com with ESMTPSA id\n\to2sm12103107pgu.76.2021.08.23.05.43.28\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 23 Aug 2021 05:43:29 -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=\"ABatlC3u\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=from:to:cc:subject:date:message-id:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=1aaEJ+CaN2f0E/lIGszK20309vFfqyGNu0O+eBZu6F8=;\n\tb=ABatlC3u4K9bOo+/2qFwLHryUelLxv2xf5qhKKVQpArif2HtXJ5rK8kBCBfZHZGOk9\n\tpeTR1t0v07kvG8CnI07SSfmpgqxLl9worxayORwwzbenWI2ctYBmQJDV9kQEyHKa96R/\n\t8x43tXpfSpViIdH1i17WbkY9HK0DoFh9SDjU8=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=1aaEJ+CaN2f0E/lIGszK20309vFfqyGNu0O+eBZu6F8=;\n\tb=PtdfAIIrHH5hY/RIngOtbl1B0mA58k3l4jLcahFPnMffNw4/f9nC8wkW/TljhRGviI\n\twveRZklt2OicCfxCmplR83wT7xgSqXVHetJIFS7n0i8D28QOrgLa55np5yBiDmvWvKQO\n\tcdXtElbrgJ3Rtv4v6m0+MkF4Gm+CBmeA4NHwm9hcozuc8RV9KUJ8iGHaByr30d0f10E/\n\tLUmI36enWqv+CYJDUi+sDrU7dok9MSNSSmc5UUpElwlsFiSYuQ+ZY9Thbe8qwA+2QGAq\n\t7PX7CcEgXnZU3ItMzS4bNj6PP/gRJ5huki6ETYgcwK5ceor5zbtKDIPpPPfV0I8vlegt\n\tdn7g==","X-Gm-Message-State":"AOAM530os5n/kBy/Hna9PBvHjE0GXGEYKDxcfeWeRip0BMTb2IKb8EaQ\n\tLcSSo3lXYwSv17vt2axcu/rueC/vJKhApw==","X-Google-Smtp-Source":"ABdhPJyPIiLcwW5ZbbvBqS25BNkz43SuqLSsgJTZd/o4xClBflJREOcRF67OZUU3wS6oMlXzK0d7DQ==","X-Received":"by 2002:a17:90a:a78b:: with SMTP id\n\tf11mr4087176pjq.91.1629722610035; \n\tMon, 23 Aug 2021 05:43:30 -0700 (PDT)","From":"Hirokazu Honda ","To":"libcamera-devel@lists.libcamera.org","Date":"Mon, 23 Aug 2021 21:43:19 +0900","Message-Id":"<20210823124321.980847-2-hiroh@chromium.org>","X-Mailer":"git-send-email 2.33.0.rc2.250.ged5fa647cd-goog","In-Reply-To":"<20210823124321.980847-1-hiroh@chromium.org>","References":"<20210823124321.980847-1-hiroh@chromium.org>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [RFC PATCH 1/3] android: generic_camera_buffer:\n\tCorrect 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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"buffer_hanlde_t doesn't provide sufficient info to map a buffer\nproperly. cros::CameraBufferManager enables handling the buffer on\nChromeOS, but no way is provided for other platforms.\n\nTherefore, we put the assumption that planes are in the same buffer\nand they are consecutive. This modifies the way of mapping in\ngeneric_camera_buffer with the assumption.\n\nSigned-off-by: Hirokazu Honda \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 | 82 +++++++++++++++++-------\n 4 files changed, 78 insertions(+), 29 deletions(-)","diff":"diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h\nindex 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\\\ndiff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\nindex 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;\ndiff --git a/src/android/mm/cros_camera_buffer.cpp b/src/android/mm/cros_camera_buffer.cpp\nindex 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)\ndiff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp\nindex b3af194c..3127069f 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,69 @@ public:\n \tSpan plane(unsigned int plane);\n \n \tsize_t jpegBufferSize(size_t maxJpegBufferSize) const;\n+\n+private:\n+\t/* \\todo remove planes_ is 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+\tint fd = -1;\n \tfor (int i = 0; i < camera3Buffer->numFds; i++) {\n-\t\tif (camera3Buffer->data[i] == -1)\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\tif (camera3Buffer->data[i] != -1) {\n+\t\t\tfd = camera3Buffer->data[i];\n \t\t\tbreak;\n \t\t}\n+\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+\tif (fd != -1) {\n+\t\terror_ = EINVAL;\n+\t\tLOG(HAL, Error) << \"No valid file descriptor\";\n+\t\treturn;\n+\t}\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+\tsize_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\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\tmaps_.emplace_back(static_cast(address),\n-\t\t\t\t static_cast(length));\n+\t\toffset += planeSize;\n \t}\n }\n \n@@ -71,15 +107,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","prefixes":["libcamera-devel","RFC","1/3"]}