[{"id":17092,"web_url":"https://patchwork.libcamera.org/comment/17092/","msgid":"","date":"2021-05-21T11:51:06","subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_device: Fix\n\tsensor frame duration","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Paul,\n\nThanks for your work.\n\nOn 2021-05-21 19:55:33 +0900, Paul Elder wrote:\n> The sensor frame duration should be set by IPA. Get the information for\n> the result metadata from libcamera.\n> \n> Signed-off-by: Paul Elder \n> ---\n> src/android/camera_device.cpp | 10 ++++++++++\n> 1 file changed, 10 insertions(+)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index b32e8be5..779ce554 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -2252,6 +2252,16 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> \t\tresultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, exposure);\n> \t}\n> \n> +\tif (metadata.contains(controls::FrameDurations)) {\n> +\t\tSpan durations =\n> +\t\t\tmetadata.get(controls::FrameDurations);\n> +\t\tif (durations[0] == durations[1]) {\n\nDo we need a todo here to expand this to work with min/max frame \ndurations at a later stage?\n\n> +\t\t\tint64_t duration = durations[0] * 1000;\n> +\t\t\tresultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,\n> +\t\t\t\t\t\t duration);\n> +\t\t}\n> +\t}\n> +\n> \tif (metadata.contains(controls::ScalerCrop)) {\n> \t\tRectangle crop = metadata.get(controls::ScalerCrop);\n> \t\tint32_t cropRect[] = {\n> -- \n> 2.27.0\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 9F5DCC31FF\n\tfor ;\n\tFri, 21 May 2021 11:51:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E40676891E;\n\tFri, 21 May 2021 13:51:09 +0200 (CEST)","from mail-lj1-x235.google.com (mail-lj1-x235.google.com\n\t[IPv6:2a00:1450:4864:20::235])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1C3AB68911\n\tfor ;\n\tFri, 21 May 2021 13:51:08 +0200 (CEST)","by mail-lj1-x235.google.com with SMTP id s25so23590440ljo.11\n\tfor ;\n\tFri, 21 May 2021 04:51:08 -0700 (PDT)","from localhost (h-155-4-209-203.A463.priv.bahnhof.se.\n\t[155.4.209.203]) by smtp.gmail.com with ESMTPSA id\n\ty19sm618944ljy.32.2021.05.21.04.51.06\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 21 May 2021 04:51:06 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"fJQWrPSe\"; 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=NKr0+RhrmZuiBluF7sRifmdAJ7VongX+e/1w/gnmsZk=;\n\tb=fJQWrPSe1YqAN4ZMXtyxw3JxPdMPxoTyeHfCJ21+9GUdYnFTgpfuQ18J2pUP/E+jcT\n\tDH234akYttSKHd1a06tJmFyifDDFkqdhonZfjpd1aOp9Br3T9tUHwmLEe9t52hiWeD6S\n\t61pH0PQrOiIB3TGyGs6j2h7W0ThxqezWmMk8ImBeSzU3CS+ZfosH6ETxNQvX5BHwU8Pk\n\taqSuVRb8wVbdfwy5DGX2kUB5q2XkWERvP15BYsncZmHTH6zqpGyIcyYOQ5N+FOdGWSEG\n\tdkUKN0DDPxmBVrYk9kBvIMoQDyK/sr6UhRtrKiyiPQXiL8mb3L8otCYwJzYZJDpBfAYX\n\tHr1g==","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=NKr0+RhrmZuiBluF7sRifmdAJ7VongX+e/1w/gnmsZk=;\n\tb=WXxnS44Oew24pRKnliO1W/rEV+xrMMY9gHa5a02m0g+/MbycOPMkoweSOx3Yia/iIh\n\tUn3uoHLblaEElWR3rS+Dm9Moud5wxU+ki5BJlwTFNrIvectg27uMlxLRKTadSmEs0oK9\n\tDaSIY3zSz7VspiBslMbyM8zFPUR3gnU9HmKbScZ960DuklsUjGsWjU0O9rf0bdgKbub7\n\tnNHLjSsBfADK5OiwAB6/NNSbBL/Ylz/0tWpun7HKGkAHiVVtEtXFTPbS/v1bkOafCrwe\n\tWS6ok4C39uL4mEPYwV5dHyYTO7y820ge06oTsoqJ3kdIjb0AgPilZIem89RPAGtJWEBc\n\tR0rg==","X-Gm-Message-State":"AOAM530hyeY3KbxD5ts5Zcww8BhBGVMhXRCwcH2AKqJwS2hWAFubQxEk\n\t6iLnMrutXe4usaaR/CZ4GTmySA==","X-Google-Smtp-Source":"ABdhPJyypFvJlXq8JmOw3tpe8Lnme1U9PrrmdwFVuziirRt4QGgADaAsprRe8GB2ftJNJnPVjidFYQ==","X-Received":"by 2002:a2e:968e:: with SMTP id\n\tq14mr6578351lji.507.1621597867521; \n\tFri, 21 May 2021 04:51:07 -0700 (PDT)","Date":"Fri, 21 May 2021 13:51:06 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Paul Elder ","Message-ID":"","References":"<20210521105534.1125303-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20210521105534.1125303-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_device: Fix\n\tsensor frame duration","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":17093,"web_url":"https://patchwork.libcamera.org/comment/17093/","msgid":"<20210521115432.shmbhtmuq6k4ycd7@uno.localdomain>","date":"2021-05-21T11:54:32","subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_device: Fix\n\tsensor frame duration","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul,\n the Fix in subject is probably not required, as the\nANDROID_SENSOR_FRAME_DURATION property was not registered at all.\n\nOn Fri, May 21, 2021 at 07:55:33PM +0900, Paul Elder wrote:\n> The sensor frame duration should be set by IPA. Get the information for\n> the result metadata from libcamera.\n\nhow about:\n\n\"Libcamera reports the frame duration through the\ncontrols::FrameDuration controls. Populate the\nandroid.sensor.frameDuration result metadata using the libcamera\ncontrol value\"\n\n>\n> Signed-off-by: Paul Elder \n> ---\n> src/android/camera_device.cpp | 10 ++++++++++\n> 1 file changed, 10 insertions(+)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index b32e8be5..779ce554 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -2252,6 +2252,16 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> \t\tresultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, exposure);\n> \t}\n>\n> +\tif (metadata.contains(controls::FrameDurations)) {\n> +\t\tSpan durations =\n> +\t\t\tmetadata.get(controls::FrameDurations);\n> +\t\tif (durations[0] == durations[1]) {\n\nYes, we have a bit of an unspecified behaviour here.\n\nWhat is the use case for reporting two different frame durations ?\n\nIndeed I see this one:\nsrc/ipa/raspberrypi/raspberrypi.cpp: libcameraMetadata_.set(controls::FrameDurations,\nsrc/ipa/raspberrypi/raspberrypi.cpp: { static_cast(minFrameDuration_),\nsrc/ipa/raspberrypi/raspberrypi.cpp: static_cast(maxFrameDuration_) });\n\nWhere the RPi IPA reports the min/max frame durations received as\ncontrols part of a Request before using them to clamp the AE computed\nexposure time and blankings.\n\n+Naush, +David\n\nWouldn't it be better to report the actual duration by knowing the\nactual exposure+blanking values once AGC has run ?\n\nThanks\n j\n\n> +\t\t\tint64_t duration = durations[0] * 1000;\n> +\t\t\tresultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,\n> +\t\t\t\t\t\t duration);\n> +\t\t}\n> +\t}\n> +\n> \tif (metadata.contains(controls::ScalerCrop)) {\n> \t\tRectangle crop = metadata.get(controls::ScalerCrop);\n> \t\tint32_t cropRect[] = {\n> --\n> 2.27.0\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 099A2C3200\n\tfor ;\n\tFri, 21 May 2021 11:53:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 926846891A;\n\tFri, 21 May 2021 13:53:48 +0200 (CEST)","from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A68CC68911\n\tfor ;\n\tFri, 21 May 2021 13:53:47 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay10.mail.gandi.net (Postfix) with ESMTPSA id 9D688240008;\n\tFri, 21 May 2021 11:53:46 +0000 (UTC)"],"Date":"Fri, 21 May 2021 13:54:32 +0200","From":"Jacopo Mondi ","To":"Paul Elder ,\n\tNaushir Patuck , david.plowman@raspberrypi.com","Message-ID":"<20210521115432.shmbhtmuq6k4ycd7@uno.localdomain>","References":"<20210521105534.1125303-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210521105534.1125303-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_device: Fix\n\tsensor frame duration","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":17096,"web_url":"https://patchwork.libcamera.org/comment/17096/","msgid":"","date":"2021-05-21T12:08:37","subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_device: Fix\n\tsensor frame duration","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo and Paul,\n\nOn Fri, 21 May 2021 at 12:53, Jacopo Mondi wrote:\n\n> Hi Paul,\n> the Fix in subject is probably not required, as the\n> ANDROID_SENSOR_FRAME_DURATION property was not registered at all.\n>\n> On Fri, May 21, 2021 at 07:55:33PM +0900, Paul Elder wrote:\n> > The sensor frame duration should be set by IPA. Get the information for\n> > the result metadata from libcamera.\n>\n> how about:\n>\n> \"Libcamera reports the frame duration through the\n> controls::FrameDuration controls. Populate the\n> android.sensor.frameDuration result metadata using the libcamera\n> control value\"\n>\n> >\n> > Signed-off-by: Paul Elder \n> > ---\n> > src/android/camera_device.cpp | 10 ++++++++++\n> > 1 file changed, 10 insertions(+)\n> >\n> > diff --git a/src/android/camera_device.cpp\n> b/src/android/camera_device.cpp\n> > index b32e8be5..779ce554 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -2252,6 +2252,16 @@ CameraDevice::getResultMetadata(const\n> Camera3RequestDescriptor &descriptor) cons\n> > resultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME,\n> exposure);\n> > }\n> >\n> > + if (metadata.contains(controls::FrameDurations)) {\n> > + Span durations =\n> > + metadata.get(controls::FrameDurations);\n> > + if (durations[0] == durations[1]) {\n>\n> Yes, we have a bit of an unspecified behaviour here.\n>\n> What is the use case for reporting two different frame durations ?\n>\n> Indeed I see this one:\n> src/ipa/raspberrypi/raspberrypi.cpp:\n> libcameraMetadata_.set(controls::FrameDurations,\n> src/ipa/raspberrypi/raspberrypi.cpp: {\n> static_cast(minFrameDuration_),\n> src/ipa/raspberrypi/raspberrypi.cpp:\n> static_cast(maxFrameDuration_) });\n>\n> Where the RPi IPA reports the min/max frame durations received as\n> controls part of a Request before using them to clamp the AE computed\n> exposure time and blankings.\n>\n> +Naush, +David\n>\n> Wouldn't it be better to report the actual duration by knowing the\n> actual exposure+blanking values once AGC has run ?\n>\n\nThis was discussed some time back, and I think we loosely agreed that the\nvalues\nreturned here in the Request metadata are the min/max frame duration values\nthat\nare actually going to be used, as the may have been clipped based on sensor\nlimitations. Note that this gets put into the metadata only when a user\nrequest comes\nthrough, and not on every frame. Here is the description I worded in the\nyaml file:\n\n> When reported in metadata, the control expresses the minimum and maximum\nframe\n> durations used after being clipped to the sensor provided frame duration\nlimits.\n\nOur IPA does not return controls::FrameDurations per-frame, instead we\nreturn\ncontrols::SensorTimestamp that (almost) gives you the inter-frame duration\nafter\nexposure+blanking.\n\nRegards,\nNaush\n\n\n\n>\n> Thanks\n> j\n>\n> > + int64_t duration = durations[0] * 1000;\n> > +\n> resultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,\n> > + duration);\n> > + }\n> > + }\n> > +\n> > if (metadata.contains(controls::ScalerCrop)) {\n> > Rectangle crop = metadata.get(controls::ScalerCrop);\n> > int32_t cropRect[] = {\n> > --\n> > 2.27.0\n> >\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 9E2D4C3200\n\tfor ;\n\tFri, 21 May 2021 12:08:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2141F6891A;\n\tFri, 21 May 2021 14:08:56 +0200 (CEST)","from mail-lj1-x22f.google.com (mail-lj1-x22f.google.com\n\t[IPv6:2a00:1450:4864:20::22f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 60F1968911\n\tfor ;\n\tFri, 21 May 2021 14:08:54 +0200 (CEST)","by mail-lj1-x22f.google.com with SMTP id 131so23676434ljj.3\n\tfor ;\n\tFri, 21 May 2021 05:08:54 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"PsC236dy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=3LMK78VL+W2UynQBENtam4IbZbwCm2rrfnny1tIQFiA=;\n\tb=PsC236dyDTHaw/4T6b7cTjOdjIw+oSeqNIzhVhDWfJCLIIRh4pdpQRpftr2BE2tcRO\n\tCcRcTBAxzmg7YvUUiLHADWn8ZU4Kmuw48FNRsCBmOaS1ucaOOUREut5xOd+OMR3nhsee\n\t2U7x5q4gN8YbJSBCIvOwN3/uAIlhYsKfajmnJm6mtTdhQAx/XJstHNn5IcKSwo99PgK9\n\thn7LfDJlIoIYEjJ2MFli4CuYEu5K+KMLMA0WVP4BFF5ZQUNXzdKMAZ6C42XZt8ABq+Ft\n\t3d/rPL6X4yZZ+fnpWA0rrF5tVeZrGdjaRL7yZ2YYuA0vl43zitDgCduqx6imKSj5kMFY\n\tL69g==","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;\n\tbh=3LMK78VL+W2UynQBENtam4IbZbwCm2rrfnny1tIQFiA=;\n\tb=LFFad5izvHLDqnU3sjsYzabXuPLXsMvOpxhcSD2DNY50u0za9wk5CB0lCe0AbVg2Xv\n\t+nXrMXIqH1sHUTId3fw+YH0fs6201hqezeDwaDSx19hgSeXYH8hOO5nTCgjP255rK2dD\n\tpKyMCgpkzfEc4X3MnFF1rVKt8aV0nxytjwiYpC8/dFg9AmpdejgST94gFwBK5my1NU60\n\ttMcO5C4MqSbbJ8qmVRpu++qNvhYWms0f5xxaS+yby/EM2nxuYpuJI0hNHyS5PlPjbfkT\n\teFGthUf0FURzJLNK/+owz327tCkb9I6o5FjCOh5YopSJVA4f8FD98kSkPCZ+vsQ5QRFS\n\t7N0w==","X-Gm-Message-State":"AOAM533mT9G0Bg1w/Dtnbx8sZd8wZ88LmkKnj+iDgcjohmmU9fUR2FP8\n\tOS/lwJjHb+a+jCDCr7BULADmie4x7Skb480cz2QN9A==","X-Google-Smtp-Source":"ABdhPJxv9cpeOMVFw5WfOVnzNj5EiFhWX5cEhsKe7SfhNT8vDCyBOc5qLyxzkBNbVqLoI5DYZt4FZxa2K5ZDRafjzlc=","X-Received":"by 2002:a2e:9145:: with SMTP id q5mr6556959ljg.400.1621598933812;\n\tFri, 21 May 2021 05:08:53 -0700 (PDT)","MIME-Version":"1.0","References":"<20210521105534.1125303-1-paul.elder@ideasonboard.com>\n\t<20210521115432.shmbhtmuq6k4ycd7@uno.localdomain>","In-Reply-To":"<20210521115432.shmbhtmuq6k4ycd7@uno.localdomain>","From":"Naushir Patuck ","Date":"Fri, 21 May 2021 13:08:37 +0100","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"multipart/alternative; boundary=\"0000000000006b195705c2d5ec0f\"","Subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_device: Fix\n\tsensor frame duration","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\" "}},{"id":17119,"web_url":"https://patchwork.libcamera.org/comment/17119/","msgid":"<4233a2f2-adae-9e42-8c70-b36d6f61da78@ideasonboard.com>","date":"2021-05-22T11:29:55","subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_device: Fix\n\tsensor frame duration","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Paul,\n\nOn 5/21/21 4:25 PM, Paul Elder wrote:\n> The sensor frame duration should be set by IPA. Get the information for\n> the result metadata from libcamera.\n>\n> Signed-off-by: Paul Elder \n> ---\n> src/android/camera_device.cpp | 10 ++++++++++\n> 1 file changed, 10 insertions(+)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index b32e8be5..779ce554 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -2252,6 +2252,16 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> \t\tresultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, exposure);\n> \t}\n> \n> +\tif (metadata.contains(controls::FrameDurations)) {\n> +\t\tSpan durations =\n> +\t\t\tmetadata.get(controls::FrameDurations);\n> +\t\tif (durations[0] == durations[1]) {\n> +\t\t\tint64_t duration = durations[0] * 1000;\n> +\t\t\tresultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,\n> +\t\t\t\t\t\t duration);\n> +\t\t}\n> +\t}\n> +\n\nI wonder if the hard coding patch [2/2] should live here instead (as a \n'else' fallback), with a \\todo in ipa/ipu3.cpp ofcourse. Since, we have \ntwo IPAs for IPU3 platform(one residing externally), we would need to \nintroduce [2/2] in the external IPA as well, I think, sooner down the \nCTS line work. As far as I can see, there are already a few hard-coded \nvalues in camera_device.cpp for resultMetadata entries. I think this \napproach would serve a better \"singular\" place to look at regarding what \nentries are hard-coded and which one aren't. What do you think?\n\n> \tif (metadata.contains(controls::ScalerCrop)) {\n> \t\tRectangle crop = metadata.get(controls::ScalerCrop);\n> \t\tint32_t cropRect[] = {","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 AE463C3201\n\tfor ;\n\tSat, 22 May 2021 11:30:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D273668919;\n\tSat, 22 May 2021 13:30: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 134FA601AA\n\tfor ;\n\tSat, 22 May 2021 13:30:02 +0200 (CEST)","from localhost.localdomain (unknown [103.251.226.203])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2B74B88F\n\tfor ;\n\tSat, 22 May 2021 13:30:00 +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=\"Kb7xiqwQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621683001;\n\tbh=po9lYUkrco1qEj+eSYln9PcmuqDn+F1i8EWvGoROKSQ=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=Kb7xiqwQn+EngoykdPwmrHqMxAcK1qNyB1Hw/O6GYWoseAJ8ojlMkxEQFYKfbnW/y\n\tFID7X5W4nszWa/SmOWmONrYJmB4WuS5C2DzQ2YY8FAKVlbbqM29YRr+ODkUx5dFZcW\n\t/M3Av33r3A8VpCi7yczLqX/t0MXhDQcfDyr6lFRM=","To":"libcamera-devel@lists.libcamera.org","References":"<20210521105534.1125303-1-paul.elder@ideasonboard.com>","From":"Umang Jain ","Message-ID":"<4233a2f2-adae-9e42-8c70-b36d6f61da78@ideasonboard.com>","Date":"Sat, 22 May 2021 16:59:55 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.11.0","MIME-Version":"1.0","In-Reply-To":"<20210521105534.1125303-1-paul.elder@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_device: Fix\n\tsensor frame duration","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\" "}},{"id":17122,"web_url":"https://patchwork.libcamera.org/comment/17122/","msgid":"","date":"2021-05-23T00:14:35","subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_device: Fix\n\tsensor frame duration","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello everybody,\n\nOn Fri, May 21, 2021 at 01:08:37PM +0100, Naushir Patuck wrote:\n> On Fri, 21 May 2021 at 12:53, Jacopo Mondi wrote:\n> \n> > Hi Paul,\n> > the Fix in subject is probably not required, as the\n> > ANDROID_SENSOR_FRAME_DURATION property was not registered at all.\n> >\n> > On Fri, May 21, 2021 at 07:55:33PM +0900, Paul Elder wrote:\n> > > The sensor frame duration should be set by IPA. Get the information for\n> > > the result metadata from libcamera.\n> >\n> > how about:\n> >\n> > \"Libcamera reports the frame duration through the\n> > controls::FrameDuration controls. Populate the\n> > android.sensor.frameDuration result metadata using the libcamera\n> > control value\"\n> >\n> > > Signed-off-by: Paul Elder \n> > > ---\n> > > src/android/camera_device.cpp | 10 ++++++++++\n> > > 1 file changed, 10 insertions(+)\n> > >\n> > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > index b32e8be5..779ce554 100644\n> > > --- a/src/android/camera_device.cpp\n> > > +++ b/src/android/camera_device.cpp\n> > > @@ -2252,6 +2252,16 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> > > resultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, exposure);\n> > > }\n> > >\n> > > + if (metadata.contains(controls::FrameDurations)) {\n> > > + Span durations =\n> > > + metadata.get(controls::FrameDurations);\n> > > + if (durations[0] == durations[1]) {\n> >\n> > Yes, we have a bit of an unspecified behaviour here.\n> >\n> > What is the use case for reporting two different frame durations ?\n> >\n> > Indeed I see this one:\n> > src/ipa/raspberrypi/raspberrypi.cpp:\n> > libcameraMetadata_.set(controls::FrameDurations,\n> > src/ipa/raspberrypi/raspberrypi.cpp: {\n> > static_cast(minFrameDuration_),\n> > src/ipa/raspberrypi/raspberrypi.cpp:\n> > static_cast(maxFrameDuration_) });\n> >\n> > Where the RPi IPA reports the min/max frame durations received as\n> > controls part of a Request before using them to clamp the AE computed\n> > exposure time and blankings.\n> >\n> > +Naush, +David\n> >\n> > Wouldn't it be better to report the actual duration by knowing the\n> > actual exposure+blanking values once AGC has run ?\n> \n> This was discussed some time back, and I think we loosely agreed that the values\n> returned here in the Request metadata are the min/max frame duration values that\n> are actually going to be used, as the may have been clipped based on sensor\n> limitations.\n\nI don't recall if you had actual use cases to consume this in\napplications, do you remember ?\n\n> Note that this gets put into the metadata only when a user request comes\n> through, and not on every frame.\n\nWhat metadata should be reported every frame or only in specific\ncircumstances is still to be discussed, but that's a topic for later :-)\n\n> Here is the description I worded in the yaml file:\n> \n> > When reported in metadata, the control expresses the minimum and maximum frame\n> > durations used after being clipped to the sensor provided frame duration limits.\n> \n> Our IPA does not return controls::FrameDurations per-frame, instead we return\n> controls::SensorTimestamp that (almost) gives you the inter-frame duration after\n> exposure+blanking.\n\nThat's subject to jitter. The timestamp is important, but reporting the\nnominal frame duration is also important.\n\nHow about renaming FrameDurations to FrameDurationLimits, and adding a\nnew FrameDuration control to report the nominal duration for the frame\nin the metadata ?\n\n> > > + int64_t duration = durations[0] * 1000;\n> > > +\n> > resultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,\n> > > + duration);\n> > > + }\n> > > + }\n> > > +\n> > > if (metadata.contains(controls::ScalerCrop)) {\n> > > Rectangle crop = metadata.get(controls::ScalerCrop);\n> > > int32_t cropRect[] = {","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 931D4C3200\n\tfor ;\n\tSun, 23 May 2021 00:14:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9197260510;\n\tSun, 23 May 2021 02:14:40 +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 11BA660510\n\tfor ;\n\tSun, 23 May 2021 02:14:39 +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 6F45F8A8;\n\tSun, 23 May 2021 02:14:38 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"di+YufKQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621728878;\n\tbh=Lu95HJYoSgDSPY3cWS+xBkbt46P6CR7n6xPDenlxw3k=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=di+YufKQodJCgdvGxa/b/USQt7Y/7rb0ZSHZI5VYz3IQNa5dgQL1TXV1FdInEQIvd\n\t+ny+HG3GMShoG1LJPPv4l3W426OuPIuVOQb+/BJmVGAXR7wcwfkwVTrIJnQuT+pEXk\n\tO+Zhg+fZ5VYF64qpqhHKbNxUNk7HfjKw/d/Urg7c=","Date":"Sun, 23 May 2021 03:14:35 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210521105534.1125303-1-paul.elder@ideasonboard.com>\n\t<20210521115432.shmbhtmuq6k4ycd7@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_device: Fix\n\tsensor frame duration","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\" "}},{"id":17125,"web_url":"https://patchwork.libcamera.org/comment/17125/","msgid":"","date":"2021-05-23T06:32:30","subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_device: Fix\n\tsensor frame duration","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi all,\n\n\n\nOn Sun, 23 May 2021, 1:14 am Laurent Pinchart, <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hello everybody,\n>\n> On Fri, May 21, 2021 at 01:08:37PM +0100, Naushir Patuck wrote:\n> > On Fri, 21 May 2021 at 12:53, Jacopo Mondi wrote:\n> >\n> > > Hi Paul,\n> > > the Fix in subject is probably not required, as the\n> > > ANDROID_SENSOR_FRAME_DURATION property was not registered at all.\n> > >\n> > > On Fri, May 21, 2021 at 07:55:33PM +0900, Paul Elder wrote:\n> > > > The sensor frame duration should be set by IPA. Get the information\n> for\n> > > > the result metadata from libcamera.\n> > >\n> > > how about:\n> > >\n> > > \"Libcamera reports the frame duration through the\n> > > controls::FrameDuration controls. Populate the\n> > > android.sensor.frameDuration result metadata using the libcamera\n> > > control value\"\n> > >\n> > > > Signed-off-by: Paul Elder \n> > > > ---\n> > > > src/android/camera_device.cpp | 10 ++++++++++\n> > > > 1 file changed, 10 insertions(+)\n> > > >\n> > > > diff --git a/src/android/camera_device.cpp\n> b/src/android/camera_device.cpp\n> > > > index b32e8be5..779ce554 100644\n> > > > --- a/src/android/camera_device.cpp\n> > > > +++ b/src/android/camera_device.cpp\n> > > > @@ -2252,6 +2252,16 @@ CameraDevice::getResultMetadata(const\n> Camera3RequestDescriptor &descriptor) cons\n> > > > resultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME,\n> exposure);\n> > > > }\n> > > >\n> > > > + if (metadata.contains(controls::FrameDurations)) {\n> > > > + Span durations =\n> > > > + metadata.get(controls::FrameDurations);\n> > > > + if (durations[0] == durations[1]) {\n> > >\n> > > Yes, we have a bit of an unspecified behaviour here.\n> > >\n> > > What is the use case for reporting two different frame durations ?\n> > >\n> > > Indeed I see this one:\n> > > src/ipa/raspberrypi/raspberrypi.cpp:\n> > > libcameraMetadata_.set(controls::FrameDurations,\n> > > src/ipa/raspberrypi/raspberrypi.cpp: {\n> > > static_cast(minFrameDuration_),\n> > > src/ipa/raspberrypi/raspberrypi.cpp:\n> > > static_cast(maxFrameDuration_) });\n> > >\n> > > Where the RPi IPA reports the min/max frame durations received as\n> > > controls part of a Request before using them to clamp the AE computed\n> > > exposure time and blankings.\n> > >\n> > > +Naush, +David\n> > >\n> > > Wouldn't it be better to report the actual duration by knowing the\n> > > actual exposure+blanking values once AGC has run ?\n> >\n> > This was discussed some time back, and I think we loosely agreed that\n> the values\n> > returned here in the Request metadata are the min/max frame duration\n> values that\n> > are actually going to be used, as the may have been clipped based on\n> sensor\n> > limitations.\n>\n> I don't recall if you had actual use cases to consume this in\n> applications, do you remember ?\n>\n> > Note that this gets put into the metadata only when a user request comes\n> > through, and not on every frame.\n>\n> What metadata should be reported every frame or only in specific\n> circumstances is still to be discussed, but that's a topic for later :-)\n>\n> > Here is the description I worded in the yaml file:\n> >\n> > > When reported in metadata, the control expresses the minimum and\n> maximum frame\n> > > durations used after being clipped to the sensor provided frame\n> duration limits.\n> >\n> > Our IPA does not return controls::FrameDurations per-frame, instead we\n> return\n> > controls::SensorTimestamp that (almost) gives you the inter-frame\n> duration after\n> > exposure+blanking.\n>\n> That's subject to jitter. The timestamp is important, but reporting the\n> nominal frame duration is also important.\n>\n> How about renaming FrameDurations to FrameDurationLimits, and adding a\n> new FrameDuration control to report the nominal duration for the frame\n> in the metadata ?\n>\n\nSounds good to me.\n\nWe will need to sync the change with our libcamera-apps as they do use this\ncontrol, but I'm sure that's manageable.\n\nNaush\n\n\n\n> > > > + int64_t duration = durations[0] * 1000;\n> > > > +\n> > > resultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,\n> > > > + duration);\n> > > > + }\n> > > > + }\n> > > > +\n> > > > if (metadata.contains(controls::ScalerCrop)) {\n> > > > Rectangle crop = metadata.get(controls::ScalerCrop);\n> > > > int32_t cropRect[] = {\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 54EC2C3200\n\tfor ;\n\tSun, 23 May 2021 06:32:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 931FC6891A;\n\tSun, 23 May 2021 08:32:43 +0200 (CEST)","from mail-lj1-x233.google.com (mail-lj1-x233.google.com\n\t[IPv6:2a00:1450:4864:20::233])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8A171602AE\n\tfor ;\n\tSun, 23 May 2021 08:32:42 +0200 (CEST)","by mail-lj1-x233.google.com with SMTP id w15so29192770ljo.10\n\tfor ;\n\tSat, 22 May 2021 23:32:42 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"k3ZLqSD7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=J9gfMbKKt1Z0i3uZeQo0u5kRqMiOQANq3YEB+9gGmYA=;\n\tb=k3ZLqSD720VU9dI4Qmlx+t29xDTZ2xmhct8YyVJAp6o0vxSUM6bzVeHWYLp3pMIS63\n\tDDOH3DkCufKpQEUK69az/viKqlPp37pBeoTP8K543YzEH683VeXhdjBxwE3wlHAxnQIa\n\t1HHao8KFLb4by9DdAULD9T9eP0wOEcn5aRd0hWtbkVV4TXmEfnOAP6r9GBuct2K9ubPA\n\t1C15lMcjk9TYmScwwmQ1CZPYLSTsi3fXHOrOq+LEqjA7pOOgU10lrTXQIovf4obOA0ZG\n\tFGkhhK2H1+vUEjl27a/LP8YMP2LLqJHMmQUEAXf0+w9f2f3jEgGdACG3Lb0Ib4Ix+6+9\n\teDSA==","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;\n\tbh=J9gfMbKKt1Z0i3uZeQo0u5kRqMiOQANq3YEB+9gGmYA=;\n\tb=Ho2Q6ljMHRNSxFc51ToAACwSnBQ4yMnizKFLAeTzADn1epBLXB1LJ0NO9v1nn9KVUQ\n\tHvDVzXEPkeSRS9VGnP+83wqXZjM2Owhc1iFToRKy9bQL5PUH81i5tIjBrVvhrrTrckcu\n\tjwd99BSpw/NPHDpnTdpeVstfbS5BQAq0aZe606ybfac3YvHANRDx8Cze3YRq6EEUwy84\n\tcb8KN94JoAz7iZDCYnjL3SjRy5mdBtZXsjkLKAq8MqkkX47o721WG22X7gW5SFYTxJQH\n\tMF2vFSU1lm6B0qSQlpa8ERuTJQEloCPrIsVj5Bxx5Y1ts+bhuyQKKZHa7lNXVgMVYI4F\n\t+SbQ==","X-Gm-Message-State":"AOAM533tHjuowaYjYsobenphW98+RWTqfEk1/Od3oNjllN4A5WfMGEnD\n\tzvVwuVdWZhLNdXWir1UOJeCTx8z1oY7TBqextU/BjiCVE64=","X-Google-Smtp-Source":"ABdhPJxXyRXIqkMclFUsK37VQGDR/ZMv2qeMuaweDUELNGKWK1XiLpxnfHFfDX2//vEwZkqXct/34B4/zjYyyTKT+kQ=","X-Received":"by 2002:a05:651c:1311:: with SMTP id\n\tu17mr12326685lja.48.1621751561794; \n\tSat, 22 May 2021 23:32:41 -0700 (PDT)","MIME-Version":"1.0","References":"<20210521105534.1125303-1-paul.elder@ideasonboard.com>\n\t<20210521115432.shmbhtmuq6k4ycd7@uno.localdomain>\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Sun, 23 May 2021 07:32:30 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"000000000000c14d7605c2f975fa\"","Subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_device: Fix\n\tsensor frame duration","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\" "}}]