[{"id":21299,"web_url":"https://patchwork.libcamera.org/comment/21299/","msgid":"","date":"2021-11-28T22:02:28","subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: Add support for\n\tancillary links to MediaLink","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Daniel,\n\nThank you for the patch.\n\nOn Fri, Nov 26, 2021 at 12:31:14AM +0000, Daniel Scally wrote:\n> Update the MediaLink class to include members suitable for the new\n> type of media_v2_link, connecting two instances of MediaEntity\n> rather than MediaPads\n> \n> Signed-off-by: Daniel Scally \n> ---\n> \n> Adding new members and a new constructor here seemed in the end like the least\n> impactful and probably cleanest method of doing this, as otherwise the source\n> and sink would need to become MediaObjects and be cast everywhere.\n\nI'm not really thrilled by this. Given that the purpose of ancillary\nlinks is to expose the relationship between a primary device and its\nancillary devices, and given that those relationships don't carry any\nother property than their existence, I think adding a\n\n\tconst std::vector ancillaryEntities() const;\n\n(naming bikeshedding accepted) function to the MediaEntity class should\nbe enough, without a need to extend the MediaLink API.\n\n> include/libcamera/internal/media_object.h | 10 ++++++++++\n> src/libcamera/media_object.cpp | 24 ++++++++++++++++++++++-\n> 2 files changed, 33 insertions(+), 1 deletion(-)\n> \n> diff --git a/include/libcamera/internal/media_object.h b/include/libcamera/internal/media_object.h\n> index 6ae22c67..79c71325 100644\n> --- a/include/libcamera/internal/media_object.h\n> +++ b/include/libcamera/internal/media_object.h\n> @@ -45,6 +45,8 @@ class MediaLink : public MediaObject\n> public:\n> \tMediaPad *source() const { return source_; }\n> \tMediaPad *sink() const { return sink_; }\n> +\tMediaEntity *primary() const { return primary_; };\n> +\tMediaEntity *ancillary() const { return ancillary_; };\n> \tunsigned int flags() const { return flags_; }\n> \tint setEnabled(bool enable);\n> \n> @@ -55,9 +57,13 @@ private:\n> \n> \tMediaLink(const struct media_v2_link *link,\n> \t\t MediaPad *source, MediaPad *sink);\n> +\tMediaLink(const struct media_v2_link *link,\n> +\t\t MediaEntity *primary, MediaEntity *ancillary);\n> \n> \tMediaPad *source_;\n> \tMediaPad *sink_;\n> +\tMediaEntity *primary_;\n> +\tMediaEntity *ancillary_;\n> \tunsigned int flags_;\n> };\n> \n> @@ -104,12 +110,15 @@ public:\n> \tunsigned int deviceMinor() const { return minor_; }\n> \n> \tconst std::vector &pads() const { return pads_; }\n> +\tconst std::vector &ancillary_links() const { return ancillary_links_; }\n> \n> \tconst MediaPad *getPadByIndex(unsigned int index) const;\n> \tconst MediaPad *getPadById(unsigned int id) const;\n> \n> \tint setDeviceNode(const std::string &deviceNode);\n> \n> +\tvoid addLink(MediaLink *link);\n> +\n> private:\n> \tLIBCAMERA_DISABLE_COPY_AND_MOVE(MediaEntity)\n> \n> @@ -129,6 +138,7 @@ private:\n> \tunsigned int minor_;\n> \n> \tstd::vector pads_;\n> +\tstd::vector ancillary_links_;\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp\n> index f425d044..e903d5ef 100644\n> --- a/src/libcamera/media_object.cpp\n> +++ b/src/libcamera/media_object.cpp\n> @@ -134,7 +134,7 @@ int MediaLink::setEnabled(bool enable)\n> }\n> \n> /**\n> - * \\brief Construct a MediaLink\n> + * \\brief Construct a MediaLink between two pads\n> * \\param[in] link The media link kernel data\n> * \\param[in] source The source pad at the origin of the link\n> * \\param[in] sink The sink pad at the destination of the link\n> @@ -146,6 +146,19 @@ MediaLink::MediaLink(const struct media_v2_link *link, MediaPad *source,\n> {\n> }\n> \n> +/**\n> + * \\brief Construct a MediaLink between two entities\n> + * \\param[in] link The media link kernel data\n> + * \\param[in] primary The primary entity at the origin of the link\n> + * \\param[in] ancillary The ancillary entity at the destination of the link\n> + */\n> +MediaLink::MediaLink(const struct media_v2_link *link, MediaEntity *primary,\n> +\t\t MediaEntity *ancillary)\n> +\t: MediaObject(primary->device(), link->id), primary_(primary),\n> +\t ancillary_(ancillary), flags_(link->flags)\n> +{\n> +}\n> +\n> /**\n> * \\fn MediaLink::source()\n> * \\brief Retrieve the link's source pad\n> @@ -378,6 +391,15 @@ int MediaEntity::setDeviceNode(const std::string &deviceNode)\n> \treturn 0;\n> }\n> \n> +/**\n> + * \\brief Add an ancillary link to the MediaEntity\n> + * \\param[in] link Pointer to the MediaLink class\n> + */\n> +void MediaEntity::addLink(MediaLink *link)\n> +{\n> +\tancillary_links_.push_back(link);\n> +}\n> +\n> /**\n> * \\brief Construct a MediaEntity\n> * \\param[in] dev The media device this entity belongs to","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 002ECBDB13\n\tfor ;\n\tSun, 28 Nov 2021 22:02:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 18E446058C;\n\tSun, 28 Nov 2021 23:02:55 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7AA6F604FC\n\tfor ;\n\tSun, 28 Nov 2021 23:02:53 +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 D98EFF1;\n\tSun, 28 Nov 2021 23:02:52 +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=\"omEuN1/4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638136973;\n\tbh=sk7RJzcKwsWC6GP+ICYB3SG/ipQlu01HQQ1srqS4HYI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=omEuN1/4Zyvfa+5iY1NEuR/4HR3dBj3cPhO1m7Tzadwfr2yQScTaFbuV1Tt5URxk1\n\tGOfkDnQb7pX/dM54CXmwC1Th42sB32FvkxntaPr8zHU1da8hWg4zsgAOf5dyHTDXVC\n\t1EFIPiibzOEH1YzfO6sni2sG3/R8+61K1XnClLV0=","Date":"Mon, 29 Nov 2021 00:02:28 +0200","From":"Laurent Pinchart ","To":"Daniel Scally ","Message-ID":"","References":"<20211126003118.42356-1-djrscally@gmail.com>\n\t<20211126003118.42356-2-djrscally@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211126003118.42356-2-djrscally@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: Add support for\n\tancillary links to MediaLink","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\" "}},{"id":21308,"web_url":"https://patchwork.libcamera.org/comment/21308/","msgid":"<0422b8e6-b5e0-137d-a639-e28de955fcd6@gmail.com>","date":"2021-11-29T08:38:49","subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: Add support for\n\tancillary links to MediaLink","submitter":{"id":90,"url":"https://patchwork.libcamera.org/api/people/90/","name":"Daniel Scally","email":"djrscally@gmail.com"},"content":"Morning Laurent\n\nOn 28/11/2021 22:02, Laurent Pinchart wrote:\n> Hi Daniel,\n>\n> Thank you for the patch.\n>\n> On Fri, Nov 26, 2021 at 12:31:14AM +0000, Daniel Scally wrote:\n>> Update the MediaLink class to include members suitable for the new\n>> type of media_v2_link, connecting two instances of MediaEntity\n>> rather than MediaPads\n>>\n>> Signed-off-by: Daniel Scally \n>> ---\n>>\n>> Adding new members and a new constructor here seemed in the end like the least\n>> impactful and probably cleanest method of doing this, as otherwise the source\n>> and sink would need to become MediaObjects and be cast everywhere.\n> I'm not really thrilled by this. Given that the purpose of ancillary\n> links is to expose the relationship between a primary device and its\n> ancillary devices, and given that those relationships don't carry any\n> other property than their existence, I think adding a\n>\n> \tconst std::vector ancillaryEntities() const;\n>\n> (naming bikeshedding accepted) function to the MediaEntity class should\n> be enough, without a need to extend the MediaLink API.\n\n\nFair enough...that would simplify patch two quite a lot too I suppose.\nI'll switch to doing it that way for the v2.\n\n>\n>> include/libcamera/internal/media_object.h | 10 ++++++++++\n>> src/libcamera/media_object.cpp | 24 ++++++++++++++++++++++-\n>> 2 files changed, 33 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/include/libcamera/internal/media_object.h b/include/libcamera/internal/media_object.h\n>> index 6ae22c67..79c71325 100644\n>> --- a/include/libcamera/internal/media_object.h\n>> +++ b/include/libcamera/internal/media_object.h\n>> @@ -45,6 +45,8 @@ class MediaLink : public MediaObject\n>> public:\n>> \tMediaPad *source() const { return source_; }\n>> \tMediaPad *sink() const { return sink_; }\n>> +\tMediaEntity *primary() const { return primary_; };\n>> +\tMediaEntity *ancillary() const { return ancillary_; };\n>> \tunsigned int flags() const { return flags_; }\n>> \tint setEnabled(bool enable);\n>> \n>> @@ -55,9 +57,13 @@ private:\n>> \n>> \tMediaLink(const struct media_v2_link *link,\n>> \t\t MediaPad *source, MediaPad *sink);\n>> +\tMediaLink(const struct media_v2_link *link,\n>> +\t\t MediaEntity *primary, MediaEntity *ancillary);\n>> \n>> \tMediaPad *source_;\n>> \tMediaPad *sink_;\n>> +\tMediaEntity *primary_;\n>> +\tMediaEntity *ancillary_;\n>> \tunsigned int flags_;\n>> };\n>> \n>> @@ -104,12 +110,15 @@ public:\n>> \tunsigned int deviceMinor() const { return minor_; }\n>> \n>> \tconst std::vector &pads() const { return pads_; }\n>> +\tconst std::vector &ancillary_links() const { return ancillary_links_; }\n>> \n>> \tconst MediaPad *getPadByIndex(unsigned int index) const;\n>> \tconst MediaPad *getPadById(unsigned int id) const;\n>> \n>> \tint setDeviceNode(const std::string &deviceNode);\n>> \n>> +\tvoid addLink(MediaLink *link);\n>> +\n>> private:\n>> \tLIBCAMERA_DISABLE_COPY_AND_MOVE(MediaEntity)\n>> \n>> @@ -129,6 +138,7 @@ private:\n>> \tunsigned int minor_;\n>> \n>> \tstd::vector pads_;\n>> +\tstd::vector ancillary_links_;\n>> };\n>> \n>> } /* namespace libcamera */\n>> diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp\n>> index f425d044..e903d5ef 100644\n>> --- a/src/libcamera/media_object.cpp\n>> +++ b/src/libcamera/media_object.cpp\n>> @@ -134,7 +134,7 @@ int MediaLink::setEnabled(bool enable)\n>> }\n>> \n>> /**\n>> - * \\brief Construct a MediaLink\n>> + * \\brief Construct a MediaLink between two pads\n>> * \\param[in] link The media link kernel data\n>> * \\param[in] source The source pad at the origin of the link\n>> * \\param[in] sink The sink pad at the destination of the link\n>> @@ -146,6 +146,19 @@ MediaLink::MediaLink(const struct media_v2_link *link, MediaPad *source,\n>> {\n>> }\n>> \n>> +/**\n>> + * \\brief Construct a MediaLink between two entities\n>> + * \\param[in] link The media link kernel data\n>> + * \\param[in] primary The primary entity at the origin of the link\n>> + * \\param[in] ancillary The ancillary entity at the destination of the link\n>> + */\n>> +MediaLink::MediaLink(const struct media_v2_link *link, MediaEntity *primary,\n>> +\t\t MediaEntity *ancillary)\n>> +\t: MediaObject(primary->device(), link->id), primary_(primary),\n>> +\t ancillary_(ancillary), flags_(link->flags)\n>> +{\n>> +}\n>> +\n>> /**\n>> * \\fn MediaLink::source()\n>> * \\brief Retrieve the link's source pad\n>> @@ -378,6 +391,15 @@ int MediaEntity::setDeviceNode(const std::string &deviceNode)\n>> \treturn 0;\n>> }\n>> \n>> +/**\n>> + * \\brief Add an ancillary link to the MediaEntity\n>> + * \\param[in] link Pointer to the MediaLink class\n>> + */\n>> +void MediaEntity::addLink(MediaLink *link)\n>> +{\n>> +\tancillary_links_.push_back(link);\n>> +}\n>> +\n>> /**\n>> * \\brief Construct a MediaEntity\n>> * \\param[in] dev The media device this entity belongs to","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 66A56BDB13\n\tfor ;\n\tMon, 29 Nov 2021 08:38:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C6B966058C;\n\tMon, 29 Nov 2021 09:38:52 +0100 (CET)","from mail-wm1-x329.google.com (mail-wm1-x329.google.com\n\t[IPv6:2a00:1450:4864:20::329])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 295E460423\n\tfor ;\n\tMon, 29 Nov 2021 09:38:51 +0100 (CET)","by mail-wm1-x329.google.com with SMTP id\n\t77-20020a1c0450000000b0033123de3425so16165811wme.0\n\tfor ;\n\tMon, 29 Nov 2021 00:38:51 -0800 (PST)","from [192.168.0.14]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net. [86.13.91.161])\n\tby smtp.gmail.com with ESMTPSA id\n\tj8sm12669703wrh.16.2021.11.29.00.38.49\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tMon, 29 Nov 2021 00:38:50 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"Yt3wxiNI\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-transfer-encoding:content-language; \n\tbh=dyfYFX8O4pPXZDzoeOwDVftlujaOfP9UzPb2uP7XFys=;\n\tb=Yt3wxiNIyL/JdXwJjmmGU3KSkWFdj0AWyINTqbumxc7LKC1vjcDabap7uDlO1VKrG9\n\tvfwJyK5bXqnGca7c5vZKEkhi8YaYJMnrAH4Ij7d1Sgohgl+1HNrcPMip9XKVWpBMsJLV\n\t69V+T21kxIPOCkgvJsxZmIYMZ08fuuEe4NZ4QBq0OSscK7YgLwuG0d9YfNZWy+PNppoj\n\tpf/CbS1gBXmxG/vGPfEZXOpZ3eSVUX1ePqmxdsgwGjFHWrDb7Pe+eacMNmRMBPBgoyS7\n\tUkwvQN4S3+Hsk78oyHNHhrloaL/rD8B4Xad/u/sYrFhTzn9xL+QNmnBDHXEfbWfm+2aA\n\tKNFQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:subject:to:cc:references:from:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-transfer-encoding\n\t:content-language;\n\tbh=dyfYFX8O4pPXZDzoeOwDVftlujaOfP9UzPb2uP7XFys=;\n\tb=mA/iVNzOtufBchZydgZI0dOFFGqQBcTEBeoUesmxvNnBcrolTVr7OVbTqDab+rPV9V\n\t6j507vcop8xyR6KiHv6DBQ21vrCGx567omwKK5IJlrHFMddK8IRIYHL+Y/AhJV36v6PZ\n\tKMWSgG/mlTwMse4UORb6JAvJaOZrXr8a+UzD6mKUmLudWNWPoordH9vsa+mYO5C01GNU\n\tuPbh+TNZmmRVwBZRq1DGBPXvuqoFt0D0EwmRClGc4yStUxe1Ak8hNAc+4G5zVqpSArxZ\n\tzh0/drka/wX2DS2uyWhNArVRgwRIEDZsg+5jonuNLsz2K3vQ18ke/eRYHt1bpFR5FKPv\n\tBd/A==","X-Gm-Message-State":"AOAM531TS6ggS8k361q9sYLhtiSfg7KA2cV6xxdsq/TtE0ZvKx+1k3ic\n\tsS/Lv+6ZNr5tunUGfyudo8vsK9wmowE=","X-Google-Smtp-Source":"ABdhPJzfEbCoBAnoUR+UU/s7GU8ot2NIpsrmt2eWU+iOThRxMmbYGxVHnCAFGjKigCZb9Fk/ctBtpw==","X-Received":"by 2002:a05:600c:1e27:: with SMTP id\n\tay39mr35291697wmb.84.1638175130724; \n\tMon, 29 Nov 2021 00:38:50 -0800 (PST)","To":"Laurent Pinchart ","References":"<20211126003118.42356-1-djrscally@gmail.com>\n\t<20211126003118.42356-2-djrscally@gmail.com>\n\t","From":"Daniel Scally ","Message-ID":"<0422b8e6-b5e0-137d-a639-e28de955fcd6@gmail.com>","Date":"Mon, 29 Nov 2021 08:38:49 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.14.0","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: Add support for\n\tancillary links to MediaLink","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\" "}}]