Patch Detail
Show a patch.
GET /api/1.1/patches/17858/?format=api
{ "id": 17858, "url": "https://patchwork.libcamera.org/api/1.1/patches/17858/?format=api", "web_url": "https://patchwork.libcamera.org/patch/17858/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20221124025133.17875-8-laurent.pinchart@ideasonboard.com>", "date": "2022-11-24T02:51:31", "name": "[libcamera-devel,v4,7/9] pipeline: rkisp1: Fix stream size validation", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "a39b13a62e551ef7f9183db0ee4377987c3fefd4", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/1.1/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/17858/mbox/", "series": [ { "id": 3634, "url": "https://patchwork.libcamera.org/api/1.1/series/3634/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3634", "date": "2022-11-24T02:51:24", "name": "Add Bayer format support for RkISP1", "version": 4, "mbox": "https://patchwork.libcamera.org/series/3634/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/17858/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/17858/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DD57FBDE6B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 24 Nov 2022 02:52:01 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7403A6331B;\n\tThu, 24 Nov 2022 03:52:01 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E9ED263325\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 24 Nov 2022 03:51:59 +0100 (CET)", "from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7DA1F496\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 24 Nov 2022 03:51:59 +0100 (CET)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669258321;\n\tbh=WptlNbyQJXl6QIbjV1sxEkl7ZEFKPR4Ew90c3gsadbU=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=lluR/EMrhhO7LfM7uOyWZIC3CfyI44qYFSctl5E1M+bYqk1mPW/tQIEzHAILEGewi\n\txC9lQkRs+dGRzqeMWk3t0gwvXzzG+om9tQEcGkhHzS7LOwzNYMhXvnj8lP+Oiw6LKn\n\t+YMlWj2U1Pvctosg3K4CuEXkUevEpJMFNADf9CD4LQTmgfMlgOH/XR0pQw2k2oKSmL\n\tAKrjv4jWe5P1wj0eaG4asXR0r1GgY6Jp+K1sFpC31r6KwbnnSt7KyzOiR8T19jJeCy\n\tkYSrLfuAcr13J20wRC2vsrw3ZImLIuHpXrvd3TjyqPTddF8EOzVrXpL6aWn0geggIg\n\tKZUIqRkwBQqFw==", "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1669258319;\n\tbh=WptlNbyQJXl6QIbjV1sxEkl7ZEFKPR4Ew90c3gsadbU=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=YrabK7AEU1gSGJqN5uOOoPbeUe2dE6Ph1u6sXQo0O72XAzzTzuFBbCec7xeiPBL/O\n\tz/3IZB6nJukAxC/4ayArjEZlBt0ehOSNn3hP7GNmPUNud13C5UmZRU5OueCeW4JBaK\n\tWvZzIoRyW9mAV2aJ6C3TpGav8qKUGwXtB1CNPDTw=" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"YrabK7AE\"; dkim-atps=neutral", "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", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v4 7/9] pipeline: rkisp1: Fix stream size\n\tvalidation", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "From": "Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>", "Reply-To": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Unlike RkISP1Path::generateConfiguration(), the validate() function\ndoesn't take the camera sensor resolution into account but only\nconsiders the absolute minimum and maximum sizes supported by the ISP to\nvalidate the stream size. Fix it by using the same logic as when\ngenerating the configuration.\n\nInstead of passing the sensor resolution to the validate() function,\npass the CameraSensor pointer to prepare for subsequent changes that\nwill require access to more camera sensor data. While at it, update the\ngenerateConfiguration() function similarly for the same reason.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n---\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 17 +++++++-------\n src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 22 +++++++++++++++----\n src/libcamera/pipeline/rkisp1/rkisp1_path.h | 6 +++--\n 3 files changed, 31 insertions(+), 14 deletions(-)", "diff": "diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 231b16eca110..3eb31a49bd92 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -411,14 +411,15 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,\n \n bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg)\n {\n+\tconst CameraSensor *sensor = data_->sensor_.get();\n \tStreamConfiguration config;\n \n \tconfig = cfg;\n-\tif (data_->mainPath_->validate(&config) != Valid)\n+\tif (data_->mainPath_->validate(sensor, &config) != Valid)\n \t\treturn false;\n \n \tconfig = cfg;\n-\tif (data_->selfPath_ && data_->selfPath_->validate(&config) != Valid)\n+\tif (data_->selfPath_ && data_->selfPath_->validate(sensor, &config) != Valid)\n \t\treturn false;\n \n \treturn true;\n@@ -466,7 +467,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n \t\t/* Try to match stream without adjusting configuration. */\n \t\tif (mainPathAvailable) {\n \t\t\tStreamConfiguration tryCfg = cfg;\n-\t\t\tif (data_->mainPath_->validate(&tryCfg) == Valid) {\n+\t\t\tif (data_->mainPath_->validate(sensor, &tryCfg) == Valid) {\n \t\t\t\tmainPathAvailable = false;\n \t\t\t\tcfg = tryCfg;\n \t\t\t\tcfg.setStream(const_cast<Stream *>(&data_->mainPathStream_));\n@@ -476,7 +477,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n \n \t\tif (selfPathAvailable) {\n \t\t\tStreamConfiguration tryCfg = cfg;\n-\t\t\tif (data_->selfPath_->validate(&tryCfg) == Valid) {\n+\t\t\tif (data_->selfPath_->validate(sensor, &tryCfg) == Valid) {\n \t\t\t\tselfPathAvailable = false;\n \t\t\t\tcfg = tryCfg;\n \t\t\t\tcfg.setStream(const_cast<Stream *>(&data_->selfPathStream_));\n@@ -487,7 +488,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n \t\t/* Try to match stream allowing adjusting configuration. */\n \t\tif (mainPathAvailable) {\n \t\t\tStreamConfiguration tryCfg = cfg;\n-\t\t\tif (data_->mainPath_->validate(&tryCfg) == Adjusted) {\n+\t\t\tif (data_->mainPath_->validate(sensor, &tryCfg) == Adjusted) {\n \t\t\t\tmainPathAvailable = false;\n \t\t\t\tcfg = tryCfg;\n \t\t\t\tcfg.setStream(const_cast<Stream *>(&data_->mainPathStream_));\n@@ -498,7 +499,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n \n \t\tif (selfPathAvailable) {\n \t\t\tStreamConfiguration tryCfg = cfg;\n-\t\t\tif (data_->selfPath_->validate(&tryCfg) == Adjusted) {\n+\t\t\tif (data_->selfPath_->validate(sensor, &tryCfg) == Adjusted) {\n \t\t\t\tselfPathAvailable = false;\n \t\t\t\tcfg = tryCfg;\n \t\t\t\tcfg.setStream(const_cast<Stream *>(&data_->selfPathStream_));\n@@ -610,11 +611,11 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n \t\tStreamConfiguration cfg;\n \t\tif (useMainPath) {\n \t\t\tcfg = data->mainPath_->generateConfiguration(\n-\t\t\t\tdata->sensor_->resolution());\n+\t\t\t\tdata->sensor_.get());\n \t\t\tmainPathAvailable = false;\n \t\t} else {\n \t\t\tcfg = data->selfPath_->generateConfiguration(\n-\t\t\t\tdata->sensor_->resolution());\n+\t\t\t\tdata->sensor_.get());\n \t\t\tselfPathAvailable = false;\n \t\t}\n \ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\nindex 7a2b0d172d84..f60680556052 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n@@ -12,6 +12,7 @@\n #include <libcamera/formats.h>\n #include <libcamera/stream.h>\n \n+#include \"libcamera/internal/camera_sensor.h\"\n #include \"libcamera/internal/media_device.h\"\n #include \"libcamera/internal/v4l2_subdevice.h\"\n #include \"libcamera/internal/v4l2_videodevice.h\"\n@@ -85,8 +86,10 @@ void RkISP1Path::populateFormats()\n \t}\n }\n \n-StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)\n+StreamConfiguration RkISP1Path::generateConfiguration(const CameraSensor *sensor)\n {\n+\tconst Size &resolution = sensor->resolution();\n+\n \tSize maxResolution = maxResolution_.boundedToAspectRatio(resolution)\n \t\t\t\t\t .boundedTo(resolution);\n \tSize minResolution = minResolution_.expandedToAspectRatio(resolution);\n@@ -104,8 +107,11 @@ StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)\n \treturn cfg;\n }\n \n-CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)\n+CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor,\n+\t\t\t\t\t\t StreamConfiguration *cfg)\n {\n+\tconst Size &resolution = sensor->resolution();\n+\n \tconst StreamConfiguration reqCfg = *cfg;\n \tCameraConfiguration::Status status = CameraConfiguration::Valid;\n \n@@ -117,8 +123,16 @@ CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)\n \tif (!streamFormats_.count(cfg->pixelFormat))\n \t\tcfg->pixelFormat = formats::NV12;\n \n-\tcfg->size.boundTo(maxResolution_);\n-\tcfg->size.expandTo(minResolution_);\n+\t/*\n+\t * Adjust the size based on the sensor resolution and absolute limits\n+\t * of the ISP.\n+\t */\n+\tSize maxResolution = maxResolution_.boundedToAspectRatio(resolution)\n+\t\t\t\t\t .boundedTo(resolution);\n+\tSize minResolution = minResolution_.expandedToAspectRatio(resolution);\n+\n+\tcfg->size.boundTo(maxResolution);\n+\tcfg->size.expandTo(minResolution);\n \tcfg->bufferCount = RKISP1_BUFFER_COUNT;\n \n \tV4L2DeviceFormat format;\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\nindex d88effbb6f56..bf4ad18fbbf2 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n@@ -23,6 +23,7 @@\n \n namespace libcamera {\n \n+class CameraSensor;\n class MediaDevice;\n class V4L2Subdevice;\n struct StreamConfiguration;\n@@ -39,8 +40,9 @@ public:\n \tint setEnabled(bool enable) { return link_->setEnabled(enable); }\n \tbool isEnabled() const { return link_->flags() & MEDIA_LNK_FL_ENABLED; }\n \n-\tStreamConfiguration generateConfiguration(const Size &resolution);\n-\tCameraConfiguration::Status validate(StreamConfiguration *cfg);\n+\tStreamConfiguration generateConfiguration(const CameraSensor *sensor);\n+\tCameraConfiguration::Status validate(const CameraSensor *sensor,\n+\t\t\t\t\t StreamConfiguration *cfg);\n \n \tint configure(const StreamConfiguration &config,\n \t\t const V4L2SubdeviceFormat &inputFormat);\n", "prefixes": [ "libcamera-devel", "v4", "7/9" ] }