[{"id":16470,"web_url":"https://patchwork.libcamera.org/comment/16470/","msgid":"","date":"2021-04-21T18:51:55","subject":"Re: [libcamera-devel] [PATCH v3 16/16] android: camera_device: Use\n\tcontrols::SensorTimestamp","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 2021-04-21 18:03:19 +0200, Jacopo Mondi wrote:\n> Use the controls::SensorTimestamp value to populate\n> ANDROID_SENSOR_TIMESTAMP result metadata.\n> \n> The Camera is assumed to provide the control in the Request metadata.\n> \n> Reviewed-by: Laurent Pinchart \n> Signed-off-by: Jacopo Mondi \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> src/android/camera_device.cpp | 19 ++++++-------------\n> src/android/camera_device.h | 3 +--\n> 2 files changed, 7 insertions(+), 15 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index a71aee2fc734..97ce34fd65a0 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -2025,7 +2025,6 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> \n> void CameraDevice::requestComplete(Request *request)\n> {\n> -\tconst Request::BufferMap &buffers = request->buffers();\n> \tcamera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK;\n> \tstd::unique_ptr resultMetadata;\n> \n> @@ -2053,14 +2052,7 @@ void CameraDevice::requestComplete(Request *request)\n> \tLOG(HAL, Debug) << \"Request \" << request->cookie() << \" completed with \"\n> \t\t\t<< descriptor.buffers_.size() << \" streams\";\n> \n> -\t/*\n> -\t * \\todo The timestamp used for the metadata is currently always taken\n> -\t * from the first buffer (which may be the first stream) in the Request.\n> -\t * It might be appropriate to return a 'correct' (as determined by\n> -\t * pipeline handlers) timestamp in the Request itself.\n> -\t */\n> -\tuint64_t timestamp = buffers.begin()->second->metadata().timestamp;\n> -\tresultMetadata = getResultMetadata(descriptor, timestamp);\n> +\tresultMetadata = getResultMetadata(descriptor);\n> \n> \t/* Handle any JPEG compression. */\n> \tfor (camera3_stream_buffer_t &buffer : descriptor.buffers_) {\n> @@ -2105,6 +2097,7 @@ void CameraDevice::requestComplete(Request *request)\n> \tcaptureResult.output_buffers = descriptor.buffers_.data();\n> \n> \tif (status == CAMERA3_BUFFER_STATUS_OK) {\n> +\t\tint64_t timestamp = request->metadata().get(controls::SensorTimestamp);\n> \t\tnotifyShutter(descriptor.frameNumber_, timestamp);\n> \n> \t\tcaptureResult.partial_result = 1;\n> @@ -2164,8 +2157,7 @@ void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream)\n> * Produce a set of fixed result metadata.\n> */\n> std::unique_ptr\n> -CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor,\n> -\t\t\t\tint64_t timestamp) const\n> +CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) const\n> {\n> \tconst ControlList &metadata = descriptor.request_->metadata();\n> \tconst CameraMetadata &settings = descriptor.settings_;\n> @@ -2291,8 +2283,6 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor,\n> \tresultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE,\n> \t\t\t\t &value32, 1);\n> \n> -\tresultMetadata->addEntry(ANDROID_SENSOR_TIMESTAMP, ×tamp, 1);\n> -\n> \tvalue = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;\n> \tresultMetadata->addEntry(ANDROID_STATISTICS_FACE_DETECT_MODE,\n> \t\t\t\t &value, 1);\n> @@ -2318,6 +2308,9 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor,\n> \t\t\t\t &rolling_shutter_skew, 1);\n> \n> \t/* Add metadata tags reported by libcamera. */\n> +\tint64_t timestamp = metadata.get(controls::SensorTimestamp);\n> +\tresultMetadata->addEntry(ANDROID_SENSOR_TIMESTAMP, ×tamp, 1);\n> +\n> \tif (metadata.contains(controls::draft::PipelineDepth)) {\n> \t\tuint8_t pipeline_depth =\n> \t\t\tmetadata.get(controls::draft::PipelineDepth);\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index c63e8e21726e..23457e47767a 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -107,8 +107,7 @@ private:\n> \tlibcamera::PixelFormat toPixelFormat(int format) const;\n> \tint processControls(Camera3RequestDescriptor *descriptor);\n> \tstd::unique_ptr getResultMetadata(\n> -\t\tconst Camera3RequestDescriptor &descriptor,\n> -\t\tint64_t timestamp) const;\n> +\t\tconst Camera3RequestDescriptor &descriptor) const;\n> \n> \tunsigned int id_;\n> \tcamera3_device_t camera3Device_;\n> -- \n> 2.31.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 C6545BDB17\n\tfor ;\n\tWed, 21 Apr 2021 18:51:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 45C0068847;\n\tWed, 21 Apr 2021 20:51:58 +0200 (CEST)","from mail-lj1-x236.google.com (mail-lj1-x236.google.com\n\t[IPv6:2a00:1450:4864:20::236])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 45BA868840\n\tfor ;\n\tWed, 21 Apr 2021 20:51:57 +0200 (CEST)","by mail-lj1-x236.google.com with SMTP id b38so7748630ljf.5\n\tfor ;\n\tWed, 21 Apr 2021 11:51:57 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id c1sm37550ljf.85.2021.04.21.11.51.56\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 21 Apr 2021 11:51:56 -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=\"oc1tXQsl\"; 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=R1gZQi71E9A5RcTmM+sqFv60KMSIrut8MDD/E6VtqO0=;\n\tb=oc1tXQsl8uUvxadrQMVqKzTU4lQ4zIiDjXdIAjs9VWm2YDatGodMjyPfiKNybH2zyt\n\tUz/snrRj9V30bl/T3oaCmy3PbXvniGz3r1vq5Yp68BSGukwhWJEdlRXL5uM46VBYG2/5\n\tnU1AN7y6fHzbl5lVRyeyciNALjKduXqdPmbOUvrEkDezz9x4RO/V5aO3rm3o/t0WlOa2\n\tLylXybWZxkNKgjVIoxcsVMicNK87jVEdhpwCaqz1qF3I61v8IfusOitTgzThNdtUSF1T\n\toVgWs8A8kxgqXAnnvr343SkbE34RLZmk6EaWkfeiMBcp8sdGRutb9mqgQEaPaZs+zI33\n\tohpg==","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=R1gZQi71E9A5RcTmM+sqFv60KMSIrut8MDD/E6VtqO0=;\n\tb=el9kU+EpIxwxrg81+fC2My72Km4QGUmM9Mcz7kD0Gft/WOg9LTgLGEdmJwBzYY25tG\n\thh0g66uOSFojnER9WPfnywsQZX/58zXd5fcBaPYB+2Jva4cDPHgu1wdTocbLEVhPXLpd\n\tq6aEwLmsU2CCnDzoWMRL7KB6Hl6KL/GfAzQHg0I5N6qtFRkp/7yIQ9mX4WjGzc076kpA\n\tnnZS4O4txbiW7KgR0BXQddRslmvcVUpuT365z7eLaT4w2+GB13cx2pwW2bw7buD8xTj0\n\tVKQQ+KdzYmN4wgAID9WXDKVwWyMsE8GNU+wT82Km5awdH9EiVvWnKTUCLuIdbWvjj8Ig\n\txLVQ==","X-Gm-Message-State":"AOAM530B7fwc0IcIHRcl+66t8J+oHgsNbM4/GxnLqui2PLuqzCIWhE21\n\t8A2yJMQqYGXXqyz99woyp2khU0UhNkP07wmi","X-Google-Smtp-Source":"ABdhPJyztqkt3stW9aeOszjhHuAP4LAmqjnoYyR8BaX6ATat2GU2xehMcfO+bLhXqksFvIbAitfN3g==","X-Received":"by 2002:a2e:8797:: with SMTP id\n\tn23mr19632733lji.347.1619031116630; \n\tWed, 21 Apr 2021 11:51:56 -0700 (PDT)","Date":"Wed, 21 Apr 2021 20:51:55 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210421160319.42251-1-jacopo@jmondi.org>\n\t<20210421160319.42251-17-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210421160319.42251-17-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v3 16/16] android: camera_device: Use\n\tcontrols::SensorTimestamp","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","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16488,"web_url":"https://patchwork.libcamera.org/comment/16488/","msgid":"","date":"2021-04-22T05:49:14","subject":"Re: [libcamera-devel] [PATCH v3 16/16] android: camera_device: Use\n\tcontrols::SensorTimestamp","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Jacopo, thank you for the work in this series.\n\nOn Thu, Apr 22, 2021 at 3:51 AM Niklas Söderlund\n wrote:\n>\n> Hi Jacopo,\n>\n> Thanks for your patch.\n>\n> On 2021-04-21 18:03:19 +0200, Jacopo Mondi wrote:\n> > Use the controls::SensorTimestamp value to populate\n> > ANDROID_SENSOR_TIMESTAMP result metadata.\n> >\n> > The Camera is assumed to provide the control in the Request metadata.\n> >\n> > Reviewed-by: Laurent Pinchart \n> > Signed-off-by: Jacopo Mondi \n>\n> Reviewed-by: Niklas Söderlund \n>\n> > ---\n> > src/android/camera_device.cpp | 19 ++++++-------------\n> > src/android/camera_device.h | 3 +--\n> > 2 files changed, 7 insertions(+), 15 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index a71aee2fc734..97ce34fd65a0 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -2025,7 +2025,6 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> >\n> > void CameraDevice::requestComplete(Request *request)\n> > {\n> > - const Request::BufferMap &buffers = request->buffers();\n> > camera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK;\n> > std::unique_ptr resultMetadata;\n> >\n> > @@ -2053,14 +2052,7 @@ void CameraDevice::requestComplete(Request *request)\n> > LOG(HAL, Debug) << \"Request \" << request->cookie() << \" completed with \"\n> > << descriptor.buffers_.size() << \" streams\";\n> >\n> > - /*\n> > - * \\todo The timestamp used for the metadata is currently always taken\n> > - * from the first buffer (which may be the first stream) in the Request.\n> > - * It might be appropriate to return a 'correct' (as determined by\n> > - * pipeline handlers) timestamp in the Request itself.\n> > - */\n> > - uint64_t timestamp = buffers.begin()->second->metadata().timestamp;\n> > - resultMetadata = getResultMetadata(descriptor, timestamp);\n> > + resultMetadata = getResultMetadata(descriptor);\n> >\n> > /* Handle any JPEG compression. */\n> > for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {\n> > @@ -2105,6 +2097,7 @@ void CameraDevice::requestComplete(Request *request)\n> > captureResult.output_buffers = descriptor.buffers_.data();\n> >\n> > if (status == CAMERA3_BUFFER_STATUS_OK) {\n> > + int64_t timestamp = request->metadata().get(controls::SensorTimestamp);\n\nnit: This must not matter, but should we explicitly cast to unsigned\n64 bit integer?\nconst uint64_t timestamp =\nstatic_cast(request->metadata().get(controls::SensorTimestamp));\n\n> > notifyShutter(descriptor.frameNumber_, timestamp);\n> >\n> > captureResult.partial_result = 1;\n> > @@ -2164,8 +2157,7 @@ void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream)\n> > * Produce a set of fixed result metadata.\n> > */\n> > std::unique_ptr\n> > -CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor,\n> > - int64_t timestamp) const\n> > +CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) const\n> > {\n> > const ControlList &metadata = descriptor.request_->metadata();\n> > const CameraMetadata &settings = descriptor.settings_;\n> > @@ -2291,8 +2283,6 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor,\n> > resultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE,\n> > &value32, 1);\n> >\n> > - resultMetadata->addEntry(ANDROID_SENSOR_TIMESTAMP, ×tamp, 1);\n> > -\n> > value = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;\n> > resultMetadata->addEntry(ANDROID_STATISTICS_FACE_DETECT_MODE,\n> > &value, 1);\n> > @@ -2318,6 +2308,9 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor,\n> > &rolling_shutter_skew, 1);\n> >\n> > /* Add metadata tags reported by libcamera. */\n> > + int64_t timestamp = metadata.get(controls::SensorTimestamp);\n\nhuge nit: const int64_t\n\n> > + resultMetadata->addEntry(ANDROID_SENSOR_TIMESTAMP, ×tamp, 1);\n> > +\n> > if (metadata.contains(controls::draft::PipelineDepth)) {\n> > uint8_t pipeline_depth =\n> > metadata.get(controls::draft::PipelineDepth);\n> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > index c63e8e21726e..23457e47767a 100644\n> > --- a/src/android/camera_device.h\n> > +++ b/src/android/camera_device.h\n> > @@ -107,8 +107,7 @@ private:\n> > libcamera::PixelFormat toPixelFormat(int format) const;\n> > int processControls(Camera3RequestDescriptor *descriptor);\n> > std::unique_ptr getResultMetadata(\n> > - const Camera3RequestDescriptor &descriptor,\n> > - int64_t timestamp) const;\n> > + const Camera3RequestDescriptor &descriptor) const;\n> >\n> > unsigned int id_;\n> > camera3_device_t camera3Device_;\n> > --\n> > 2.31.1\n\nReviewed-by: Hirokazu Honda \n\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\n>\n> --\n> Regards,\n> Niklas Söderlund\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 93C87BDB15\n\tfor ;\n\tThu, 22 Apr 2021 05:49:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 238996884C;\n\tThu, 22 Apr 2021 07:49:27 +0200 (CEST)","from mail-ej1-x62e.google.com (mail-ej1-x62e.google.com\n\t[IPv6:2a00:1450:4864:20::62e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 62E726884B\n\tfor ;\n\tThu, 22 Apr 2021 07:49:25 +0200 (CEST)","by mail-ej1-x62e.google.com with SMTP id w3so66959754ejc.4\n\tfor ;\n\tWed, 21 Apr 2021 22:49:25 -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=\"Qe+FSy4M\"; 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:content-transfer-encoding;\n\tbh=XOdAd5WGHA10AlHSZhTZsYL3yjmgxBzWNj7icl/0EbM=;\n\tb=Qe+FSy4MMdz9rjTEgKPFkfTNJlCMFMynFPFSlSyyUHyhmqKY3OnzmYfiu+7cHXJxlC\n\tvZXFnozMxCk300Nppzbpdp3+vIN0MMVeFNPbLDH48bH+ONGrA1tLd324qtWd1n2ib0W5\n\tvezAXmroRHgpspa+nRoVrWVoaS9keyxfA2/jA=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=XOdAd5WGHA10AlHSZhTZsYL3yjmgxBzWNj7icl/0EbM=;\n\tb=TKQ5oNm71zx+m6cSEXUCbogqRqUdCvr95Pvfs042wHlPtVus9EVPqcMbIPncG8cy8U\n\t6rc2zavmmr95FhmGwTEFwSXQmX0YA7aGKU5Q9xuDKWOBVs24CgDwgoAJXRa5H0c5get3\n\tBBWj88ovN9K8NPL9lsFTjqdJRpx2PnDT2GZ4zE2Iug2JcaqHoxAtxc4vP5B9wnx2b4KY\n\togI+FCXHcCXr/Mn1H0VNpVT1VPfEkh9Bd/mxc5XYljzWCpsAcJGJc2OEMj+ucZIWAPtS\n\tXs9x8Okq/qL/bDXLyBOV4cHp7Y4vhr+whL2hxZ7VKNfkVdgaDlc7wMOP6tJWMB7P5kL8\n\tXIzQ==","X-Gm-Message-State":"AOAM531F7f/safTz8zDyPNk2Z0CZtpmVqKWlq30+jbs4YTJ+NF1MS1Em\n\tIf/KKKl1WmhVygnlvOlLAlaiADuQE6/8BEbd7IkW5w==","X-Google-Smtp-Source":"ABdhPJwA9pgU7IuacYOKvAZH9/e9TZGIeM30JHHZzLx8TYYFZHqa0tOO//17XI4jib7hrYhBabQIGIWD2EDb+lmKfqo=","X-Received":"by 2002:a17:906:fcc4:: with SMTP id\n\tqx4mr1505581ejb.42.1619070565039; \n\tWed, 21 Apr 2021 22:49:25 -0700 (PDT)","MIME-Version":"1.0","References":"<20210421160319.42251-1-jacopo@jmondi.org>\n\t<20210421160319.42251-17-jacopo@jmondi.org>\n\t","In-Reply-To":"","From":"Hirokazu Honda ","Date":"Thu, 22 Apr 2021 14:49:14 +0900","Message-ID":"","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= ","Subject":"Re: [libcamera-devel] [PATCH v3 16/16] android: camera_device: Use\n\tcontrols::SensorTimestamp","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 ","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]