[{"id":17253,"web_url":"https://patchwork.libcamera.org/comment/17253/","msgid":"","date":"2021-05-26T06:53:13","subject":"Re: [libcamera-devel] [PATCH v4 1/3] controls: Split FrameDurations\n\tinto FrameDuration and FrameDurationLimits","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Paul, thank you for the patch.\n\nOn Wed, May 26, 2021 at 12:47 PM Paul Elder \nwrote:\n\n> We need a separate control to report the nominal frame duration, but\n> it's also useful to report the min/max frame duration values that will\n> be used. Split the FrameDurations control into FrameDuration and\n> FrameDurationLimits respectively to support both of these.\n>\n> Signed-off-by: Paul Elder \n> Reviewed-by: Laurent Pinchart \n>\n>\nReviewed-by: Hirokazu Honda \n\n\n> ---\n> Changes in v4:\n> - set FrameDurationLimits (instead of FrameDuration) in the ipu3\n> pipeline handler\n>\n> Changes in v3:\n> - mention that the control is meant to be returned in metadata\n> - s/nominal/instantaneous\n> ---\n> include/libcamera/ipa/raspberrypi.h | 2 +-\n> src/android/camera_device.cpp | 2 +-\n> src/ipa/raspberrypi/raspberrypi.cpp | 4 ++--\n> src/libcamera/control_ids.yaml | 9 ++++++++-\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 7 ++++---\n> 5 files changed, 16 insertions(+), 8 deletions(-)\n>\n> diff --git a/include/libcamera/ipa/raspberrypi.h\n> b/include/libcamera/ipa/raspberrypi.h\n> index d10c1733..a8790000 100644\n> --- a/include/libcamera/ipa/raspberrypi.h\n> +++ b/include/libcamera/ipa/raspberrypi.h\n> @@ -44,7 +44,7 @@ static const ControlInfoMap Controls = {\n> { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },\n> { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },\n> { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535,\n> 65535, 65535, 65535), Rectangle{}) },\n> - { &controls::FrameDurations, ControlInfo(INT64_C(1000),\n> INT64_C(1000000000)) },\n> + { &controls::FrameDurationLimits, ControlInfo(INT64_C(1000),\n> INT64_C(1000000000)) },\n> { &controls::draft::NoiseReductionMode,\n> ControlInfo(controls::draft::NoiseReductionModeValues) },\n> };\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index dc0c8f5f..9e267b5f 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -864,7 +864,7 @@ const camera_metadata_t\n> *CameraDevice::getStaticMetadata()\n>\n> int64_t minFrameDurationNsec = -1;\n> int64_t maxFrameDurationNsec = -1;\n> - const auto frameDurationsInfo =\n> controlsInfo.find(&controls::FrameDurations);\n> + const auto frameDurationsInfo =\n> controlsInfo.find(&controls::FrameDurationLimits);\n> if (frameDurationsInfo != controlsInfo.end()) {\n> minFrameDurationNsec =\n> frameDurationsInfo->second.min().get() * 1000;\n> maxFrameDurationNsec =\n> frameDurationsInfo->second.max().get() * 1000;\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> index e5bb8159..0c4752ec 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -859,7 +859,7 @@ void IPARPi::queueRequest(const ControlList &controls)\n> break;\n> }\n>\n> - case controls::FRAME_DURATIONS: {\n> + case controls::FRAME_DURATION_LIMITS: {\n> auto frameDurations = ctrl.second.get int64_t>>();\n> applyFrameDurations(frameDurations[0],\n> frameDurations[1]);\n> break;\n> @@ -1074,7 +1074,7 @@ void IPARPi::applyFrameDurations(double\n> minFrameDuration, double maxFrameDuratio\n> maxFrameDuration_ = std::max(maxFrameDuration_, minFrameDuration_);\n>\n> /* Return the validated limits via metadata. */\n> - libcameraMetadata_.set(controls::FrameDurations,\n> + libcameraMetadata_.set(controls::FrameDurationLimits,\n> { static_cast(minFrameDuration_),\n> static_cast(maxFrameDuration_)\n> });\n>\n> diff --git a/src/libcamera/control_ids.yaml\n> b/src/libcamera/control_ids.yaml\n> index 88d81ac4..f62ade48 100644\n> --- a/src/libcamera/control_ids.yaml\n> +++ b/src/libcamera/control_ids.yaml\n> @@ -323,7 +323,14 @@ controls:\n> step to respect the received gain factor and shall report\n> their total value in the request metadata.\n>\n> - - FrameDurations:\n> + - FrameDuration:\n> + type: int64_t\n> + description: |\n> + The instantaneous frame duration from start of frame exposure to\n> start\n> + of next exposure, expressed in microseconds. This control is\n> meant to\n> + be returned in metadata.\n> +\n> + - FrameDurationLimits:\n> type: int64_t\n> description: |\n> The minimum and maximum (in that order) frame duration,\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 25203256..2d15d488 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -983,9 +983,10 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData\n> *data)\n> frameDurations[i] = frameSize / (sensorInfo.pixelRate /\n> 1000000U);\n> }\n>\n> - controls[&controls::FrameDurations] =\n> ControlInfo(frameDurations[0],\n> -\n> frameDurations[1],\n> -\n> frameDurations[2]);\n> + controls[&controls::FrameDurationLimits] =\n> + ControlInfo(frameDurations[0],\n> + frameDurations[1],\n> + frameDurations[2]);\n>\n> /*\n> * Compute the scaler crop limits.\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 51F96C3203\n\tfor ;\n\tWed, 26 May 2021 06:53:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0D03568922;\n\tWed, 26 May 2021 08:53:26 +0200 (CEST)","from mail-ed1-x529.google.com (mail-ed1-x529.google.com\n\t[IPv6:2a00:1450:4864:20::529])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F1C4C6891B\n\tfor ;\n\tWed, 26 May 2021 08:53:24 +0200 (CEST)","by mail-ed1-x529.google.com with SMTP id h16so39134204edr.6\n\tfor ;\n\tTue, 25 May 2021 23:53:24 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"DEUn1jE3\"; 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=506/sHfDIdT8HBSPLLUa2ODSFqnwpR9QfEv65PbocbI=;\n\tb=DEUn1jE3Z//GNYpOZODY1Ce0PQaQKNEIjPGWVQsh61jxkXS4Quy3Mp/qCon53t5Xr1\n\tBrIUe5gKKmwRVZZdjFrLx3OEC6EQd6J0V89kNWCo5Xj5I6lnVOlAzLan16BSlEl8Wq6L\n\tyYbhi7iofo+ZTBMJt3bS1T3mksPGZxXmC9lOM=","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=506/sHfDIdT8HBSPLLUa2ODSFqnwpR9QfEv65PbocbI=;\n\tb=R34NehC/UFl4sKusKSQeYFX2Q3JZmG3gX9aSGWtldmDS03C8Vmv/1ckMWQ6ehybfju\n\tnNlqNdvXL1sSuOUEP6GdNxW+PlLmff1R/fZ984xq4S+tYnlJ6kMJ701BWuQgujlJz4Sd\n\thVG9D83Jh09zyYRceUMXEfyaCmTi83X7kPqFmySCMlPeQLT1Com8JlP/WZ3krbUBZDvO\n\t+p9HXvl5vjPsdvOYxpCpFUrSyI/hu3SXDR52Qq4vldI9llNMWLzZSxuiav/XjsBOEgJt\n\tJQU+oIT8pfkEYJoP8D1m+3Z5+mjmXUFREuMkse0zgEPVMYa1Z9ptDxHcHP1Hr19rn32V\n\t1qEg==","X-Gm-Message-State":"AOAM532QRyTr9EC3TYKqWISzZSLbx61NyHbaklDucw4Q/Nd0IApTTNXM\n\tVpgf1D5v8I+rYuA2bVUIUmQqOMOotCHobzA+494UZQ==","X-Google-Smtp-Source":"ABdhPJwqi2+M7MldO35i+D0eE3dgcb8rmTUXI9G7L/gg/30d0I0z0RR3QCCf3Dy3PmOxIGhRTsYTSvbPMnY4i8Mybm4=","X-Received":"by 2002:a05:6402:95d:: with SMTP id\n\th29mr33900617edz.233.1622012004663; \n\tTue, 25 May 2021 23:53:24 -0700 (PDT)","MIME-Version":"1.0","References":"<20210526034720.1253094-1-paul.elder@ideasonboard.com>","In-Reply-To":"<20210526034720.1253094-1-paul.elder@ideasonboard.com>","From":"Hirokazu Honda ","Date":"Wed, 26 May 2021 15:53:13 +0900","Message-ID":"","To":"Paul Elder ","Content-Type":"multipart/alternative; boundary=\"0000000000005c188905c3361909\"","Subject":"Re: [libcamera-devel] [PATCH v4 1/3] controls: Split FrameDurations\n\tinto FrameDuration and FrameDurationLimits","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":17259,"web_url":"https://patchwork.libcamera.org/comment/17259/","msgid":"<20210526081127.xx225irwoltixsdn@uno.localdomain>","date":"2021-05-26T08:11:27","subject":"Re: [libcamera-devel] [PATCH v4 1/3] controls: Split FrameDurations\n\tinto FrameDuration and FrameDurationLimits","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul,\n\nOn Wed, May 26, 2021 at 12:47:18PM +0900, Paul Elder wrote:\n> We need a separate control to report the nominal frame duration, but\n>\n> -\tcontrols[&controls::FrameDurations] = ControlInfo(frameDurations[0],\n> -\t\t\t\t\t\t\t frameDurations[1],\n> -\t\t\t\t\t\t\t frameDurations[2]);\n> +\tcontrols[&controls::FrameDurationLimits] =\n> +\t\tControlInfo(frameDurations[0],\n> +\t\t\t frameDurations[1],\n> +\t\t\t frameDurations[2]);\n\nThat's a weird alignement :)\nWhatever, I don't like going over 80 cols neither (but maybe the\nControlInfo(...) fits on one line now?)\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n>\n> \t/*\n> \t * Compute the scaler crop limits.\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 87DB9C3200\n\tfor ;\n\tWed, 26 May 2021 08:10:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 121A068921;\n\tWed, 26 May 2021 10:10:44 +0200 (CEST)","from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E54406891F\n\tfor ;\n\tWed, 26 May 2021 10:10:41 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 57731E000B;\n\tWed, 26 May 2021 08:10:41 +0000 (UTC)"],"Date":"Wed, 26 May 2021 10:11:27 +0200","From":"Jacopo Mondi ","To":"Paul Elder ","Message-ID":"<20210526081127.xx225irwoltixsdn@uno.localdomain>","References":"<20210526034720.1253094-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210526034720.1253094-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 1/3] controls: Split FrameDurations\n\tinto FrameDuration and FrameDurationLimits","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\" "}}]