[{"id":21654,"web_url":"https://patchwork.libcamera.org/comment/21654/","msgid":"","date":"2021-12-08T07:51:46","subject":"Re: [libcamera-devel] [PATCH v3 1/9] libcamera: Add members to\n\tMediaEntity to support ancillary entities","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 Tue, Dec 07, 2021 at 10:45:04PM +0000, Daniel Scally wrote:\n> With kernel support for ancillary links, we can describe the\n> relationship between two devices represented individually as instances\n> of MediaEntity. As the only property of that relationship is its\n> existence, describe those relationships in libcamera simply as a\n> vector of MediaEntity pointers to the ancillary devices.\n> \n> Signed-off-by: Daniel Scally \n> ---\n> Changes in v3:\n> \n> \t- Fixed some style issues\n> \t- Made addAncillaryEntity() private\n\nGeneral note, you could keep the changelog for previous versions. I\nusually include them in my commit message (below ---) when I develop,\nand only strip them off when upstreaming.\n\n> include/libcamera/internal/media_object.h | 4 ++++\n> src/libcamera/media_object.cpp | 15 +++++++++++++++\n> 2 files changed, 19 insertions(+)\n> \n> diff --git a/include/libcamera/internal/media_object.h b/include/libcamera/internal/media_object.h\n> index 90c63598..b1572968 100644\n> --- a/include/libcamera/internal/media_object.h\n> +++ b/include/libcamera/internal/media_object.h\n> @@ -104,6 +104,7 @@ public:\n> \tunsigned int deviceMinor() const { return minor_; }\n> \n> \tconst std::vector &pads() const { return pads_; }\n> +\tconst std::vector ancillaryEntities() const { return ancillaryEntities_; }\n> \n> \tconst MediaPad *getPadByIndex(unsigned int index) const;\n> \tconst MediaPad *getPadById(unsigned int id) const;\n> @@ -120,6 +121,8 @@ private:\n> \n> \tvoid addPad(MediaPad *pad);\n> \n> +\tvoid addAncillaryEntity(MediaEntity *ancillaryEntity);\n> +\n> \tstd::string name_;\n> \tunsigned int function_;\n> \tunsigned int flags_;\n> @@ -129,6 +132,7 @@ private:\n> \tunsigned int minor_;\n> \n> \tstd::vector pads_;\n> +\tstd::vector ancillaryEntities_;\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp\n> index f425d044..dbcf10e2 100644\n> --- a/src/libcamera/media_object.cpp\n> +++ b/src/libcamera/media_object.cpp\n> @@ -326,6 +326,21 @@ void MediaPad::addLink(MediaLink *link)\n> * \\return The list of the entity's pads\n> */\n> \n> +/**\n> + * \\fn MediaEntity::ancillaryEntities()\n> + * \\brief Retrieve all ancillary entities of the entity\n> + * \\return The list of the entity's ancillary entities\n> + */\n> +\n> +/**\n> + * \\brief Add a MediaEntity to the list of ancillary entities\n> + * \\param[in] ancillaryEntity The instance of MediaEntity to add\n> + */\n> +void MediaEntity::addAncillaryEntity(MediaEntity *ancillaryEntity)\n> +{\n> +\tancillaryEntities_.push_back(ancillaryEntity);\n> +}\n> +\n\nWe try to match the order of functions in the .h and .cpp file (at least\nwithin the public, protected and private categories, the categories\nthemselves can be interleaved). This function should thus be moved right\nafter addPad().\n\nReviewed-by: Laurent Pinchart \n\n> /**\n> * \\brief Get a pad in this entity by its index\n> * \\param[in] index The 0-based pad index","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 6180FBDB13\n\tfor ;\n\tWed, 8 Dec 2021 07:52:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 396D060868;\n\tWed, 8 Dec 2021 08:52:18 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 01A6260115\n\tfor ;\n\tWed, 8 Dec 2021 08:52:15 +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 5361E8BB;\n\tWed, 8 Dec 2021 08:52:15 +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=\"loU2wBI4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638949935;\n\tbh=dRotdmyGIsvO7kxr1HzpquKvulBIXNLu69jpDvvlVes=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=loU2wBI4jOd+lCpDmerKiH9lHQP3HgCq+Xd6EYT8fAQ6xtVz4YjgykefZwV/CmelG\n\tLrr4hU6rCHgKSYZhe9D6RxfQOKRuDaucFeX2POrqPzclXePA4HT02hPbpg9n0JRzg7\n\tm14gKeizyI4bNJ9iWlk2GdJnUw9WwCD2YV99wxHw=","Date":"Wed, 8 Dec 2021 09:51:46 +0200","From":"Laurent Pinchart ","To":"Daniel Scally ","Message-ID":"","References":"<20211207224512.753979-1-djrscally@gmail.com>\n\t<20211207224512.753979-2-djrscally@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211207224512.753979-2-djrscally@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v3 1/9] libcamera: Add members to\n\tMediaEntity to support ancillary entities","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":21675,"web_url":"https://patchwork.libcamera.org/comment/21675/","msgid":"<0b0e3035-e97c-0aaa-2675-4e3d31e6ca84@gmail.com>","date":"2021-12-08T12:42:23","subject":"Re: [libcamera-devel] [PATCH v3 1/9] libcamera: Add members to\n\tMediaEntity to support ancillary entities","submitter":{"id":90,"url":"https://patchwork.libcamera.org/api/people/90/","name":"Daniel Scally","email":"djrscally@gmail.com"},"content":"Hi Laurent\n\nOn 08/12/2021 07:51, Laurent Pinchart wrote:\n> Hi Daniel,\n>\n> Thank you for the patch.\n>\n> On Tue, Dec 07, 2021 at 10:45:04PM +0000, Daniel Scally wrote:\n>> With kernel support for ancillary links, we can describe the\n>> relationship between two devices represented individually as instances\n>> of MediaEntity. As the only property of that relationship is its\n>> existence, describe those relationships in libcamera simply as a\n>> vector of MediaEntity pointers to the ancillary devices.\n>>\n>> Signed-off-by: Daniel Scally \n>> ---\n>> Changes in v3:\n>>\n>> \t- Fixed some style issues\n>> \t- Made addAncillaryEntity() private\n> General note, you could keep the changelog for previous versions. I\n> usually include them in my commit message (below ---) when I develop,\n> and only strip them off when upstreaming.\n\n\nSure, I'll start doing that\n\n>\n>> include/libcamera/internal/media_object.h | 4 ++++\n>> src/libcamera/media_object.cpp | 15 +++++++++++++++\n>> 2 files changed, 19 insertions(+)\n>>\n>> diff --git a/include/libcamera/internal/media_object.h b/include/libcamera/internal/media_object.h\n>> index 90c63598..b1572968 100644\n>> --- a/include/libcamera/internal/media_object.h\n>> +++ b/include/libcamera/internal/media_object.h\n>> @@ -104,6 +104,7 @@ public:\n>> \tunsigned int deviceMinor() const { return minor_; }\n>> \n>> \tconst std::vector &pads() const { return pads_; }\n>> +\tconst std::vector ancillaryEntities() const { return ancillaryEntities_; }\n>> \n>> \tconst MediaPad *getPadByIndex(unsigned int index) const;\n>> \tconst MediaPad *getPadById(unsigned int id) const;\n>> @@ -120,6 +121,8 @@ private:\n>> \n>> \tvoid addPad(MediaPad *pad);\n>> \n>> +\tvoid addAncillaryEntity(MediaEntity *ancillaryEntity);\n>> +\n>> \tstd::string name_;\n>> \tunsigned int function_;\n>> \tunsigned int flags_;\n>> @@ -129,6 +132,7 @@ private:\n>> \tunsigned int minor_;\n>> \n>> \tstd::vector pads_;\n>> +\tstd::vector ancillaryEntities_;\n>> };\n>> \n>> } /* namespace libcamera */\n>> diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp\n>> index f425d044..dbcf10e2 100644\n>> --- a/src/libcamera/media_object.cpp\n>> +++ b/src/libcamera/media_object.cpp\n>> @@ -326,6 +326,21 @@ void MediaPad::addLink(MediaLink *link)\n>> * \\return The list of the entity's pads\n>> */\n>> \n>> +/**\n>> + * \\fn MediaEntity::ancillaryEntities()\n>> + * \\brief Retrieve all ancillary entities of the entity\n>> + * \\return The list of the entity's ancillary entities\n>> + */\n>> +\n>> +/**\n>> + * \\brief Add a MediaEntity to the list of ancillary entities\n>> + * \\param[in] ancillaryEntity The instance of MediaEntity to add\n>> + */\n>> +void MediaEntity::addAncillaryEntity(MediaEntity *ancillaryEntity)\n>> +{\n>> +\tancillaryEntities_.push_back(ancillaryEntity);\n>> +}\n>> +\n> We try to match the order of functions in the .h and .cpp file (at least\n> within the public, protected and private categories, the categories\n> themselves can be interleaved). This function should thus be moved right\n> after addPad().\n\n\nOrdering may or may not be my strong suit...\n\n> Reviewed-by: Laurent Pinchart \n\n\nThanks!\n\n>\n>> /**\n>> * \\brief Get a pad in this entity by its index\n>> * \\param[in] index The 0-based pad index","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 7CA5CBF415\n\tfor ;\n\tWed, 8 Dec 2021 12:42:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C4E716086A;\n\tWed, 8 Dec 2021 13:42:26 +0100 (CET)","from mail-wm1-x32f.google.com (mail-wm1-x32f.google.com\n\t[IPv6:2a00:1450:4864:20::32f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 46FFD60225\n\tfor ;\n\tWed, 8 Dec 2021 13:42:25 +0100 (CET)","by mail-wm1-x32f.google.com with SMTP id\n\to19-20020a1c7513000000b0033a93202467so1698977wmc.2\n\tfor ;\n\tWed, 08 Dec 2021 04:42:25 -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\tr17sm2918953wmq.11.2021.12.08.04.42.24\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tWed, 08 Dec 2021 04:42:24 -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=\"FmKFYqdM\"; 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=h9oeuLxSjTkjpk1F50R1/Whwnrl86gxo8s/9vazDPi0=;\n\tb=FmKFYqdM3JtFhyc3ldcXZv7XMkCgli3rEPhJ+YZwP3rvxYO/Y/GRWTFaA3IzDhIQsN\n\twjnkrWDaSxWPDLMq1zg8DBRDUmtaoNuYmlWvnRg6xveqprLHT45TazMtp414XwYPzpWC\n\toLtzyxo7KCcGYSC72GyvZWRZYRIWUEmIcq1WKGUiv8RfvPEFdWFN74hkYPTvvXRyHHb/\n\tpREXoJWOHPedtUfqgO5AhwZ+ZMhkucZF0xbfTDnXv+uADyxN2Pd4ZkB09KQwB1m3il0g\n\t+SUNKhpVFfU9miEV5Nmr14VgXJH2U65ezsJBFuCzjCKYxhm+aKV0brqAgW57+dTxplcY\n\t9/ZQ==","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=h9oeuLxSjTkjpk1F50R1/Whwnrl86gxo8s/9vazDPi0=;\n\tb=mrJ7+85+nZWPQVDcM6gS4jZO1zHBVLGpwBwzguwmmCGxwShoavDbUUvwGMWrMHui0M\n\tFsgBjamjkddCqEsuWFfQzqinbtLfQ+gpGlX0Nktv4BZSiCCbE9Q+5sVBNuAkhevi5sBQ\n\ttbcdj0UPO9LYwNO7zt0/1Jl6xBcv/5AvAoI/AGJQ3WViSZJU+JLBHR6j8KQZwzD5fyCB\n\tNuTnNsKw1xsIJu9cuLSNhKE0eJlP1KwYydqFQ/A6RjPpufPnIt4fXUE1i/8DuYj8BI/s\n\tUSgsFqKjDYFa344qy/OjFGGYuAx5A/FiksedqRnISCFTRrS9iZ7hjNKOkCIr87HGC/U+\n\tD+VA==","X-Gm-Message-State":"AOAM5307Y7xmejvQg56V5n0AAzLaydcZXs8+zsXCHgWUwKvwhTpaMBq9\n\tJSRS5/hp9b7aokOtvcbEoPPxXfHIGbI=","X-Google-Smtp-Source":"ABdhPJyW4aVw3sJdIj2OGHs3O67Jp/tZ+vfU8HDgE+8blCptemzi0l5fyuYd4eE30PNCAK4LeDPLrQ==","X-Received":"by 2002:a1c:7ed3:: with SMTP id\n\tz202mr15692303wmc.110.1638967344908; \n\tWed, 08 Dec 2021 04:42:24 -0800 (PST)","To":"Laurent Pinchart ","References":"<20211207224512.753979-1-djrscally@gmail.com>\n\t<20211207224512.753979-2-djrscally@gmail.com>\n\t","From":"Daniel Scally ","Message-ID":"<0b0e3035-e97c-0aaa-2675-4e3d31e6ca84@gmail.com>","Date":"Wed, 8 Dec 2021 12:42:23 +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 v3 1/9] libcamera: Add members to\n\tMediaEntity to support ancillary entities","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\" "}}]