From patchwork Thu Jan 28 09:10:46 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 11038 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 A6FD0C33BC for ; Thu, 28 Jan 2021 09:10:57 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 7453D68384; Thu, 28 Jan 2021 10:10:57 +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="lhm3lI2t"; dkim-atps=neutral Received: from mail-wr1-x431.google.com (mail-wr1-x431.google.com [IPv6:2a00:1450:4864:20::431]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 50EFF68379 for ; Thu, 28 Jan 2021 10:10:55 +0100 (CET) Received: by mail-wr1-x431.google.com with SMTP id l12so4613736wry.2 for ; Thu, 28 Jan 2021 01:10:55 -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=lhm3lI2tx7uzFFCx6rd+ZIjrfX2OmgV5Y3K2KjSjhVYriKvLZcErIzaBoq/2G787w9 Jk6TFMIBkV18jBWdXRjz4KZMs4ZJrvi4AfHvWzKdcs7/uYXDwA5JHg7ZO7RVouFVoeGB 8CsPKzsZ0E8p+CdTwQihFk094h53rjwqeNUGYQCvvCsgKksBjAVZXXM+e9IuVgfwqZyw Ea2E7oudCi6HcSltyn2NDE8xi89t4fmsnSDNNI3SPIAYGKGocHK/0LoESPuRQySra4mW dIBf3WWnnlwq8fdKCFhlYVRpEEsoo1xi6OlHsCzjOx4bhqTU/h+Y4Yx2uQNfe8z3AHlE IuTg== 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=AoJNRjaxpHMXZBKzEZ84DkopqufqfHHabXMncNW6/N76nTQykDR4zjS1oaEwl6Z28G +n7pBNZ03YnaROaQfQ1C9Ojg82qeWmk6APOux85lDYqsijVzFGz/Gz0v6NZZY/ll5RrM a7LgfnQtgxYJCkkmZQqOajIFS2Dx0kQEQi5V/vGYvOId+NPvhjJxquEMvSf0eJ21w133 Au+mHvcxENHUt77RsuNRtA6mplLwWGrpycrYOedcoyQIhr7EYI1kuO7jqcpY1qeCeNcA N7If8Mscs8topxDyQ/rUR5Rp/0aZ3y+UsCrMCQ+tMSkeCE2QWl/TTlXCx5J9NZKB0Yat mFmQ== X-Gm-Message-State: AOAM532fA/SoqDKh8FSeBhgYfIHy5GAB+5tliO8wJNl2EMqJIzCssFP/ L1cvEnUo2EJcTeHHUwwFrzthmRLdEakEUUZR X-Google-Smtp-Source: ABdhPJxM1WZAMzhOeNKV/tBrpZg0kzTKo//Mn62+fVvsh++1FCWZUE5lNcRnYXeMCo+5a/KOLqsNgQ== X-Received: by 2002:adf:eb4e:: with SMTP id u14mr15241568wrn.99.1611825054785; Thu, 28 Jan 2021 01:10:54 -0800 (PST) Received: from naushir-VirtualBox.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id f17sm6517056wrv.0.2021.01.28.01.10.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Jan 2021 01:10:54 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Thu, 28 Jan 2021 09:10:46 +0000 Message-Id: <20210128091050.881815-2-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210128091050.881815-1-naush@raspberrypi.com> References: <20210128091050.881815-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 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 --- 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 Thu Jan 28 09:10:47 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 11039 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 42212BD808 for ; Thu, 28 Jan 2021 09:10:59 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1167868383; Thu, 28 Jan 2021 10:10:59 +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="rNuqhqHv"; dkim-atps=neutral Received: from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com [IPv6:2a00:1450:4864:20::42e]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6FB406837A for ; Thu, 28 Jan 2021 10:10:56 +0100 (CET) Received: by mail-wr1-x42e.google.com with SMTP id v15so4612796wrx.4 for ; Thu, 28 Jan 2021 01:10:56 -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=Vjiy0X5vCxiIh0kqLnE5/yh2+p1/5u96Km9Vbm5Nhag=; b=rNuqhqHvbXxbnOQzcjoGf2m0c2lBIWouTC9R6KUmb8A/uuAPTU4dzDDPFN4Y8A97xK 8Iu5gGHrpbxmoHDOPCp/LRjDKe0YCOrHTJqgmA2JaytisdAjCY12laajaCwS0oFXxoK2 OE7yVwyCVouaQOPBCfEcAFIgdqFdnM5mNAEjIDgdyLyVV5VarsE8Vs+orvH6bb7LlI2s 1gSoR8wQ2KKEO4+v4L0SmWVjcwxHTpTwoWFfzlpJuX4ViipkcUtF7w3WueqvQw+qE+EV XIxhB8G5az+Rl2I6R0qqkNdsESjgH1PLEN+lCYVsZ2YbYSnka58ILoQNcjWi06W9YZ8/ 3MQA== 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=Vjiy0X5vCxiIh0kqLnE5/yh2+p1/5u96Km9Vbm5Nhag=; b=J3RHv/u3hLgWrbUEkTVmHw6htTED6A//dGiFjAva5T+qJaPviTHSwRJ8hVEeo4+rzy 9ZFKSptcTYVe2wO7dmE9o6DSj6I+4QhdMBfbU5LJ5YiUUfbT37YGH2yO2dNYgkh/pCuB bstStYzkYHBurlk/oL5gI2JlJxqAjue5H12iOySB4WafKO7xBoOokqE+gkVRy8au4ioQ 1YWbGD4S3tf2nxC72VCjfL/QH/htJlYSniV2I9EIgjsQmbV4nTVdHwVrZUntImEG0F5i avpnjIujJmj34o3EGs11yu3WlFYujQAkeNwR1UPyJ1UcKmPP9rc8ugADJRSiO60LeeAK nORA== X-Gm-Message-State: AOAM532NExsq9ylpumnWhvHiHps5KoH3GqLMURdrh9APVPB4Xw+LrQ2Z PtlP8qmJlDtN1B4lVwOlHlglr201Yex2Qmdf X-Google-Smtp-Source: ABdhPJxM2OmdeaqnGJFd8KfMV4KA5jahZlTVgMLoPxScmDkH09vQGCWAnxQetyCgg0uRQ5O3sIdIXw== X-Received: by 2002:adf:fc8a:: with SMTP id g10mr15702631wrr.189.1611825055865; Thu, 28 Jan 2021 01:10:55 -0800 (PST) Received: from naushir-VirtualBox.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id f17sm6517056wrv.0.2021.01.28.01.10.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Jan 2021 01:10:55 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Thu, 28 Jan 2021 09:10:47 +0000 Message-Id: <20210128091050.881815-3-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210128091050.881815-1-naush@raspberrypi.com> References: <20210128091050.881815-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 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 --- 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 Thu Jan 28 09:10:48 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 11040 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 B7088BD808 for ; Thu, 28 Jan 2021 09:11:01 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 879D768390; Thu, 28 Jan 2021 10:11:01 +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="UzntPPn6"; dkim-atps=neutral Received: from mail-wm1-x32c.google.com (mail-wm1-x32c.google.com [IPv6:2a00:1450:4864:20::32c]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E1E8F68383 for ; Thu, 28 Jan 2021 10:10:57 +0100 (CET) Received: by mail-wm1-x32c.google.com with SMTP id i9so3958888wmq.1 for ; Thu, 28 Jan 2021 01:10:57 -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=vLoV02H7yVVWHeI2CE7xbapt2Jd5AA/m9VmsfsTEGDg=; b=UzntPPn6gxvzXTZCcZR2idty+q6NWIiLUwO6PBEiVW26WfiStHRTAsiUIhVocqtbhS lblj2oYF99aANk5xE84T1z8+EH3NjrWGLbZwrAZ1wC0FHJhIbvBCBVxX6z9ZpMAqS68F wBFWZedwtddB5ypChCQQUWannEra473YxLKCKiamRfw5hlzEvNWvMOd8LAhYi/Rw4e2F fUfiAQ2hoKDBc7aTF06Wpa01Z09yqD7daTuWYYucClb/DqSyAzj5SBnAcLeg3Js5ROff YknlbMRjafM644CexFQRPV/voWp9Wgn19rEEi/75In3PYMB65iDnzkfeUMoFHwxLTzcx D77Q== 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=vLoV02H7yVVWHeI2CE7xbapt2Jd5AA/m9VmsfsTEGDg=; b=JW+7SePBTo0UGZd+bVz6iJgv8Ga1LknpKBqCwPiTug8gwZfZ2WMgy9d+5ntnfob1HC DubewIjt18ezeyFMhb0RDpjKKJP1GXsDHGkelPJY++Qusm34Aks8wU4WyrFzuks1i41p 95heUOvAgge8gyR9pYG9wE5bIW1KXQgs8RtXeblTQVs7oqoXmoGUDGeC42Xk72UaEUPM FoBI7LQWcJpHBy1/JqWlGCSw//5Q47+VmKXZXw62DfWiTDTxk/wNo3REVQOSvgEA91U+ +MVLIE5Sbn4wX4SplnUQPwAYmWuK/XeAFSWywrPgqbXOije2OiIVfLQESoa5Uq80n7/L lm2g== X-Gm-Message-State: AOAM5320lfWT40OHDUspBseNAzgnixuIyZSMTd4r9An5kR2g9bEyyea5 Nq5tnxDLoBikGDI3L0g0pzDkb3eOkKS3Yfjz X-Google-Smtp-Source: ABdhPJxLZZ9agbXfclSKIgWVgK3+Vbg3ofn/XeC5nAMZgVAPEbl9/K+v6WEXx+fJUoIdHj/54u72IQ== X-Received: by 2002:a1c:a593:: with SMTP id o141mr7855987wme.92.1611825056893; Thu, 28 Jan 2021 01:10:56 -0800 (PST) Received: from naushir-VirtualBox.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id f17sm6517056wrv.0.2021.01.28.01.10.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Jan 2021 01:10:56 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Thu, 28 Jan 2021 09:10:48 +0000 Message-Id: <20210128091050.881815-4-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210128091050.881815-1-naush@raspberrypi.com> References: <20210128091050.881815-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 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 --- 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..3e17255737b2 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 will have been clamped to the + * maximum allowable values in the sensor for this 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 Thu Jan 28 09:10:49 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 11041 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 6A799BD808 for ; Thu, 28 Jan 2021 09:11:02 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 011AB68385; Thu, 28 Jan 2021 10:11:02 +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="ATAAJXeQ"; dkim-atps=neutral Received: from mail-wm1-x331.google.com (mail-wm1-x331.google.com [IPv6:2a00:1450:4864:20::331]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9DA3368383 for ; Thu, 28 Jan 2021 10:10:58 +0100 (CET) Received: by mail-wm1-x331.google.com with SMTP id u14so3942965wmq.4 for ; Thu, 28 Jan 2021 01:10:58 -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=irRZPJbnLcJSSH4oOs1b7hMuxfFXIX3RyNQhx7GBNIY=; b=ATAAJXeQsOxlYw5xZuA+I9bPx4McfYLjCrzSTdN1vi9QdFiNlp2KRdzXGLLvHx7UzQ JQjdASB1qb84NhWW+TQZxo5hOAXsWlUZThFy2VCNA9sUrTgpU2mtkzpvspwc8ROOBMJ6 q4XYe1+6iAeOp0dgbYkzPYt6ptQok9EP4hgmR611d8FAR0toroDE1fhd5soKZ2r/K+Wl dn1CW/dS7b+e26fKkSu/daFEWjbglX3EGP785Y7zJmAU7oRITkNGRIKLZTndCiv4BPoX DggvjTkvyExQb1/L2deHXNdQOB1clXLDDNBizMl+KfbJ+Kc80WmClI/Ofz8DzlgDcRMv aXDA== 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=irRZPJbnLcJSSH4oOs1b7hMuxfFXIX3RyNQhx7GBNIY=; b=Y7GEqSBMsi55noDvUdnZCagJz2qCRA40U6ZrOufoFXZKUyluEUlkk7zRjHDKz0HlBQ lmqvIDwYmIyUVoE643avBpo9M+4h8hqHj6AtXdfANDtC6cls0j4UO9vWIJSz7aLAcVk+ kGE4svi7gNggxoI5CM873X+539+82G2juaDcC1DpQzlYyNBJnRz1UJZHsztbTj0QjDAN MgZKdzCxdXTA1ck24vJ2lBCnO5C22FJqirCCgCRoAAPt69U0SstO/rR3M8aEqBt91Cxg 0LJpQ0vHxdstKYKErIhCGyGxNxByRZiXJTy6nfMZrsGRJyMGkmxAJI3Qb9QsARkmunfD 7S4g== X-Gm-Message-State: AOAM531MfoIiwulTg7dmniP3q7mUEq4thOwX8XQlFyxo3t+EhbQpWDai gU0iEGL2fSW9Rqs75q7lwYcD1UM9pqDHYZh0 X-Google-Smtp-Source: ABdhPJzauSEtvjJbm3W5THvYfuB7yAvi7grpR/JQqrI41IKZvNB/wNv9G3gdJfvsszDh4R1Lbiy43Q== X-Received: by 2002:a05:600c:4e92:: with SMTP id f18mr7668427wmq.126.1611825057779; Thu, 28 Jan 2021 01:10:57 -0800 (PST) Received: from naushir-VirtualBox.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id f17sm6517056wrv.0.2021.01.28.01.10.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Jan 2021 01:10:57 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Thu, 28 Jan 2021 09:10:49 +0000 Message-Id: <20210128091050.881815-5-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210128091050.881815-1-naush@raspberrypi.com> References: <20210128091050.881815-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 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 --- .../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 Thu Jan 28 09:10:50 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 11042 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 1BBB5BD808 for ; Thu, 28 Jan 2021 09:11:03 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id CC0DC68394; Thu, 28 Jan 2021 10:11:02 +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="g6F7+v0X"; 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 5D3686838C for ; Thu, 28 Jan 2021 10:10:59 +0100 (CET) Received: by mail-wm1-x330.google.com with SMTP id s24so4731209wmj.0 for ; Thu, 28 Jan 2021 01:10:59 -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=G0NLRn1CkpdPEZDB7Iv8jjTSZZtCx6D+TwurDlEp7/Q=; b=g6F7+v0X37O/dMM0MaCfp8n9H0b4UbP93PHovoOc83cFGf/kN0nzuxsqY7lWRe4R45 c0DGGMSOnOH47sDVaea3TJyXdelskP+ShPuVnjzP9m2d7zJA6OnN6OJ506PhGaEQAOzW SGvK06c2nM3t6q/NZNacf7acdur39UUZo1fKDxnjv86NWjBPS3JQI4uUuqkv+UKsrWgW ZCEefLuC278UP8Pa0lrgpMiLfUgB5ov4I00AAvxtUd1QRhOrFJkI+WXIENUZj2lTxuBV gOqt0BS+jOAkktcrmeM7FRbeU7EP63V78VUZnevsFfKb9u9chUW9RORIqq4I8+ogw4/V qaCQ== 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=G0NLRn1CkpdPEZDB7Iv8jjTSZZtCx6D+TwurDlEp7/Q=; b=Ruk5/Kpe8z5/uXAd1NeIkWCQZSSvB06nptPduzQUWdCz/dN0y9QMhcpzdmFpadMQnm 2Hm+nu5zpTkEiqXyyGP1eWJmfDOZIqMDm0zcOzMUR8YRMjO3fXFqnSrZbx1LJoIxCl72 n8RXN0BYw3JlNcsfw6VTafKFmfZM424iRV303mfJDLEIyMYTO9RSzL3POc/8eZnisPdo rQ4x/bPoGWNvlAP6JAk8tLM6ImmIceOeqXTATps5ZMGob/4KlXs6JSBZp0xaIoz8YHul aOu7my7J2y8rBIHe5odj1RCRhfuH73vziV3qFNLWQgy7X6tZ9efB2kgHQxzk6W9y63kp 6etA== X-Gm-Message-State: AOAM530R8Vtv+1PriRtn/seTa0NIbSEyYCA7Mebj+Y+ZXJnDwpcs+bp/ 1UP5oygweZ6hsJxIX5aAdBwGWig4WkzONGPv X-Google-Smtp-Source: ABdhPJwt2z/bX7K9JNOVOcfJYj1V61DkT5V6pvnBkw7TqEgQO0sbkOoI1dSZ6yFDa6bkn+xDaOF7yw== X-Received: by 2002:a05:600c:215:: with SMTP id 21mr7749851wmi.54.1611825058593; Thu, 28 Jan 2021 01:10:58 -0800 (PST) Received: from naushir-VirtualBox.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id f17sm6517056wrv.0.2021.01.28.01.10.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Jan 2021 01:10:58 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Thu, 28 Jan 2021 09:10:50 +0000 Message-Id: <20210128091050.881815-6-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210128091050.881815-1-naush@raspberrypi.com> References: <20210128091050.881815-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 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_; };