[{"id":898,"web_url":"https://patchwork.libcamera.org/comment/898/","msgid":"<20190226193136.GO899@bigcity.dyn.berto.se>","date":"2019-02-26T19:31:37","subject":"Re: [libcamera-devel] [PATCH v3 1/8] libcamera: v4l2_subdevice:\n\tStore media entity","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-02-26 17:26:34 +0100, Jacopo Mondi wrote:\n> Store the media entity backing the V4L2Subdevice and add a deviceName()\n> method to retrieve the human readable name of the subdevice, which is\n> created using the name of the associated media entity.\n> \n> Use the deviceName() in error messages where appropriate, as the entity\n> name is sometimes more informative than the device node path.\n\nI would keep the device node in the error messages instead of the device \nname. As a user diagnosing the problem I would find it easier to \nassociate the errno to a device node rather then a name. Also going from \nthe device node to the device name is easier then the reveres using v4l2 \ntools. Possibly you could print both.\n\nWith this fixed,\n\nReviewed-by: Niklas Söderlund \n\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/include/v4l2_subdevice.h | 8 +++++---\n> src/libcamera/v4l2_subdevice.cpp | 17 ++++++++++++-----\n> 2 files changed, 17 insertions(+), 8 deletions(-)\n> \n> diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h\n> index 82fa6685ab52..eac699a06109 100644\n> --- a/src/libcamera/include/v4l2_subdevice.h\n> +++ b/src/libcamera/include/v4l2_subdevice.h\n> @@ -9,9 +9,10 @@\n> \n> #include \n> \n> +#include \"media_object.h\"\n> +\n> namespace libcamera {\n> \n> -class MediaEntity;\n> struct Rectangle;\n> \n> struct V4L2SubdeviceFormat {\n> @@ -31,7 +32,8 @@ public:\n> \tbool isOpen() const;\n> \tvoid close();\n> \n> -\tstd::string deviceNode() const { return deviceNode_; }\n> +\tstd::string deviceNode() const { return entity_->deviceNode(); }\n> +\tstd::string deviceName() const { return entity_->name(); }\n> \n> \tint setCrop(unsigned int pad, Rectangle *rect);\n> \tint setCompose(unsigned int pad, Rectangle *rect);\n> @@ -43,7 +45,7 @@ private:\n> \tint setSelection(unsigned int pad, unsigned int target,\n> \t\t\t Rectangle *rect);\n> \n> -\tstd::string deviceNode_;\n> +\tconst MediaEntity *entity_;\n> \tint fd_;\n> };\n> \n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index b436f73cc75f..56ecf3851cb0 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -88,7 +88,7 @@ LOG_DEFINE_CATEGORY(V4L2Subdev)\n> * path\n> */\n> V4L2Subdevice::V4L2Subdevice(const MediaEntity *entity)\n> -\t: deviceNode_(entity->deviceNode()), fd_(-1)\n> +\t: entity_(entity), fd_(-1)\n> {\n> }\n> \n> @@ -106,11 +106,11 @@ int V4L2Subdevice::open()\n> \t\treturn -EBUSY;\n> \t}\n> \n> -\tret = ::open(deviceNode_.c_str(), O_RDWR);\n> +\tret = ::open(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<< \"Failed to open V4L2 subdevice '\" << deviceNode()\n> \t\t\t<< \"': \" << strerror(-ret);\n> \t\treturn ret;\n> \t}\n> @@ -147,6 +147,13 @@ void V4L2Subdevice::close()\n> * \\return The subdevice's device node system path\n> */\n> \n> +/**\n> + * \\fn V4L2Subdevice::deviceName()\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> + */\n> +\n> /**\n> * \\brief Set a crop rectangle on one of the V4L2 subdevice pads\n> * \\param[in] pad The 0-indexed pad number the rectangle is to be applied to\n> @@ -189,7 +196,7 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format)\n> \t\tret = -errno;\n> \t\tLOG(V4L2Subdev, Error)\n> \t\t\t<< \"Unable to get format on pad \" << pad\n> -\t\t\t<< \" of \" << deviceNode_ << \": \" << strerror(-ret);\n> +\t\t\t<< \" of \" << deviceName() << \": \" << strerror(-ret);\n> \t\treturn ret;\n> \t}\n> \n> @@ -255,7 +262,7 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,\n> \t\tret = -errno;\n> \t\tLOG(V4L2Subdev, Error)\n> \t\t\t<< \"Unable to set rectangle \" << target << \" on pad \"\n> -\t\t\t<< pad << \" of \" << deviceNode_ << \": \"\n> +\t\t\t<< pad << \" of \" << deviceName() << \": \"\n> \t\t\t<< strerror(-ret);\n> \t\treturn ret;\n> \t}\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-x141.google.com (mail-lf1-x141.google.com\n\t[IPv6:2a00:1450:4864:20::141])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0D4EF610B2\n\tfor ;\n\tTue, 26 Feb 2019 20:31:39 +0100 (CET)","by mail-lf1-x141.google.com with SMTP id q12so10574035lfm.0\n\tfor ;\n\tTue, 26 Feb 2019 11:31:38 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tl20sm3075294ljb.24.2019.02.26.11.31.37\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tTue, 26 Feb 2019 11:31:37 -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=3JZ1Cnzb4ordPnFHzGdVJ5WgySF9WyFtfe0JCxaSCwY=;\n\tb=t63t6D+wMTgNSU7he1rp+Zca7wHL1VTDBOdrmFI05npQItc+0D7E5pk5NFNOeHMjaV\n\t/IrIP2omME6pX2TtVw5enO0Kx00Yjc2SidIEKCeKQDkfw6BK85NELj+e0E/OPwkj4Nf+\n\tr6p73zweUffb1DpVmGhazTFTPDZ94GJ5C4ZtWR+6TYyTUdNeTycTcNAlaT0pdthvZUf9\n\tiJwbYIEOXc/OekWrqKJubOLeZK1dO4EzYsIxz8/yy5OZxj3grIDtlZyEA9nVuKUgC7jv\n\tPwdWhrKG0nMui/iMwybf2dkxN7v2E9BPb4rAv03S/oGzx+dQIx40dWcCaVk1oXWbsv1j\n\tqrEQ==","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=3JZ1Cnzb4ordPnFHzGdVJ5WgySF9WyFtfe0JCxaSCwY=;\n\tb=WLoNON5d+roBq8b3edINhLy2LeD7kdeibGgZNoyqVUsDCVdhD/cTh+/IL6XI1vZBrX\n\tOvr9b9N4y39KAlDeqBV1P8IF3P1WaSsIXFFrb72Bj/bJAQ1Doy75HkuI76WjZkTOuxX2\n\tA20hsz7WlRl6pAmWVmhd+NCQPID1TaAfLFgPN/KMca1KMfywipWVkBXzZXzNkVD2ufGJ\n\tOIvt0K2DnLpXtUHtC1lWFCu1AO00qu1M5RFZoFqE2O3m5iEU8htSeLBAPGYZLzLZgGNh\n\tRDJkC00r/cTY7yIGBux2C02IPCZCk7bH7+AnkWe/SJasmMvpqI96mwQItsLbGw9emljN\n\tNy9A==","X-Gm-Message-State":"AHQUAuY58XVFKyk/3MB1WXeIBkqdTBy6EgDF7rY2BflUNORv1IRvPA47\n\tCjT6OiLjuTZwfcBJ8Y8Lf+y0oKMP3BY=","X-Google-Smtp-Source":"AHgI3IYJO8I77TPi2datxpO5HXUzQYt6CE3N56cnIvmP2kujuE8khNBCNm2f0vHUkBNfZVPwxucDEw==","X-Received":"by 2002:a19:2105:: with SMTP id h5mr8232793lfh.17.1551209498176; \n\tTue, 26 Feb 2019 11:31:38 -0800 (PST)","Date":"Tue, 26 Feb 2019 20:31:37 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190226193136.GO899@bigcity.dyn.berto.se>","References":"<20190226162641.12116-1-jacopo@jmondi.org>\n\t<20190226162641.12116-2-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":"<20190226162641.12116-2-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 1/8] libcamera: v4l2_subdevice:\n\tStore 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, 26 Feb 2019 19:31:39 -0000"}},{"id":899,"web_url":"https://patchwork.libcamera.org/comment/899/","msgid":"","date":"2019-02-26T20:07:49","subject":"Re: [libcamera-devel] [PATCH v3 1/8] libcamera: v4l2_subdevice:\n\tStore media 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 26/02/2019 16:26, Jacopo Mondi wrote:\n> Store the media entity backing the V4L2Subdevice and add a deviceName()\n> method to retrieve the human readable name of the subdevice, which is\n> created using the name of the associated media entity.\n> \n> Use the deviceName() in error messages where appropriate, as the entity\n> name is sometimes more informative than the device node path.\n\nI did something similar locally in the V4L2Device perhaps I it might be\nuseful in mainline.\n\nReviewed-by: Kieran Bingham \n\n\n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/include/v4l2_subdevice.h | 8 +++++---\n> src/libcamera/v4l2_subdevice.cpp | 17 ++++++++++++-----\n> 2 files changed, 17 insertions(+), 8 deletions(-)\n> \n> diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h\n> index 82fa6685ab52..eac699a06109 100644\n> --- a/src/libcamera/include/v4l2_subdevice.h\n> +++ b/src/libcamera/include/v4l2_subdevice.h\n> @@ -9,9 +9,10 @@\n> \n> #include \n> \n> +#include \"media_object.h\"\n> +\n> namespace libcamera {\n> \n> -class MediaEntity;\n> struct Rectangle;\n> \n> struct V4L2SubdeviceFormat {\n> @@ -31,7 +32,8 @@ public:\n> \tbool isOpen() const;\n> \tvoid close();\n> \n> -\tstd::string deviceNode() const { return deviceNode_; }\n> +\tstd::string deviceNode() const { return entity_->deviceNode(); }\n> +\tstd::string deviceName() const { return entity_->name(); }\n\ndeviceName() calling name() bothers me a little. But I'm sure it's fine.\nPerhaps we might do a global s/deviceName()/name()/ or the inverse sometime.\n\n\n\n> \n> \tint setCrop(unsigned int pad, Rectangle *rect);\n> \tint setCompose(unsigned int pad, Rectangle *rect);\n> @@ -43,7 +45,7 @@ private:\n> \tint setSelection(unsigned int pad, unsigned int target,\n> \t\t\t Rectangle *rect);\n> \n> -\tstd::string deviceNode_;\n> +\tconst MediaEntity *entity_;\n> \tint fd_;\n> };\n> \n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index b436f73cc75f..56ecf3851cb0 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -88,7 +88,7 @@ LOG_DEFINE_CATEGORY(V4L2Subdev)\n> * path\n> */\n> V4L2Subdevice::V4L2Subdevice(const MediaEntity *entity)\n> -\t: deviceNode_(entity->deviceNode()), fd_(-1)\n> +\t: entity_(entity), fd_(-1)\n> {\n> }\n> \n> @@ -106,11 +106,11 @@ int V4L2Subdevice::open()\n> \t\treturn -EBUSY;\n> \t}\n> \n> -\tret = ::open(deviceNode_.c_str(), O_RDWR);\n> +\tret = ::open(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<< \"Failed to open V4L2 subdevice '\" << deviceNode()\n> \t\t\t<< \"': \" << strerror(-ret);\n> \t\treturn ret;\n> \t}\n> @@ -147,6 +147,13 @@ void V4L2Subdevice::close()\n> * \\return The subdevice's device node system path\n> */\n> \n> +/**\n> + * \\fn V4L2Subdevice::deviceName()\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> + */\n> +\n> /**\n> * \\brief Set a crop rectangle on one of the V4L2 subdevice pads\n> * \\param[in] pad The 0-indexed pad number the rectangle is to be applied to\n> @@ -189,7 +196,7 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format)\n> \t\tret = -errno;\n> \t\tLOG(V4L2Subdev, Error)\n> \t\t\t<< \"Unable to get format on pad \" << pad\n> -\t\t\t<< \" of \" << deviceNode_ << \": \" << strerror(-ret);\n> +\t\t\t<< \" of \" << deviceName() << \": \" << strerror(-ret);\n> \t\treturn ret;\n> \t}\n> \n> @@ -255,7 +262,7 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,\n> \t\tret = -errno;\n> \t\tLOG(V4L2Subdev, Error)\n> \t\t\t<< \"Unable to set rectangle \" << target << \" on pad \"\n> -\t\t\t<< pad << \" of \" << deviceNode_ << \": \"\n> +\t\t\t<< pad << \" of \" << deviceName() << \": \"> \t\t\t<< strerror(-ret);\n> \t\treturn ret;\n> \t}\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 9FABA610B2\n\tfor ;\n\tTue, 26 Feb 2019 21:07:52 +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 0132367;\n\tTue, 26 Feb 2019 21:07:51 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1551211672;\n\tbh=rbJ76avm5uPgWt8RwXUKem1XN3K2MSfyRzvAnpoGxjU=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=sm66dS3UDzsbq739I79MnrNwY5S44KsLIlamTT1VOLmXrmoiafYGqVhvSf+9rr41c\n\tt1jaTBsprj8+CJh52C5V+aUd9HyhtT56IAT/b5l9NSPQvptwaimk/7BbIzUzFG/I6B\n\tIghs1ueDPXH7dZbbYGzDMBCGey8PuQ3OXJIvsoUQ=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","References":"<20190226162641.12116-1-jacopo@jmondi.org>\n\t<20190226162641.12116-2-jacopo@jmondi.org>","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":"Tue, 26 Feb 2019 20:07:49 +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":"<20190226162641.12116-2-jacopo@jmondi.org>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v3 1/8] libcamera: v4l2_subdevice:\n\tStore 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, 26 Feb 2019 20:07:52 -0000"}},{"id":908,"web_url":"https://patchwork.libcamera.org/comment/908/","msgid":"<20190227080959.zx3jmt67uk3ihn53@uno.localdomain>","date":"2019-02-27T08:09:59","subject":"Re: [libcamera-devel] [PATCH v3 1/8] libcamera: v4l2_subdevice:\n\tStore media entity","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Tue, Feb 26, 2019 at 08:31:37PM +0100, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> Thanks for your work.\n>\n> On 2019-02-26 17:26:34 +0100, Jacopo Mondi wrote:\n> > Store the media entity backing the V4L2Subdevice and add a deviceName()\n> > method to retrieve the human readable name of the subdevice, which is\n> > created using the name of the associated media entity.\n> >\n> > Use the deviceName() in error messages where appropriate, as the entity\n> > name is sometimes more informative than the device node path.\n>\n> I would keep the device node in the error messages instead of the device\n> name. As a user diagnosing the problem I would find it easier to\n> associate the errno to a device node rather then a name. Also going from\n> the device node to the device name is easier then the reveres using v4l2\n> tools. Possibly you could print both.\n\nI would agree if we were talking about video device nodes, but here\nwe're talking about subdevices.\n\nIt's very easy to go from entities from the devnode path with\nmedia-ctl, and subdevices are inspected with usually with media-ctl\nthat presents them by their entities name.\n\nHaving the crude v4l-subdev path, is much less informative imho.\n\n\n>\n> With this fixed,\n>\n> Reviewed-by: Niklas Söderlund \n>\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/include/v4l2_subdevice.h | 8 +++++---\n> > src/libcamera/v4l2_subdevice.cpp | 17 ++++++++++++-----\n> > 2 files changed, 17 insertions(+), 8 deletions(-)\n> >\n> > diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h\n> > index 82fa6685ab52..eac699a06109 100644\n> > --- a/src/libcamera/include/v4l2_subdevice.h\n> > +++ b/src/libcamera/include/v4l2_subdevice.h\n> > @@ -9,9 +9,10 @@\n> >\n> > #include \n> >\n> > +#include \"media_object.h\"\n> > +\n> > namespace libcamera {\n> >\n> > -class MediaEntity;\n> > struct Rectangle;\n> >\n> > struct V4L2SubdeviceFormat {\n> > @@ -31,7 +32,8 @@ public:\n> > \tbool isOpen() const;\n> > \tvoid close();\n> >\n> > -\tstd::string deviceNode() const { return deviceNode_; }\n> > +\tstd::string deviceNode() const { return entity_->deviceNode(); }\n> > +\tstd::string deviceName() const { return entity_->name(); }\n> >\n> > \tint setCrop(unsigned int pad, Rectangle *rect);\n> > \tint setCompose(unsigned int pad, Rectangle *rect);\n> > @@ -43,7 +45,7 @@ private:\n> > \tint setSelection(unsigned int pad, unsigned int target,\n> > \t\t\t Rectangle *rect);\n> >\n> > -\tstd::string deviceNode_;\n> > +\tconst MediaEntity *entity_;\n> > \tint fd_;\n> > };\n> >\n> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > index b436f73cc75f..56ecf3851cb0 100644\n> > --- a/src/libcamera/v4l2_subdevice.cpp\n> > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > @@ -88,7 +88,7 @@ LOG_DEFINE_CATEGORY(V4L2Subdev)\n> > * path\n> > */\n> > V4L2Subdevice::V4L2Subdevice(const MediaEntity *entity)\n> > -\t: deviceNode_(entity->deviceNode()), fd_(-1)\n> > +\t: entity_(entity), fd_(-1)\n> > {\n> > }\n> >\n> > @@ -106,11 +106,11 @@ int V4L2Subdevice::open()\n> > \t\treturn -EBUSY;\n> > \t}\n> >\n> > -\tret = ::open(deviceNode_.c_str(), O_RDWR);\n> > +\tret = ::open(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<< \"Failed to open V4L2 subdevice '\" << deviceNode()\n> > \t\t\t<< \"': \" << strerror(-ret);\n> > \t\treturn ret;\n> > \t}\n> > @@ -147,6 +147,13 @@ void V4L2Subdevice::close()\n> > * \\return The subdevice's device node system path\n> > */\n> >\n> > +/**\n> > + * \\fn V4L2Subdevice::deviceName()\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> > + */\n> > +\n> > /**\n> > * \\brief Set a crop rectangle on one of the V4L2 subdevice pads\n> > * \\param[in] pad The 0-indexed pad number the rectangle is to be applied to\n> > @@ -189,7 +196,7 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format)\n> > \t\tret = -errno;\n> > \t\tLOG(V4L2Subdev, Error)\n> > \t\t\t<< \"Unable to get format on pad \" << pad\n> > -\t\t\t<< \" of \" << deviceNode_ << \": \" << strerror(-ret);\n> > +\t\t\t<< \" of \" << deviceName() << \": \" << strerror(-ret);\n> > \t\treturn ret;\n> > \t}\n> >\n> > @@ -255,7 +262,7 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,\n> > \t\tret = -errno;\n> > \t\tLOG(V4L2Subdev, Error)\n> > \t\t\t<< \"Unable to set rectangle \" << target << \" on pad \"\n> > -\t\t\t<< pad << \" of \" << deviceNode_ << \": \"\n> > +\t\t\t<< pad << \" of \" << deviceName() << \": \"\n> > \t\t\t<< strerror(-ret);\n> > \t\treturn ret;\n> > \t}\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 relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DA98F601E2\n\tfor ;\n\tWed, 27 Feb 2019 09:09:30 +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 relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 5B39C1C0005;\n\tWed, 27 Feb 2019 08:09:30 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Wed, 27 Feb 2019 09:09:59 +0100","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190227080959.zx3jmt67uk3ihn53@uno.localdomain>","References":"<20190226162641.12116-1-jacopo@jmondi.org>\n\t<20190226162641.12116-2-jacopo@jmondi.org>\n\t<20190226193136.GO899@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"wvsicei46jtlznmt\"","Content-Disposition":"inline","In-Reply-To":"<20190226193136.GO899@bigcity.dyn.berto.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v3 1/8] libcamera: v4l2_subdevice:\n\tStore 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":"Wed, 27 Feb 2019 08:09:31 -0000"}},{"id":928,"web_url":"https://patchwork.libcamera.org/comment/928/","msgid":"<20190227173413.GK4813@pendragon.ideasonboard.com>","date":"2019-02-27T17:34:13","subject":"Re: [libcamera-devel] [PATCH v3 1/8] libcamera: v4l2_subdevice:\n\tStore 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\nThank you for the patch.\n\nOn Tue, Feb 26, 2019 at 05:26:34PM +0100, Jacopo Mondi wrote:\n> Store the media entity backing the V4L2Subdevice and add a deviceName()\n> method to retrieve the human readable name of the subdevice, which is\n> created using the name of the associated media entity.\n> \n> Use the deviceName() in error messages where appropriate, as the entity\n> name is sometimes more informative than the device node path.\n> \n> Signed-off-by: Jacopo Mondi \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/libcamera/include/v4l2_subdevice.h | 8 +++++---\n> src/libcamera/v4l2_subdevice.cpp | 17 ++++++++++++-----\n> 2 files changed, 17 insertions(+), 8 deletions(-)\n> \n> diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h\n> index 82fa6685ab52..eac699a06109 100644\n> --- a/src/libcamera/include/v4l2_subdevice.h\n> +++ b/src/libcamera/include/v4l2_subdevice.h\n> @@ -9,9 +9,10 @@\n> \n> #include \n> \n> +#include \"media_object.h\"\n> +\n> namespace libcamera {\n> \n> -class MediaEntity;\n> struct Rectangle;\n> \n> struct V4L2SubdeviceFormat {\n> @@ -31,7 +32,8 @@ public:\n> \tbool isOpen() const;\n> \tvoid close();\n> \n> -\tstd::string deviceNode() const { return deviceNode_; }\n> +\tstd::string deviceNode() const { return entity_->deviceNode(); }\n> +\tstd::string deviceName() const { return entity_->name(); }\n> \n> \tint setCrop(unsigned int pad, Rectangle *rect);\n> \tint setCompose(unsigned int pad, Rectangle *rect);\n> @@ -43,7 +45,7 @@ private:\n> \tint setSelection(unsigned int pad, unsigned int target,\n> \t\t\t Rectangle *rect);\n> \n> -\tstd::string deviceNode_;\n> +\tconst MediaEntity *entity_;\n> \tint fd_;\n> };\n> \n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index b436f73cc75f..56ecf3851cb0 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -88,7 +88,7 @@ LOG_DEFINE_CATEGORY(V4L2Subdev)\n> * path\n> */\n> V4L2Subdevice::V4L2Subdevice(const MediaEntity *entity)\n> -\t: deviceNode_(entity->deviceNode()), fd_(-1)\n> +\t: entity_(entity), fd_(-1)\n> {\n> }\n> \n> @@ -106,11 +106,11 @@ int V4L2Subdevice::open()\n> \t\treturn -EBUSY;\n> \t}\n> \n> -\tret = ::open(deviceNode_.c_str(), O_RDWR);\n> +\tret = ::open(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<< \"Failed to open V4L2 subdevice '\" << deviceNode()\n> \t\t\t<< \"': \" << strerror(-ret);\n> \t\treturn ret;\n> \t}\n> @@ -147,6 +147,13 @@ void V4L2Subdevice::close()\n> * \\return The subdevice's device node system path\n> */\n> \n> +/**\n> + * \\fn V4L2Subdevice::deviceName()\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> + */\n> +\n> /**\n> * \\brief Set a crop rectangle on one of the V4L2 subdevice pads\n> * \\param[in] pad The 0-indexed pad number the rectangle is to be applied to\n> @@ -189,7 +196,7 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format)\n> \t\tret = -errno;\n> \t\tLOG(V4L2Subdev, Error)\n> \t\t\t<< \"Unable to get format on pad \" << pad\n> -\t\t\t<< \" of \" << deviceNode_ << \": \" << strerror(-ret);\n> +\t\t\t<< \" of \" << deviceName() << \": \" << strerror(-ret);\n> \t\treturn ret;\n> \t}\n> \n> @@ -255,7 +262,7 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,\n> \t\tret = -errno;\n> \t\tLOG(V4L2Subdev, Error)\n> \t\t\t<< \"Unable to set rectangle \" << target << \" on pad \"\n> -\t\t\t<< pad << \" of \" << deviceNode_ << \": \"\n> +\t\t\t<< pad << \" of \" << deviceName() << \": \"\n> \t\t\t<< strerror(-ret);\n> \t\treturn ret;\n> \t}","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 077D8610B3\n\tfor ;\n\tWed, 27 Feb 2019 18:34:23 +0100 (CET)","from pendragon.ideasonboard.com (unknown [83.145.195.18])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5B2B249;\n\tWed, 27 Feb 2019 18:34:22 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1551288862;\n\tbh=E8f1Hrn6ZsNSBxDdSSaTsJHfgtLh3Aj7wwb6zaeUvns=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kMi8BBLl5TqabBfEZQIIcCT9EfwsvcexB9KIRomoy/sGSBMxcun0tXDXyobCjnO8h\n\tecqQhIaqUlVUc15cd6wKEgHDE4S+eXHbzvC8DCKBzXstSrL+MciEIze0/QnyVWAlOL\n\tEuFj8f3wjRrF2odjGdLcCRX9DnDuMNl8MIOv+aPw=","Date":"Wed, 27 Feb 2019 19:34:13 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190227173413.GK4813@pendragon.ideasonboard.com>","References":"<20190226162641.12116-1-jacopo@jmondi.org>\n\t<20190226162641.12116-2-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190226162641.12116-2-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 1/8] libcamera: v4l2_subdevice:\n\tStore 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":"Wed, 27 Feb 2019 17:34:23 -0000"}}]