[{"id":201,"web_url":"https://patchwork.libcamera.org/comment/201/","msgid":"<20190104163257.GM22790@bigcity.dyn.berto.se>","date":"2019-01-04T16:33:16","subject":"Re: [libcamera-devel] [PATCH 2/5] libcamera: media_device: Add\n\tfunction to get a MediaLink","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your work.\n\nOn 2019-01-03 18:38:56 +0100, Jacopo Mondi wrote:\n> Add three overloaded functions 'link()' to retrieve a link between two\n> pads. Each overloaded implementation exposes a different method to\n> identify the source and sink entities.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/include/media_device.h | 6 ++\n> src/libcamera/media_device.cpp | 119 +++++++++++++++++++++++++++\n> 2 files changed, 125 insertions(+)\n> \n> diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h\n> index 9f45fc7..3228ad5 100644\n> --- a/src/libcamera/include/media_device.h\n> +++ b/src/libcamera/include/media_device.h\n> @@ -40,6 +40,12 @@ public:\n> \tconst std::vector &entities() const { return entities_; }\n> \tMediaEntity *getEntityByName(const std::string &name) const;\n> \n> +\tMediaLink *link(const std::string sourceName, unsigned int sourceIdx,\n> +\t\t\tconst std::string sinkName, unsigned int sinkIdx) const;\n> +\tMediaLink *link(const MediaEntity *source, unsigned int sourceIdx,\n> +\t\t\tconst MediaEntity *sink, unsigned int sinkIdx) const;\n> +\tMediaLink *link(const MediaPad *source, const MediaPad *sink) const;\n> +\n> private:\n> \tstd::string driver_;\n> \tstd::string devnode_;\n> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp\n> index 2f9490c..6892300 100644\n> --- a/src/libcamera/media_device.cpp\n> +++ b/src/libcamera/media_device.cpp\n> @@ -306,6 +306,125 @@ MediaEntity *MediaDevice::getEntityByName(const std::string &name) const\n> \treturn nullptr;\n> }\n> \n> +/**\n> + * \\brief Return the MediaLink that connects two entities identified by name\n> + * \\param sourceName The source entity name\n> + * \\param sourceIdx The index of the source pad\n> + * \\param sinkName The sink entity name\n> + * \\param sinkIdx The index of the sink pad\n> + *\n> + * Find link that connects the pads at index \\a sourceIdx of the source\n> + * entity with name \\a sourceName, to the pad at index \\a sinkIdx of the\n> + * sink entity with name \\a sinkName, if any.\n> + *\n> + * \\sa MediaDevice::link(const MediaEntity *source, unsigned int sourceIdx, const MediaEntity *sink, unsigned int sinkIdx) const\n> + * \\sa MediaDevice::link(const MediaPad *source, const MediaPad *sink) const\n> + *\n> + * \\return The link that connects the two entities, nullptr otherwise\n> + */\n> +MediaLink *MediaDevice::link(const std::string sourceName, unsigned int sourceIdx,\n> +\t\t\t const std::string sinkName, unsigned int sinkIdx) const\n> +{\n> +\tconst MediaEntity *source = getEntityByName(sourceName);\n> +\tif (!source) {\n> +\t\tLOG(Error) << \"Failed to find entity with name: \"\n> +\t\t\t << sourceName << \"\\n\";\n> +\t\treturn nullptr;\n> +\t}\n> +\n> +\tconst MediaPad *sourcePad = source->getPadByIndex(sourceIdx);\n> +\tif (!sourcePad) {\n> +\t\tLOG(Error) << \"Entity \\\"\" << sourceName << \"\\\" \"\n> +\t\t\t << \"has no pad at index \" << sourceIdx << \"\\n\";\n> +\t\treturn nullptr;\n> +\t}\n> +\n> +\tconst MediaEntity *sink = getEntityByName(sinkName);\n> +\tif (!sink) {\n> +\t\tLOG(Error) << \"Failed to find entity with name: \"\n> +\t\t\t << sinkName << \"\\n\";\n> +\t\treturn nullptr;\n> +\t}\n> +\n> +\tconst MediaPad *sinkPad = sink->getPadByIndex(sinkIdx);\n> +\tif (!sinkPad) {\n> +\t\tLOG(Error) << \"Entity \\\"\" << sinkName << \"\\\" \"\n> +\t\t\t << \"has no pad at index \" << sinkIdx << \"\\n\";\n> +\t\treturn nullptr;\n> +\t}\n> +\n> +\treturn link(sourcePad, sinkPad);\n\nHow about just resolving the source and sink MediaEntity's here and then\n\n return link(source, sourceIdx, sink, sinkIdx);\n\nThis would reduce the code duplication.\n\n> +}\n> +\n> +/**\n> + * \\brief Return the MediaLink that connects two entities\n> + * \\param source The source entity\n> + * \\param sourceIdx The index of the source pad\n> + * \\param sink The sink entity\n> + * \\param sinkIdx The index of the sink pad\n> + *\n> + * Find link that connects the pads at index \\a sourceIdx of the source\n> + * entity \\a source, to the pad at index \\a sinkIdx of the sink entity \\a\n> + * sink, if any.\n> + *\n> + * \\sa MediaDevice::link(const std::string sourceName, unsigned int sourceIdx, const std::string sinkName, unsigned int sinkIdx) const\n> + * \\sa MediaDevice::link(const MediaPad *source, const MediaPad *sink) const\n> + *\n> + * \\return The link that connects the two entities, nullptr otherwise\n> + */\n> +MediaLink *MediaDevice::link(const MediaEntity *source, unsigned int sourceIdx,\n> +\t\t\t const MediaEntity *sink, unsigned int sinkIdx) const\n> +{\n> +\n> +\tconst MediaPad *sourcePad = source->getPadByIndex(sourceIdx);\n> +\tif (!sourcePad) {\n> +\t\tLOG(Error) << \"Entity \\\"\" << source->name() << \"\\\" \"\n> +\t\t\t << \"has no pad at index \" << sourceIdx << \"\\n\";\n> +\t\treturn nullptr;\n> +\t}\n> +\n> +\tconst MediaPad *sinkPad = sink->getPadByIndex(sinkIdx);\n> +\tif (!sinkPad) {\n> +\t\tLOG(Error) << \"Entity \\\"\" << sink->name() << \"\\\" \"\n> +\t\t\t << \"has no pad at index \" << sinkIdx << \"\\n\";\n> +\t\treturn nullptr;\n> +\t}\n> +\n> +\treturn link(sourcePad, sinkPad);\n> +}\n> +\n> +/**\n> + * \\brief Return the MediaLink that connects two pads\n> + * \\param source The source pad\n> + * \\param sink The sink pad\n> + *\n> + * \\sa MediaDevice::link(const std::string sourceName, unsigned int sourceIdx, const std::string sinkName, unsigned int sinkIdx) const\n> + * \\sa MediaDevice::link(const MediaEntity *source, unsigned int sourceIdx, const MediaEntity *sink, unsigned int sinkIdx) const\n> + *\n> + * \\return The link that connects the two pads, nullptr otherwise\n> + */\n> +MediaLink *MediaDevice::link(const MediaPad *source, const MediaPad *sink) const\n> +{\n> +\tif (!source || !sink)\n> +\t\treturn nullptr;\n> +\n> +\t/*\n> +\t * Now that we have made sure all instances are valid, compare\n> +\t * their ids to find a matching link.\n> +\t */\n\nI like comments in the code it self. I don't like (but I still write \nthem my self) comments that are written in this tens, how about.\n\n Compare pad ids to find the link which connects the source and sink \n pads.\n\n> +\tfor (MediaLink *link : source->links()) {\n> +\t\tif (link->sink()->id() != sink->id())\n> +\t\t\tcontinue;\n> +\n> +\t\tif (link->sink()->entity()->id() != sink->entity()->id())\n> +\t\t\tcontinue;\n> +\n> +\t\treturn link;\n> +\t}\n> +\n> +\treturn nullptr;\n> +}\n> +\n> /**\n> * \\var MediaDevice::objects_\n> * \\brief Global map of media objects (entities, pads, links) keyed by their\n> -- \n> 2.20.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lf1-x143.google.com (mail-lf1-x143.google.com\n\t[IPv6:2a00:1450:4864:20::143])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1703F60B0B\n\tfor ;\n\tFri, 4 Jan 2019 17:33:18 +0100 (CET)","by mail-lf1-x143.google.com with SMTP id u18so25798500lff.10\n\tfor ;\n\tFri, 04 Jan 2019 08:33:18 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tb20sm11125907lfj.61.2019.01.04.08.33.16\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tFri, 04 Jan 2019 08:33:16 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=N+GUb2OTz9YpJgEVmmBKMb+7/UBPfXpUvfp35UBR0D4=;\n\tb=Lj5q0JsGDUXWg/7EccgsS5fhlJqQlZNJ2UvQGv3/1v0hZEg3zNdl8AO8KdxeXFwSJN\n\t6cAZ0JpXioivdr9YjEEAR6IsvhqnJcLqIKw0n/sVyETuhkbnk5H6TORNnYd3nO+aU66N\n\t02t947DYBG3rtRF12rH1Q4lFzNBn3VOtHzmytutglCDFz9qWqhE9ncRlBHu7YA9bK39s\n\tCobmjeWQbSxwTD4jNQVVqLAWJJlJWNIh+H2TYJDvN9Tq4fMLN471kwmJNF1dl51KDIdi\n\t9UTAd9v9CkpNbEx+xH74aS4Rdp/wcjNRlhLTv8JNW7eD+00Xg2yoIeFXu31InphwjpgA\n\tGFqQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=N+GUb2OTz9YpJgEVmmBKMb+7/UBPfXpUvfp35UBR0D4=;\n\tb=OATGmQjYf1WhjANH1r2s9k+aqxizU+26oxLM/uombkDDkxngHNKUNJx/9jaNcdnaJ9\n\tEdfZHNyL7lXuTdb42fv/arHYza6WLWbyZcCF6Q4Oy05nfDSyJ6HPe1a/xu41Kpusk+Sw\n\tvvOmKxo5nQR2TGDygYWXZNFB3dmjSuTLn26Cvf8bTmoMdL9cBf4WcMB83gCtRxKhr38X\n\tVx94SUr59Df5i926RyB6jLsFUbzZWNuVA7HCB8HVsUA0DSvVzX96LrNWVF+IFViGd6m8\n\thvWyZfjPpSiU26DRwwoc87goZ/p0zJCZL3356X8RZWSbVIUEcD10Lg2MrhTIU7o3XlHh\n\tvvEg==","X-Gm-Message-State":"AA+aEWYRI4ZJ5FJQAzIxPCxyVjlLJ2M25K3DTgEETtt4L3aIKAz3BsN8\n\teWCdU2GFnf4sHePw1KjMBL1We6UrQm0=","X-Google-Smtp-Source":"AFSGD/UUmDrYOekndXbfHwrYPXt4iSdm9HNw+Uo3ZTh//jHtdEHIR5IaDd9AFvRAJDSwDXuxCylbGQ==","X-Received":"by 2002:ac2:4343:: with SMTP id\n\to3mr27036329lfl.129.1546619597238; \n\tFri, 04 Jan 2019 08:33:17 -0800 (PST)","Date":"Fri, 4 Jan 2019 17:33:16 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190104163257.GM22790@bigcity.dyn.berto.se>","References":"<20190103173859.22624-1-jacopo@jmondi.org>\n\t<20190103173859.22624-3-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190103173859.22624-3-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 2/5] libcamera: media_device: Add\n\tfunction to get a MediaLink","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Fri, 04 Jan 2019 16:33:18 -0000"}},{"id":217,"web_url":"https://patchwork.libcamera.org/comment/217/","msgid":"<20190107085730.ykny5lo42gk2iwq7@uno.localdomain>","date":"2019-01-07T08:57:30","subject":"Re: [libcamera-devel] [PATCH 2/5] libcamera: media_device: Add\n\tfunction to get a MediaLink","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Fri, Jan 04, 2019 at 05:33:16PM +0100, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> Thanks for your work.\n>\n> On 2019-01-03 18:38:56 +0100, Jacopo Mondi wrote:\n> > Add three overloaded functions 'link()' to retrieve a link between two\n> > pads. Each overloaded implementation exposes a different method to\n> > identify the source and sink entities.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/include/media_device.h | 6 ++\n> > src/libcamera/media_device.cpp | 119 +++++++++++++++++++++++++++\n> > 2 files changed, 125 insertions(+)\n> >\n> > diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h\n> > index 9f45fc7..3228ad5 100644\n> > --- a/src/libcamera/include/media_device.h\n> > +++ b/src/libcamera/include/media_device.h\n> > @@ -40,6 +40,12 @@ public:\n> > \tconst std::vector &entities() const { return entities_; }\n> > \tMediaEntity *getEntityByName(const std::string &name) const;\n> >\n> > +\tMediaLink *link(const std::string sourceName, unsigned int sourceIdx,\n> > +\t\t\tconst std::string sinkName, unsigned int sinkIdx) const;\n> > +\tMediaLink *link(const MediaEntity *source, unsigned int sourceIdx,\n> > +\t\t\tconst MediaEntity *sink, unsigned int sinkIdx) const;\n> > +\tMediaLink *link(const MediaPad *source, const MediaPad *sink) const;\n> > +\n> > private:\n> > \tstd::string driver_;\n> > \tstd::string devnode_;\n> > diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp\n> > index 2f9490c..6892300 100644\n> > --- a/src/libcamera/media_device.cpp\n> > +++ b/src/libcamera/media_device.cpp\n> > @@ -306,6 +306,125 @@ MediaEntity *MediaDevice::getEntityByName(const std::string &name) const\n> > \treturn nullptr;\n> > }\n> >\n> > +/**\n> > + * \\brief Return the MediaLink that connects two entities identified by name\n> > + * \\param sourceName The source entity name\n> > + * \\param sourceIdx The index of the source pad\n> > + * \\param sinkName The sink entity name\n> > + * \\param sinkIdx The index of the sink pad\n> > + *\n> > + * Find link that connects the pads at index \\a sourceIdx of the source\n> > + * entity with name \\a sourceName, to the pad at index \\a sinkIdx of the\n> > + * sink entity with name \\a sinkName, if any.\n> > + *\n> > + * \\sa MediaDevice::link(const MediaEntity *source, unsigned int sourceIdx, const MediaEntity *sink, unsigned int sinkIdx) const\n> > + * \\sa MediaDevice::link(const MediaPad *source, const MediaPad *sink) const\n> > + *\n> > + * \\return The link that connects the two entities, nullptr otherwise\n> > + */\n> > +MediaLink *MediaDevice::link(const std::string sourceName, unsigned int sourceIdx,\n> > +\t\t\t const std::string sinkName, unsigned int sinkIdx) const\n> > +{\n> > +\tconst MediaEntity *source = getEntityByName(sourceName);\n> > +\tif (!source) {\n> > +\t\tLOG(Error) << \"Failed to find entity with name: \"\n> > +\t\t\t << sourceName << \"\\n\";\n> > +\t\treturn nullptr;\n> > +\t}\n> > +\n> > +\tconst MediaPad *sourcePad = source->getPadByIndex(sourceIdx);\n> > +\tif (!sourcePad) {\n> > +\t\tLOG(Error) << \"Entity \\\"\" << sourceName << \"\\\" \"\n> > +\t\t\t << \"has no pad at index \" << sourceIdx << \"\\n\";\n> > +\t\treturn nullptr;\n> > +\t}\n> > +\n> > +\tconst MediaEntity *sink = getEntityByName(sinkName);\n> > +\tif (!sink) {\n> > +\t\tLOG(Error) << \"Failed to find entity with name: \"\n> > +\t\t\t << sinkName << \"\\n\";\n> > +\t\treturn nullptr;\n> > +\t}\n> > +\n> > +\tconst MediaPad *sinkPad = sink->getPadByIndex(sinkIdx);\n> > +\tif (!sinkPad) {\n> > +\t\tLOG(Error) << \"Entity \\\"\" << sinkName << \"\\\" \"\n> > +\t\t\t << \"has no pad at index \" << sinkIdx << \"\\n\";\n> > +\t\treturn nullptr;\n> > +\t}\n> > +\n> > +\treturn link(sourcePad, sinkPad);\n>\n> How about just resolving the source and sink MediaEntity's here and then\n>\n> return link(source, sourceIdx, sink, sinkIdx);\n>\n> This would reduce the code duplication.\n>\n\nAh, right, no need to search for the source and sink pads here.\nI'll fix.\n\n> > +}\n> > +\n> > +/**\n> > + * \\brief Return the MediaLink that connects two entities\n> > + * \\param source The source entity\n> > + * \\param sourceIdx The index of the source pad\n> > + * \\param sink The sink entity\n> > + * \\param sinkIdx The index of the sink pad\n> > + *\n> > + * Find link that connects the pads at index \\a sourceIdx of the source\n> > + * entity \\a source, to the pad at index \\a sinkIdx of the sink entity \\a\n> > + * sink, if any.\n> > + *\n> > + * \\sa MediaDevice::link(const std::string sourceName, unsigned int sourceIdx, const std::string sinkName, unsigned int sinkIdx) const\n> > + * \\sa MediaDevice::link(const MediaPad *source, const MediaPad *sink) const\n> > + *\n> > + * \\return The link that connects the two entities, nullptr otherwise\n> > + */\n> > +MediaLink *MediaDevice::link(const MediaEntity *source, unsigned int sourceIdx,\n> > +\t\t\t const MediaEntity *sink, unsigned int sinkIdx) const\n> > +{\n> > +\n> > +\tconst MediaPad *sourcePad = source->getPadByIndex(sourceIdx);\n> > +\tif (!sourcePad) {\n> > +\t\tLOG(Error) << \"Entity \\\"\" << source->name() << \"\\\" \"\n> > +\t\t\t << \"has no pad at index \" << sourceIdx << \"\\n\";\n> > +\t\treturn nullptr;\n> > +\t}\n> > +\n> > +\tconst MediaPad *sinkPad = sink->getPadByIndex(sinkIdx);\n> > +\tif (!sinkPad) {\n> > +\t\tLOG(Error) << \"Entity \\\"\" << sink->name() << \"\\\" \"\n> > +\t\t\t << \"has no pad at index \" << sinkIdx << \"\\n\";\n> > +\t\treturn nullptr;\n> > +\t}\n> > +\n> > +\treturn link(sourcePad, sinkPad);\n> > +}\n> > +\n> > +/**\n> > + * \\brief Return the MediaLink that connects two pads\n> > + * \\param source The source pad\n> > + * \\param sink The sink pad\n> > + *\n> > + * \\sa MediaDevice::link(const std::string sourceName, unsigned int sourceIdx, const std::string sinkName, unsigned int sinkIdx) const\n> > + * \\sa MediaDevice::link(const MediaEntity *source, unsigned int sourceIdx, const MediaEntity *sink, unsigned int sinkIdx) const\n> > + *\n> > + * \\return The link that connects the two pads, nullptr otherwise\n> > + */\n> > +MediaLink *MediaDevice::link(const MediaPad *source, const MediaPad *sink) const\n> > +{\n> > +\tif (!source || !sink)\n> > +\t\treturn nullptr;\n> > +\n> > +\t/*\n> > +\t * Now that we have made sure all instances are valid, compare\n> > +\t * their ids to find a matching link.\n> > +\t */\n>\n> I like comments in the code it self. I don't like (but I still write\n> them my self) comments that are written in this tens, how about.\n>\n> Compare pad ids to find the link which connects the source and sink\n> pads.\n>\n\nAck for this and for the comments style in general.\n\nThanks\n j\n\n> > +\tfor (MediaLink *link : source->links()) {\n> > +\t\tif (link->sink()->id() != sink->id())\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tif (link->sink()->entity()->id() != sink->entity()->id())\n> > +\t\t\tcontinue;\n> > +\n> > +\t\treturn link;\n> > +\t}\n> > +\n> > +\treturn nullptr;\n> > +}\n> > +\n> > /**\n> > * \\var MediaDevice::objects_\n> > * \\brief Global map of media objects (entities, pads, links) keyed by their\n> > --\n> > 2.20.1\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\n>\n> --\n> Regards,\n> Niklas Söderlund","headers":{"Return-Path":"","Received":["from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BE22660B0B\n\tfor ;\n\tMon, 7 Jan 2019 09:57:25 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 48670C0007;\n\tMon, 7 Jan 2019 08:57:25 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Mon, 7 Jan 2019 09:57:30 +0100","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190107085730.ykny5lo42gk2iwq7@uno.localdomain>","References":"<20190103173859.22624-1-jacopo@jmondi.org>\n\t<20190103173859.22624-3-jacopo@jmondi.org>\n\t<20190104163257.GM22790@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"x3mqoampvsdova4y\"","Content-Disposition":"inline","In-Reply-To":"<20190104163257.GM22790@bigcity.dyn.berto.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 2/5] libcamera: media_device: Add\n\tfunction to get a MediaLink","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 07 Jan 2019 08:57:25 -0000"}},{"id":236,"web_url":"https://patchwork.libcamera.org/comment/236/","msgid":"<2020023.d3Q2KelfCx@avalon>","date":"2019-01-07T21:39:43","subject":"Re: [libcamera-devel] [PATCH 2/5] libcamera: media_device: Add\n\tfunction to get a MediaLink","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Monday, 7 January 2019 10:57:30 EET Jacopo Mondi wrote:\n> On Fri, Jan 04, 2019 at 05:33:16PM +0100, Niklas Söderlund wrote:\n> > On 2019-01-03 18:38:56 +0100, Jacopo Mondi wrote:\n> > > Add three overloaded functions 'link()' to retrieve a link between two\n> > > pads. Each overloaded implementation exposes a different method to\n> > > identify the source and sink entities.\n> > > \n> > > Signed-off-by: Jacopo Mondi \n> > > ---\n> > > \n> > > src/libcamera/include/media_device.h | 6 ++\n> > > src/libcamera/media_device.cpp | 119 +++++++++++++++++++++++++++\n> > > 2 files changed, 125 insertions(+)\n\n[snip]\n\n> > > diff --git a/src/libcamera/media_device.cpp\n> > > b/src/libcamera/media_device.cpp index 2f9490c..6892300 100644\n> > > --- a/src/libcamera/media_device.cpp\n> > > +++ b/src/libcamera/media_device.cpp\n> > > @@ -306,6 +306,125 @@ MediaEntity *MediaDevice::getEntityByName(const\n> > > std::string &name) const\n> > > \treturn nullptr;\n> > > }\n> > > \n> > > +/**\n> > > + * \\brief Return the MediaLink that connects two entities identified by\n> > > name\n\ns/Return/Retrieve/\n\nThe link doesn't actually connect the two entities directly, it connects two \npads. I'd write this as\n\n\"Retrieve the MediaLink connecting two pads, identified by entity name and pad \nindex.\"\n\n> > > + * \\param sourceName The source entity name\n> > > + * \\param sourceIdx The index of the source pad\n> > > + * \\param sinkName The sink entity name\n> > > + * \\param sinkIdx The index of the sink pad\n> > > + *\n> > > + * Find link that connects the pads at index \\a sourceIdx of the source\n\ns/Find/Find the/\n\n> > > + * entity with name \\a sourceName, to the pad at index \\a sinkIdx of\n> > > the\n> > > + * sink entity with name \\a sinkName, if any.\n> > > + *\n> > > + * \\sa MediaDevice::link(const MediaEntity *source, unsigned int\n> > > sourceIdx, const MediaEntity *sink, unsigned int sinkIdx) const + * \\sa\n> > > MediaDevice::link(const MediaPad *source, const MediaPad *sink) const +\n> > > *\n> > > + * \\return The link that connects the two entities, nullptr otherwise\n\n\", or nullptr if no such link exists\"\n\n> > > + */\n> > > +MediaLink *MediaDevice::link(const std::string sourceName, unsigned int\n> > > sourceIdx,\n> > > +\t\t\t const std::string sinkName, unsigned int sinkIdx)\n> > > const\n\nIsn't it dangerous to create a function that allows modifying a link from a \nconst MediaDevice ? Should you provide both\n\n\tMediaLink *MediaDevice::link(...)\n\tconst MediaLink *MediaDevice::link(...) const\n\n? If you don't want to provide both I think you could go for the former only, \nas I don't think we'll have to deal with const MediaDevice pointers.\n\n> > > +{\n> > > +\tconst MediaEntity *source = getEntityByName(sourceName);\n> > > +\tif (!source) {\n> > > +\t\tLOG(Error) << \"Failed to find entity with name: \"\n> > > +\t\t\t << sourceName << \"\\n\";\n\nI'd go for LOG(Debug) at most. I think it's a valid use case for a user to \ncall this function to get a link without knowing whether it is present in the \ngraph. This can be used to test for the availability of a link in a media \ndevice that supports multiple configurations. Even as a debug message I'm not \nsure how valuable this is. Same for the messages below.\n\nAll these comments apply to the other two functions below.\n\n> > > +\t\treturn nullptr;\n> > > +\t}\n> > > +\n> > > +\tconst MediaPad *sourcePad = source->getPadByIndex(sourceIdx);\n> > > +\tif (!sourcePad) {\n> > > +\t\tLOG(Error) << \"Entity \\\"\" << sourceName << \"\\\" \"\n> > > +\t\t\t << \"has no pad at index \" << sourceIdx << \"\\n\";\n> > > +\t\treturn nullptr;\n> > > +\t}\n> > > +\n> > > +\tconst MediaEntity *sink = getEntityByName(sinkName);\n> > > +\tif (!sink) {\n> > > +\t\tLOG(Error) << \"Failed to find entity with name: \"\n> > > +\t\t\t << sinkName << \"\\n\";\n> > > +\t\treturn nullptr;\n> > > +\t}\n> > > +\n> > > +\tconst MediaPad *sinkPad = sink->getPadByIndex(sinkIdx);\n> > > +\tif (!sinkPad) {\n> > > +\t\tLOG(Error) << \"Entity \\\"\" << sinkName << \"\\\" \"\n> > > +\t\t\t << \"has no pad at index \" << sinkIdx << \"\\n\";\n> > > +\t\treturn nullptr;\n> > > +\t}\n> > > +\n> > > +\treturn link(sourcePad, sinkPad);\n> > \n> > How about just resolving the source and sink MediaEntity's here and then\n> > \n> > return link(source, sourceIdx, sink, sinkIdx);\n> > \n> > This would reduce the code duplication.\n> \n> Ah, right, no need to search for the source and sink pads here.\n> I'll fix.\n> \n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Return the MediaLink that connects two entities\n> > > + * \\param source The source entity\n> > > + * \\param sourceIdx The index of the source pad\n> > > + * \\param sink The sink entity\n> > > + * \\param sinkIdx The index of the sink pad\n> > > + *\n> > > + * Find link that connects the pads at index \\a sourceIdx of the source\n> > > + * entity \\a source, to the pad at index \\a sinkIdx of the sink entity\n> > > \\a\n> > > + * sink, if any.\n> > > + *\n> > > + * \\sa MediaDevice::link(const std::string sourceName, unsigned int\n> > > sourceIdx, const std::string sinkName, unsigned int sinkIdx) const + *\n> > > \\sa MediaDevice::link(const MediaPad *source, const MediaPad *sink)\n> > > const + *\n> > > + * \\return The link that connects the two entities, nullptr otherwise\n> > > + */\n> > > +MediaLink *MediaDevice::link(const MediaEntity *source, unsigned int\n> > > sourceIdx,\n> > > +\t\t\t const MediaEntity *sink, unsigned int sinkIdx)\n> > > const\n> > > +{\n> > > +\n> > > +\tconst MediaPad *sourcePad = source->getPadByIndex(sourceIdx);\n> > > +\tif (!sourcePad) {\n> > > +\t\tLOG(Error) << \"Entity \\\"\" << source->name() << \"\\\" \"\n> > > +\t\t\t << \"has no pad at index \" << sourceIdx << \"\\n\";\n> > > +\t\treturn nullptr;\n> > > +\t}\n> > > +\n> > > +\tconst MediaPad *sinkPad = sink->getPadByIndex(sinkIdx);\n> > > +\tif (!sinkPad) {\n> > > +\t\tLOG(Error) << \"Entity \\\"\" << sink->name() << \"\\\" \"\n> > > +\t\t\t << \"has no pad at index \" << sinkIdx << \"\\n\";\n> > > +\t\treturn nullptr;\n> > > +\t}\n> > > +\n> > > +\treturn link(sourcePad, sinkPad);\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Return the MediaLink that connects two pads\n> > > + * \\param source The source pad\n> > > + * \\param sink The sink pad\n> > > + *\n> > > + * \\sa MediaDevice::link(const std::string sourceName, unsigned int\n> > > sourceIdx, const std::string sinkName, unsigned int sinkIdx) const + *\n> > > \\sa MediaDevice::link(const MediaEntity *source, unsigned int\n> > > sourceIdx, const MediaEntity *sink, unsigned int sinkIdx) const\n> > > + *\n> > > + * \\return The link that connects the two pads, nullptr otherwise\n> > > + */\n> > > +MediaLink *MediaDevice::link(const MediaPad *source, const MediaPad\n> > > *sink) const\n> > > +{\n> > > +\tif (!source || !sink)\n> > > +\t\treturn nullptr;\n\nCan this happen ?\n\n> > > +\t/*\n> > > +\t * Now that we have made sure all instances are valid, compare\n> > > +\t * their ids to find a matching link.\n> > > +\t */\n> > \n> > I like comments in the code it self. I don't like (but I still write\n> > them my self) comments that are written in this tens, how about.\n> > \n> > Compare pad ids to find the link which connects the source and sink\n> > pads.\n> \n> Ack for this and for the comments style in general.\n> \n> > > +\tfor (MediaLink *link : source->links()) {\n> > > +\t\tif (link->sink()->id() != sink->id())\n> > > +\t\t\tcontinue;\n> > > +\n> > > +\t\tif (link->sink()->entity()->id() != sink->entity()->id())\n> > > +\t\t\tcontinue;\n\nDo you need this check given that the id is unique ? It seems to be only \nneeded if you search based on pad index, not when searching on pad id. You \ncould thus write\n\n\t\tif (link->sink()->id() == sink->id())\n\t\t\treturn link;\n\n> > > +\t\treturn link;\n> > > +\t}\n> > > +\n> > > +\treturn nullptr;\n> > > +}\n> > > +\n> > > /**\n> > > * \\var MediaDevice::objects_\n> > > * \\brief Global map of media objects (entities, pads, links) keyed by\n> > > their","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CB58D600CC\n\tfor ;\n\tMon, 7 Jan 2019 22:38:37 +0100 (CET)","from avalon.localnet (dfj612ybrt5fhg77mgycy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:2e86:4862:ef6a:2804])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7715FE22;\n\tMon, 7 Jan 2019 22:38:36 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1546897116;\n\tbh=oqBDUF3QrpODDdwdX27da9ker38Bh0yFWO+fsm47lKg=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=OpVEDPUdic99o2Id3Fuxg1lJAo5TFcbgOqWcRKiBJdSXOMJGsvWNjVvVvqDzPDde6\n\t1QYceBxWP0Pk5mkTaIdXc3zXk5NTQndJcNxWyup+dBypekgaXpxRfdMlZFJCRmyhqk\n\tAUvnybwAkhoXdJxMxU51XhHNHJEnpBGhzpoQ7pfA=","From":"Laurent Pinchart ","To":"libcamera-devel@lists.libcamera.org","Date":"Mon, 07 Jan 2019 23:39:43 +0200","Message-ID":"<2020023.d3Q2KelfCx@avalon>","Organization":"Ideas on Board Oy","In-Reply-To":"<20190107085730.ykny5lo42gk2iwq7@uno.localdomain>","References":"<20190103173859.22624-1-jacopo@jmondi.org>\n\t<20190104163257.GM22790@bigcity.dyn.berto.se>\n\t<20190107085730.ykny5lo42gk2iwq7@uno.localdomain>","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","Content-Type":"text/plain; charset=\"iso-8859-1\"","Subject":"Re: [libcamera-devel] [PATCH 2/5] libcamera: media_device: Add\n\tfunction to get a MediaLink","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 07 Jan 2019 21:38:38 -0000"}},{"id":252,"web_url":"https://patchwork.libcamera.org/comment/252/","msgid":"<20190108134933.fw26rr5u7cvhvhn4@uno.localdomain>","date":"2019-01-08T13:49:33","subject":"Re: [libcamera-devel] [PATCH 2/5] libcamera: media_device: Add\n\tfunction to get a MediaLink","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Mon, Jan 07, 2019 at 11:39:43PM +0200, Laurent Pinchart wrote:\n> Hello,\n>\n[snip]\n\n> > > > + */\n> > > > +MediaLink *MediaDevice::link(const std::string sourceName, unsigned int\n> > > > sourceIdx,\n> > > > +\t\t\t const std::string sinkName, unsigned int sinkIdx)\n> > > > const\n>\n> Isn't it dangerous to create a function that allows modifying a link from a\n> const MediaDevice ? Should you provide both\n>\n> \tMediaLink *MediaDevice::link(...)\n> \tconst MediaLink *MediaDevice::link(...) const\n>\n> ? If you don't want to provide both I think you could go for the former only,\n> as I don't think we'll have to deal with const MediaDevice pointers.\n\nThe function is declared as:\nMediaLink *MediaDevice::link(...) const\nand not\nconst MediaLink *MediaDevice::link(...) const\n\nand indeed the function itself does not change the MediaDevice\ninstance state. Would you drop the const at end of the declaration?\n\nThanks\n j\n\n\n>\n> > > > +{\n> > > > +\tconst MediaEntity *source = getEntityByName(sourceName);\n> > > > +\tif (!source) {\n> > > > +\t\tLOG(Error) << \"Failed to find entity with name: \"\n> > > > +\t\t\t << sourceName << \"\\n\";\n>\n> I'd go for LOG(Debug) at most. I think it's a valid use case for a user to\n> call this function to get a link without knowing whether it is present in the\n> graph. This can be used to test for the availability of a link in a media\n> device that supports multiple configurations. Even as a debug message I'm not\n> sure how valuable this is. Same for the messages below.\n>\n> All these comments apply to the other two functions below.\n>\n> > > > +\t\treturn nullptr;\n> > > > +\t}\n> > > > +\n> > > > +\tconst MediaPad *sourcePad = source->getPadByIndex(sourceIdx);\n> > > > +\tif (!sourcePad) {\n> > > > +\t\tLOG(Error) << \"Entity \\\"\" << sourceName << \"\\\" \"\n> > > > +\t\t\t << \"has no pad at index \" << sourceIdx << \"\\n\";\n> > > > +\t\treturn nullptr;\n> > > > +\t}\n> > > > +\n> > > > +\tconst MediaEntity *sink = getEntityByName(sinkName);\n> > > > +\tif (!sink) {\n> > > > +\t\tLOG(Error) << \"Failed to find entity with name: \"\n> > > > +\t\t\t << sinkName << \"\\n\";\n> > > > +\t\treturn nullptr;\n> > > > +\t}\n> > > > +\n> > > > +\tconst MediaPad *sinkPad = sink->getPadByIndex(sinkIdx);\n> > > > +\tif (!sinkPad) {\n> > > > +\t\tLOG(Error) << \"Entity \\\"\" << sinkName << \"\\\" \"\n> > > > +\t\t\t << \"has no pad at index \" << sinkIdx << \"\\n\";\n> > > > +\t\treturn nullptr;\n> > > > +\t}\n> > > > +\n> > > > +\treturn link(sourcePad, sinkPad);\n> > >\n> > > How about just resolving the source and sink MediaEntity's here and then\n> > >\n> > > return link(source, sourceIdx, sink, sinkIdx);\n> > >\n> > > This would reduce the code duplication.\n> >\n> > Ah, right, no need to search for the source and sink pads here.\n> > I'll fix.\n> >\n> > > > +}\n> > > > +\n> > > > +/**\n> > > > + * \\brief Return the MediaLink that connects two entities\n> > > > + * \\param source The source entity\n> > > > + * \\param sourceIdx The index of the source pad\n> > > > + * \\param sink The sink entity\n> > > > + * \\param sinkIdx The index of the sink pad\n> > > > + *\n> > > > + * Find link that connects the pads at index \\a sourceIdx of the source\n> > > > + * entity \\a source, to the pad at index \\a sinkIdx of the sink entity\n> > > > \\a\n> > > > + * sink, if any.\n> > > > + *\n> > > > + * \\sa MediaDevice::link(const std::string sourceName, unsigned int\n> > > > sourceIdx, const std::string sinkName, unsigned int sinkIdx) const + *\n> > > > \\sa MediaDevice::link(const MediaPad *source, const MediaPad *sink)\n> > > > const + *\n> > > > + * \\return The link that connects the two entities, nullptr otherwise\n> > > > + */\n> > > > +MediaLink *MediaDevice::link(const MediaEntity *source, unsigned int\n> > > > sourceIdx,\n> > > > +\t\t\t const MediaEntity *sink, unsigned int sinkIdx)\n> > > > const\n> > > > +{\n> > > > +\n> > > > +\tconst MediaPad *sourcePad = source->getPadByIndex(sourceIdx);\n> > > > +\tif (!sourcePad) {\n> > > > +\t\tLOG(Error) << \"Entity \\\"\" << source->name() << \"\\\" \"\n> > > > +\t\t\t << \"has no pad at index \" << sourceIdx << \"\\n\";\n> > > > +\t\treturn nullptr;\n> > > > +\t}\n> > > > +\n> > > > +\tconst MediaPad *sinkPad = sink->getPadByIndex(sinkIdx);\n> > > > +\tif (!sinkPad) {\n> > > > +\t\tLOG(Error) << \"Entity \\\"\" << sink->name() << \"\\\" \"\n> > > > +\t\t\t << \"has no pad at index \" << sinkIdx << \"\\n\";\n> > > > +\t\treturn nullptr;\n> > > > +\t}\n> > > > +\n> > > > +\treturn link(sourcePad, sinkPad);\n> > > > +}\n> > > > +\n> > > > +/**\n> > > > + * \\brief Return the MediaLink that connects two pads\n> > > > + * \\param source The source pad\n> > > > + * \\param sink The sink pad\n> > > > + *\n> > > > + * \\sa MediaDevice::link(const std::string sourceName, unsigned int\n> > > > sourceIdx, const std::string sinkName, unsigned int sinkIdx) const + *\n> > > > \\sa MediaDevice::link(const MediaEntity *source, unsigned int\n> > > > sourceIdx, const MediaEntity *sink, unsigned int sinkIdx) const\n> > > > + *\n> > > > + * \\return The link that connects the two pads, nullptr otherwise\n> > > > + */\n> > > > +MediaLink *MediaDevice::link(const MediaPad *source, const MediaPad\n> > > > *sink) const\n> > > > +{\n> > > > +\tif (!source || !sink)\n> > > > +\t\treturn nullptr;\n>\n> Can this happen ?\n>\n> > > > +\t/*\n> > > > +\t * Now that we have made sure all instances are valid, compare\n> > > > +\t * their ids to find a matching link.\n> > > > +\t */\n> > >\n> > > I like comments in the code it self. I don't like (but I still write\n> > > them my self) comments that are written in this tens, how about.\n> > >\n> > > Compare pad ids to find the link which connects the source and sink\n> > > pads.\n> >\n> > Ack for this and for the comments style in general.\n> >\n> > > > +\tfor (MediaLink *link : source->links()) {\n> > > > +\t\tif (link->sink()->id() != sink->id())\n> > > > +\t\t\tcontinue;\n> > > > +\n> > > > +\t\tif (link->sink()->entity()->id() != sink->entity()->id())\n> > > > +\t\t\tcontinue;\n>\n> Do you need this check given that the id is unique ? It seems to be only\n> needed if you search based on pad index, not when searching on pad id. You\n> could thus write\n>\n> \t\tif (link->sink()->id() == sink->id())\n> \t\t\treturn link;\n>\n> > > > +\t\treturn link;\n> > > > +\t}\n> > > > +\n> > > > +\treturn nullptr;\n> > > > +}\n> > > > +\n> > > > /**\n> > > > * \\var MediaDevice::objects_\n> > > > * \\brief Global map of media objects (entities, pads, links) keyed by\n> > > > their\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n>\n>","headers":{"Return-Path":"","Received":["from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D3DF060B2E\n\tfor ;\n\tTue, 8 Jan 2019 14:49:28 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 2095AFF806;\n\tTue, 8 Jan 2019 13:49:27 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Tue, 8 Jan 2019 14:49:33 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org, Niklas =?utf-8?q?S=C3=B6derlund?=\n\t","Message-ID":"<20190108134933.fw26rr5u7cvhvhn4@uno.localdomain>","References":"<20190103173859.22624-1-jacopo@jmondi.org>\n\t<20190104163257.GM22790@bigcity.dyn.berto.se>\n\t<20190107085730.ykny5lo42gk2iwq7@uno.localdomain>\n\t<2020023.d3Q2KelfCx@avalon>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"ojd2hnhvxrd4t63y\"","Content-Disposition":"inline","In-Reply-To":"<2020023.d3Q2KelfCx@avalon>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 2/5] libcamera: media_device: Add\n\tfunction to get a MediaLink","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 08 Jan 2019 13:49:29 -0000"}},{"id":255,"web_url":"https://patchwork.libcamera.org/comment/255/","msgid":"<1594782.p22lul1AVk@avalon>","date":"2019-01-08T14:33:01","subject":"Re: [libcamera-devel] [PATCH 2/5] libcamera: media_device: Add\n\tfunction to get a MediaLink","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Tuesday, 8 January 2019 15:49:33 EET Jacopo Mondi wrote:\n> On Mon, Jan 07, 2019 at 11:39:43PM +0200, Laurent Pinchart wrote:\n> > Hello,\n> \n> [snip]\n> \n> >>>> + */\n> >>>> +MediaLink *MediaDevice::link(const std::string sourceName, unsigned\n> >>>> int sourceIdx,\n> >>>> +\t\t\t const std::string sinkName, unsigned int sinkIdx)\n> >>>> const\n> > \n> > Isn't it dangerous to create a function that allows modifying a link from\n> > a const MediaDevice ? Should you provide both\n> > \n> > \tMediaLink *MediaDevice::link(...)\n> > \tconst MediaLink *MediaDevice::link(...) const\n> > \n> > ? If you don't want to provide both I think you could go for the former\n> > only, as I don't think we'll have to deal with const MediaDevice\n> > pointers.\n> \n> The function is declared as:\n> MediaLink *MediaDevice::link(...) const\n> and not\n> const MediaLink *MediaDevice::link(...) const\n> \n> and indeed the function itself does not change the MediaDevice\n> instance state. Would you drop the const at end of the declaration?\n\nI think I would do so, as it's a bit misleading otherwise. True, the function \ndoesn't change the MediaDevice instance itself, but it allows non-const access \nto a link, which is part of the graph. If we give a const MediaDevice * \npointer to someone, I wouldn't expect the API to allow changing links.\n\n[snip]","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1E06360B2E\n\tfor ;\n\tTue, 8 Jan 2019 15:31:53 +0100 (CET)","from avalon.localnet (dfj612ybrt5fhg77mgycy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:2e86:4862:ef6a:2804])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8064D586;\n\tTue, 8 Jan 2019 15:31:52 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1546957912;\n\tbh=9cczT2eQbmH607HC3/VSgypJpPxl6zIe5+Vb0T7ohHE=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=HcRR2+v0bMNDaAlgFlvDHNfP/i8LpuVXLCQBPdP8YC37Ta7tUEnrltJUAiQitgfGo\n\tXEVIYFwchw3CH/WBh3zsEbfahINDeH6lOw36GzAouM13VRNCTps3tol4QiXPkVQR4u\n\tk02o5unDuPNvJUG8SIXrw2CzdIcgqX8NIgUzQ7Zg=","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org, Niklas =?iso-8859-1?q?S=F6derlund?=\n\t","Date":"Tue, 08 Jan 2019 16:33:01 +0200","Message-ID":"<1594782.p22lul1AVk@avalon>","Organization":"Ideas on Board Oy","In-Reply-To":"<20190108134933.fw26rr5u7cvhvhn4@uno.localdomain>","References":"<20190103173859.22624-1-jacopo@jmondi.org>\n\t<2020023.d3Q2KelfCx@avalon>\n\t<20190108134933.fw26rr5u7cvhvhn4@uno.localdomain>","MIME-Version":"1.0","Content-Transfer-Encoding":"7Bit","Content-Type":"text/plain; charset=\"us-ascii\"","Subject":"Re: [libcamera-devel] [PATCH 2/5] libcamera: media_device: Add\n\tfunction to get a MediaLink","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 08 Jan 2019 14:31:53 -0000"}}]