From patchwork Thu Nov 24 02:51:31 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 17858 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 DD57FBDE6B for ; Thu, 24 Nov 2022 02:52:01 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 7403A6331B; Thu, 24 Nov 2022 03:52:01 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1669258321; bh=WptlNbyQJXl6QIbjV1sxEkl7ZEFKPR4Ew90c3gsadbU=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=lluR/EMrhhO7LfM7uOyWZIC3CfyI44qYFSctl5E1M+bYqk1mPW/tQIEzHAILEGewi xC9lQkRs+dGRzqeMWk3t0gwvXzzG+om9tQEcGkhHzS7LOwzNYMhXvnj8lP+Oiw6LKn +YMlWj2U1Pvctosg3K4CuEXkUevEpJMFNADf9CD4LQTmgfMlgOH/XR0pQw2k2oKSmL AKrjv4jWe5P1wj0eaG4asXR0r1GgY6Jp+K1sFpC31r6KwbnnSt7KyzOiR8T19jJeCy kYSrLfuAcr13J20wRC2vsrw3ZImLIuHpXrvd3TjyqPTddF8EOzVrXpL6aWn0geggIg KZUIqRkwBQqFw== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E9ED263325 for ; Thu, 24 Nov 2022 03:51:59 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="YrabK7AE"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 7DA1F496 for ; Thu, 24 Nov 2022 03:51:59 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1669258319; bh=WptlNbyQJXl6QIbjV1sxEkl7ZEFKPR4Ew90c3gsadbU=; h=From:To:Subject:Date:In-Reply-To:References:From; b=YrabK7AEU1gSGJqN5uOOoPbeUe2dE6Ph1u6sXQo0O72XAzzTzuFBbCec7xeiPBL/O z/3IZB6nJukAxC/4ayArjEZlBt0ehOSNn3hP7GNmPUNud13C5UmZRU5OueCeW4JBaK WvZzIoRyW9mAV2aJ6C3TpGav8qKUGwXtB1CNPDTw= To: libcamera-devel@lists.libcamera.org Date: Thu, 24 Nov 2022 04:51:31 +0200 Message-Id: <20221124025133.17875-8-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.37.4 In-Reply-To: <20221124025133.17875-1-laurent.pinchart@ideasonboard.com> References: <20221124025133.17875-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 7/9] pipeline: rkisp1: Fix stream size validation 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: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Unlike RkISP1Path::generateConfiguration(), the validate() function doesn't take the camera sensor resolution into account but only considers the absolute minimum and maximum sizes supported by the ISP to validate the stream size. Fix it by using the same logic as when generating the configuration. Instead of passing the sensor resolution to the validate() function, pass the CameraSensor pointer to prepare for subsequent changes that will require access to more camera sensor data. While at it, update the generateConfiguration() function similarly for the same reason. Signed-off-by: Laurent Pinchart Reviewed-by: Paul Elder Reviewed-by: Jacopo Mondi --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 17 +++++++------- src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 22 +++++++++++++++---- src/libcamera/pipeline/rkisp1/rkisp1_path.h | 6 +++-- 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 231b16eca110..3eb31a49bd92 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -411,14 +411,15 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera, bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg) { + const CameraSensor *sensor = data_->sensor_.get(); StreamConfiguration config; config = cfg; - if (data_->mainPath_->validate(&config) != Valid) + if (data_->mainPath_->validate(sensor, &config) != Valid) return false; config = cfg; - if (data_->selfPath_ && data_->selfPath_->validate(&config) != Valid) + if (data_->selfPath_ && data_->selfPath_->validate(sensor, &config) != Valid) return false; return true; @@ -466,7 +467,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() /* Try to match stream without adjusting configuration. */ if (mainPathAvailable) { StreamConfiguration tryCfg = cfg; - if (data_->mainPath_->validate(&tryCfg) == Valid) { + if (data_->mainPath_->validate(sensor, &tryCfg) == Valid) { mainPathAvailable = false; cfg = tryCfg; cfg.setStream(const_cast(&data_->mainPathStream_)); @@ -476,7 +477,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() if (selfPathAvailable) { StreamConfiguration tryCfg = cfg; - if (data_->selfPath_->validate(&tryCfg) == Valid) { + if (data_->selfPath_->validate(sensor, &tryCfg) == Valid) { selfPathAvailable = false; cfg = tryCfg; cfg.setStream(const_cast(&data_->selfPathStream_)); @@ -487,7 +488,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() /* Try to match stream allowing adjusting configuration. */ if (mainPathAvailable) { StreamConfiguration tryCfg = cfg; - if (data_->mainPath_->validate(&tryCfg) == Adjusted) { + if (data_->mainPath_->validate(sensor, &tryCfg) == Adjusted) { mainPathAvailable = false; cfg = tryCfg; cfg.setStream(const_cast(&data_->mainPathStream_)); @@ -498,7 +499,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() if (selfPathAvailable) { StreamConfiguration tryCfg = cfg; - if (data_->selfPath_->validate(&tryCfg) == Adjusted) { + if (data_->selfPath_->validate(sensor, &tryCfg) == Adjusted) { selfPathAvailable = false; cfg = tryCfg; cfg.setStream(const_cast(&data_->selfPathStream_)); @@ -610,11 +611,11 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera, StreamConfiguration cfg; if (useMainPath) { cfg = data->mainPath_->generateConfiguration( - data->sensor_->resolution()); + data->sensor_.get()); mainPathAvailable = false; } else { cfg = data->selfPath_->generateConfiguration( - data->sensor_->resolution()); + data->sensor_.get()); selfPathAvailable = false; } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp index 7a2b0d172d84..f60680556052 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp @@ -12,6 +12,7 @@ #include #include +#include "libcamera/internal/camera_sensor.h" #include "libcamera/internal/media_device.h" #include "libcamera/internal/v4l2_subdevice.h" #include "libcamera/internal/v4l2_videodevice.h" @@ -85,8 +86,10 @@ void RkISP1Path::populateFormats() } } -StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution) +StreamConfiguration RkISP1Path::generateConfiguration(const CameraSensor *sensor) { + const Size &resolution = sensor->resolution(); + Size maxResolution = maxResolution_.boundedToAspectRatio(resolution) .boundedTo(resolution); Size minResolution = minResolution_.expandedToAspectRatio(resolution); @@ -104,8 +107,11 @@ StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution) return cfg; } -CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg) +CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, + StreamConfiguration *cfg) { + const Size &resolution = sensor->resolution(); + const StreamConfiguration reqCfg = *cfg; CameraConfiguration::Status status = CameraConfiguration::Valid; @@ -117,8 +123,16 @@ CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg) if (!streamFormats_.count(cfg->pixelFormat)) cfg->pixelFormat = formats::NV12; - cfg->size.boundTo(maxResolution_); - cfg->size.expandTo(minResolution_); + /* + * Adjust the size based on the sensor resolution and absolute limits + * of the ISP. + */ + Size maxResolution = maxResolution_.boundedToAspectRatio(resolution) + .boundedTo(resolution); + Size minResolution = minResolution_.expandedToAspectRatio(resolution); + + cfg->size.boundTo(maxResolution); + cfg->size.expandTo(minResolution); cfg->bufferCount = RKISP1_BUFFER_COUNT; V4L2DeviceFormat format; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h index d88effbb6f56..bf4ad18fbbf2 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h @@ -23,6 +23,7 @@ namespace libcamera { +class CameraSensor; class MediaDevice; class V4L2Subdevice; struct StreamConfiguration; @@ -39,8 +40,9 @@ public: int setEnabled(bool enable) { return link_->setEnabled(enable); } bool isEnabled() const { return link_->flags() & MEDIA_LNK_FL_ENABLED; } - StreamConfiguration generateConfiguration(const Size &resolution); - CameraConfiguration::Status validate(StreamConfiguration *cfg); + StreamConfiguration generateConfiguration(const CameraSensor *sensor); + CameraConfiguration::Status validate(const CameraSensor *sensor, + StreamConfiguration *cfg); int configure(const StreamConfiguration &config, const V4L2SubdeviceFormat &inputFormat);