[{"id":19720,"web_url":"https://patchwork.libcamera.org/comment/19720/","msgid":"","date":"2021-09-20T19:11:12","subject":"Re: [libcamera-devel] [PATCH v3 05/10] android: camera_device:\n\tStore results metadata into Camera3RequestDescriptor","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Uman,\n\nThank you for the patch.\n\nOn Mon, Sep 20, 2021 at 11:07:47PM +0530, Umang Jain wrote:\n> Store metadata which is sent as capture results into the\n> Camera3RequestDescriptor. This is will remove the need to send the\n\ns/is will/will/\n\n> result metadata pointer separately to CameraStream::process().\n> In the subsequent commit, a Camera3RequestDescriptor pointer\n> will be passed to CameraStream::process() and the result metadata\n> can be directly accessed from there.\n\nI'd have squashed 05/10 and 06/10 together, as this patch alone is very\nsmall, and the combination of the two shows the intent of 05/10 better.\nUp to you.\n\nReviewed-by: Laurent Pinchart \n\n> Signed-off-by: Umang Jain \n> ---\n> src/android/camera_device.cpp | 6 ++++--\n> src/android/camera_device.h | 1 +\n> 2 files changed, 5 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 3d6a2bed..1ae4ac73 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1131,6 +1131,8 @@ void CameraDevice::requestComplete(Request *request)\n> \t\tresultMetadata = std::make_unique(0, 0);\n> \t}\n> \n> +\tdescriptor->resultMetadata_ = std::move(resultMetadata);\n> +\n> \t/* Handle any JPEG compression. */\n> \tfor (camera3_stream_buffer_t &buffer : descriptor->buffers_) {\n> \t\tCameraStream *cameraStream =\n> @@ -1150,7 +1152,7 @@ void CameraDevice::requestComplete(Request *request)\n> \n> \t\tint ret = cameraStream->process(src, *buffer.buffer,\n> \t\t\t\t\t\tdescriptor->settings_,\n> -\t\t\t\t\t\tresultMetadata.get());\n> +\t\t\t\t\t\tdescriptor->resultMetadata_.get());\n> \t\t/*\n> \t\t * Return the FrameBuffer to the CameraStream now that we're\n> \t\t * done processing it.\n> @@ -1165,7 +1167,7 @@ void CameraDevice::requestComplete(Request *request)\n> \t\t}\n> \t}\n> \n> -\tcaptureResult.result = resultMetadata->get();\n> +\tcaptureResult.result = descriptor->resultMetadata_->get();\n> \tcallbacks_->process_capture_result(callbacks_, &captureResult);\n> \n> \tdescriptors_.pop_front();\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index 59d6cd39..b2871e52 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -47,6 +47,7 @@ struct Camera3RequestDescriptor {\n> \tstd::vector> frameBuffers_;\n> \tCameraMetadata settings_;\n> \tstd::unique_ptr request_;\n> +\tstd::unique_ptr resultMetadata_;\n> };\n> \n> class CameraDevice : protected libcamera::Loggable","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 84692BDC71\n\tfor ;\n\tMon, 20 Sep 2021 19:11:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CF88E6918A;\n\tMon, 20 Sep 2021 21:11:43 +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 98B1B69186\n\tfor ;\n\tMon, 20 Sep 2021 21:11:42 +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 123E4E57;\n\tMon, 20 Sep 2021 21:11:42 +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=\"pjNyxAPO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1632165102;\n\tbh=egEAqw9/brz9H8Ok69kSxJsF/D7P2oormbV9l3n6EGU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pjNyxAPOmu8Kz3Xot8A66jyYCbdtWSBZIHMABS2wTnFVRUx1Mv89nEnk8Mu+2nlYW\n\tMtx4KVAMQeI5Eu5mN3uHv65rxGDLuW+09ZhWH8EEEt7dZ1mdCg/LFJlE98bq/IK32W\n\twLWgOoNGjUlQUmYY8/iUtcBX1S8kDYJ9WE+b48K0=","Date":"Mon, 20 Sep 2021 22:11:12 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"","References":"<20210920173752.1346190-1-umang.jain@ideasonboard.com>\n\t<20210920173752.1346190-6-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210920173752.1346190-6-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 05/10] android: camera_device:\n\tStore results metadata into Camera3RequestDescriptor","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":19730,"web_url":"https://patchwork.libcamera.org/comment/19730/","msgid":"<20210921111314.man3dw6ij2s5gxqd@uno.localdomain>","date":"2021-09-21T11:13:14","subject":"Re: [libcamera-devel] [PATCH v3 05/10] android: camera_device:\n\tStore results metadata into Camera3RequestDescriptor","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Umang,\n\nOn Mon, Sep 20, 2021 at 11:07:47PM +0530, Umang Jain wrote:\n> Store metadata which is sent as capture results into the\n> Camera3RequestDescriptor. This is will remove the need to send the\n> result metadata pointer separately to CameraStream::process().\n> In the subsequent commit, a Camera3RequestDescriptor pointer\n> will be passed to CameraStream::process() and the result metadata\n> can be directly accessed from there.\n>\n> Signed-off-by: Umang Jain \n> ---\n> src/android/camera_device.cpp | 6 ++++--\n> src/android/camera_device.h | 1 +\n> 2 files changed, 5 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 3d6a2bed..1ae4ac73 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1131,6 +1131,8 @@ void CameraDevice::requestComplete(Request *request)\n> \t\tresultMetadata = std::make_unique(0, 0);\n> \t}\n>\n> +\tdescriptor->resultMetadata_ = std::move(resultMetadata);\n> +\n\nCan't you\n \tdescriptor->resultMetadata_ = getResultMetadata(descriptor);\n\n> \t/* Handle any JPEG compression. */\n> \tfor (camera3_stream_buffer_t &buffer : descriptor->buffers_) {\n> \t\tCameraStream *cameraStream =\n> @@ -1150,7 +1152,7 @@ void CameraDevice::requestComplete(Request *request)\n>\n> \t\tint ret = cameraStream->process(src, *buffer.buffer,\n> \t\t\t\t\t\tdescriptor->settings_,\n> -\t\t\t\t\t\tresultMetadata.get());\n> +\t\t\t\t\t\tdescriptor->resultMetadata_.get());\n\nIt's probably more tedious work, as it requires fixing the whole post\nprocessor hierarchy, but this could be just\n\n cameraStream->process(src, *buffer.buffer,\n descriptor);\n\n\n> \t\t/*\n> \t\t * Return the FrameBuffer to the CameraStream now that we're\n> \t\t * done processing it.\n> @@ -1165,7 +1167,7 @@ void CameraDevice::requestComplete(Request *request)\n> \t\t}\n> \t}\n>\n> -\tcaptureResult.result = resultMetadata->get();\n> +\tcaptureResult.result = descriptor->resultMetadata_->get();\n> \tcallbacks_->process_capture_result(callbacks_, &captureResult);\n>\n> \tdescriptors_.pop_front();\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index 59d6cd39..b2871e52 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -47,6 +47,7 @@ struct Camera3RequestDescriptor {\n> \tstd::vector> frameBuffers_;\n> \tCameraMetadata settings_;\n> \tstd::unique_ptr request_;\n> +\tstd::unique_ptr resultMetadata_;\n> };\n>\n> class CameraDevice : protected libcamera::Loggable\n> --\n> 2.31.1\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 3F35BBF01C\n\tfor ;\n\tTue, 21 Sep 2021 11:12:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8F50A6918A;\n\tTue, 21 Sep 2021 13:12:29 +0200 (CEST)","from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1AD6960247\n\tfor ;\n\tTue, 21 Sep 2021 13:12:28 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 8F1B81C0003;\n\tTue, 21 Sep 2021 11:12:27 +0000 (UTC)"],"Date":"Tue, 21 Sep 2021 13:13:14 +0200","From":"Jacopo Mondi ","To":"Umang Jain ","Message-ID":"<20210921111314.man3dw6ij2s5gxqd@uno.localdomain>","References":"<20210920173752.1346190-1-umang.jain@ideasonboard.com>\n\t<20210920173752.1346190-6-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210920173752.1346190-6-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 05/10] android: camera_device:\n\tStore results metadata into Camera3RequestDescriptor","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":19859,"web_url":"https://patchwork.libcamera.org/comment/19859/","msgid":"","date":"2021-09-27T06:31:29","subject":"Re: [libcamera-devel] [PATCH v3 05/10] android: camera_device:\n\tStore results metadata into Camera3RequestDescriptor","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Umang, thank you for the patch.\n\nOn Tue, Sep 21, 2021 at 8:12 PM Jacopo Mondi wrote:\n>\n> Hi Umang,\n>\n> On Mon, Sep 20, 2021 at 11:07:47PM +0530, Umang Jain wrote:\n> > Store metadata which is sent as capture results into the\n> > Camera3RequestDescriptor. This is will remove the need to send the\n> > result metadata pointer separately to CameraStream::process().\n> > In the subsequent commit, a Camera3RequestDescriptor pointer\n> > will be passed to CameraStream::process() and the result metadata\n> > can be directly accessed from there.\n> >\n> > Signed-off-by: Umang Jain \n\nReviewed-by: Hirokazu Honda \n\n> > ---\n> > src/android/camera_device.cpp | 6 ++++--\n> > src/android/camera_device.h | 1 +\n> > 2 files changed, 5 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 3d6a2bed..1ae4ac73 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -1131,6 +1131,8 @@ void CameraDevice::requestComplete(Request *request)\n> > resultMetadata = std::make_unique(0, 0);\n> > }\n> >\n> > + descriptor->resultMetadata_ = std::move(resultMetadata);\n> > +\n>\n> Can't you\n> descriptor->resultMetadata_ = getResultMetadata(descriptor);\n>\n> > /* Handle any JPEG compression. */\n> > for (camera3_stream_buffer_t &buffer : descriptor->buffers_) {\n> > CameraStream *cameraStream =\n> > @@ -1150,7 +1152,7 @@ void CameraDevice::requestComplete(Request *request)\n> >\n> > int ret = cameraStream->process(src, *buffer.buffer,\n> > descriptor->settings_,\n> > - resultMetadata.get());\n> > + descriptor->resultMetadata_.get());\n>\n> It's probably more tedious work, as it requires fixing the whole post\n> processor hierarchy, but this could be just\n>\n> cameraStream->process(src, *buffer.buffer,\n> descriptor);\n>\n>\n> > /*\n> > * Return the FrameBuffer to the CameraStream now that we're\n> > * done processing it.\n> > @@ -1165,7 +1167,7 @@ void CameraDevice::requestComplete(Request *request)\n> > }\n> > }\n> >\n> > - captureResult.result = resultMetadata->get();\n> > + captureResult.result = descriptor->resultMetadata_->get();\n> > callbacks_->process_capture_result(callbacks_, &captureResult);\n> >\n> > descriptors_.pop_front();\n> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > index 59d6cd39..b2871e52 100644\n> > --- a/src/android/camera_device.h\n> > +++ b/src/android/camera_device.h\n> > @@ -47,6 +47,7 @@ struct Camera3RequestDescriptor {\n> > std::vector> frameBuffers_;\n> > CameraMetadata settings_;\n> > std::unique_ptr request_;\n> > + std::unique_ptr resultMetadata_;\n> > };\n> >\n> > class CameraDevice : protected libcamera::Loggable\n> > --\n> > 2.31.1\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 7A35CBDC71\n\tfor ;\n\tMon, 27 Sep 2021 06:31:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C46146918E;\n\tMon, 27 Sep 2021 08:31:41 +0200 (CEST)","from mail-ed1-x52a.google.com (mail-ed1-x52a.google.com\n\t[IPv6:2a00:1450:4864:20::52a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4B520684C8\n\tfor ;\n\tMon, 27 Sep 2021 08:31:40 +0200 (CEST)","by mail-ed1-x52a.google.com with SMTP id v18so29537154edc.11\n\tfor ;\n\tSun, 26 Sep 2021 23:31:40 -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=\"YnSevJoU\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=qYAua+mZ50CWMS8a9Rap+A47tzt231HPZx+eJQA0EgE=;\n\tb=YnSevJoUTuSWjrP/OwYFFNZL9H1vdZ44HBa1kswiSSU/4Ciyu1rGhhTBuP9RWbVTF+\n\tOeV/CrPi+EhphQz2sko+nOGr5I8/IIB+Amez2ynEmmgTwlRaw8ADoVC9ggsVg2S7G5Rl\n\t2liFrIRdV4JHhHEt+dKx3ZhHOlUrGPtQwxgD4=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=qYAua+mZ50CWMS8a9Rap+A47tzt231HPZx+eJQA0EgE=;\n\tb=f+82h2saxwOByo0fRBcFoKhg/XPCVwDO3ZZREeOGQia4rG/Om5Sk7akI3lrIDnYIXC\n\t8PeNNj2OZuFbUo+vXk1UkQd0i4m3Daddw4iCBGFW0SvFwwQaVOPxLGB4l3A1v84p/H+j\n\t6HMjfGwYPjeeiGlE1Q71VCmud4GnsQQdFqjqOpy96Ucn783Bou2z63AyiRRuXW4KyeQI\n\t8gB7EgQr9vS2ZPFdOxM/aR+sfLHiCrfxz+BIcVvv/lqKaArvvskU1QSbQ943FcgRWMEf\n\tkQFrsNNWi7qaTIpVNFBWEJPFip6+IHA6XflUXSV2reBrNzjePVHi8dOAJSMNpoZKWfjd\n\tnSdw==","X-Gm-Message-State":"AOAM530rMQyUrmJ+rip7uJq75uagSrVXFe6OAI8aIaorXNSKH+D8ztN0\n\tf86cwF0nOJDM/B+Ui5g/vbA4jNTGl6G3SZGT3XrnBq+tPmU=","X-Google-Smtp-Source":"ABdhPJwJDTY2Y1fX9zrgOTsadTYQAUQN/jvoB5lphlAiXqTownXBdx6mgEcy7tdgor5qEwrVUGiidKdpg2gzhbyfOcA=","X-Received":"by 2002:a05:6402:1254:: with SMTP id\n\tl20mr8183366edw.202.1632724299932; \n\tSun, 26 Sep 2021 23:31:39 -0700 (PDT)","MIME-Version":"1.0","References":"<20210920173752.1346190-1-umang.jain@ideasonboard.com>\n\t<20210920173752.1346190-6-umang.jain@ideasonboard.com>\n\t<20210921111314.man3dw6ij2s5gxqd@uno.localdomain>","In-Reply-To":"<20210921111314.man3dw6ij2s5gxqd@uno.localdomain>","From":"Hirokazu Honda ","Date":"Mon, 27 Sep 2021 15:31:29 +0900","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v3 05/10] android: camera_device:\n\tStore results metadata into Camera3RequestDescriptor","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]