[{"id":1367,"web_url":"https://patchwork.libcamera.org/comment/1367/","msgid":"<20190415205936.GY1980@bigcity.dyn.berto.se>","date":"2019-04-15T20:59:37","subject":"Re: [libcamera-devel] [PATCH 08/11] libcamera: v4l2-subdevice: Add\n\tmethod to retrieve the media entity","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2019-04-15 19:56:57 +0300, Laurent Pinchart wrote:\n> Add a method to retrieve the media entity associated with a subdevice.\n> The entityName() and deviceNode() methods are not needed anymore as they\n> can be accessed through the media entity, remove them.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> src/libcamera/include/v4l2_subdevice.h | 3 +--\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--\n> src/libcamera/v4l2_subdevice.cpp | 22 +++++++--------------\n> test/v4l2_subdevice/list_formats.cpp | 6 +++---\n> test/v4l2_subdevice/v4l2_subdevice_test.cpp | 2 +-\n> 5 files changed, 14 insertions(+), 23 deletions(-)\n> \n> diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h\n> index c71dce7d8644..31bc9d298e5b 100644\n> --- a/src/libcamera/include/v4l2_subdevice.h\n> +++ b/src/libcamera/include/v4l2_subdevice.h\n> @@ -40,8 +40,7 @@ public:\n> \tbool isOpen() const;\n> \tvoid close();\n> \n> -\tconst std::string &deviceNode() const { return entity_->deviceNode(); }\n> -\tconst std::string &entityName() const { return entity_->name(); }\n> +\tconst MediaEntity *entity() const { return entity_; }\n> \n> \tint setCrop(unsigned int pad, Rectangle *rect);\n> \tint setCompose(unsigned int pad, Rectangle *rect);\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 4ddd1ede1c81..e3c79f93963e 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -579,7 +579,7 @@ int PipelineHandlerIPU3::registerCameras()\n> \t\t\t\t\t&IPU3CameraData::imguOutputBufferReady);\n> \n> \t\t/* Create and register the Camera instance. */\n> -\t\tstd::string cameraName = cio2->sensor_->entityName() + \" \"\n> +\t\tstd::string cameraName = cio2->sensor_->entity()->name() + \" \"\n> \t\t\t\t + std::to_string(id);\n> \t\tstd::shared_ptr camera = Camera::create(this,\n> \t\t\t\t\t\t\t\tcameraName,\n> @@ -1055,7 +1055,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)\n> \t\t}\n> \t}\n> \tif (maxSize_.width == 0) {\n> -\t\tLOG(IPU3, Info) << \"Sensor '\" << sensor_->entityName()\n> +\t\tLOG(IPU3, Info) << \"Sensor '\" << sensor_->entity()->name()\n> \t\t\t\t<< \"' detected, but no supported image format \"\n> \t\t\t\t<< \" found: skip camera creation\";\n> \t\treturn -ENODEV;\n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index e34cc1693b46..6b2d7cce6794 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -126,12 +126,12 @@ int V4L2Subdevice::open()\n> \t\treturn -EBUSY;\n> \t}\n> \n> -\tret = ::open(deviceNode().c_str(), O_RDWR);\n> +\tret = ::open(entity_->deviceNode().c_str(), O_RDWR);\n> \tif (ret < 0) {\n> \t\tret = -errno;\n> \t\tLOG(V4L2Subdev, Error)\n> -\t\t\t<< \"Failed to open V4L2 subdevice '\" << deviceNode()\n> -\t\t\t<< \"': \" << strerror(-ret);\n> +\t\t\t<< \"Failed to open V4L2 subdevice '\"\n> +\t\t\t<< entity_->deviceNode() << \"': \" << strerror(-ret);\n> \t\treturn ret;\n> \t}\n> \tfd_ = ret;\n> @@ -161,17 +161,9 @@ void V4L2Subdevice::close()\n> }\n> \n> /**\n> - * \\fn V4L2Subdevice::deviceNode()\n> - * \\brief Retrieve the path of the device node associated with the subdevice\n> - *\n> - * \\return The subdevice's device node system path\n> - */\n> -\n> -/**\n> - * \\fn V4L2Subdevice::entityName()\n> - * \\brief Retrieve the name of the media entity associated with the subdevice\n> - *\n> - * \\return The name of the media entity the subdevice is associated to\n> + * \\fn V4L2Subdevice::entity()\n> + * \\brief Retrieve the media entity associated with the subdevice\n> + * \\return The subdevice's associated media entity.\n> */\n> \n> /**\n> @@ -338,7 +330,7 @@ V4L2Subdevice *V4L2Subdevice::fromEntityName(const MediaDevice *media,\n> \n> std::string V4L2Subdevice::logPrefix() const\n> {\n> -\treturn \"'\" + entityName() + \"'\";\n> +\treturn \"'\" + entity_->name() + \"'\";\n> }\n> \n> int V4L2Subdevice::enumPadSizes(unsigned int pad,unsigned int code,\n> diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp\n> index 09dec9abe854..18dd8761a8ab 100644\n> --- a/test/v4l2_subdevice/list_formats.cpp\n> +++ b/test/v4l2_subdevice/list_formats.cpp\n> @@ -52,7 +52,7 @@ int ListFormatsTest::run()\n> \tformats = scaler_->formats(0);\n> \tif (formats.empty()) {\n> \t\tcerr << \"Failed to list formats on pad 0 of subdevice \"\n> -\t\t << scaler_->entityName() << endl;\n> +\t\t << scaler_->entity()->name() << endl;\n> \t\treturn TestFail;\n> \t}\n> \tfor (auto it = formats.begin(); it != formats.end(); ++it)\n> @@ -61,7 +61,7 @@ int ListFormatsTest::run()\n> \tformats = scaler_->formats(1);\n> \tif (formats.empty()) {\n> \t\tcerr << \"Failed to list formats on pad 1 of subdevice \"\n> -\t\t << scaler_->entityName() << endl;\n> +\t\t << scaler_->entity()->name() << endl;\n> \t\treturn TestFail;\n> \t}\n> \tfor (auto it = formats.begin(); it != formats.end(); ++it)\n> @@ -71,7 +71,7 @@ int ListFormatsTest::run()\n> \tformats = scaler_->formats(2);\n> \tif (!formats.empty()) {\n> \t\tcerr << \"Listing formats on non-existing pad 2 of subdevice \"\n> -\t\t << scaler_->entityName()\n> +\t\t << scaler_->entity()->name()\n> \t\t << \" should return an empty format list\" << endl;\n> \t\treturn TestFail;\n> \t}\n> diff --git a/test/v4l2_subdevice/v4l2_subdevice_test.cpp b/test/v4l2_subdevice/v4l2_subdevice_test.cpp\n> index 06582969eb45..dfcf779af95e 100644\n> --- a/test/v4l2_subdevice/v4l2_subdevice_test.cpp\n> +++ b/test/v4l2_subdevice/v4l2_subdevice_test.cpp\n> @@ -66,7 +66,7 @@ int V4L2SubdeviceTest::init()\n> \tret = scaler_->open();\n> \tif (ret) {\n> \t\tcerr << \"Unable to open video subdevice \"\n> -\t\t << scaler_->deviceNode() << endl;\n> +\t\t << scaler_->entity()->deviceNode() << endl;\n> \t\tmedia_->release();\n> \t\treturn TestSkip;\n> \t}\n> -- \n> Regards,\n> \n> Laurent Pinchart\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-x129.google.com (mail-lf1-x129.google.com\n\t[IPv6:2a00:1450:4864:20::129])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2988160B2E\n\tfor ;\n\tMon, 15 Apr 2019 22:59:39 +0200 (CEST)","by mail-lf1-x129.google.com with SMTP id j20so1315864lfh.2\n\tfor ;\n\tMon, 15 Apr 2019 13:59:39 -0700 (PDT)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tf18sm10009358lja.91.2019.04.15.13.59.37\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tMon, 15 Apr 2019 13:59:37 -0700 (PDT)"],"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=5qF2zFO2YfwD0AqKk/fzz+gdUJ02TwyJ9WAicRWHO3A=;\n\tb=y7OR+6JdFYMjvMY9y49TczHJFH+PkNpB6TUraay7Ekrp4arLCsLc3/gkqY4WGS8OEE\n\tI4Fq//vS010fsOfz0QVmpQsijz5XUsj+459rCrsCAStA05Bj+02P80DLjf1C9sMGLQZa\n\t1v181Zpjt/B/4TwJE7UHgsQX+7YhSs5EKKDMLlXPj3ju+RTqk8P1JGpMc//A/uYFCbuf\n\tS8a/uuIV5FYbpvRkz/kNWFhClRIcSF2fCXKVgXn83YWFRu56WZr2ynC3nWunZJLwW0Ve\n\t/HGaob/qfDzj7v1xASkhNvFbKNQh51kS2oRhtNb6b2WJ6EZpvF8f2pcHOToD+EOPXA99\n\tqocA==","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=5qF2zFO2YfwD0AqKk/fzz+gdUJ02TwyJ9WAicRWHO3A=;\n\tb=cnnqodSCZVUJIu2gUQq3otl+GwXxbY65qeGcXljQ1w9Ix80eLgbIznsZVImDQxKtxz\n\tdVPcdf5zosXode16I7FzUvT958vTyhmgHvjsVYDLW5xaL29BCktAJWp0SnchKcDBdJBx\n\tdYvzMeKEuInSK/8C3jbt3/a4wcMRo7yoeaSoCQitIVTaSO1oooVNJOb+r29dcoaWTyil\n\tED1/MH5E/13i8cJve3h5/pm1FWNiFadwp8wlO4n9avukAeH5ghFcOQbAW0EAOhxCDMkz\n\tIVYNMCCX7iu8PcOOLzxp8TSjNim+WyYYuli0algsJw2mKmxzRoiDvHoqUvj/PgMhN6mw\n\tiGlg==","X-Gm-Message-State":"APjAAAWq1pt3EDb4S7aES4JGtLGPbPYHnH/TgqIvAMFpJNevAOvrjiOh\n\tRVEPrAtAAwDejK3BbZ02+ynpug==","X-Google-Smtp-Source":"APXvYqysJg8PktOwTReUEvpHhr+eoi435sdYoegGpvcXBAFs3j5GVvDanxJccupvElX2FOZO0a1CGA==","X-Received":"by 2002:a19:cc52:: with SMTP id\n\tc79mr15200560lfg.30.1555361978311; \n\tMon, 15 Apr 2019 13:59:38 -0700 (PDT)","Date":"Mon, 15 Apr 2019 22:59:37 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190415205936.GY1980@bigcity.dyn.berto.se>","References":"<20190415165700.22970-1-laurent.pinchart@ideasonboard.com>\n\t<20190415165700.22970-9-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190415165700.22970-9-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.11.3 (2019-02-01)","Subject":"Re: [libcamera-devel] [PATCH 08/11] libcamera: v4l2-subdevice: Add\n\tmethod to retrieve the media 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, 15 Apr 2019 20:59:39 -0000"}},{"id":1394,"web_url":"https://patchwork.libcamera.org/comment/1394/","msgid":"<20190416152058.o7ff5mb4ayqv6omo@uno.localdomain>","date":"2019-04-16T15:20:58","subject":"Re: [libcamera-devel] [PATCH 08/11] libcamera: v4l2-subdevice: Add\n\tmethod to retrieve the media entity","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Mon, Apr 15, 2019 at 07:56:57PM +0300, Laurent Pinchart wrote:\n> Add a method to retrieve the media entity associated with a subdevice.\n> The entityName() and deviceNode() methods are not needed anymore as they\n> can be accessed through the media entity, remove them.\n>\n\nI might have missed what's the benefit of this patch. I mean, maybe in\nnext patches you need entity() and this is fine, but what's the\nbenefit of removing deviceNode() and entityName(), if not writing\nlonger lines when accessing them?\n\nThanks\n j\n\n\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/include/v4l2_subdevice.h | 3 +--\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--\n> src/libcamera/v4l2_subdevice.cpp | 22 +++++++--------------\n> test/v4l2_subdevice/list_formats.cpp | 6 +++---\n> test/v4l2_subdevice/v4l2_subdevice_test.cpp | 2 +-\n> 5 files changed, 14 insertions(+), 23 deletions(-)\n>\n> diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h\n> index c71dce7d8644..31bc9d298e5b 100644\n> --- a/src/libcamera/include/v4l2_subdevice.h\n> +++ b/src/libcamera/include/v4l2_subdevice.h\n> @@ -40,8 +40,7 @@ public:\n> \tbool isOpen() const;\n> \tvoid close();\n>\n> -\tconst std::string &deviceNode() const { return entity_->deviceNode(); }\n> -\tconst std::string &entityName() const { return entity_->name(); }\n> +\tconst MediaEntity *entity() const { return entity_; }\n>\n> \tint setCrop(unsigned int pad, Rectangle *rect);\n> \tint setCompose(unsigned int pad, Rectangle *rect);\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 4ddd1ede1c81..e3c79f93963e 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -579,7 +579,7 @@ int PipelineHandlerIPU3::registerCameras()\n> \t\t\t\t\t&IPU3CameraData::imguOutputBufferReady);\n>\n> \t\t/* Create and register the Camera instance. */\n> -\t\tstd::string cameraName = cio2->sensor_->entityName() + \" \"\n> +\t\tstd::string cameraName = cio2->sensor_->entity()->name() + \" \"\n> \t\t\t\t + std::to_string(id);\n> \t\tstd::shared_ptr camera = Camera::create(this,\n> \t\t\t\t\t\t\t\tcameraName,\n> @@ -1055,7 +1055,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)\n> \t\t}\n> \t}\n> \tif (maxSize_.width == 0) {\n> -\t\tLOG(IPU3, Info) << \"Sensor '\" << sensor_->entityName()\n> +\t\tLOG(IPU3, Info) << \"Sensor '\" << sensor_->entity()->name()\n> \t\t\t\t<< \"' detected, but no supported image format \"\n> \t\t\t\t<< \" found: skip camera creation\";\n> \t\treturn -ENODEV;\n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index e34cc1693b46..6b2d7cce6794 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -126,12 +126,12 @@ int V4L2Subdevice::open()\n> \t\treturn -EBUSY;\n> \t}\n>\n> -\tret = ::open(deviceNode().c_str(), O_RDWR);\n> +\tret = ::open(entity_->deviceNode().c_str(), O_RDWR);\n> \tif (ret < 0) {\n> \t\tret = -errno;\n> \t\tLOG(V4L2Subdev, Error)\n> -\t\t\t<< \"Failed to open V4L2 subdevice '\" << deviceNode()\n> -\t\t\t<< \"': \" << strerror(-ret);\n> +\t\t\t<< \"Failed to open V4L2 subdevice '\"\n> +\t\t\t<< entity_->deviceNode() << \"': \" << strerror(-ret);\n> \t\treturn ret;\n> \t}\n> \tfd_ = ret;\n> @@ -161,17 +161,9 @@ void V4L2Subdevice::close()\n> }\n>\n> /**\n> - * \\fn V4L2Subdevice::deviceNode()\n> - * \\brief Retrieve the path of the device node associated with the subdevice\n> - *\n> - * \\return The subdevice's device node system path\n> - */\n> -\n> -/**\n> - * \\fn V4L2Subdevice::entityName()\n> - * \\brief Retrieve the name of the media entity associated with the subdevice\n> - *\n> - * \\return The name of the media entity the subdevice is associated to\n> + * \\fn V4L2Subdevice::entity()\n> + * \\brief Retrieve the media entity associated with the subdevice\n> + * \\return The subdevice's associated media entity.\n> */\n>\n> /**\n> @@ -338,7 +330,7 @@ V4L2Subdevice *V4L2Subdevice::fromEntityName(const MediaDevice *media,\n>\n> std::string V4L2Subdevice::logPrefix() const\n> {\n> -\treturn \"'\" + entityName() + \"'\";\n> +\treturn \"'\" + entity_->name() + \"'\";\n> }\n>\n> int V4L2Subdevice::enumPadSizes(unsigned int pad,unsigned int code,\n> diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp\n> index 09dec9abe854..18dd8761a8ab 100644\n> --- a/test/v4l2_subdevice/list_formats.cpp\n> +++ b/test/v4l2_subdevice/list_formats.cpp\n> @@ -52,7 +52,7 @@ int ListFormatsTest::run()\n> \tformats = scaler_->formats(0);\n> \tif (formats.empty()) {\n> \t\tcerr << \"Failed to list formats on pad 0 of subdevice \"\n> -\t\t << scaler_->entityName() << endl;\n> +\t\t << scaler_->entity()->name() << endl;\n> \t\treturn TestFail;\n> \t}\n> \tfor (auto it = formats.begin(); it != formats.end(); ++it)\n> @@ -61,7 +61,7 @@ int ListFormatsTest::run()\n> \tformats = scaler_->formats(1);\n> \tif (formats.empty()) {\n> \t\tcerr << \"Failed to list formats on pad 1 of subdevice \"\n> -\t\t << scaler_->entityName() << endl;\n> +\t\t << scaler_->entity()->name() << endl;\n> \t\treturn TestFail;\n> \t}\n> \tfor (auto it = formats.begin(); it != formats.end(); ++it)\n> @@ -71,7 +71,7 @@ int ListFormatsTest::run()\n> \tformats = scaler_->formats(2);\n> \tif (!formats.empty()) {\n> \t\tcerr << \"Listing formats on non-existing pad 2 of subdevice \"\n> -\t\t << scaler_->entityName()\n> +\t\t << scaler_->entity()->name()\n> \t\t << \" should return an empty format list\" << endl;\n> \t\treturn TestFail;\n> \t}\n> diff --git a/test/v4l2_subdevice/v4l2_subdevice_test.cpp b/test/v4l2_subdevice/v4l2_subdevice_test.cpp\n> index 06582969eb45..dfcf779af95e 100644\n> --- a/test/v4l2_subdevice/v4l2_subdevice_test.cpp\n> +++ b/test/v4l2_subdevice/v4l2_subdevice_test.cpp\n> @@ -66,7 +66,7 @@ int V4L2SubdeviceTest::init()\n> \tret = scaler_->open();\n> \tif (ret) {\n> \t\tcerr << \"Unable to open video subdevice \"\n> -\t\t << scaler_->deviceNode() << endl;\n> +\t\t << scaler_->entity()->deviceNode() << endl;\n> \t\tmedia_->release();\n> \t\treturn TestSkip;\n> \t}\n> --\n> Regards,\n>\n> Laurent Pinchart\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 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 333CB60DB4\n\tfor ;\n\tTue, 16 Apr 2019 17:20:07 +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 relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 9C862FF80B;\n\tTue, 16 Apr 2019 15:20:06 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Tue, 16 Apr 2019 17:20:58 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190416152058.o7ff5mb4ayqv6omo@uno.localdomain>","References":"<20190415165700.22970-1-laurent.pinchart@ideasonboard.com>\n\t<20190415165700.22970-9-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"mt6sue4jhg6opayv\"","Content-Disposition":"inline","In-Reply-To":"<20190415165700.22970-9-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 08/11] libcamera: v4l2-subdevice: Add\n\tmethod to retrieve the media 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, 16 Apr 2019 15:20:07 -0000"}},{"id":1404,"web_url":"https://patchwork.libcamera.org/comment/1404/","msgid":"<20190416201404.GC4822@pendragon.ideasonboard.com>","date":"2019-04-16T20:14:04","subject":"Re: [libcamera-devel] [PATCH 08/11] libcamera: v4l2-subdevice: Add\n\tmethod to retrieve the media entity","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Tue, Apr 16, 2019 at 05:20:58PM +0200, Jacopo Mondi wrote:\n> On Mon, Apr 15, 2019 at 07:56:57PM +0300, Laurent Pinchart wrote:\n> > Add a method to retrieve the media entity associated with a subdevice.\n> > The entityName() and deviceNode() methods are not needed anymore as they\n> > can be accessed through the media entity, remove them.\n> \n> I might have missed what's the benefit of this patch. I mean, maybe in\n> next patches you need entity() and this is fine, but what's the\n> benefit of removing deviceNode() and entityName(), if not writing\n> longer lines when accessing them?\n\nAs you've correctly noted, adding the entity() function is needed. I\nthen decided to remove the entityName() and deviceNode() functions are\nthey can be accessed through entity(), without much extra hassle (apart\nfrom slightly longer lines, especially for deviceNode()). I believe that\nin general it is best not to offer different ways to access the same\ninformation when those multiple ways have similar complexities,\notherwise it creates more complex APIs for little benefit.\n\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/include/v4l2_subdevice.h | 3 +--\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--\n> > src/libcamera/v4l2_subdevice.cpp | 22 +++++++--------------\n> > test/v4l2_subdevice/list_formats.cpp | 6 +++---\n> > test/v4l2_subdevice/v4l2_subdevice_test.cpp | 2 +-\n> > 5 files changed, 14 insertions(+), 23 deletions(-)\n> >\n> > diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h\n> > index c71dce7d8644..31bc9d298e5b 100644\n> > --- a/src/libcamera/include/v4l2_subdevice.h\n> > +++ b/src/libcamera/include/v4l2_subdevice.h\n> > @@ -40,8 +40,7 @@ public:\n> > \tbool isOpen() const;\n> > \tvoid close();\n> >\n> > -\tconst std::string &deviceNode() const { return entity_->deviceNode(); }\n> > -\tconst std::string &entityName() const { return entity_->name(); }\n> > +\tconst MediaEntity *entity() const { return entity_; }\n> >\n> > \tint setCrop(unsigned int pad, Rectangle *rect);\n> > \tint setCompose(unsigned int pad, Rectangle *rect);\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 4ddd1ede1c81..e3c79f93963e 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -579,7 +579,7 @@ int PipelineHandlerIPU3::registerCameras()\n> > \t\t\t\t\t&IPU3CameraData::imguOutputBufferReady);\n> >\n> > \t\t/* Create and register the Camera instance. */\n> > -\t\tstd::string cameraName = cio2->sensor_->entityName() + \" \"\n> > +\t\tstd::string cameraName = cio2->sensor_->entity()->name() + \" \"\n> > \t\t\t\t + std::to_string(id);\n> > \t\tstd::shared_ptr camera = Camera::create(this,\n> > \t\t\t\t\t\t\t\tcameraName,\n> > @@ -1055,7 +1055,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)\n> > \t\t}\n> > \t}\n> > \tif (maxSize_.width == 0) {\n> > -\t\tLOG(IPU3, Info) << \"Sensor '\" << sensor_->entityName()\n> > +\t\tLOG(IPU3, Info) << \"Sensor '\" << sensor_->entity()->name()\n> > \t\t\t\t<< \"' detected, but no supported image format \"\n> > \t\t\t\t<< \" found: skip camera creation\";\n> > \t\treturn -ENODEV;\n> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > index e34cc1693b46..6b2d7cce6794 100644\n> > --- a/src/libcamera/v4l2_subdevice.cpp\n> > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > @@ -126,12 +126,12 @@ int V4L2Subdevice::open()\n> > \t\treturn -EBUSY;\n> > \t}\n> >\n> > -\tret = ::open(deviceNode().c_str(), O_RDWR);\n> > +\tret = ::open(entity_->deviceNode().c_str(), O_RDWR);\n> > \tif (ret < 0) {\n> > \t\tret = -errno;\n> > \t\tLOG(V4L2Subdev, Error)\n> > -\t\t\t<< \"Failed to open V4L2 subdevice '\" << deviceNode()\n> > -\t\t\t<< \"': \" << strerror(-ret);\n> > +\t\t\t<< \"Failed to open V4L2 subdevice '\"\n> > +\t\t\t<< entity_->deviceNode() << \"': \" << strerror(-ret);\n> > \t\treturn ret;\n> > \t}\n> > \tfd_ = ret;\n> > @@ -161,17 +161,9 @@ void V4L2Subdevice::close()\n> > }\n> >\n> > /**\n> > - * \\fn V4L2Subdevice::deviceNode()\n> > - * \\brief Retrieve the path of the device node associated with the subdevice\n> > - *\n> > - * \\return The subdevice's device node system path\n> > - */\n> > -\n> > -/**\n> > - * \\fn V4L2Subdevice::entityName()\n> > - * \\brief Retrieve the name of the media entity associated with the subdevice\n> > - *\n> > - * \\return The name of the media entity the subdevice is associated to\n> > + * \\fn V4L2Subdevice::entity()\n> > + * \\brief Retrieve the media entity associated with the subdevice\n> > + * \\return The subdevice's associated media entity.\n> > */\n> >\n> > /**\n> > @@ -338,7 +330,7 @@ V4L2Subdevice *V4L2Subdevice::fromEntityName(const MediaDevice *media,\n> >\n> > std::string V4L2Subdevice::logPrefix() const\n> > {\n> > -\treturn \"'\" + entityName() + \"'\";\n> > +\treturn \"'\" + entity_->name() + \"'\";\n> > }\n> >\n> > int V4L2Subdevice::enumPadSizes(unsigned int pad,unsigned int code,\n> > diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp\n> > index 09dec9abe854..18dd8761a8ab 100644\n> > --- a/test/v4l2_subdevice/list_formats.cpp\n> > +++ b/test/v4l2_subdevice/list_formats.cpp\n> > @@ -52,7 +52,7 @@ int ListFormatsTest::run()\n> > \tformats = scaler_->formats(0);\n> > \tif (formats.empty()) {\n> > \t\tcerr << \"Failed to list formats on pad 0 of subdevice \"\n> > -\t\t << scaler_->entityName() << endl;\n> > +\t\t << scaler_->entity()->name() << endl;\n> > \t\treturn TestFail;\n> > \t}\n> > \tfor (auto it = formats.begin(); it != formats.end(); ++it)\n> > @@ -61,7 +61,7 @@ int ListFormatsTest::run()\n> > \tformats = scaler_->formats(1);\n> > \tif (formats.empty()) {\n> > \t\tcerr << \"Failed to list formats on pad 1 of subdevice \"\n> > -\t\t << scaler_->entityName() << endl;\n> > +\t\t << scaler_->entity()->name() << endl;\n> > \t\treturn TestFail;\n> > \t}\n> > \tfor (auto it = formats.begin(); it != formats.end(); ++it)\n> > @@ -71,7 +71,7 @@ int ListFormatsTest::run()\n> > \tformats = scaler_->formats(2);\n> > \tif (!formats.empty()) {\n> > \t\tcerr << \"Listing formats on non-existing pad 2 of subdevice \"\n> > -\t\t << scaler_->entityName()\n> > +\t\t << scaler_->entity()->name()\n> > \t\t << \" should return an empty format list\" << endl;\n> > \t\treturn TestFail;\n> > \t}\n> > diff --git a/test/v4l2_subdevice/v4l2_subdevice_test.cpp b/test/v4l2_subdevice/v4l2_subdevice_test.cpp\n> > index 06582969eb45..dfcf779af95e 100644\n> > --- a/test/v4l2_subdevice/v4l2_subdevice_test.cpp\n> > +++ b/test/v4l2_subdevice/v4l2_subdevice_test.cpp\n> > @@ -66,7 +66,7 @@ int V4L2SubdeviceTest::init()\n> > \tret = scaler_->open();\n> > \tif (ret) {\n> > \t\tcerr << \"Unable to open video subdevice \"\n> > -\t\t << scaler_->deviceNode() << endl;\n> > +\t\t << scaler_->entity()->deviceNode() << endl;\n> > \t\tmedia_->release();\n> > \t\treturn TestSkip;\n> > \t}","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 42F1960004\n\tfor ;\n\tTue, 16 Apr 2019 22:14:13 +0200 (CEST)","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 AB718E2;\n\tTue, 16 Apr 2019 22:14:12 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1555445652;\n\tbh=bnYdO6jsKzv5sm/mZdtrjf2wnYLuqLeibrZUeZCiS6M=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Kr2SiDNJvVpD/x+EJ2ASNddNy/qNkAF/0wRGh3Tpz77x0pgdsDybanYU3DcETOXyN\n\ttLzKzFzdj5xLLGSYuTIvJzn6++BImIVXV06vydQkp4RpIAnVY0cinoh1kg6Pw3n2VG\n\tRpxWsEVfQY+PIdqb9WKRISGmwTVLC2/Jf6v255Ik=","Date":"Tue, 16 Apr 2019 23:14:04 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190416201404.GC4822@pendragon.ideasonboard.com>","References":"<20190415165700.22970-1-laurent.pinchart@ideasonboard.com>\n\t<20190415165700.22970-9-laurent.pinchart@ideasonboard.com>\n\t<20190416152058.o7ff5mb4ayqv6omo@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190416152058.o7ff5mb4ayqv6omo@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 08/11] libcamera: v4l2-subdevice: Add\n\tmethod to retrieve the media 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, 16 Apr 2019 20:14:13 -0000"}}]