[{"id":763,"web_url":"https://patchwork.libcamera.org/comment/763/","msgid":"<20190207220620.livvzjoe5cacjt4w@uno.localdomain>","date":"2019-02-07T22:06:20","subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: media_device: Provide\n\tthe default entity","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran,\n ignorant question, is FL_DEFAULT an UVC specific thing?\n\nA quick grep on kernel sources returns:\n\n$ git grep FL_DEFAULT drivers/media/\ndrivers/media/platform/omap3isp/ispresizer.c: res->video_out.video.entity.flags |= MEDIA_ENT_FL_DEFAULT;\ndrivers/media/usb/uvc/uvc_entity.c: entity->vdev->entity.flags |= MEDIA_ENT_FL_DEFAULT;\n\nSo I guess not, as omap3 seems also omap3 uses that as well.\n\nIf that's not a UVC specificity, I welcome this change.\n\nOn Thu, Feb 07, 2019 at 09:21:15PM +0000, Kieran Bingham wrote:\n> Add a helper to identify the default entity from a media device.\n>\n> Utilise it in the two places which iterate the entities manually, to allocated\n> a V4L2Device from the default entity.\n>\n> Signed-off-by: Kieran Bingham \n> ---\n> src/libcamera/include/media_device.h | 1 +\n> src/libcamera/media_device.cpp | 15 +++++++++++++++\n> src/libcamera/pipeline/uvcvideo.cpp | 10 ++++------\n> test/v4l2_device/v4l2_device_test.cpp | 12 +++++-------\n> 4 files changed, 25 insertions(+), 13 deletions(-)\n>\n> diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h\n> index 9f038093b2b2..361e8f4a4b86 100644\n> --- a/src/libcamera/include/media_device.h\n> +++ b/src/libcamera/include/media_device.h\n> @@ -42,6 +42,7 @@ public:\n>\n> \tconst std::vector &entities() const { return entities_; }\n> \tMediaEntity *getEntityByName(const std::string &name) const;\n> +\tMediaEntity *defaultEntity() const;\n>\n> \tMediaLink *link(const std::string &sourceName, unsigned int sourceIdx,\n> \t\t\tconst std::string &sinkName, unsigned int sinkIdx);\n> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp\n> index 800ed330fe6d..4af90e1590a1 100644\n> --- a/src/libcamera/media_device.cpp\n> +++ b/src/libcamera/media_device.cpp\n> @@ -319,6 +319,21 @@ MediaEntity *MediaDevice::getEntityByName(const std::string &name) const\n> \treturn nullptr;\n> }\n>\n> +/**\n> + * \\brief Return the default MediaEntity within a MediaDevice\n> + * \\return The default entity if specified, or a nullptr otherwise\n> + */\n> +MediaEntity *MediaDevice::defaultEntity() const\n> +{\n> +\tfor (MediaEntity *entity : entities_) {\n> +\t\tif (entity->flags() & MEDIA_ENT_FL_DEFAULT) {\n> +\t\t\treturn entity;\n> +\t\t}\n\nYou could drop the inner braces { } here\n> +\t}\n> +\n> +\treturn nullptr;\n> +}\n> +\n> /**\n> * \\brief Retrieve the MediaLink connecting two pads, identified by entity\n> * names and pad indexes\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index fc31c52c0ecd..ed8228bb2fc6 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -146,13 +146,11 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n>\n> \tmedia_->acquire();\n>\n> -\tfor (MediaEntity *entity : media_->entities()) {\n> -\t\tif (entity->flags() & MEDIA_ENT_FL_DEFAULT) {\n> -\t\t\tvideo_ = new V4L2Device(entity);\n> -\t\t\tbreak;\n> -\t\t}\n> -\t}\n> +\tMediaEntity *entity = media_->defaultEntity();\n> +\tif (!entity)\n\nLooking at existing code if you don't find any DEFAULT entity, you\nreturn false as you do here, but also release the media device.\n\nShould the same be done here?\n\n> +\t\treturn false;\n>\n> +\tvideo_ = new V4L2Device(entity);\n> \tif (!video_ || video_->open()) {\n> \t\tif (!video_)\n> \t\t\tLOG(UVC, Error) << \"Could not find a default video device\";\n> diff --git a/test/v4l2_device/v4l2_device_test.cpp b/test/v4l2_device/v4l2_device_test.cpp\n> index 18d014caf4c8..dd28cccada6b 100644\n> --- a/test/v4l2_device/v4l2_device_test.cpp\n> +++ b/test/v4l2_device/v4l2_device_test.cpp\n> @@ -46,15 +46,13 @@ int V4L2DeviceTest::init()\n>\n> \tmedia_->acquire();\n>\n> -\tfor (MediaEntity *entity : media_->entities()) {\n> -\t\tif (entity->flags() & MEDIA_ENT_FL_DEFAULT) {\n> -\t\t\tdev_ = new V4L2Device(entity);\n> -\t\t\tbreak;\n> -\t\t}\n> -\t}\n> +\tMediaEntity *entity = media_->defaultEntity();\n> +\tif (!entity)\n> +\t\treturn TestFail;\n>\n> +\tdev_ = new V4L2Device(entity);\n> \tif (!dev_)\n> -\t\treturn TestSkip;\n> +\t\treturn TestFail;\n>\n> \treturn dev_->open();\n> }\n> --\n> 2.19.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 relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A6304610B2\n\tfor ;\n\tThu, 7 Feb 2019 23:06:00 +0100 (CET)","from uno.localdomain (d51A4137F.access.telenet.be [81.164.19.127])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id 1D9D5100004;\n\tThu, 7 Feb 2019 22:05:59 +0000 (UTC)"],"Date":"Thu, 7 Feb 2019 23:06:20 +0100","From":"Jacopo Mondi ","To":"Kieran Bingham ","Cc":"LibCamera Devel ","Message-ID":"<20190207220620.livvzjoe5cacjt4w@uno.localdomain>","References":"<20190207212119.30299-1-kieran.bingham@ideasonboard.com>\n\t<20190207212119.30299-2-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"nn6iy2atprnsifpb\"","Content-Disposition":"inline","In-Reply-To":"<20190207212119.30299-2-kieran.bingham@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: media_device: Provide\n\tthe default entity","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":"Thu, 07 Feb 2019 22:06:00 -0000"}},{"id":769,"web_url":"https://patchwork.libcamera.org/comment/769/","msgid":"<20190208154417.GF4562@pendragon.ideasonboard.com>","date":"2019-02-08T15:44:17","subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: media_device: Provide\n\tthe default entity","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Thu, Feb 07, 2019 at 11:06:20PM +0100, Jacopo Mondi wrote:\n> Hi Kieran,\n> ignorant question, is FL_DEFAULT an UVC specific thing?\n\nIt isn't, but it isn't widely used either. It would thus be fine using\nthe default entity in UVC-specific tests or in the UVC pipeline handler,\nbut not in the rest of the code, at least not without a fallback.\n\n> A quick grep on kernel sources returns:\n> \n> $ git grep FL_DEFAULT drivers/media/\n> drivers/media/platform/omap3isp/ispresizer.c: res->video_out.video.entity.flags |= MEDIA_ENT_FL_DEFAULT;\n> drivers/media/usb/uvc/uvc_entity.c: entity->vdev->entity.flags |= MEDIA_ENT_FL_DEFAULT;\n> \n> So I guess not, as omap3 seems also omap3 uses that as well.\n> \n> If that's not a UVC specificity, I welcome this change.\n> \n> On Thu, Feb 07, 2019 at 09:21:15PM +0000, Kieran Bingham wrote:\n> > Add a helper to identify the default entity from a media device.\n> >\n> > Utilise it in the two places which iterate the entities manually, to allocated\n> > a V4L2Device from the default entity.\n> >\n> > Signed-off-by: Kieran Bingham \n> > ---\n> > src/libcamera/include/media_device.h | 1 +\n> > src/libcamera/media_device.cpp | 15 +++++++++++++++\n> > src/libcamera/pipeline/uvcvideo.cpp | 10 ++++------\n> > test/v4l2_device/v4l2_device_test.cpp | 12 +++++-------\n> > 4 files changed, 25 insertions(+), 13 deletions(-)\n> >\n> > diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h\n> > index 9f038093b2b2..361e8f4a4b86 100644\n> > --- a/src/libcamera/include/media_device.h\n> > +++ b/src/libcamera/include/media_device.h\n> > @@ -42,6 +42,7 @@ public:\n> >\n> > \tconst std::vector &entities() const { return entities_; }\n> > \tMediaEntity *getEntityByName(const std::string &name) const;\n> > +\tMediaEntity *defaultEntity() const;\n\nThis should take an entity type as a parameter as the default flag is\ndefined as \"Default entity for its type\". It could also be used, for\ninstance, to discover the default camera sensor.\n\n> >\n> > \tMediaLink *link(const std::string &sourceName, unsigned int sourceIdx,\n> > \t\t\tconst std::string &sinkName, unsigned int sinkIdx);\n> > diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp\n> > index 800ed330fe6d..4af90e1590a1 100644\n> > --- a/src/libcamera/media_device.cpp\n> > +++ b/src/libcamera/media_device.cpp\n> > @@ -319,6 +319,21 @@ MediaEntity *MediaDevice::getEntityByName(const std::string &name) const\n> > \treturn nullptr;\n> > }\n> >\n> > +/**\n> > + * \\brief Return the default MediaEntity within a MediaDevice\n> > + * \\return The default entity if specified, or a nullptr otherwise\n> > + */\n> > +MediaEntity *MediaDevice::defaultEntity() const\n> > +{\n> > +\tfor (MediaEntity *entity : entities_) {\n> > +\t\tif (entity->flags() & MEDIA_ENT_FL_DEFAULT) {\n> > +\t\t\treturn entity;\n> > +\t\t}\n> \n> You could drop the inner braces { } here\n> > +\t}\n> > +\n> > +\treturn nullptr;\n> > +}\n> > +\n> > /**\n> > * \\brief Retrieve the MediaLink connecting two pads, identified by entity\n> > * names and pad indexes\n> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> > index fc31c52c0ecd..ed8228bb2fc6 100644\n> > --- a/src/libcamera/pipeline/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> > @@ -146,13 +146,11 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> >\n> > \tmedia_->acquire();\n> >\n> > -\tfor (MediaEntity *entity : media_->entities()) {\n> > -\t\tif (entity->flags() & MEDIA_ENT_FL_DEFAULT) {\n> > -\t\t\tvideo_ = new V4L2Device(entity);\n> > -\t\t\tbreak;\n> > -\t\t}\n> > -\t}\n> > +\tMediaEntity *entity = media_->defaultEntity();\n> > +\tif (!entity)\n> \n> Looking at existing code if you don't find any DEFAULT entity, you\n> return false as you do here, but also release the media device.\n> \n> Should the same be done here?\n\nI think it should, and a message should likely be logged too.\n\n> > +\t\treturn false;\n> >\n> > +\tvideo_ = new V4L2Device(entity);\n> > \tif (!video_ || video_->open()) {\n> > \t\tif (!video_)\n> > \t\t\tLOG(UVC, Error) << \"Could not find a default video device\";\n> > diff --git a/test/v4l2_device/v4l2_device_test.cpp b/test/v4l2_device/v4l2_device_test.cpp\n> > index 18d014caf4c8..dd28cccada6b 100644\n> > --- a/test/v4l2_device/v4l2_device_test.cpp\n> > +++ b/test/v4l2_device/v4l2_device_test.cpp\n> > @@ -46,15 +46,13 @@ int V4L2DeviceTest::init()\n> >\n> > \tmedia_->acquire();\n> >\n> > -\tfor (MediaEntity *entity : media_->entities()) {\n> > -\t\tif (entity->flags() & MEDIA_ENT_FL_DEFAULT) {\n> > -\t\t\tdev_ = new V4L2Device(entity);\n> > -\t\t\tbreak;\n> > -\t\t}\n> > -\t}\n> > +\tMediaEntity *entity = media_->defaultEntity();\n> > +\tif (!entity)\n> > +\t\treturn TestFail;\n\nNote that we're planning to use vivid instead of uvcvideo for V4L2Device\ntests, so this may not be a good idea.\n\n> > +\tdev_ = new V4L2Device(entity);\n> > \tif (!dev_)\n> > -\t\treturn TestSkip;\n> > +\t\treturn TestFail;\n> >\n> > \treturn dev_->open();\n> > }","headers":{"Return-Path":"","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 72E2D6101F\n\tfor ;\n\tFri, 8 Feb 2019 16:44:19 +0100 (CET)","from pendragon.ideasonboard.com (d51A4137F.access.telenet.be\n\t[81.164.19.127])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CFA79F9;\n\tFri, 8 Feb 2019 16:44:18 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1549640659;\n\tbh=2mP4wCzocoItuK995UZC0KHGy00DLtou0sPT4N5jdSI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LDt4tBB2q70dATRXYGbvDSjYHVpLqACh/t3fISstrjzgFrd5de4y2sBMa6ToHVRa1\n\tDFVJRw07tY1v7K23xltibkb9GqLJ4prYkremoiE4hN9OQLjny27D6QqBBLtY8m3L6n\n\tc2I72qK4Zi90Gj7vt/Dqo6a72Ki89tdZSG3ymukY=","Date":"Fri, 8 Feb 2019 17:44:17 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"Kieran Bingham ,\n\tLibCamera Devel ","Message-ID":"<20190208154417.GF4562@pendragon.ideasonboard.com>","References":"<20190207212119.30299-1-kieran.bingham@ideasonboard.com>\n\t<20190207212119.30299-2-kieran.bingham@ideasonboard.com>\n\t<20190207220620.livvzjoe5cacjt4w@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190207220620.livvzjoe5cacjt4w@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: media_device: Provide\n\tthe default entity","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, 08 Feb 2019 15:44:19 -0000"}},{"id":777,"web_url":"https://patchwork.libcamera.org/comment/777/","msgid":"","date":"2019-02-11T11:43:08","subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: media_device: Provide\n\tthe default entity","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 07/02/2019 22:06, Jacopo Mondi wrote:\n> Hi Kieran,\n> ignorant question, is FL_DEFAULT an UVC specific thing?\n\nI think it's a media controller thing. I'm equally ignorant.\nThis code caused my pain of not being able to import the buffers when I\nwas working on it in the morning, and it made sense to refactor it out\nso that it did not hide my issue, as 'getting the default entity' for\nwhatever purpose seems like a common thing to do at the MediaDevice level.\n\n\n\n> \n> A quick grep on kernel sources returns:\n> \n> $ git grep FL_DEFAULT drivers/media/\n> drivers/media/platform/omap3isp/ispresizer.c: res->video_out.video.entity.flags |= MEDIA_ENT_FL_DEFAULT;\n> drivers/media/usb/uvc/uvc_entity.c: entity->vdev->entity.flags |= MEDIA_ENT_FL_DEFAULT;\n> \n\nYes, I saw a similar (I guess exactingly so) grep when I did this :)\n\n\n> So I guess not, as omap3 seems also omap3 uses that as well.\n> \n> If that's not a UVC specificity, I welcome this change.\n> \n> On Thu, Feb 07, 2019 at 09:21:15PM +0000, Kieran Bingham wrote:\n>> Add a helper to identify the default entity from a media device.\n>>\n>> Utilise it in the two places which iterate the entities manually, to allocated\n>> a V4L2Device from the default entity.\n>>\n>> Signed-off-by: Kieran Bingham \n>> ---\n>> src/libcamera/include/media_device.h | 1 +\n>> src/libcamera/media_device.cpp | 15 +++++++++++++++\n>> src/libcamera/pipeline/uvcvideo.cpp | 10 ++++------\n>> test/v4l2_device/v4l2_device_test.cpp | 12 +++++-------\n>> 4 files changed, 25 insertions(+), 13 deletions(-)\n>>\n>> diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h\n>> index 9f038093b2b2..361e8f4a4b86 100644\n>> --- a/src/libcamera/include/media_device.h\n>> +++ b/src/libcamera/include/media_device.h\n>> @@ -42,6 +42,7 @@ public:\n>>\n>> \tconst std::vector &entities() const { return entities_; }\n>> \tMediaEntity *getEntityByName(const std::string &name) const;\n>> +\tMediaEntity *defaultEntity() const;\n>>\n>> \tMediaLink *link(const std::string &sourceName, unsigned int sourceIdx,\n>> \t\t\tconst std::string &sinkName, unsigned int sinkIdx);\n>> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp\n>> index 800ed330fe6d..4af90e1590a1 100644\n>> --- a/src/libcamera/media_device.cpp\n>> +++ b/src/libcamera/media_device.cpp\n>> @@ -319,6 +319,21 @@ MediaEntity *MediaDevice::getEntityByName(const std::string &name) const\n>> \treturn nullptr;\n>> }\n>>\n>> +/**\n>> + * \\brief Return the default MediaEntity within a MediaDevice\n>> + * \\return The default entity if specified, or a nullptr otherwise\n>> + */\n>> +MediaEntity *MediaDevice::defaultEntity() const\n>> +{\n>> +\tfor (MediaEntity *entity : entities_) {\n>> +\t\tif (entity->flags() & MEDIA_ENT_FL_DEFAULT) {\n>> +\t\t\treturn entity;\n>> +\t\t}\n> \n> You could drop the inner braces { } here\n\nAck.\n\n\n>> +\t}\n>> +\n>> +\treturn nullptr;\n>> +}\n>> +\n>> /**\n>> * \\brief Retrieve the MediaLink connecting two pads, identified by entity\n>> * names and pad indexes\n>> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n>> index fc31c52c0ecd..ed8228bb2fc6 100644\n>> --- a/src/libcamera/pipeline/uvcvideo.cpp\n>> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n>> @@ -146,13 +146,11 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n>>\n>> \tmedia_->acquire();\n>>\n>> -\tfor (MediaEntity *entity : media_->entities()) {\n>> -\t\tif (entity->flags() & MEDIA_ENT_FL_DEFAULT) {\n>> -\t\t\tvideo_ = new V4L2Device(entity);\n>> -\t\t\tbreak;\n>> -\t\t}\n>> -\t}\n>> +\tMediaEntity *entity = media_->defaultEntity();\n>> +\tif (!entity)\n> \n> Looking at existing code if you don't find any DEFAULT entity, you\n> return false as you do here, but also release the media device.\n> \n> Should the same be done here?\n> \n\nAh yes, quite likely.\n\n\n>> +\t\treturn false;\n>>\n>> +\tvideo_ = new V4L2Device(entity);\n>> \tif (!video_ || video_->open()) {\n>> \t\tif (!video_)\n>> \t\t\tLOG(UVC, Error) << \"Could not find a default video device\";\n>> diff --git a/test/v4l2_device/v4l2_device_test.cpp b/test/v4l2_device/v4l2_device_test.cpp\n>> index 18d014caf4c8..dd28cccada6b 100644\n>> --- a/test/v4l2_device/v4l2_device_test.cpp\n>> +++ b/test/v4l2_device/v4l2_device_test.cpp\n>> @@ -46,15 +46,13 @@ int V4L2DeviceTest::init()\n>>\n>> \tmedia_->acquire();\n>>\n>> -\tfor (MediaEntity *entity : media_->entities()) {\n>> -\t\tif (entity->flags() & MEDIA_ENT_FL_DEFAULT) {\n>> -\t\t\tdev_ = new V4L2Device(entity);\n>> -\t\t\tbreak;\n>> -\t\t}\n>> -\t}\n>> +\tMediaEntity *entity = media_->defaultEntity();\n>> +\tif (!entity)\n>> +\t\treturn TestFail;\n>>\n>> +\tdev_ = new V4L2Device(entity);\n>> \tif (!dev_)\n>> -\t\treturn TestSkip;\n>> +\t\treturn TestFail;\n>>\n>> \treturn dev_->open();\n>> }\n>> --\n>> 2.19.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 perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5F81D610B2\n\tfor ;\n\tMon, 11 Feb 2019 12:43:12 +0100 (CET)","from [192.168.0.21]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A890D2E4;\n\tMon, 11 Feb 2019 12:43:11 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1549885391;\n\tbh=81ndNJH09gtTl1d4PcH9XABws5ENYTjqOpFqpfuUgiw=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=kGmaWiFs90n5kR9y6Rjdo4M3g9AeK5JR9PgN1uXukAv/AdnRkJvDeT9dEvziYlxBG\n\tO2ocWpmWyo1H/Wh6pSOlspYayGoYVJEToADb0us2RJ7Dx9AE5LXOS5IU1slGonYPxF\n\tDdjtz3fAIa0xVfP78xEo8m2/zpA5RMFuoZUY8YJU=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Jacopo Mondi ","Cc":"LibCamera Devel ","References":"<20190207212119.30299-1-kieran.bingham@ideasonboard.com>\n\t<20190207212119.30299-2-kieran.bingham@ideasonboard.com>\n\t<20190207220620.livvzjoe5cacjt4w@uno.localdomain>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"","Date":"Mon, 11 Feb 2019 11:43:08 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.4.0","MIME-Version":"1.0","In-Reply-To":"<20190207220620.livvzjoe5cacjt4w@uno.localdomain>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: media_device: Provide\n\tthe default entity","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, 11 Feb 2019 11:43:12 -0000"}},{"id":778,"web_url":"https://patchwork.libcamera.org/comment/778/","msgid":"","date":"2019-02-11T11:47:35","subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: media_device: Provide\n\tthe default entity","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 08/02/2019 15:44, Laurent Pinchart wrote:\n> Hello,\n> \n> On Thu, Feb 07, 2019 at 11:06:20PM +0100, Jacopo Mondi wrote:\n>> Hi Kieran,\n>> ignorant question, is FL_DEFAULT an UVC specific thing?\n> \n> It isn't, but it isn't widely used either. It would thus be fine using\n> the default entity in UVC-specific tests or in the UVC pipeline handler,\n> but not in the rest of the code, at least not without a fallback.\n\nThe only places that use this helper are the uvcvideo pipeline, and the\nV4L2DeviceTest which is currently coded to skip if not operated on a UVC\ndevice...\n\nWhen this stops being coded to a UVC device - the helper can be removed\nor updated as necessary.\n\n\n>> A quick grep on kernel sources returns:\n>>\n>> $ git grep FL_DEFAULT drivers/media/\n>> drivers/media/platform/omap3isp/ispresizer.c: res->video_out.video.entity.flags |= MEDIA_ENT_FL_DEFAULT;\n>> drivers/media/usb/uvc/uvc_entity.c: entity->vdev->entity.flags |= MEDIA_ENT_FL_DEFAULT;\n>>\n>> So I guess not, as omap3 seems also omap3 uses that as well.\n>>\n>> If that's not a UVC specificity, I welcome this change.\n>>\n>> On Thu, Feb 07, 2019 at 09:21:15PM +0000, Kieran Bingham wrote:\n>>> Add a helper to identify the default entity from a media device.\n>>>\n>>> Utilise it in the two places which iterate the entities manually, to allocated\n>>> a V4L2Device from the default entity.\n>>>\n>>> Signed-off-by: Kieran Bingham \n>>> ---\n>>> src/libcamera/include/media_device.h | 1 +\n>>> src/libcamera/media_device.cpp | 15 +++++++++++++++\n>>> src/libcamera/pipeline/uvcvideo.cpp | 10 ++++------\n>>> test/v4l2_device/v4l2_device_test.cpp | 12 +++++-------\n>>> 4 files changed, 25 insertions(+), 13 deletions(-)\n>>>\n>>> diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h\n>>> index 9f038093b2b2..361e8f4a4b86 100644\n>>> --- a/src/libcamera/include/media_device.h\n>>> +++ b/src/libcamera/include/media_device.h\n>>> @@ -42,6 +42,7 @@ public:\n>>>\n>>> \tconst std::vector &entities() const { return entities_; }\n>>> \tMediaEntity *getEntityByName(const std::string &name) const;\n>>> +\tMediaEntity *defaultEntity() const;\n> \n> This should take an entity type as a parameter as the default flag is\n> defined as \"Default entity for its type\". It could also be used, for\n> instance, to discover the default camera sensor.\n\nI don't understand how that would be implemented.\n\nWhat extra item does it then check against? the entity type?\n\n\n>>> \tMediaLink *link(const std::string &sourceName, unsigned int sourceIdx,\n>>> \t\t\tconst std::string &sinkName, unsigned int sinkIdx);\n>>> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp\n>>> index 800ed330fe6d..4af90e1590a1 100644\n>>> --- a/src/libcamera/media_device.cpp\n>>> +++ b/src/libcamera/media_device.cpp\n>>> @@ -319,6 +319,21 @@ MediaEntity *MediaDevice::getEntityByName(const std::string &name) const\n>>> \treturn nullptr;\n>>> }\n>>>\n>>> +/**\n>>> + * \\brief Return the default MediaEntity within a MediaDevice\n>>> + * \\return The default entity if specified, or a nullptr otherwise\n>>> + */\n>>> +MediaEntity *MediaDevice::defaultEntity() const\n>>> +{\n>>> +\tfor (MediaEntity *entity : entities_) {\n>>> +\t\tif (entity->flags() & MEDIA_ENT_FL_DEFAULT) {\n>>> +\t\t\treturn entity;\n>>> +\t\t}\n>>\n>> You could drop the inner braces { } here\n>>> +\t}\n>>> +\n>>> +\treturn nullptr;\n>>> +}\n>>> +\n>>> /**\n>>> * \\brief Retrieve the MediaLink connecting two pads, identified by entity\n>>> * names and pad indexes\n>>> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n>>> index fc31c52c0ecd..ed8228bb2fc6 100644\n>>> --- a/src/libcamera/pipeline/uvcvideo.cpp\n>>> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n>>> @@ -146,13 +146,11 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n>>>\n>>> \tmedia_->acquire();\n>>>\n>>> -\tfor (MediaEntity *entity : media_->entities()) {\n>>> -\t\tif (entity->flags() & MEDIA_ENT_FL_DEFAULT) {\n>>> -\t\t\tvideo_ = new V4L2Device(entity);\n>>> -\t\t\tbreak;\n>>> -\t\t}\n>>> -\t}\n>>> +\tMediaEntity *entity = media_->defaultEntity();\n>>> +\tif (!entity)\n>>\n>> Looking at existing code if you don't find any DEFAULT entity, you\n>> return false as you do here, but also release the media device.\n>>\n>> Should the same be done here?\n> \n> I think it should, and a message should likely be logged too.\n\nAck.\n\n\n> \n>>> +\t\treturn false;\n>>>\n>>> +\tvideo_ = new V4L2Device(entity);\n>>> \tif (!video_ || video_->open()) {\n>>> \t\tif (!video_)\n>>> \t\t\tLOG(UVC, Error) << \"Could not find a default video device\";\n>>> diff --git a/test/v4l2_device/v4l2_device_test.cpp b/test/v4l2_device/v4l2_device_test.cpp\n>>> index 18d014caf4c8..dd28cccada6b 100644\n>>> --- a/test/v4l2_device/v4l2_device_test.cpp\n>>> +++ b/test/v4l2_device/v4l2_device_test.cpp\n>>> @@ -46,15 +46,13 @@ int V4L2DeviceTest::init()\n>>>\n>>> \tmedia_->acquire();\n>>>\n>>> -\tfor (MediaEntity *entity : media_->entities()) {\n>>> -\t\tif (entity->flags() & MEDIA_ENT_FL_DEFAULT) {\n>>> -\t\t\tdev_ = new V4L2Device(entity);\n>>> -\t\t\tbreak;\n>>> -\t\t}\n>>> -\t}\n>>> +\tMediaEntity *entity = media_->defaultEntity();\n>>> +\tif (!entity)\n>>> +\t\treturn TestFail;\n> \n> Note that we're planning to use vivid instead of uvcvideo for V4L2Device\n> tests, so this may not be a good idea.\n\n\nOk - so that should change at that point. Here to me it's a failure,\nbecause we will not know which UVC device to check against.\n\nI can add a comment to say that this check is UVC specific.\n\n\n\n>>> +\tdev_ = new V4L2Device(entity);\n>>> \tif (!dev_)\n>>> -\t\treturn TestSkip;\n>>> +\t\treturn TestFail;\n>>>\n>>> \treturn dev_->open();\n>>> }\n>","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 DC10F610B2\n\tfor ;\n\tMon, 11 Feb 2019 12:47:39 +0100 (CET)","from [192.168.0.21]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 006C52E4;\n\tMon, 11 Feb 2019 12:47:38 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1549885659;\n\tbh=ymsuuioAN9LkQw5nAClnTKhRuzn75AE4usvFfaooazc=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=kYGnLGaENQYNlCbm+1jZQFBHy/Wk2ybTZnnFLP3QgMSOCj20Y+zlTmY7/lnLx/23a\n\tZqCE2EQXIMxZMNC3jKX9NsRFGKzSoZjyY2lp5xVd+ifkOFqdiaXOCegjKX+RRngwXl\n\t6jTmAmQyYQwSGDxfl6wPIhTvasVHCBpeN9bnZFHo=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart ,\n\tJacopo Mondi ","Cc":"LibCamera Devel ","References":"<20190207212119.30299-1-kieran.bingham@ideasonboard.com>\n\t<20190207212119.30299-2-kieran.bingham@ideasonboard.com>\n\t<20190207220620.livvzjoe5cacjt4w@uno.localdomain>\n\t<20190208154417.GF4562@pendragon.ideasonboard.com>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"","Date":"Mon, 11 Feb 2019 11:47:35 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.4.0","MIME-Version":"1.0","In-Reply-To":"<20190208154417.GF4562@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: media_device: Provide\n\tthe default entity","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, 11 Feb 2019 11:47:40 -0000"}},{"id":784,"web_url":"https://patchwork.libcamera.org/comment/784/","msgid":"<20190212091520.GB6279@pendragon.ideasonboard.com>","date":"2019-02-12T09:15:20","subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: media_device: Provide\n\tthe default entity","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Mon, Feb 11, 2019 at 11:47:35AM +0000, Kieran Bingham wrote:\n> On 08/02/2019 15:44, Laurent Pinchart wrote:\n> > On Thu, Feb 07, 2019 at 11:06:20PM +0100, Jacopo Mondi wrote:\n> >> Hi Kieran,\n> >> ignorant question, is FL_DEFAULT an UVC specific thing?\n> > \n> > It isn't, but it isn't widely used either. It would thus be fine using\n> > the default entity in UVC-specific tests or in the UVC pipeline handler,\n> > but not in the rest of the code, at least not without a fallback.\n> \n> The only places that use this helper are the uvcvideo pipeline, and the\n\nThere it makes complete sense.\n\n> V4L2DeviceTest which is currently coded to skip if not operated on a UVC\n> device...\n> \n> When this stops being coded to a UVC device - the helper can be removed\n> or updated as necessary.\n\nFine with me.\n\n> >> A quick grep on kernel sources returns:\n> >>\n> >> $ git grep FL_DEFAULT drivers/media/\n> >> drivers/media/platform/omap3isp/ispresizer.c: res->video_out.video.entity.flags |= MEDIA_ENT_FL_DEFAULT;\n> >> drivers/media/usb/uvc/uvc_entity.c: entity->vdev->entity.flags |= MEDIA_ENT_FL_DEFAULT;\n> >>\n> >> So I guess not, as omap3 seems also omap3 uses that as well.\n> >>\n> >> If that's not a UVC specificity, I welcome this change.\n> >>\n> >> On Thu, Feb 07, 2019 at 09:21:15PM +0000, Kieran Bingham wrote:\n> >>> Add a helper to identify the default entity from a media device.\n> >>>\n> >>> Utilise it in the two places which iterate the entities manually, to allocated\n> >>> a V4L2Device from the default entity.\n> >>>\n> >>> Signed-off-by: Kieran Bingham \n> >>> ---\n> >>> src/libcamera/include/media_device.h | 1 +\n> >>> src/libcamera/media_device.cpp | 15 +++++++++++++++\n> >>> src/libcamera/pipeline/uvcvideo.cpp | 10 ++++------\n> >>> test/v4l2_device/v4l2_device_test.cpp | 12 +++++-------\n> >>> 4 files changed, 25 insertions(+), 13 deletions(-)\n> >>>\n> >>> diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h\n> >>> index 9f038093b2b2..361e8f4a4b86 100644\n> >>> --- a/src/libcamera/include/media_device.h\n> >>> +++ b/src/libcamera/include/media_device.h\n> >>> @@ -42,6 +42,7 @@ public:\n> >>>\n> >>> \tconst std::vector &entities() const { return entities_; }\n> >>> \tMediaEntity *getEntityByName(const std::string &name) const;\n> >>> +\tMediaEntity *defaultEntity() const;\n> > \n> > This should take an entity type as a parameter as the default flag is\n> > defined as \"Default entity for its type\". It could also be used, for\n> > instance, to discover the default camera sensor.\n> \n> I don't understand how that would be implemented.\n> \n> What extra item does it then check against? the entity type?\n\nThe entity type, yes.\n\n> >>> \tMediaLink *link(const std::string &sourceName, unsigned int sourceIdx,\n> >>> \t\t\tconst std::string &sinkName, unsigned int sinkIdx);\n> >>> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp\n> >>> index 800ed330fe6d..4af90e1590a1 100644\n> >>> --- a/src/libcamera/media_device.cpp\n> >>> +++ b/src/libcamera/media_device.cpp\n> >>> @@ -319,6 +319,21 @@ MediaEntity *MediaDevice::getEntityByName(const std::string &name) const\n> >>> \treturn nullptr;\n> >>> }\n> >>>\n> >>> +/**\n> >>> + * \\brief Return the default MediaEntity within a MediaDevice\n> >>> + * \\return The default entity if specified, or a nullptr otherwise\n> >>> + */\n> >>> +MediaEntity *MediaDevice::defaultEntity() const\n> >>> +{\n> >>> +\tfor (MediaEntity *entity : entities_) {\n> >>> +\t\tif (entity->flags() & MEDIA_ENT_FL_DEFAULT) {\n> >>> +\t\t\treturn entity;\n> >>> +\t\t}\n> >>\n> >> You could drop the inner braces { } here\n> >>> +\t}\n> >>> +\n> >>> +\treturn nullptr;\n> >>> +}\n> >>> +\n> >>> /**\n> >>> * \\brief Retrieve the MediaLink connecting two pads, identified by entity\n> >>> * names and pad indexes\n> >>> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> >>> index fc31c52c0ecd..ed8228bb2fc6 100644\n> >>> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> >>> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> >>> @@ -146,13 +146,11 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> >>>\n> >>> \tmedia_->acquire();\n> >>>\n> >>> -\tfor (MediaEntity *entity : media_->entities()) {\n> >>> -\t\tif (entity->flags() & MEDIA_ENT_FL_DEFAULT) {\n> >>> -\t\t\tvideo_ = new V4L2Device(entity);\n> >>> -\t\t\tbreak;\n> >>> -\t\t}\n> >>> -\t}\n> >>> +\tMediaEntity *entity = media_->defaultEntity();\n> >>> +\tif (!entity)\n> >>\n> >> Looking at existing code if you don't find any DEFAULT entity, you\n> >> return false as you do here, but also release the media device.\n> >>\n> >> Should the same be done here?\n> > \n> > I think it should, and a message should likely be logged too.\n> \n> Ack.\n> \n> >>> +\t\treturn false;\n> >>>\n> >>> +\tvideo_ = new V4L2Device(entity);\n> >>> \tif (!video_ || video_->open()) {\n> >>> \t\tif (!video_)\n> >>> \t\t\tLOG(UVC, Error) << \"Could not find a default video device\";\n> >>> diff --git a/test/v4l2_device/v4l2_device_test.cpp b/test/v4l2_device/v4l2_device_test.cpp\n> >>> index 18d014caf4c8..dd28cccada6b 100644\n> >>> --- a/test/v4l2_device/v4l2_device_test.cpp\n> >>> +++ b/test/v4l2_device/v4l2_device_test.cpp\n> >>> @@ -46,15 +46,13 @@ int V4L2DeviceTest::init()\n> >>>\n> >>> \tmedia_->acquire();\n> >>>\n> >>> -\tfor (MediaEntity *entity : media_->entities()) {\n> >>> -\t\tif (entity->flags() & MEDIA_ENT_FL_DEFAULT) {\n> >>> -\t\t\tdev_ = new V4L2Device(entity);\n> >>> -\t\t\tbreak;\n> >>> -\t\t}\n> >>> -\t}\n> >>> +\tMediaEntity *entity = media_->defaultEntity();\n> >>> +\tif (!entity)\n> >>> +\t\treturn TestFail;\n> > \n> > Note that we're planning to use vivid instead of uvcvideo for V4L2Device\n> > tests, so this may not be a good idea.\n> \n> Ok - so that should change at that point. Here to me it's a failure,\n> because we will not know which UVC device to check against.\n> \n> I can add a comment to say that this check is UVC specific.\n\nThat should be enough for now.\n\n> >>> +\tdev_ = new V4L2Device(entity);\n> >>> \tif (!dev_)\n> >>> -\t\treturn TestSkip;\n> >>> +\t\treturn TestFail;\n> >>>\n> >>> \treturn dev_->open();\n> >>> }","headers":{"Return-Path":"","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 D824F60DBB\n\tfor ;\n\tTue, 12 Feb 2019 10:15:24 +0100 (CET)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id ED8B685;\n\tTue, 12 Feb 2019 10:15:23 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1549962924;\n\tbh=sVEjVjUM0fP+esgVmg1z3hi9AOiq2tFJZLN0GZSfybs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BMnSyrhp7WtpnXdmR1nZCFu+b0x3RfQ5PkAH7jCH6Vv6HVo7OxZqDClrVIY9FzzMW\n\tM5LOmEuEZHRoxiTK3AU8RkbUvwv1BWXih/EZ6dII2tG5CBKwUdf9+5xQQF+izKLG9J\n\tyqX2fW3fVyKi49vifQXl85kKxWX07v+Detg7187s=","Date":"Tue, 12 Feb 2019 11:15:20 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"Jacopo Mondi ,\n\tLibCamera Devel ","Message-ID":"<20190212091520.GB6279@pendragon.ideasonboard.com>","References":"<20190207212119.30299-1-kieran.bingham@ideasonboard.com>\n\t<20190207212119.30299-2-kieran.bingham@ideasonboard.com>\n\t<20190207220620.livvzjoe5cacjt4w@uno.localdomain>\n\t<20190208154417.GF4562@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: media_device: Provide\n\tthe default entity","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, 12 Feb 2019 09:15:25 -0000"}}]