[{"id":22113,"web_url":"https://patchwork.libcamera.org/comment/22113/","msgid":"","date":"2022-02-03T00:18:04","subject":"Re: [libcamera-devel] [PATCH v4 3/9] libcamera: media_device:\n\tHandle ancillary links in populateLinks()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Dan,\n\nThank you for the patch.\n\nOn Mon, Jan 31, 2022 at 10:33:24PM +0000, Daniel Scally wrote:\n> The populateLinks() function can't currently handle ancillary links\n> which causes an error to be thrown in process() when the MediaObject\n> cannot be cast to a MediaPad.\n> \n> Add explicit handling for the different link types, creating either\n> pad-2-pad links or else storing the pointer to the ancillary device\n> MediaEntity in the ancillaryEntities_ member of the primary.\n> \n> Signed-off-by: Daniel Scally \n> ---\n> \n> Changes in v4:\n> \n> \t- Added some error checking to confirm that the casts to pad or entity\n> \treturn valid pointers (Laurent)\n> \n> Changes in v3:\n> \n> \t- Split out the new macro\n> \t- Fixed some style errors and comments\n> \t- Added a default case\n> \n> src/libcamera/media_device.cpp | 67 ++++++++++++++++++++++++++--------\n> 1 file changed, 51 insertions(+), 16 deletions(-)\n> \n> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp\n> index 941f86c2..a008383a 100644\n> --- a/src/libcamera/media_device.cpp\n> +++ b/src/libcamera/media_device.cpp\n> @@ -683,40 +683,75 @@ bool MediaDevice::populateLinks(const struct media_v2_topology &topology)\n> \t\t\t\t\t (topology.ptr_links);\n> \n> \tfor (unsigned int i = 0; i < topology.num_links; ++i) {\n> -\t\t/* We only care about pad-2-pad links here. */\n> -\t\tif ((mediaLinks[i].flags & MEDIA_LNK_FL_LINK_TYPE) !=\n> -\t\t MEDIA_LNK_FL_DATA_LINK)\n> +\t\t/*\n> +\t\t * Skip links between entities and interfaces: interfaces are\n> +\t\t * not created as MediaObjects at this time, so the source and\n\ns/MediaObjects/MediaObject instances/\n\n> +\t\t * sink pointers would never be found.\n\nIsn't it only the sink pointer that can't be found ?\n\n> +\t\t */\n> +\t\tif ((mediaLinks[i].flags & MEDIA_LNK_FL_LINK_TYPE) ==\n> +\t\t MEDIA_LNK_FL_INTERFACE_LINK)\n> \t\t\tcontinue;\n> \n> -\t\t/* Store references to source and sink pads in the link. */\n> +\t\t/* Look up the source and sink objects. */\n> \t\tunsigned int source_id = mediaLinks[i].source_id;\n> -\t\tMediaPad *source = dynamic_cast\n> -\t\t\t\t (object(source_id));\n> +\t\tMediaObject *source = object(source_id);\n> \t\tif (!source) {\n> \t\t\tLOG(MediaDevice, Error)\n> -\t\t\t\t<< \"Failed to find pad with id: \"\n> +\t\t\t\t<< \"Failed to find MediaObject with id: \"\n\nWhile at it, s/://. Same below.\n\n> \t\t\t\t<< source_id;\n> \t\t\treturn false;\n> \t\t}\n> \n> \t\tunsigned int sink_id = mediaLinks[i].sink_id;\n> -\t\tMediaPad *sink = dynamic_cast\n> -\t\t\t\t (object(sink_id));\n> +\t\tMediaObject *sink = object(sink_id);\n> \t\tif (!sink) {\n> \t\t\tLOG(MediaDevice, Error)\n> -\t\t\t\t<< \"Failed to find pad with id: \"\n> +\t\t\t\t<< \"Failed to find MediaObject with id: \"\n> \t\t\t\t<< sink_id;\n> \t\t\treturn false;\n> \t\t}\n> \n> -\t\tMediaLink *link = new MediaLink(&mediaLinks[i], source, sink);\n> -\t\tif (!addObject(link)) {\n> -\t\t\tdelete link;\n> -\t\t\treturn false;\n> +\t\tswitch (mediaLinks[i].flags & MEDIA_LNK_FL_LINK_TYPE) {\n> +\t\tcase MEDIA_LNK_FL_DATA_LINK: {\n> +\t\t\tMediaPad *sourcePad = dynamic_cast(source);\n> +\t\t\tMediaPad *sinkPad = dynamic_cast(sink);\n> +\t\t\tif (!source || !sink) {\n> +\t\t\t\tLOG(MediaDevice, Error)\n> +\t\t\t\t\t<< \"Source or sink is not a pad\";\n> +\t\t\t\treturn false;\n> +\t\t\t}\n> +\n> +\t\t\tMediaLink *link = new MediaLink(&mediaLinks[i],\n> +\t\t\t\t\t\t\tsourcePad, sinkPad);\n> +\t\t\tif (!addObject(link)) {\n> +\t\t\t\tdelete link;\n> +\t\t\t\treturn false;\n> +\t\t\t}\n> +\n> +\t\t\tlink->source()->addLink(link);\n> +\t\t\tlink->sink()->addLink(link);\n> +\n> +\t\t\tbreak;\n> +\t\t}\n\nBlank line here.\n\n> +\t\tcase MEDIA_LNK_FL_ANCILLARY_LINK: {\n> +\t\t\tMediaEntity *primary = dynamic_cast(source);\n> +\t\t\tMediaEntity *ancillary = dynamic_cast(sink);\n> +\t\t\tif (!primary || !ancillary) {\n> +\t\t\t\tLOG(MediaDevice, Error)\n> +\t\t\t\t\t<< \"Source or sink is not an entity\";\n> +\t\t\t\treturn false;\n> +\t\t\t}\n> +\n> +\t\t\tprimary->addAncillaryEntity(ancillary);\n> +\n> +\t\t\tbreak;\n> \t\t}\n\nHere too.\n\nReviewed-by: Laurent Pinchart \n\n> +\t\tdefault:\n> +\t\t\tLOG(MediaDevice, Warning)\n> +\t\t\t\t<< \"Unknown media link type\";\n> \n> -\t\tsource->addLink(link);\n> -\t\tsink->addLink(link);\n> +\t\t\tbreak;\n> +\t\t}\n> \t}\n> \n> \treturn true;","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 5FB82BF415\n\tfor ;\n\tThu, 3 Feb 2022 00:18:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C9A07609E1;\n\tThu, 3 Feb 2022 01:18:28 +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 8CB76609C3\n\tfor ;\n\tThu, 3 Feb 2022 01:18:27 +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 EAD0749C;\n\tThu, 3 Feb 2022 01:18:26 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"XMrjzpMO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1643847507;\n\tbh=YUQDCe/aYLLYInDr8+DOJy6jg2ImSkfpnOUnsFeT25w=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XMrjzpMOWNeFqdzUeBW/hn5htz4cLZGo8cNn3LaSyfYXNAv1vy80TKA3FVs9Bp3De\n\tKQQCY/h5AOGcT+ATDuMeYhfO8APNjhfntPCf7vRDs8vo8tFwHn8jreynjzW2A2Gr5c\n\tfFDjy3ZnHBX0ybZPkhl87lfWDIcUvs/GGP24SXgI=","Date":"Thu, 3 Feb 2022 02:18:04 +0200","From":"Laurent Pinchart ","To":"Daniel Scally ","Message-ID":"","References":"<20220131223330.61563-1-djrscally@gmail.com>\n\t<20220131223330.61563-4-djrscally@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220131223330.61563-4-djrscally@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v4 3/9] libcamera: media_device:\n\tHandle ancillary links in populateLinks()","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]