From patchwork Fri Jan 29 11:16:12 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 11051 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 6C117BD808 for ; Fri, 29 Jan 2021 11:16:27 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3E75D683AA; Fri, 29 Jan 2021 12:16:27 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="PhjfmVro"; dkim-atps=neutral Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E6C75683A2 for ; Fri, 29 Jan 2021 12:16:25 +0100 (CET) Received: by mail-wr1-x434.google.com with SMTP id m13so8432076wro.12 for ; Fri, 29 Jan 2021 03:16:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=x05WP2jGPAHR3jvXFT93jDzxY90RnftgMznUPHjSXzM=; b=PhjfmVrosEtWrcb6JwldGS0FPuiYnEA+anMREJDA9YBMb5tD1LXHhFZGkj+31vBKBl crkAcpivc0aWxDa+4AbUSAQBoNNcZWBzVcRtbwAsGFB5zi796bH+Hzz6YtwpZSPFM+xC FKsCiSJnTYvKUL35KUMj823ST+sxBRyeyJ+szIlLR8nQCtMzVBz5x4daYRXqf4jIF/1N NMxZbNmhV58V2N29b/BkTywmozq8alikDxlHx6f7gCfSwsZI/VgwCzWNxC7jaSwXLKuG 3fh2togvCsf0MMW3fEU+L5c246YqzNsaEEPPgkoCAJeIyvYabbpKFlUqk03FdpqfVUAO KCdQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=x05WP2jGPAHR3jvXFT93jDzxY90RnftgMznUPHjSXzM=; b=uj8FO/pPSSGJh335SUCpccyvW2oW1L4DMkstSpnE7Q4rWXasQHFv1y7bssTqxeGYU2 5ZQ5bpmIu4ReuQcSNCBeWQsdxLtboqCOp6/o3sWW6gX4Zyb1EpyMm+h5M6/ZR2Pssrwr IaCLyHbrWSFhZe9gPY6bl5vtvh/cSQ0XoGEx27ukjn8JLLT5cp+0qS8QhztYZHg6T23a egaEsO8DAq+Q3q/OAtrnmOZkfxf7DIOwGC5Ibk3H07npmJUZhG5CLi1HTCzg2XNRoaQx gU4RCXpdsEslhoCZw2Grn1Ouaq+6NSO3jO7B5u7K2dKjoiQ186Slbz8KUKVTR30WgPBG TCgA== X-Gm-Message-State: AOAM531bjxCq0V8espyBoWcZJ39+xm9pzCs2nWpNLbQRZDJBbWMM2Uy5 oGWQx3xuV5jTxBYv1N+8bV6m5EurZNWi5YLf X-Google-Smtp-Source: ABdhPJxFEvlzy38LE3dp1AwHGQkoun9SU/Rvuhirk6jtAUACosC1C1CZ6Dv414ayYNUAuuBQTAVtQQ== X-Received: by 2002:a5d:4a0c:: with SMTP id m12mr3935345wrq.309.1611918985479; Fri, 29 Jan 2021 03:16:25 -0800 (PST) Received: from naushir-VirtualBox.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id f14sm11324007wre.69.2021.01.29.03.16.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 29 Jan 2021 03:16:23 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Fri, 29 Jan 2021 11:16:12 +0000 Message-Id: <20210129111616.1047483-2-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210129111616.1047483-1-naush@raspberrypi.com> References: <20210129111616.1047483-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 1/5] libcamera: camera_sensor: Make VBLANK mandatory X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Jacopo Mondi Add the V4L2_CID_VBLANK control to the list of mandatory controls the sensor driver has to report and document the new requirement. The vertical blanking control is used to calculate the frame duration. Signed-off-by: Jacopo Mondi Signed-off-by: Naushir Patuck Reviewed-by: Naushir Patuck Reviewed-by: Laurent Pinchart --- Documentation/sensor_driver_requirements.rst | 6 ++++-- src/libcamera/camera_sensor.cpp | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Documentation/sensor_driver_requirements.rst b/Documentation/sensor_driver_requirements.rst index 6dcd4e68d64d..590797f054ce 100644 --- a/Documentation/sensor_driver_requirements.rst +++ b/Documentation/sensor_driver_requirements.rst @@ -27,18 +27,20 @@ The sensor driver shall support the following V4L2 controls: * `V4L2_CID_EXPOSURE`_ * `V4L2_CID_HBLANK`_ * `V4L2_CID_PIXEL_RATE`_ +* `V4L2_CID_VBLANK`_ .. _V4L2_CID_EXPOSURE: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/control.html .. _V4L2_CID_HBLANK: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-image-source.html .. _V4L2_CID_PIXEL_RATE: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-image-process.html +.. _V4L2_CID_VBLANK: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-image-source.html While V4L2 doesn't specify a unit for the `EXPOSURE` control, libcamera requires it to be expressed as a number of image lines. Camera sensor drivers that do not comply with this requirement will need to be adapted or will produce incorrect results. -The `HBLANK` and `PIXEL_RATE` controls are used to compute the sensor output -timings. +The `HBLANK`, `PIXEL_RATE` and `VBLANK` controls are used to compute the sensor +output timings. Optional Requirements --------------------- diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index ca33c0094088..ab315bdc468c 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -244,6 +244,7 @@ int CameraSensor::validateSensorDriver() V4L2_CID_EXPOSURE, V4L2_CID_HBLANK, V4L2_CID_PIXEL_RATE, + V4L2_CID_VBLANK, }; ControlList ctrls = subdev_->getControls(mandatoryControls); From patchwork Fri Jan 29 11:16:13 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 11052 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id CEE39BD808 for ; Fri, 29 Jan 2021 11:16:29 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9AD31683AD; Fri, 29 Jan 2021 12:16:29 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="t4xD5WZw"; dkim-atps=neutral Received: from mail-wr1-x42d.google.com (mail-wr1-x42d.google.com [IPv6:2a00:1450:4864:20::42d]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C28FA683A2 for ; Fri, 29 Jan 2021 12:16:27 +0100 (CET) Received: by mail-wr1-x42d.google.com with SMTP id s7so5447320wru.5 for ; Fri, 29 Jan 2021 03:16:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=lB1gEBJZ04IbOiJJDY++VCv0eADH9q7VDwMF3UMW9ZE=; b=t4xD5WZwzNgzr08U7mXeyzSFdEtEu0JWdOMyWkEH4cfCushPHvdnFBp8yUD+IgjQgD Xt6YNHjljrkQ09LUV6WAOkdGn5Dt4d1NqP3XgFZvItSWi4yFPAePW76K+CXE5qlJ3am2 WmePCBVrjjLsZn0Nj4ec06ng1rLcQGkuSjMmToMfeL21loOdnEHOPts0O0n5QWmmxXpg GkiypFM+DVtzEP6SCBE7MrfJT9yRyyL+EgaYdipnnG6MLBzf+2vDupwXwhRAtMd8z9lm /SPqwma5xLbzfhV/UpyLdYY1eHVgJlH/mbvs2sk/3wUzuUNW44/Z4xjs5mx7ezhvgK7N NRMw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=lB1gEBJZ04IbOiJJDY++VCv0eADH9q7VDwMF3UMW9ZE=; b=RTYPGZsHXPHto/dNnUOGhN/RDTjEh3BCb5FEVeTQKKVa2OWV54vci3dsuS6iGRo9A5 2e+vG9phTP7sM0Sb1fiFd2dYnL1DGaiqYEkxD4gXq5/6Smnh0+mZbKSNdQI4hHimG7VW f6CirUKUKPmsTmyxT3DKBIgVW6zAkB79/S1pza6nVCgZ7poJvJwBI3vi1KuUXwOXt4Rs 87iDGEDzXvTQepfg+DMhJKQPNlfITd7TA8U3Ws7PEPhHph9SDsmH4eYhpbTFwsLzqZSf ZZDp3jHRZsZtmxW6onQR+BgN+xc7rwDSQU4IQbk2N485vYROMJ0L+HxyLZ21I3jYxmCn Ey3Q== X-Gm-Message-State: AOAM530IwMlJalkiDeXZpmP40cf2tHjAfRUY8qNuEe+dBqqoy2cqi9ta zVlKHyP7ojxfUeVoyuF0sKwR9uFI0max3SX6 X-Google-Smtp-Source: ABdhPJw1O1IrHI7ijkdUgWmbG6b8b+0a8G/SaMfEVAzZSNEULQz+KKBFz49AhqiASlkxCixfsLqEVA== X-Received: by 2002:adf:ee43:: with SMTP id w3mr4201606wro.200.1611918987251; Fri, 29 Jan 2021 03:16:27 -0800 (PST) Received: from naushir-VirtualBox.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id f14sm11324007wre.69.2021.01.29.03.16.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 29 Jan 2021 03:16:26 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Fri, 29 Jan 2021 11:16:13 +0000 Message-Id: <20210129111616.1047483-3-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210129111616.1047483-1-naush@raspberrypi.com> References: <20210129111616.1047483-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 2/5] libcamera: camera_sensor: Add frame length limits to CameraSensorInfo X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Sensor frame length is made up of active and inactive (blanking) lines. The minimum and maximum frame length values may be used by pipeline handlers to limit frame durations based on the sensor mode capabilities. Store the minimum and maximum allowable frame length values (in lines) in the CameraSensorInfo structure. These values are computed in CameraSensor::sensorInfo() by querying the sensor subdevice V4L2_CID_VBLANK control limits. This in turn means that V4L2_CID_VBLANK is now a mandatory subdevice control. Signed-off-by: Naushir Patuck Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- include/libcamera/internal/camera_sensor.h | 3 ++ src/libcamera/camera_sensor.cpp | 43 ++++++++++++++++++++-- test/ipa/ipa_wrappers_test.cpp | 2 + 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h index fed36bf26e47..5d8c9b1a3121 100644 --- a/include/libcamera/internal/camera_sensor.h +++ b/include/libcamera/internal/camera_sensor.h @@ -33,6 +33,9 @@ struct CameraSensorInfo { uint64_t pixelRate; uint32_t lineLength; + + uint32_t minFrameLength; + uint32_t maxFrameLength; }; class CameraSensor : protected Loggable diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index ab315bdc468c..f60d0cc9c6fa 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -113,6 +113,36 @@ LOG_DEFINE_CATEGORY(CameraSensor) * The total line length in pixel clock periods, including blanking. */ +/** + * \var CameraSensorInfo::minFrameLength + * \brief The minimum allowable frame length in units of lines + * + * The sensor frame length comprises of active output lines and blanking lines + * in a frame. The minimum frame length value dictates the minimum allowable + * frame duration of the sensor mode. + * + * To obtain the minimum frame duration: + * + * \verbatim + frameDuration(s) = minFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second) + \endverbatim + */ + +/** + * \var CameraSensorInfo::maxFrameLength + * \brief The maximum allowable frame length in units of lines + * + * The sensor frame length comprises of active output lines and blanking lines + * in a frame. The maximum frame length value dictates the maximum allowable + * frame duration of the sensor mode. + * + * To obtain the maximum frame duration: + * + * \verbatim + frameDuration(s) = maxFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second) + \endverbatim + */ + /** * \class CameraSensor * \brief A camera sensor based on V4L2 subdevices @@ -699,12 +729,13 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const info->outputSize = format.size; /* - * Retrieve the pixel rate and the line length through V4L2 controls. - * Support for the V4L2_CID_PIXEL_RATE and V4L2_CID_HBLANK controls is - * mandatory. + * Retrieve the pixel rate, line length and minimum/maximum frame + * duration through V4L2 controls. Support for the V4L2_CID_PIXEL_RATE, + * V4L2_CID_HBLANK and V4L2_CID_VBLANK controls is mandatory. */ ControlList ctrls = subdev_->getControls({ V4L2_CID_PIXEL_RATE, - V4L2_CID_HBLANK }); + V4L2_CID_HBLANK, + V4L2_CID_VBLANK }); if (ctrls.empty()) { LOG(CameraSensor, Error) << "Failed to retrieve camera info controls"; @@ -715,6 +746,10 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const info->lineLength = info->outputSize.width + hblank; info->pixelRate = ctrls.get(V4L2_CID_PIXEL_RATE).get(); + const ControlInfo vblank = ctrls.infoMap()->at(V4L2_CID_VBLANK); + info->minFrameLength = info->outputSize.height + vblank.min().get(); + info->maxFrameLength = info->outputSize.height + vblank.max().get(); + return 0; } diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp index 47533d105d03..eb6d783e8489 100644 --- a/test/ipa/ipa_wrappers_test.cpp +++ b/test/ipa/ipa_wrappers_test.cpp @@ -313,6 +313,8 @@ protected: .outputSize = { 2560, 1940 }, .pixelRate = 96000000, .lineLength = 2918, + .minFrameLength = 1940, + .maxFrameLength = 2880 }; std::map config{ { 1, { V4L2_PIX_FMT_YUYV, { 1024, 768 } } }, From patchwork Fri Jan 29 11:16:14 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 11053 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 49E41BD808 for ; Fri, 29 Jan 2021 11:16:33 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1769C683B2; Fri, 29 Jan 2021 12:16:33 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="qpHzXIAK"; dkim-atps=neutral Received: from mail-wr1-x42d.google.com (mail-wr1-x42d.google.com [IPv6:2a00:1450:4864:20::42d]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D4F7C683A2 for ; Fri, 29 Jan 2021 12:16:30 +0100 (CET) Received: by mail-wr1-x42d.google.com with SMTP id a1so8464748wrq.6 for ; Fri, 29 Jan 2021 03:16:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=JTTOH31m7p+bu3QZbZYeF2LaEMpxFjrTVsaqweEBDIo=; b=qpHzXIAKqHbxIEOukqmcnYJf96XztdOh2iYAeWN+Oh7pzDVSe8gmIBFMAzi9GrrZDh IfzrQHhsAcGF0ru0isgp4dLsAuyIl6xYDmh9JVOdrQTVaX36YfMYrmpmzG9B7ayBzA43 UOZoNfWoPeVm2wmsncDj0SFkGfnUeihAMM46+sqfHFiqkMBcuyekd3dNk9Yi/PcpCzHk 0gw9EIuaCwm0fWkgGQmJLpY4RQPUFCnImXIL4lToETPokXoRr7yzuoVk6fzV6CxKWXu7 5k5mtV0XQqn5Q/D8vMyBaLnmTRQabNIoiM86aXu4rbciwYrKC6gxOcF1SnKe8JPZeQWK sY3g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=JTTOH31m7p+bu3QZbZYeF2LaEMpxFjrTVsaqweEBDIo=; b=sV9xE2+BfzxIyynURpvDfSAAomID/PfiYTtrDLMzA/OB2K5zNQ1qzQyGN/YTUVCSOh ciJfyBvuIc0Z2pbh2nlsIZ3R8juy1HjulNctaBBpKShtvmHY7cD1I8atozlr3Y5r/1ss S3WSaDitVGlgFPusSOEGwujzOWysPO/d5+vfkNqDvRjIe4R4fdrlS+rb/u/CVsFxPLsC 4KD9JPLoAR0BqVHaSxf99lS7hvtcHzELedlBRlBasPNM/q1tzVQA0/OHCeOKY3dLs/Ao 7Q8doHgpK412oZ/2hGjqs5p0S/LoMATvnT+BglCd0K/b99XzKGamWnIh118Ls8Q3GIlv jO2A== X-Gm-Message-State: AOAM532wnq+xMOB7XEUsibY9NaAoApE9lS1Z7X9F7WbvdnciWfEmIayp 2wp7/6bFhxA7rtU/WQNkQHMg67rdKyC0nRBt X-Google-Smtp-Source: ABdhPJz1aDqKWFWXKqeGtv8+dJ4jlBJADZCpPsKDNbA+hyQED4EQFMVz1rmDpJjprZcLAtopoMLklg== X-Received: by 2002:a5d:58c2:: with SMTP id o2mr3956475wrf.31.1611918990196; Fri, 29 Jan 2021 03:16:30 -0800 (PST) Received: from naushir-VirtualBox.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id f14sm11324007wre.69.2021.01.29.03.16.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 29 Jan 2021 03:16:28 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Fri, 29 Jan 2021 11:16:14 +0000 Message-Id: <20210129111616.1047483-4-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210129111616.1047483-1-naush@raspberrypi.com> References: <20210129111616.1047483-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 3/5] ipa: raspberrypi: Limit the calculated vblank based on the sensor mode X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The existing framerate/vblank calculations did not account for the sensor mode limits. This commit extracts vblank limits from the sensor v4l2 controls, and stores it in the camera modes structure. Exposure and vblank value calculations now use values clamped to the sensor mode limits. The libcamera::controls::FrameDurations metadata return values now reflect the minimum and maximum after clamping. Signed-off-by: Naushir Patuck Reviewed-by: David Plowman Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- src/ipa/raspberrypi/cam_helper.cpp | 16 +++---- src/ipa/raspberrypi/cam_helper.hpp | 5 +- src/ipa/raspberrypi/cam_helper_imx219.cpp | 6 +-- src/ipa/raspberrypi/cam_helper_imx477.cpp | 4 +- src/ipa/raspberrypi/cam_helper_ov5647.cpp | 4 +- src/ipa/raspberrypi/controller/camera_mode.h | 2 + src/ipa/raspberrypi/raspberrypi.cpp | 49 +++++++++++++------- 7 files changed, 47 insertions(+), 39 deletions(-) diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp index b7b8faf09c69..93d1b7b0296a 100644 --- a/src/ipa/raspberrypi/cam_helper.cpp +++ b/src/ipa/raspberrypi/cam_helper.cpp @@ -34,9 +34,8 @@ CamHelper *CamHelper::Create(std::string const &cam_name) return nullptr; } -CamHelper::CamHelper(MdParser *parser, unsigned int maxFrameLength, - unsigned int frameIntegrationDiff) - : parser_(parser), initialized_(false), maxFrameLength_(maxFrameLength), +CamHelper::CamHelper(MdParser *parser, unsigned int frameIntegrationDiff) + : parser_(parser), initialized_(false), frameIntegrationDiff_(frameIntegrationDiff) { } @@ -67,13 +66,12 @@ uint32_t CamHelper::GetVBlanking(double &exposure, double minFrameDuration, assert(initialized_); /* - * Clamp frame length by the frame duration range and the maximum allowable - * value in the sensor, given by maxFrameLength_. + * minFrameDuration and maxFrameDuration are clamped by the caller + * based on the limits for the active sensor mode. */ - frameLengthMin = std::clamp(1e3 * minFrameDuration / mode_.line_length, - mode_.height, maxFrameLength_); - frameLengthMax = std::clamp(1e3 * maxFrameDuration / mode_.line_length, - mode_.height, maxFrameLength_); + frameLengthMin = 1e3 * minFrameDuration / mode_.line_length; + frameLengthMax = 1e3 * maxFrameDuration / mode_.line_length; + /* * Limit the exposure to the maximum frame duration requested, and * re-calculate if it has been clipped. diff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp index b1739a57e342..14d70112cb62 100644 --- a/src/ipa/raspberrypi/cam_helper.hpp +++ b/src/ipa/raspberrypi/cam_helper.hpp @@ -62,8 +62,7 @@ class CamHelper { public: static CamHelper *Create(std::string const &cam_name); - CamHelper(MdParser *parser, unsigned int maxFrameLength, - unsigned int frameIntegrationDiff); + CamHelper(MdParser *parser, unsigned int frameIntegrationDiff); virtual ~CamHelper(); void SetCameraMode(const CameraMode &mode); MdParser &Parser() const { return *parser_; } @@ -86,8 +85,6 @@ protected: private: bool initialized_; - /* Largest possible frame length, in units of lines. */ - unsigned int maxFrameLength_; /* * Smallest difference between the frame length and integration time, * in units of lines. diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp index 8688ec091c44..95b8e698fe3b 100644 --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp @@ -56,15 +56,13 @@ private: * in units of lines. */ static constexpr int frameIntegrationDiff = 4; - /* Largest possible frame length, in units of lines. */ - static constexpr int maxFrameLength = 0xffff; }; CamHelperImx219::CamHelperImx219() #if ENABLE_EMBEDDED_DATA - : CamHelper(new MdParserImx219(), maxFrameLength, frameIntegrationDiff) + : CamHelper(new MdParserImx219(), frameIntegrationDiff) #else - : CamHelper(new MdParserRPi(), maxFrameLength, frameIntegrationDiff) + : CamHelper(new MdParserRPi(), frameIntegrationDiff) #endif { } diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp index 5396131003f1..eaa7be16d20e 100644 --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp @@ -45,12 +45,10 @@ private: * in units of lines. */ static constexpr int frameIntegrationDiff = 22; - /* Largest possible frame length, in units of lines. */ - static constexpr int maxFrameLength = 0xffdc; }; CamHelperImx477::CamHelperImx477() - : CamHelper(new MdParserImx477(), maxFrameLength, frameIntegrationDiff) + : CamHelper(new MdParserImx477(), frameIntegrationDiff) { } diff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp b/src/ipa/raspberrypi/cam_helper_ov5647.cpp index 2bd8a754f133..a7f417324048 100644 --- a/src/ipa/raspberrypi/cam_helper_ov5647.cpp +++ b/src/ipa/raspberrypi/cam_helper_ov5647.cpp @@ -30,8 +30,6 @@ private: * in units of lines. */ static constexpr int frameIntegrationDiff = 4; - /* Largest possible frame length, in units of lines. */ - static constexpr int maxFrameLength = 0xffff; }; /* @@ -40,7 +38,7 @@ private: */ CamHelperOv5647::CamHelperOv5647() - : CamHelper(new MdParserRPi(), maxFrameLength, frameIntegrationDiff) + : CamHelper(new MdParserRPi(), frameIntegrationDiff) { } diff --git a/src/ipa/raspberrypi/controller/camera_mode.h b/src/ipa/raspberrypi/controller/camera_mode.h index 920f11beb0b0..256438c931d9 100644 --- a/src/ipa/raspberrypi/controller/camera_mode.h +++ b/src/ipa/raspberrypi/controller/camera_mode.h @@ -37,6 +37,8 @@ struct CameraMode { double line_length; // any camera transform *not* reflected already in the camera tuning libcamera::Transform transform; + // minimum and maximum fame lengths in units of lines + uint32_t min_frame_length, max_frame_length; }; #ifdef __cplusplus diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index 5ccc7a6551f5..e4911b734e20 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -102,6 +102,7 @@ private: void reportMetadata(); bool parseEmbeddedData(unsigned int bufferId, struct DeviceStatus &deviceStatus); void processStats(unsigned int bufferId); + void applyFrameDurations(double minFrameDuration, double maxFrameDuration); void applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls); void applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls); void applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls); @@ -279,6 +280,13 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo) * the line length in pixels and the pixel rate. */ mode_.line_length = 1e9 * sensorInfo.lineLength / sensorInfo.pixelRate; + + /* + * Set the frame length limits for the mode to ensure exposure and + * framerate calculations are clipped appropriately. + */ + mode_.min_frame_length = sensorInfo.minFrameLength; + mode_.max_frame_length = sensorInfo.maxFrameLength; } void IPARPi::configure(const CameraSensorInfo &sensorInfo, @@ -384,8 +392,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo, controller_.Initialise(); controllerInit_ = true; - minFrameDuration_ = defaultMinFrameDuration; - maxFrameDuration_ = defaultMaxFrameDuration; + /* Supply initial values for frame durations. */ + applyFrameDurations(defaultMinFrameDuration, defaultMaxFrameDuration); /* Supply initial values for gain and exposure. */ ControlList ctrls(sensorCtrls_); @@ -819,20 +827,7 @@ void IPARPi::queueRequest(const ControlList &controls) case controls::FRAME_DURATIONS: { auto frameDurations = ctrl.second.get>(); - - /* This will be applied once AGC recalculations occur. */ - minFrameDuration_ = frameDurations[0] ? frameDurations[0] : defaultMinFrameDuration; - maxFrameDuration_ = frameDurations[1] ? frameDurations[1] : defaultMaxFrameDuration; - maxFrameDuration_ = std::max(maxFrameDuration_, minFrameDuration_); - - /* - * \todo The values returned in the metadata below must be - * correctly clipped by what the sensor mode supports and - * what the AGC exposure mode or manual shutter speed limits - */ - libcameraMetadata_.set(controls::FrameDurations, - { static_cast(minFrameDuration_), - static_cast(maxFrameDuration_) }); + applyFrameDurations(frameDurations[0], frameDurations[1]); break; } @@ -991,6 +986,28 @@ void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls) static_cast(awbStatus->gain_b * 1000)); } +void IPARPi::applyFrameDurations(double minFrameDuration, double maxFrameDuration) +{ + const double minSensorFrameDuration = 1e-3 * mode_.min_frame_length * mode_.line_length; + const double maxSensorFrameDuration = 1e-3 * mode_.max_frame_length * mode_.line_length; + + /* + * This will only be applied once AGC recalculations occur. + * The values may be clamped based on the sensor mode capabilities as well. + */ + minFrameDuration_ = minFrameDuration ? minFrameDuration : defaultMaxFrameDuration; + maxFrameDuration_ = maxFrameDuration ? maxFrameDuration : defaultMinFrameDuration; + minFrameDuration_ = std::clamp(minFrameDuration_, + minSensorFrameDuration, maxSensorFrameDuration); + maxFrameDuration_ = std::clamp(maxFrameDuration_, + minSensorFrameDuration, maxSensorFrameDuration); + + /* Return the validated limits out though metadata. */ + libcameraMetadata_.set(controls::FrameDurations, + { static_cast(minFrameDuration_), + static_cast(maxFrameDuration_) }); +} + void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls) { int32_t gainCode = helper_->GainCode(agcStatus->analogue_gain); From patchwork Fri Jan 29 11:16:15 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 11054 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id DA791BD808 for ; Fri, 29 Jan 2021 11:16:33 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A210C683AB; Fri, 29 Jan 2021 12:16:33 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="iJ9Z3IB/"; dkim-atps=neutral Received: from mail-wm1-x334.google.com (mail-wm1-x334.google.com [IPv6:2a00:1450:4864:20::334]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 52240683B2 for ; Fri, 29 Jan 2021 12:16:32 +0100 (CET) Received: by mail-wm1-x334.google.com with SMTP id o10so6822602wmc.1 for ; Fri, 29 Jan 2021 03:16:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=sokBqjo6NSR747y41vXTQwh17vFWOqvUtMH/XwPfKPg=; b=iJ9Z3IB/Ggeyz0yzpsWz7CntxPtbS51uPCX7Nc5GZL4KWYoB27iOsWbm9xFy4wjx5u e5GlQad9CfupTLMwd+uK0JgsBSG9iJVt90Qb5Qy+LYPSEWQwWjwPKFbBzkJ5duij2oh2 NgQgSgomHYfHCQAXdkUsORaGxdT3kdKad0Q0ocmtcIys/0gOvUqKTckop6An0YXIruyT YUABpp2uZjihHyj1pc9CcZCTHV+Lw3q+li95DWIQIGfqf2Vy2AB9sr4fSDSQD72Dpoo6 pzkWUQAKqqb04po2cOTDpZNBFe0s66PHqpGxxF3bYePBnvE+S0uue/b9aVY8vRV/rmoe 1fiQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=sokBqjo6NSR747y41vXTQwh17vFWOqvUtMH/XwPfKPg=; b=Jzg9g6YlQOIELC9j0cYwyJ95NsI6LPiF06k76aG6ViaStpiduOys+GRE9GYyqybFAv G7kpVVXfSY1yPeJrsKYPv3J8ATHcx8CYnH7cKOIpnk6yRGjl5qLJLyD85vTlQ83gcZM2 OCKBkH2RyWwoFae1oKH5SgmZs2Y7UvJN0q6QTa9Q1uKeKcH0ylEI5aiw+xUDeB2iYaaf bePBO03Pds4YH9wMo2Pyb7bKcs62lCTXi9SIgWgsdqst9orTYSuwqRuPVvzkurusNR2L wDW/bIi9WWjSAkyeRfTWhAOjWynvcw5MqNWozZdsfaQX2amv+el31LaussXFyRDpWf5H LYjg== X-Gm-Message-State: AOAM5306lUTMgSwGrukvJmtVopnSbNthDxCk4K0yoPIjDW03GZE8R0ln cMcEQeQkQXgZ1VAGco99srAzufU27xzx1R6L X-Google-Smtp-Source: ABdhPJztkDPP3Z0UsU/b7hdBRrRZsfQzky/Kb94jjno5YfDhQgdG4Z6b80MRXa6KDpfbhTz5vRK93w== X-Received: by 2002:a1c:2288:: with SMTP id i130mr3313013wmi.181.1611918991627; Fri, 29 Jan 2021 03:16:31 -0800 (PST) Received: from naushir-VirtualBox.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id f14sm11324007wre.69.2021.01.29.03.16.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 29 Jan 2021 03:16:30 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Fri, 29 Jan 2021 11:16:15 +0000 Message-Id: <20210129111616.1047483-5-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210129111616.1047483-1-naush@raspberrypi.com> References: <20210129111616.1047483-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 4/5] ipa: raspberrypi: Pass the maximum allowable shutter speed into the AGC X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" In order to provide an optimal split between shutter speed and gain, the AGC must know the maximum allowable shutter speed, as limited by the maximum frame duration (either application provided or the default). Add a new API function, SetMaxShutter, to the AgcAlgorithm class. The IPA provides the the maximum shutter speed for AGC calculations. This applies to both the manual and auto AGC modes. Signed-off-by: Naushir Patuck Reviewed-by: David Plowman Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- .../raspberrypi/controller/agc_algorithm.hpp | 1 + src/ipa/raspberrypi/controller/rpi/agc.cpp | 48 +++++++++++++------ src/ipa/raspberrypi/controller/rpi/agc.hpp | 3 ++ src/ipa/raspberrypi/raspberrypi.cpp | 12 +++++ 4 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/ipa/raspberrypi/controller/agc_algorithm.hpp b/src/ipa/raspberrypi/controller/agc_algorithm.hpp index 981f1de2f0e4..f97deb0fca59 100644 --- a/src/ipa/raspberrypi/controller/agc_algorithm.hpp +++ b/src/ipa/raspberrypi/controller/agc_algorithm.hpp @@ -19,6 +19,7 @@ public: virtual void SetEv(double ev) = 0; virtual void SetFlickerPeriod(double flicker_period) = 0; virtual void SetFixedShutter(double fixed_shutter) = 0; // microseconds + virtual void SetMaxShutter(double max_shutter) = 0; // microseconds virtual void SetFixedAnalogueGain(double fixed_analogue_gain) = 0; virtual void SetMeteringMode(std::string const &metering_mode_name) = 0; virtual void SetExposureMode(std::string const &exposure_mode_name) = 0; diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp index eddd1684da12..0023d50029f1 100644 --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp @@ -157,7 +157,7 @@ Agc::Agc(Controller *controller) frame_count_(0), lock_count_(0), last_target_exposure_(0.0), ev_(1.0), flicker_period_(0.0), - fixed_shutter_(0), fixed_analogue_gain_(0.0) + max_shutter_(0), fixed_shutter_(0), fixed_analogue_gain_(0.0) { memset(&awb_, 0, sizeof(awb_)); // Setting status_.total_exposure_value_ to zero initially tells us @@ -227,11 +227,16 @@ void Agc::SetFlickerPeriod(double flicker_period) flicker_period_ = flicker_period; } +void Agc::SetMaxShutter(double max_shutter) +{ + max_shutter_ = max_shutter; +} + void Agc::SetFixedShutter(double fixed_shutter) { fixed_shutter_ = fixed_shutter; // Set this in case someone calls Pause() straight after. - status_.shutter_time = fixed_shutter; + status_.shutter_time = clipShutter(fixed_shutter_); } void Agc::SetFixedAnalogueGain(double fixed_analogue_gain) @@ -261,7 +266,8 @@ void Agc::SwitchMode([[maybe_unused]] CameraMode const &camera_mode, { housekeepConfig(); - if (fixed_shutter_ != 0.0 && fixed_analogue_gain_ != 0.0) { + double fixed_shutter = clipShutter(fixed_shutter_); + if (fixed_shutter != 0.0 && fixed_analogue_gain_ != 0.0) { // We're going to reset the algorithm here with these fixed values. fetchAwbStatus(metadata); @@ -269,14 +275,14 @@ void Agc::SwitchMode([[maybe_unused]] CameraMode const &camera_mode, ASSERT(min_colour_gain != 0.0); // This is the equivalent of computeTargetExposure and applyDigitalGain. - target_.total_exposure_no_dg = fixed_shutter_ * fixed_analogue_gain_; + target_.total_exposure_no_dg = fixed_shutter * fixed_analogue_gain_; target_.total_exposure = target_.total_exposure_no_dg / min_colour_gain; // Equivalent of filterExposure. This resets any "history". filtered_ = target_; // Equivalent of divideUpExposure. - filtered_.shutter = fixed_shutter_; + filtered_.shutter = fixed_shutter; filtered_.analogue_gain = fixed_analogue_gain_; } else if (status_.total_exposure_value) { // On a mode switch, it's possible the exposure profile could change, @@ -290,7 +296,7 @@ void Agc::SwitchMode([[maybe_unused]] CameraMode const &camera_mode, // for any that weren't set. // Equivalent of divideUpExposure. - filtered_.shutter = fixed_shutter_ ? fixed_shutter_ : config_.default_exposure_time; + filtered_.shutter = fixed_shutter ? fixed_shutter : config_.default_exposure_time; filtered_.analogue_gain = fixed_analogue_gain_ ? fixed_analogue_gain_ : config_.default_analogue_gain; } @@ -403,7 +409,8 @@ void Agc::housekeepConfig() { // First fetch all the up-to-date settings, so no one else has to do it. status_.ev = ev_; - status_.fixed_shutter = fixed_shutter_; + double fixed_shutter = clipShutter(fixed_shutter_); + status_.fixed_shutter = fixed_shutter; status_.fixed_analogue_gain = fixed_analogue_gain_; status_.flicker_period = flicker_period_; LOG(RPiAgc, Debug) << "ev " << status_.ev << " fixed_shutter " @@ -582,13 +589,15 @@ void Agc::computeTargetExposure(double gain) target_.total_exposure = current_.total_exposure_no_dg * gain; // The final target exposure is also limited to what the exposure // mode allows. + double max_shutter = status_.fixed_shutter != 0.0 + ? status_.fixed_shutter + : exposure_mode_->shutter.back(); + max_shutter = clipShutter(max_shutter); double max_total_exposure = - (status_.fixed_shutter != 0.0 - ? status_.fixed_shutter - : exposure_mode_->shutter.back()) * + max_shutter * (status_.fixed_analogue_gain != 0.0 - ? status_.fixed_analogue_gain - : exposure_mode_->gain.back()); + ? status_.fixed_analogue_gain + : exposure_mode_->gain.back()); target_.total_exposure = std::min(target_.total_exposure, max_total_exposure); } @@ -671,6 +680,7 @@ void Agc::divideUpExposure() shutter_time = status_.fixed_shutter != 0.0 ? status_.fixed_shutter : exposure_mode_->shutter[0]; + shutter_time = clipShutter(shutter_time); analogue_gain = status_.fixed_analogue_gain != 0.0 ? status_.fixed_analogue_gain : exposure_mode_->gain[0]; @@ -678,14 +688,15 @@ void Agc::divideUpExposure() for (unsigned int stage = 1; stage < exposure_mode_->gain.size(); stage++) { if (status_.fixed_shutter == 0.0) { - if (exposure_mode_->shutter[stage] * - analogue_gain >= + double stage_shutter = + clipShutter(exposure_mode_->shutter[stage]); + if (stage_shutter * analogue_gain >= exposure_value) { shutter_time = exposure_value / analogue_gain; break; } - shutter_time = exposure_mode_->shutter[stage]; + shutter_time = stage_shutter; } if (status_.fixed_analogue_gain == 0.0) { if (exposure_mode_->gain[stage] * @@ -740,6 +751,13 @@ void Agc::writeAndFinish(Metadata *image_metadata, bool desaturate) << " analogue gain " << filtered_.analogue_gain; } +double Agc::clipShutter(double shutter) +{ + if (max_shutter_) + shutter = std::min(shutter, max_shutter_); + return shutter; +} + // Register algorithm with the system. static Algorithm *Create(Controller *controller) { diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp b/src/ipa/raspberrypi/controller/rpi/agc.hpp index 05c334e4a1de..0427fb59ec1b 100644 --- a/src/ipa/raspberrypi/controller/rpi/agc.hpp +++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp @@ -78,6 +78,7 @@ public: unsigned int GetConvergenceFrames() const override; void SetEv(double ev) override; void SetFlickerPeriod(double flicker_period) override; + void SetMaxShutter(double max_shutter) override; // microseconds void SetFixedShutter(double fixed_shutter) override; // microseconds void SetFixedAnalogueGain(double fixed_analogue_gain) override; void SetMeteringMode(std::string const &metering_mode_name) override; @@ -100,6 +101,7 @@ private: void filterExposure(bool desaturate); void divideUpExposure(); void writeAndFinish(Metadata *image_metadata, bool desaturate); + double clipShutter(double shutter); AgcMeteringMode *metering_mode_; AgcExposureMode *exposure_mode_; AgcConstraintMode *constraint_mode_; @@ -126,6 +128,7 @@ private: std::string constraint_mode_name_; double ev_; double flicker_period_; + double max_shutter_; double fixed_shutter_; double fixed_analogue_gain_; }; diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index e4911b734e20..8c0e699184f6 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -1006,6 +1006,18 @@ void IPARPi::applyFrameDurations(double minFrameDuration, double maxFrameDuratio libcameraMetadata_.set(controls::FrameDurations, { static_cast(minFrameDuration_), static_cast(maxFrameDuration_) }); + + /* + * Calculate the maximum exposure time possible for the AGC to use. + * GetVBlanking() will update maxShutter with the largest exposure + * value possible. + */ + double maxShutter = std::numeric_limits::max(); + helper_->GetVBlanking(maxShutter, minFrameDuration_, maxFrameDuration_); + + RPiController::AgcAlgorithm *agc = dynamic_cast( + controller_.GetAlgorithm("agc")); + agc->SetMaxShutter(maxShutter); } void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls) From patchwork Fri Jan 29 11:16:16 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 11055 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 52F13BD808 for ; Fri, 29 Jan 2021 11:16:36 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 222FA683B8; Fri, 29 Jan 2021 12:16:36 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="rapxOpop"; dkim-atps=neutral Received: from mail-wm1-x330.google.com (mail-wm1-x330.google.com [IPv6:2a00:1450:4864:20::330]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6C8F5683A2 for ; Fri, 29 Jan 2021 12:16:34 +0100 (CET) Received: by mail-wm1-x330.google.com with SMTP id 190so6543968wmz.0 for ; Fri, 29 Jan 2021 03:16:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=RUpOb87XUfU7JD7k/ixnooM+rLIMqRySp8eMpIpMBPI=; b=rapxOpop637A6TR6rsDgJe9eXcwqnRMPG9ycolHWAphZeDHK6Hw8BeqXAfaZeXzXR3 TDLbrv5zXaqQ6MV5XsJfJpEru0JSgQPfYQQp/mhc+36ZpkwygdbMBVv/TAxSSgF1OAb8 wgJEhn2QEMP2ec6UMRC4Wtk1LBW2bWiBRGvqa8yCXIE1kKt/H8MVFBXfYvUUIYM3T2bF nGMVAsN5OppRyb7psk1dH/z2yyeluRylu+IoTdz4ud0XpTVlUIrtK1Nk0hb4QkWHJPL0 2BdPofbnX66cC9Lcypc7JiCzPA1igC17FLPSmfLjgONY1iQaeVgoNanLpXONryppKrcP LCPA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=RUpOb87XUfU7JD7k/ixnooM+rLIMqRySp8eMpIpMBPI=; b=DmR1/JtZxxZ6Hh4oPz726AA84XDRPTDg1FYgW0TZgOdIZvgmvymaufsD4b9p9c2TZn O9iJT7Jlrci2dFchP+y8buyX2fz1vgIwYX78QoiA+upkbOtCkZPkykoGjXXN98mW/DNV iIwHFCRFDp+rRi7JiKmpTeJusNKP/d41YzX5yg5kumM5Doq76vUFxhBLrAQV+GeHseKW wXdDgzQSvM+b+49P2365WlUN1cwa9KxNyapsgMWfhstA/VAI6XKFMTvvAbNcVDVoTcbO zdzguZZyWqNZDwHPS2uVtn3gQ3FaE0N5VfStRpCN5/3o/ge7210zmvktCIMdeQz6y+5n Wa0A== X-Gm-Message-State: AOAM533zngdmWE1uG2336MkPkk3G8pys0sjRTAeztZaypjLYQu1oNARR fS34BaEGkSePAtokT1N3XTIhaNvt5j/8k6dB X-Google-Smtp-Source: ABdhPJzuFdGScnh1p1QFRXNHRjSf13ft2T4vJ06m60fYg7mf0sjfqyMKZ18v6B40Ob5Q8FCH51n7NA== X-Received: by 2002:a05:600c:2888:: with SMTP id g8mr3320782wmd.169.1611918993600; Fri, 29 Jan 2021 03:16:33 -0800 (PST) Received: from naushir-VirtualBox.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id f14sm11324007wre.69.2021.01.29.03.16.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 29 Jan 2021 03:16:32 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Fri, 29 Jan 2021 11:16:16 +0000 Message-Id: <20210129111616.1047483-6-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210129111616.1047483-1-naush@raspberrypi.com> References: <20210129111616.1047483-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 5/5] pipeline: raspberrypi: Add notion of priority write to StaggeredCtrl X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" If an exposure time change adjusts the vblanking limits, and we write both VBLANK and EXPOSURE controls as a set, the latter may fail if the value is outside of the limits calculated by the old VBLANK value. This is a limitation in V4L2 and cannot be fixed by writing VBLANK before EXPOSURE. The workaround here is to have the StaggeredCtrl mark the VBLANK control as "priority write", which then write VBLANK separately from (and ahead of) any other controls. This way, the sensor driver will update the EXPOSURE control with new limits before the new values is presented, and will thus be seen as valid. In addition to this, the following changes have also been made to the module: - The initializer list passed into init() now uses a structure type instead of a std::pair. - Use unsigned int to store control delays to avoid unnecessary casts. StaggeredCtrl is due to be deprecated and replaced by DelayedCtrl, so this change serves more a working proof-of-concept on the workaround, and not much care has been taken to provide a nice new API for applying this "priority write" flag to the control. A similar workaround must be available to DelayedCtrl eventually. Signed-off-by: Naushir Patuck Reviewed-by: Jacopo Mondi --- src/ipa/raspberrypi/raspberrypi.cpp | 5 ++- .../pipeline/raspberrypi/raspberrypi.cpp | 12 ++++-- .../pipeline/raspberrypi/staggered_ctrl.cpp | 41 +++++++++++++------ .../pipeline/raspberrypi/staggered_ctrl.h | 17 ++++++-- 4 files changed, 54 insertions(+), 21 deletions(-) diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index 8c0e699184f6..2ad7b7dabb3e 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -1038,8 +1038,9 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls) /* * Due to the behavior of V4L2, the current value of VBLANK could clip the - * exposure time without us knowing. The next time though this function should - * clip exposure correctly. + * exposure time without us knowing. We get around this by ensuring the + * staggered write component submits VBLANK separately from, and before the + * EXPOSURE control. */ ctrls.set(V4L2_CID_VBLANK, vblanking); ctrls.set(V4L2_CID_EXPOSURE, exposureLines); diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 524cc960dd37..2118f2e72486 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1229,12 +1229,18 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config) /* * Setup our staggered control writer with the sensor default * gain and exposure delays. + * + * VBLANK must be flagged as "priority write" to allow it to + * be set ahead of (and separate from) all other controls that + * are batched together. This is needed so that any update to the + * EXPOSURE control will be validated based on the new VBLANK + * control value. */ if (!staggeredCtrl_) { staggeredCtrl_.init(unicam_[Unicam::Image].dev(), - { { V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] }, - { V4L2_CID_EXPOSURE, result.data[resultIdx++] }, - { V4L2_CID_VBLANK, result.data[resultIdx++] } }); + { { V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++], false }, + { V4L2_CID_EXPOSURE, result.data[resultIdx++], false }, + { V4L2_CID_VBLANK, result.data[resultIdx++], true } }); sensorMetadata_ = result.data[resultIdx++]; } } diff --git a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp b/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp index 62605c0fceee..498cd65b4cb6 100644 --- a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp +++ b/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp @@ -22,21 +22,23 @@ LOG_DEFINE_CATEGORY(RPI_S_W) namespace RPi { void StaggeredCtrl::init(V4L2VideoDevice *dev, - std::initializer_list> delayList) + std::initializer_list ctrlList) { std::lock_guard lock(lock_); dev_ = dev; - delay_ = delayList; + ctrlParams_.clear(); ctrl_.clear(); - /* Find the largest delay across all controls. */ maxDelay_ = 0; - for (auto const &p : delay_) { + for (auto const &c : ctrlList) { LOG(RPI_S_W, Info) << "Init ctrl " - << utils::hex(p.first) << " with delay " - << static_cast(p.second); - maxDelay_ = std::max(maxDelay_, p.second); + << utils::hex(c.id) << " with delay " + << static_cast(c.delay); + + ctrlParams_[c.id] = { c.delay, c.priorityWrite }; + /* Find the largest delay across all controls. */ + maxDelay_ = std::max(maxDelay_, c.delay); } init_ = true; @@ -67,7 +69,7 @@ bool StaggeredCtrl::set(uint32_t ctrl, int32_t value) std::lock_guard lock(lock_); /* Can we find this ctrl as one that is registered? */ - if (delay_.find(ctrl) == delay_.end()) + if (ctrlParams_.find(ctrl) == ctrlParams_.end()) return false; ctrl_[ctrl][setCount_].value = value; @@ -82,7 +84,7 @@ bool StaggeredCtrl::set(std::initializer_list for (auto const &p : ctrlList) { /* Can we find this ctrl? */ - if (delay_.find(p.first) == delay_.end()) + if (ctrlParams_.find(p.first) == ctrlParams_.end()) return false; ctrl_[p.first][setCount_] = CtrlInfo(p.second); @@ -97,7 +99,7 @@ bool StaggeredCtrl::set(const ControlList &controls) for (auto const &p : controls) { /* Can we find this ctrl? */ - if (delay_.find(p.first) == delay_.end()) + if (ctrlParams_.find(p.first) == ctrlParams_.end()) return false; ctrl_[p.first][setCount_] = CtrlInfo(p.second.get()); @@ -117,12 +119,25 @@ int StaggeredCtrl::write() ControlList controls(dev_->controls()); for (auto &p : ctrl_) { - int delayDiff = maxDelay_ - delay_[p.first]; + int delayDiff = maxDelay_ - ctrlParams_[p.first].delay; int index = std::max(0, setCount_ - delayDiff); if (p.second[index].updated) { - /* We need to write this value out. */ - controls.set(p.first, p.second[index].value); + if (ctrlParams_[p.first].priorityWrite) { + /* + * This control must be written now, it could + * affect validity of the other controls. + */ + ControlList immediate(dev_->controls()); + immediate.set(p.first, p.second[index].value); + dev_->setControls(&immediate); + } else { + /* + * Batch up the list of controls and write them + * at the end of the function. + */ + controls.set(p.first, p.second[index].value); + } p.second[index].updated = false; LOG(RPI_S_W, Debug) << "Writing ctrl " << utils::hex(p.first) << " to " diff --git a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h b/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h index 382fa31a6853..637629c0d9a8 100644 --- a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h +++ b/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h @@ -23,6 +23,12 @@ namespace RPi { class StaggeredCtrl { public: + struct CtrlInitParams { + unsigned int id; + unsigned int delay; + bool priorityWrite; + }; + StaggeredCtrl() : init_(false), setCount_(0), getCount_(0), maxDelay_(0) { @@ -34,7 +40,7 @@ public: } void init(V4L2VideoDevice *dev, - std::initializer_list> delayList); + std::initializer_list ctrlList); void reset(); void get(std::unordered_map &ctrl, uint8_t offset = 0); @@ -79,12 +85,17 @@ private: } }; + struct CtrlParams { + unsigned int delay; + bool priorityWrite; + }; + bool init_; uint32_t setCount_; uint32_t getCount_; - uint8_t maxDelay_; + unsigned int maxDelay_; V4L2VideoDevice *dev_; - std::unordered_map delay_; + std::unordered_map ctrlParams_; std::unordered_map ctrl_; std::mutex lock_; };