[{"id":34643,"web_url":"https://patchwork.libcamera.org/comment/34643/","msgid":"<078aa88b-0e07-4b88-801f-ae7233f3c84b@igalia.com>","date":"2025-06-26T06:52:58","subject":"Re: [PATCH v1 1/3] pipeline: rpi: Fix for enumerating the media\n\tgraphs","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn 6/25/25 2:11 PM, Naushir Patuck wrote:\n> When there are multiple entities between the sensor and CFE device (e.g.\n> a seraliser and deserialiser or multiple mux devices), the media graph\n> enumeration would work incorrectly and report that the frontend entity\n> was not found. This is beause the found flag was stored locally in a\ns/beause/because/\n> boolean and got lost in the recursion.\n>\n> Fix this by explicitly tracking and returning the frontend found flag\n> through the return value of enumerateVideoDevices(). This ensures the\n> flag does not get lost through nested recursion.\n>\n> This flag can also be used to fail a camera registration if the frontend\n> is not found.\n>\n> Signed-off-by: Naushir Patuck \n\n\nLooks good to me,\n\nReviewed-by: Umang Jain \n\n> ---\n> .../pipeline/rpi/common/pipeline_base.cpp | 22 +++++++++++--------\n> .../pipeline/rpi/common/pipeline_base.h | 2 +-\n> 2 files changed, 14 insertions(+), 10 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index e14d3b913aaa..eafe94427dbc 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -806,7 +806,8 @@ int PipelineHandlerBase::registerCamera(std::unique_ptr &camera\n> \t * chain. There may be a cascade of devices in this chain!\n> \t */\n> \tMediaLink *link = sensorEntity->getPadByIndex(0)->links()[0];\n> -\tdata->enumerateVideoDevices(link, frontendName);\n> +\tif (!data->enumerateVideoDevices(link, frontendName))\n> +\t\treturn -EINVAL;\n> \n> \tipa::RPi::InitResult result;\n> \tif (data->loadIPA(&result)) {\n> @@ -1018,16 +1019,20 @@ void CameraData::freeBuffers()\n> * | Sensor2 | | Sensor3 |\n> * +---------+ +---------+\n> */\n> -void CameraData::enumerateVideoDevices(MediaLink *link, const std::string &frontend)\n> +bool CameraData::enumerateVideoDevices(MediaLink *link, const std::string &frontend)\n> {\n> \tconst MediaPad *sinkPad = link->sink();\n> \tconst MediaEntity *entity = sinkPad->entity();\n> \tbool frontendFound = false;\n> \n> +\t/* Once we reach the Frontend entity, we are done. */\n> +\tif (link->sink()->entity()->name() == frontend)\n> +\t\treturn true;\n> +\n> \t/* We only deal with Video Mux and Bridge devices in cascade. */\n> \tif (entity->function() != MEDIA_ENT_F_VID_MUX &&\n> \t entity->function() != MEDIA_ENT_F_VID_IF_BRIDGE)\n> -\t\treturn;\n> +\t\treturn false;\n> \n> \t/* Find the source pad for this Video Mux or Bridge device. */\n> \tconst MediaPad *sourcePad = nullptr;\n> @@ -1039,7 +1044,7 @@ void CameraData::enumerateVideoDevices(MediaLink *link, const std::string &front\n> \t\t\t * and this branch in the cascade.\n> \t\t\t */\n> \t\t\tif (sourcePad)\n> -\t\t\t\treturn;\n> +\t\t\t\treturn false;\n> \n> \t\t\tsourcePad = pad;\n> \t\t}\n> @@ -1056,12 +1061,9 @@ void CameraData::enumerateVideoDevices(MediaLink *link, const std::string &front\n> \t * other Video Mux and Bridge devices.\n> \t */\n> \tfor (MediaLink *l : sourcePad->links()) {\n> -\t\tenumerateVideoDevices(l, frontend);\n> -\t\t/* Once we reach the Frontend entity, we are done. */\n> -\t\tif (l->sink()->entity()->name() == frontend) {\n> -\t\t\tfrontendFound = true;\n> +\t\tfrontendFound = enumerateVideoDevices(l, frontend);\n> +\t\tif (frontendFound)\n> \t\t\tbreak;\n> -\t\t}\n> \t}\n> \n> \t/* This identifies the end of our entity enumeration recursion. */\n> @@ -1076,6 +1078,8 @@ void CameraData::enumerateVideoDevices(MediaLink *link, const std::string &front\n> \t\t\tbridgeDevices_.clear();\n> \t\t}\n> \t}\n> +\n> +\treturn frontendFound;\n> }\n> \n> int CameraData::loadPipelineConfiguration()\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> index 4c5743e04f86..4bce4ec4f978 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> @@ -68,7 +68,7 @@ public:\n> \tvoid freeBuffers();\n> \tvirtual void platformFreeBuffers() = 0;\n> \n> -\tvoid enumerateVideoDevices(MediaLink *link, const std::string &frontend);\n> +\tbool enumerateVideoDevices(MediaLink *link, const std::string &frontend);\n> \n> \tint loadPipelineConfiguration();\n> \tint loadIPA(ipa::RPi::InitResult *result);","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 B7637C3237\n\tfor ;\n\tThu, 26 Jun 2025 06:52:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3576F68DF3;\n\tThu, 26 Jun 2025 08:52:58 +0200 (CEST)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5ED7A61529\n\tfor ;\n\tThu, 26 Jun 2025 08:52:54 +0200 (CEST)","from [49.36.69.141] (helo=[192.168.29.108])\n\tby fanzine2.igalia.com with esmtpsa \n\t(Cipher TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_128_GCM:128)\n\t(Exim) id 1uUgTd-008s6R-8l; Thu, 26 Jun 2025 08:52:53 +0200"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=igalia.com header.i=@igalia.com\n\theader.b=\"PWUZJuuG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com;\n\ts=20170329;\n\th=Content-Transfer-Encoding:Content-Type:In-Reply-To:From:\n\tReferences:Cc:To:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To:\n\tContent-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender:\n\tResent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:\n\tList-Subscribe:List-Post:List-Owner:List-Archive;\n\tbh=D+x4NrEOUlaisQ2vGqNpVBBFxwmtpke0VxEgP9vZPR8=;\n\tb=PWUZJuuGCC1UqOrkU9yO8M6wr1\n\toA0AgeDVwfafY7nFOFYrIffTPkHS0f5S88SdPRYcVSiURnZYu7wZ8idee3U9V3LNqMDzQ/FtmuvYi\n\t7/IoxLvimupzW8TG3pBdguYvAT6xxilz45Nl3A7JJZMrYcq6+m0HjqrSf+jP5JX9ehQBiZboNEj2J\n\tg2eATKe+8ZAcsd0OYgzyUsD3AmwqVBEuGFlpS9SP8r5oNaPzgCb6AekHizQM/IAtD3KqesnC5/wLu\n\tpYzcP272n7SMlZIWJpEjo+TS+fkqH70NnjgJmLBgvjpyg1rXEkv3kMhFbxvNU9QF9oVbQ98ZLf6dd\n\tGvQ1IWtg==;","Message-ID":"<078aa88b-0e07-4b88-801f-ae7233f3c84b@igalia.com>","Date":"Thu, 26 Jun 2025 12:22:58 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v1 1/3] pipeline: rpi: Fix for enumerating the media\n\tgraphs","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","Cc":"dave.stevenson@raspberrypi.com","References":"<20250625084212.858487-1-naush@raspberrypi.com>\n\t<20250625084212.858487-2-naush@raspberrypi.com>","Content-Language":"en-US","From":"Umang Jain ","In-Reply-To":"<20250625084212.858487-2-naush@raspberrypi.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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\" "}},{"id":34659,"web_url":"https://patchwork.libcamera.org/comment/34659/","msgid":"<20250626125349.GM8738@pendragon.ideasonboard.com>","date":"2025-06-26T12:53:49","subject":"Re: [PATCH v1 1/3] pipeline: rpi: Fix for enumerating the media\n\tgraphs","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Wed, Jun 25, 2025 at 09:41:16AM +0100, Naushir Patuck wrote:\n> When there are multiple entities between the sensor and CFE device (e.g.\n> a seraliser and deserialiser or multiple mux devices), the media graph\n\ns/seraliser/serialiser/\n\n> enumeration would work incorrectly and report that the frontend entity\n> was not found. This is beause the found flag was stored locally in a\n\ns/beause/because/\n\n> boolean and got lost in the recursion.\n> \n> Fix this by explicitly tracking and returning the frontend found flag\n> through the return value of enumerateVideoDevices(). This ensures the\n> flag does not get lost through nested recursion.\n> \n> This flag can also be used to fail a camera registration if the frontend\n> is not found.\n> \n> Signed-off-by: Naushir Patuck \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> .../pipeline/rpi/common/pipeline_base.cpp | 22 +++++++++++--------\n> .../pipeline/rpi/common/pipeline_base.h | 2 +-\n> 2 files changed, 14 insertions(+), 10 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index e14d3b913aaa..eafe94427dbc 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -806,7 +806,8 @@ int PipelineHandlerBase::registerCamera(std::unique_ptr &camera\n> \t * chain. There may be a cascade of devices in this chain!\n> \t */\n> \tMediaLink *link = sensorEntity->getPadByIndex(0)->links()[0];\n> -\tdata->enumerateVideoDevices(link, frontendName);\n> +\tif (!data->enumerateVideoDevices(link, frontendName))\n> +\t\treturn -EINVAL;\n> \n> \tipa::RPi::InitResult result;\n> \tif (data->loadIPA(&result)) {\n> @@ -1018,16 +1019,20 @@ void CameraData::freeBuffers()\n> * | Sensor2 | | Sensor3 |\n> * +---------+ +---------+\n> */\n> -void CameraData::enumerateVideoDevices(MediaLink *link, const std::string &frontend)\n> +bool CameraData::enumerateVideoDevices(MediaLink *link, const std::string &frontend)\n> {\n> \tconst MediaPad *sinkPad = link->sink();\n> \tconst MediaEntity *entity = sinkPad->entity();\n> \tbool frontendFound = false;\n> \n> +\t/* Once we reach the Frontend entity, we are done. */\n> +\tif (link->sink()->entity()->name() == frontend)\n> +\t\treturn true;\n> +\n> \t/* We only deal with Video Mux and Bridge devices in cascade. */\n> \tif (entity->function() != MEDIA_ENT_F_VID_MUX &&\n> \t entity->function() != MEDIA_ENT_F_VID_IF_BRIDGE)\n> -\t\treturn;\n> +\t\treturn false;\n> \n> \t/* Find the source pad for this Video Mux or Bridge device. */\n> \tconst MediaPad *sourcePad = nullptr;\n> @@ -1039,7 +1044,7 @@ void CameraData::enumerateVideoDevices(MediaLink *link, const std::string &front\n> \t\t\t * and this branch in the cascade.\n> \t\t\t */\n> \t\t\tif (sourcePad)\n> -\t\t\t\treturn;\n> +\t\t\t\treturn false;\n> \n> \t\t\tsourcePad = pad;\n> \t\t}\n> @@ -1056,12 +1061,9 @@ void CameraData::enumerateVideoDevices(MediaLink *link, const std::string &front\n> \t * other Video Mux and Bridge devices.\n> \t */\n> \tfor (MediaLink *l : sourcePad->links()) {\n> -\t\tenumerateVideoDevices(l, frontend);\n> -\t\t/* Once we reach the Frontend entity, we are done. */\n> -\t\tif (l->sink()->entity()->name() == frontend) {\n> -\t\t\tfrontendFound = true;\n> +\t\tfrontendFound = enumerateVideoDevices(l, frontend);\n> +\t\tif (frontendFound)\n> \t\t\tbreak;\n> -\t\t}\n> \t}\n> \n> \t/* This identifies the end of our entity enumeration recursion. */\n> @@ -1076,6 +1078,8 @@ void CameraData::enumerateVideoDevices(MediaLink *link, const std::string &front\n> \t\t\tbridgeDevices_.clear();\n> \t\t}\n> \t}\n> +\n> +\treturn frontendFound;\n> }\n> \n> int CameraData::loadPipelineConfiguration()\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> index 4c5743e04f86..4bce4ec4f978 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> @@ -68,7 +68,7 @@ public:\n> \tvoid freeBuffers();\n> \tvirtual void platformFreeBuffers() = 0;\n> \n> -\tvoid enumerateVideoDevices(MediaLink *link, const std::string &frontend);\n> +\tbool enumerateVideoDevices(MediaLink *link, const std::string &frontend);\n> \n> \tint loadPipelineConfiguration();\n> \tint loadIPA(ipa::RPi::InitResult *result);","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 BF9D5C3237\n\tfor ;\n\tThu, 26 Jun 2025 12:54:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 559A068DF6;\n\tThu, 26 Jun 2025 14:54:16 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9C87E68DE8\n\tfor ;\n\tThu, 26 Jun 2025 14:54:13 +0200 (CEST)","from pendragon.ideasonboard.com (85-76-34-12-nat.elisa-mobile.fi\n\t[85.76.34.12])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 87E80743;\n\tThu, 26 Jun 2025 14:53:54 +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=\"RyqwZITP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1750942434;\n\tbh=uZjE/UXToCKFyA8rblp3WaEwJ+zHWSIEpI/zmZjt5IQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RyqwZITPmbewlSBFRNKpWHr+jZvLCXAOYmNvVM12O5aG2NxDra+3uytjPeAlV71P6\n\t0nv2ezXyYdQg45s8BsHOWYB0DnI4eheNbJzQhxXa4VbWLfCBAqt6br6tfBeflxDIQr\n\tMB258CMOkXDGjeGLev7Boz1rm8ttBUi7gxPJVqMg=","Date":"Thu, 26 Jun 2025 15:53:49 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org, dave.stevenson@raspberrypi.com","Subject":"Re: [PATCH v1 1/3] pipeline: rpi: Fix for enumerating the media\n\tgraphs","Message-ID":"<20250626125349.GM8738@pendragon.ideasonboard.com>","References":"<20250625084212.858487-1-naush@raspberrypi.com>\n\t<20250625084212.858487-2-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250625084212.858487-2-naush@raspberrypi.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\" "}}]