[{"id":1163,"web_url":"https://patchwork.libcamera.org/comment/1163/","msgid":"<20190401220137.GM4787@pendragon.ideasonboard.com>","date":"2019-04-01T22:01:37","subject":"Re: [libcamera-devel] [PATCH v5 06/19] libcamera: v4l2_device:\n\tCreate device from entity name","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Tue, Mar 26, 2019 at 09:38:49AM +0100, Jacopo Mondi wrote:\n> Add a static method to V4L2Device class to create a V4L2Device instance\n> from a media entity name.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  src/libcamera/include/v4l2_device.h |  4 ++++\n>  src/libcamera/v4l2_device.cpp       | 22 ++++++++++++++++++++++\n>  2 files changed, 26 insertions(+)\n> \n> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> index 258deee8d461..482d7891dd49 100644\n> --- a/src/libcamera/include/v4l2_device.h\n> +++ b/src/libcamera/include/v4l2_device.h\n> @@ -22,6 +22,7 @@ namespace libcamera {\n>  class Buffer;\n>  class BufferPool;\n>  class EventNotifier;\n> +class MediaDevice;\n>  class MediaEntity;\n>  \n>  struct V4L2Capability final : v4l2_capability {\n> @@ -107,6 +108,9 @@ public:\n>  class V4L2Device : protected Loggable\n>  {\n>  public:\n> +\tstatic V4L2Device *fromEntityName(const MediaDevice *media,\n> +\t\t\t\t\t  const std::string &name);\n> +\n\nI would move this down, as static methods are usually put after the\nnon-static ones.\n\nI wonder if there would be an advantage in turning this into a\nconstructor for the V4L2Device class, but in any case that could be done\nlater, so, with the above fixed,\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n>  \texplicit V4L2Device(const std::string &deviceNode);\n>  \texplicit V4L2Device(const MediaEntity *entity);\n>  \tV4L2Device(const V4L2Device &) = delete;\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 93d81517fedb..68be5a064258 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -19,6 +19,7 @@\n>  #include <libcamera/event_notifier.h>\n>  \n>  #include \"log.h\"\n> +#include \"media_device.h\"\n>  #include \"media_object.h\"\n>  #include \"v4l2_device.h\"\n>  \n> @@ -264,6 +265,27 @@ const std::string V4L2DeviceFormat::toString() const\n>   * released.\n>   */\n>  \n> +/**\n> + * \\brief Create a new video device instance from entity with \\a name in\n> + * media device \\a media\n> + * \\param[in] media The media device where the entity is registered\n> + * \\param[in] name The media entity name\n> + *\n> + * Releasing memory of the newly created instance is responsibility of the\n> + * caller of this function.\n> + *\n> + * \\return A newly created V4L2Device on success, nullptr otherwise\n> + */\n> +V4L2Device *V4L2Device::fromEntityName(const MediaDevice *media,\n> +\t\t\t\t       const std::string &name)\n> +{\n> +\tMediaEntity *entity = media->getEntityByName(name);\n> +\tif (!entity)\n> +\t\treturn nullptr;\n> +\n> +\treturn new V4L2Device(entity);\n> +}\n> +\n>  /**\n>   * \\brief Construct a V4L2Device\n>   * \\param deviceNode The file-system path to the video device node","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["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 24506610B3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  2 Apr 2019 00:01:49 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4E837542;\n\tTue,  2 Apr 2019 00:01:48 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1554156108;\n\tbh=TOaPXid4L1JHR7FptJfzGb2iNlVb5nDKPWoeD+Qcokw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WxWuaYnn1fJ8RaD3bAgS5tPhYr7K/cZyU7kgrRp6bgZx+6pGfML2SkWwk0qoTbh/3\n\tlay9oyfMbVQR30v5A8gbfF1o1qTX1By58B+IDaTjLKt75iqAET7a9cMJJqdJOEo+d5\n\t626BuTr3hYfKuBq53SAOfw1pRndrnDqDySTfxmL8=","Date":"Tue, 2 Apr 2019 01:01:37 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190401220137.GM4787@pendragon.ideasonboard.com>","References":"<20190326083902.26121-1-jacopo@jmondi.org>\n\t<20190326083902.26121-7-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190326083902.26121-7-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v5 06/19] libcamera: v4l2_device:\n\tCreate device from entity name","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 01 Apr 2019 22:01:49 -0000"}},{"id":1171,"web_url":"https://patchwork.libcamera.org/comment/1171/","msgid":"<20190402082332.r5ucfejsffonygbi@uno.localdomain>","date":"2019-04-02T08:23:32","subject":"Re: [libcamera-devel] [PATCH v5 06/19] libcamera: v4l2_device:\n\tCreate device from entity name","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Tue, Apr 02, 2019 at 01:01:37AM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Tue, Mar 26, 2019 at 09:38:49AM +0100, Jacopo Mondi wrote:\n> > Add a static method to V4L2Device class to create a V4L2Device instance\n> > from a media entity name.\n> >\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > ---\n> >  src/libcamera/include/v4l2_device.h |  4 ++++\n> >  src/libcamera/v4l2_device.cpp       | 22 ++++++++++++++++++++++\n> >  2 files changed, 26 insertions(+)\n> >\n> > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> > index 258deee8d461..482d7891dd49 100644\n> > --- a/src/libcamera/include/v4l2_device.h\n> > +++ b/src/libcamera/include/v4l2_device.h\n> > @@ -22,6 +22,7 @@ namespace libcamera {\n> >  class Buffer;\n> >  class BufferPool;\n> >  class EventNotifier;\n> > +class MediaDevice;\n> >  class MediaEntity;\n> >\n> >  struct V4L2Capability final : v4l2_capability {\n> > @@ -107,6 +108,9 @@ public:\n> >  class V4L2Device : protected Loggable\n> >  {\n> >  public:\n> > +\tstatic V4L2Device *fromEntityName(const MediaDevice *media,\n> > +\t\t\t\t\t  const std::string &name);\n> > +\n>\n> I would move this down, as static methods are usually put after the\n> non-static ones.\n>\n\nI haven't found this rule anywhere in the code style guides we refer\nto. I can move them down if that's how we generally do things.\n\n> I wonder if there would be an advantage in turning this into a\n> constructor for the V4L2Device class, but in any case that could be done\n> later, so, with the above fixed,\n>\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nThanks\n  j\n\n>\n> >  \texplicit V4L2Device(const std::string &deviceNode);\n> >  \texplicit V4L2Device(const MediaEntity *entity);\n> >  \tV4L2Device(const V4L2Device &) = delete;\n> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > index 93d81517fedb..68be5a064258 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -19,6 +19,7 @@\n> >  #include <libcamera/event_notifier.h>\n> >\n> >  #include \"log.h\"\n> > +#include \"media_device.h\"\n> >  #include \"media_object.h\"\n> >  #include \"v4l2_device.h\"\n> >\n> > @@ -264,6 +265,27 @@ const std::string V4L2DeviceFormat::toString() const\n> >   * released.\n> >   */\n> >\n> > +/**\n> > + * \\brief Create a new video device instance from entity with \\a name in\n> > + * media device \\a media\n> > + * \\param[in] media The media device where the entity is registered\n> > + * \\param[in] name The media entity name\n> > + *\n> > + * Releasing memory of the newly created instance is responsibility of the\n> > + * caller of this function.\n> > + *\n> > + * \\return A newly created V4L2Device on success, nullptr otherwise\n> > + */\n> > +V4L2Device *V4L2Device::fromEntityName(const MediaDevice *media,\n> > +\t\t\t\t       const std::string &name)\n> > +{\n> > +\tMediaEntity *entity = media->getEntityByName(name);\n> > +\tif (!entity)\n> > +\t\treturn nullptr;\n> > +\n> > +\treturn new V4L2Device(entity);\n> > +}\n> > +\n> >  /**\n> >   * \\brief Construct a V4L2Device\n> >   * \\param deviceNode The file-system path to the video device node\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7D48160DB3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  2 Apr 2019 10:22:47 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 11F1D24000F;\n\tTue,  2 Apr 2019 08:22:46 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Tue, 2 Apr 2019 10:23:32 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190402082332.r5ucfejsffonygbi@uno.localdomain>","References":"<20190326083902.26121-1-jacopo@jmondi.org>\n\t<20190326083902.26121-7-jacopo@jmondi.org>\n\t<20190401220137.GM4787@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"uklmh7xzvm7num3a\"","Content-Disposition":"inline","In-Reply-To":"<20190401220137.GM4787@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v5 06/19] libcamera: v4l2_device:\n\tCreate device from entity name","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 02 Apr 2019 08:22:47 -0000"}}]