[{"id":30718,"web_url":"https://patchwork.libcamera.org/comment/30718/","msgid":"<20240810122953.GC7180@pendragon.ideasonboard.com>","date":"2024-08-10T12:29:53","subject":"Re: [PATCH 1/2] pipeline: simple: Remove media member variable","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"hi Kieran and Paul,\n\nThank you for the patch.\n\nOn Thu, Aug 08, 2024 at 06:39:20PM +0100, Kieran Bingham wrote:\n> From: Paul Elder \n> \n> There is no need for the simple pipeline handler to save the media\n> device. Remove it.\n\nI suspect there's a hidden reason behind this change. Well, maybe not\nthat hidden, given there's a second patch posted in the same series :-)\n\n> \n> Signed-off-by: Paul Elder \n> Signed-off-by: Kieran Bingham \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 21 +++++++++++----------\n> 1 file changed, 11 insertions(+), 10 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 9910900e0e5e..222052ce02f8 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -363,13 +363,12 @@ private:\n> \t\treturn static_cast(camera->_d());\n> \t}\n> \n> -\tstd::vector locateSensors();\n> +\tstd::vector locateSensors(MediaDevice *media);\n> \tstatic int resetRoutingTable(V4L2Subdevice *subdev);\n> \n> \tconst MediaPad *acquirePipeline(SimpleCameraData *data);\n> \tvoid releasePipeline(SimpleCameraData *data);\n> \n> -\tMediaDevice *media_;\n> \tstd::map entities_;\n> \n> \tMediaDevice *converter_;\n> @@ -1424,7 +1423,8 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)\n> * Match and Setup\n> */\n> \n> -std::vector SimplePipelineHandler::locateSensors()\n> +std::vector\n> +SimplePipelineHandler::locateSensors(MediaDevice *media)\n> {\n> \tstd::vector entities;\n> \n> @@ -1432,7 +1432,7 @@ std::vector SimplePipelineHandler::locateSensors()\n> \t * Gather all the camera sensor entities based on the function they\n> \t * expose.\n> \t */\n> -\tfor (MediaEntity *entity : media_->entities()) {\n> +\tfor (MediaEntity *entity : media->entities()) {\n> \t\tif (entity->function() == MEDIA_ENT_F_CAM_SENSOR)\n> \t\t\tentities.push_back(entity);\n> \t}\n> @@ -1520,17 +1520,18 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> {\n> \tconst SimplePipelineInfo *info = nullptr;\n> \tunsigned int numStreams = 1;\n> +\tMediaDevice *media;\n> \n> \tfor (const SimplePipelineInfo &inf : supportedDevices) {\n> \t\tDeviceMatch dm(inf.driver);\n> -\t\tmedia_ = acquireMediaDevice(enumerator, dm);\n> -\t\tif (media_) {\n> +\t\tmedia = acquireMediaDevice(enumerator, dm);\n> +\t\tif (media) {\n> \t\t\tinfo = &inf;\n> \t\t\tbreak;\n> \t\t}\n> \t}\n> \n> -\tif (!media_)\n> +\tif (!media)\n> \t\treturn false;\n> \n> \tfor (const auto &[name, streams] : info->converters) {\n> @@ -1545,13 +1546,13 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> \tswIspEnabled_ = info->swIspEnabled;\n> \n> \t/* Locate the sensors. */\n> -\tstd::vector sensors = locateSensors();\n> +\tstd::vector sensors = locateSensors(media);\n> \tif (sensors.empty()) {\n> -\t\tLOG(SimplePipeline, Info) << \"No sensor found for \" << media_->deviceNode();\n> +\t\tLOG(SimplePipeline, Info) << \"No sensor found for \" << media->deviceNode();\n> \t\treturn false;\n> \t}\n> \n> -\tLOG(SimplePipeline, Debug) << \"Sensor found for \" << media_->deviceNode();\n> +\tLOG(SimplePipeline, Debug) << \"Sensor found for \" << media->deviceNode();\n> \n> \t/*\n> \t * Create one camera data instance for each sensor and gather all","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 51A2BBE173\n\tfor ;\n\tSat, 10 Aug 2024 12:30:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7CD94633BA;\n\tSat, 10 Aug 2024 14:30:19 +0200 (CEST)","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 01A9F63393\n\tfor ;\n\tSat, 10 Aug 2024 14:30:17 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DB7CB2D5;\n\tSat, 10 Aug 2024 14:29:22 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"MknpxraM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1723292963;\n\tbh=+43b6ZxI0/UrTm5FKrn4ZzYeFBH+53MePuZbXRVT43s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=MknpxraMoy4nyL/UifdIV1LpbL5hzm3Vu7vYGrBhrtjpbqxuUWT0GCyimc9oWdnui\n\t4RVkQ5voaFVOkfK36rHdpFihM0dKH/NPh8RFKelwR6RsRz02OR8KpjTx+A+/dcEfQo\n\t1w6K4JY4SeHVMX1AVN5cNFhWN7tSmqoP1oKB1si4=","Date":"Sat, 10 Aug 2024 15:29:53 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"libcamera devel ,\n\tPaul Elder ","Subject":"Re: [PATCH 1/2] pipeline: simple: Remove media member variable","Message-ID":"<20240810122953.GC7180@pendragon.ideasonboard.com>","References":"<20240808173921.2519957-1-kieran.bingham@ideasonboard.com>\n\t<20240808173921.2519957-2-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240808173921.2519957-2-kieran.bingham@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]