[{"id":12596,"web_url":"https://patchwork.libcamera.org/comment/12596/","msgid":"<20200920004821.GD2692107@oden.dyn.berto.se>","date":"2020-09-20T00:48:21","subject":"Re: [libcamera-devel] [PATCH v2 7/8] android: camera_device: Use\n\tlibcamera buffer pool","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your patch,\n\nOn 2020-09-18 15:50:40 +0200, Jacopo Mondi wrote:\n> Now that we have reserved and made available to the camera HAL a\n> pool of libcamera allocated buffers, use them when a CameraStream\n> instance that requires internal allocation is processed.\n> \n> Reviewed-by: Hirokazu Honda <hiroh@chromium.org>\n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  src/android/camera_device.cpp | 47 +++++++++++++++++++++++++----------\n>  1 file changed, 34 insertions(+), 13 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 6ed56ff57dab..f7e9a1e5f494 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1477,6 +1477,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n>  \tfor (unsigned int i = 0; i < descriptor->numBuffers; ++i) {\n>  \t\tCameraStream *cameraStream =\n>  \t\t\tstatic_cast<CameraStream *>(camera3Buffers[i].stream->priv);\n> +\t\tconst StreamConfiguration &config = config_->at(cameraStream->index());\n> +\t\tStream *stream = config.stream();\n>  \n>  \t\t/*\n>  \t\t * Keep track of which stream the request belongs to and store\n> @@ -1485,27 +1487,39 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n>  \t\tdescriptor->buffers[i].stream = camera3Buffers[i].stream;\n>  \t\tdescriptor->buffers[i].buffer = camera3Buffers[i].buffer;\n>  \n> -\t\t/* Software streams are handled after hardware streams complete. */\n> -\t\tif (cameraStream->format() == formats::MJPEG)\n> +\t\t/* Mapped streams don't need to be added to the Request. */\n> +\t\tif (cameraStream->type() == CameraStream::Type::Mapped)\n>  \t\t\tcontinue;\n>  \n> -\t\t/*\n> -\t\t * Create a libcamera buffer using the dmabuf descriptors of\n> -\t\t * the camera3Buffer for each stream. The FrameBuffer is\n> -\t\t * directly associated with the Camera3RequestDescriptor for\n> -\t\t * lifetime management only.\n> -\t\t */\n> -\t\tFrameBuffer *buffer = createFrameBuffer(*camera3Buffers[i].buffer);\n> +\t\tFrameBuffer *buffer;\n> +\t\tif (cameraStream->type() == CameraStream::Type::Direct) {\n> +\t\t\t/*\n> +\t\t\t * Create a libcamera buffer using the dmabuf\n> +\t\t\t * descriptors of the camera3Buffer for each stream and\n> +\t\t\t * associate it with the Camera3RequestDescriptor for\n> +\t\t\t * lifetime management only.\n> +\t\t\t */\n> +\t\t\tbuffer = createFrameBuffer(*camera3Buffers[i].buffer);\n> +\t\t\tdescriptor->frameBuffers.emplace_back(buffer);\n> +\n> +\t\t} else {\n> +\t\t\t/*\n> +\t\t\t * Get the frame buffer from the CameraStream internal\n> +\t\t\t * buffer pool. The lifetime management of internal\n> +\t\t\t * buffers is connected to the one of the\n> +\t\t\t * FrameBufferAllocator instance.\n> +\t\t\t *\n> +\t\t\t * The retrieved buffer has to be returned to the\n> +\t\t\t * allocator once it has been processed.\n> +\t\t\t */\n> +\t\t\tbuffer = getBuffer(stream);\n> +\t\t}\n\nI think this could be turned into a switch (cameraStream->type()) \nstatement?\n\n>  \t\tif (!buffer) {\n>  \t\t\tLOG(HAL, Error) << \"Failed to create buffer\";\n>  \t\t\tdelete request;\n>  \t\t\tdelete descriptor;\n>  \t\t\treturn -ENOMEM;\n>  \t\t}\n> -\t\tdescriptor->frameBuffers.emplace_back(buffer);\n> -\n> -\t\tStreamConfiguration *streamConfiguration = &config_->at(cameraStream->index());\n> -\t\tStream *stream = streamConfiguration->stream();\n>  \n>  \t\trequest->addBuffer(stream, buffer);\n>  \t}\n> @@ -1632,6 +1646,13 @@ void CameraDevice::requestComplete(Request *request)\n>  \t\tconst uint32_t jpeg_orientation = 0;\n>  \t\tresultMetadata->addEntry(ANDROID_JPEG_ORIENTATION,\n>  \t\t\t\t\t &jpeg_orientation, 1);\n> +\n> +\t\t/*\n> +\t\t * Return the FrameBuffer to the CameraStream now that we're\n> +\t\t * done processing it.\n> +\t\t */\n> +\t\tif (cameraStream->type() == CameraStream::Type::Internal)\n> +\t\t\treturnBuffer(stream, buffer);\n>  \t}\n>  \n>  \t/* Prepare to call back the Android camera stack. */\n> -- \n> 2.28.0\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 7F310BF01C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 20 Sep 2020 00:48:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1B01862FBF;\n\tSun, 20 Sep 2020 02:48:23 +0200 (CEST)","from mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7769262FAB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 20 Sep 2020 02:48:22 +0200 (CEST)","by mail-lj1-x241.google.com with SMTP id y4so8126826ljk.8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 19 Sep 2020 17:48:22 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\ty25sm1525991ljd.79.2020.09.19.17.48.21\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 19 Sep 2020 17:48:21 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"OS0MncbU\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=k1WVDJaJM0LS049mZD4mOl07F4uGnvIySiU/CnQcbjc=;\n\tb=OS0MncbUkPnAQf//ChvhZ8Mpb8w8ohpX6AX9+tqmAlBHfsribvYRF1AhLHAyEcsdWM\n\tIHiRJatJ9A58MpASZfSVCxUEJ38AWo/aZpjKFVL1ql9HJj2rMJfyNXNDZ2Vh5uNudVkE\n\t4Q5vMOE3Ise6sHuYDEyVrBzi9M8mTh20HkrdJJbq1EnakRLB5KKuG3Mtwp1uwdaFdnDk\n\tEkKLkTZGIkhOICiFzpxBlFv9e2Ix1sG4DvQFs+DZ2Cmdxn31SCT46tWCrFz7VX/SKasH\n\tYNlf6Cr7BzDSDLnJKnlHaL4HGzjf3B5SbsNwnIeVREFO9G9pUvDcSLXNAqS2JU4luGOG\n\tRaHg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=k1WVDJaJM0LS049mZD4mOl07F4uGnvIySiU/CnQcbjc=;\n\tb=Af06CKVwQDXR7wBBdEHb629pKeUhrc0WUkkPOmwizn7oqjSJ2RdJpMsXwpgtbXvDSo\n\tOcbyAl8RYSKIKW+0QFNfozxEam0Z3lnWuQ4M4zomincPFzj/Yc9CQQQ+6tD5q641EWPb\n\twgoQbMouiRQC6tDdyg2M2J/SxjdeaQE3AoPW5zcgd4cwOT3HzsEUYL7o0y7+PRm3WR+r\n\t60X8c6WEyqVpmmL+QJX94EgshYN4CZlF1mQ3E5wntCvxN1g5YYvKvwJviE9XjU4b9r6X\n\t2TUP231Uas3Bn3Xl6h0DkOG2YfISCR5WccOUTR3WKiCyfeaPPGIIKufE+QZXWT0S+Tfa\n\t8yvQ==","X-Gm-Message-State":"AOAM530jqHe6vSGb9Ljrsww85f+HgkybSEci7NL+HPq1YWZB03JHZtyB\n\t58+oEJ19GQbu0tCC44QO1dbLz9YLXqYkMw==","X-Google-Smtp-Source":"ABdhPJz2SXjdLCloBml9vBaUBJEYZUaA8JKnsXWWr2wt27qdD2Q3sLT4I0mJZw61N9fWfniy37FCMw==","X-Received":"by 2002:a2e:880d:: with SMTP id\n\tx13mr14619201ljh.20.1600562901923; \n\tSat, 19 Sep 2020 17:48:21 -0700 (PDT)","Date":"Sun, 20 Sep 2020 02:48:21 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<20200920004821.GD2692107@oden.dyn.berto.se>","References":"<20200918135041.91492-1-jacopo@jmondi.org>\n\t<20200918135041.91492-8-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200918135041.91492-8-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 7/8] android: camera_device: Use\n\tlibcamera buffer pool","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"hanlinchen@chromium.org, libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]