[{"id":1693,"web_url":"https://patchwork.libcamera.org/comment/1693/","msgid":"<20190524075624.6lkxyyws5czawt63@uno.localdomain>","date":"2019-05-24T07:56:24","subject":"Re: [libcamera-devel] [PATCH v2 3/3] libcamera: pipeline: uvc: Use\n\tthe device to validate formats","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran,\n\nOn Thu, May 23, 2019 at 02:59:00PM +0100, Kieran Bingham wrote:\n> UVC pipelines are highly variable, and we can not define their\n> configuration restrictions within the UVC pipeline handler directly.\n>\n> Update the UVCCameraConfiguration to store the UVCCameraData (storing\n> a reference to the camera as a means of borrowing a reference to the\n> camera data).\n>\n> We then validate the configuration by the tryFormat() operation on the\n> UVC V4L2Device.\n>\n> Signed-off-by: Kieran Bingham \n> ---\n> src/libcamera/pipeline/uvcvideo.cpp | 36 +++++++++++++++++++++--------\n> 1 file changed, 26 insertions(+), 10 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index 45260f34c8f5..df321c6e64a6 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -42,9 +42,18 @@ public:\n> class UVCCameraConfiguration : public CameraConfiguration\n> {\n> public:\n> -\tUVCCameraConfiguration();\n> +\tUVCCameraConfiguration(Camera *camera, UVCCameraData *data);\n\nMinor: do you need 'camera' in UVCCameraConfiguration or just 'data' ?\nYou can pass data to the UVCCameraConfiguration constructor at\ngenerateConfiguration() time, and save storing one shared pointer.\n\nApart from this minor thing:\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n>\n> \tStatus validate() override;\n> +\n> +private:\n> +\t/*\n> +\t * The UVCCameraConfiguration instance is guaranteed to be valid as long\n> +\t * as the corresponding Camera instance is valid. In order to borrow a\n> +\t * reference to the camera data, store a new reference to the camera.\n> +\t */\n> +\tstd::shared_ptr camera_;\n> +\tconst UVCCameraData *data_;\n> };\n>\n> class PipelineHandlerUVC : public PipelineHandler\n> @@ -76,9 +85,12 @@ private:\n> \t}\n> };\n>\n> -UVCCameraConfiguration::UVCCameraConfiguration()\n> +UVCCameraConfiguration::UVCCameraConfiguration(Camera *camera,\n> +\t\t\t\t\t UVCCameraData *data)\n> \t: CameraConfiguration()\n> {\n> +\tcamera_ = camera->shared_from_this();\n> +\tdata_ = data;\n> }\n>\n> CameraConfiguration::Status UVCCameraConfiguration::validate()\n> @@ -96,17 +108,20 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()\n>\n> \tStreamConfiguration &cfg = config_[0];\n>\n> -\t/* \\todo: Validate the configuration against the device capabilities. */\n> -\tconst unsigned int pixelFormat = cfg.pixelFormat;\n> -\tconst Size size = cfg.size;\n> +\tV4L2DeviceFormat format;\n> +\tformat.fourcc = cfg.pixelFormat;\n> +\tformat.size = cfg.size;\n>\n> -\tcfg.pixelFormat = V4L2_PIX_FMT_YUYV;\n> -\tcfg.size = { 640, 480 };\n> +\t/* Validate the format on the device. */\n> +\tdata_->video_->tryFormat(&format);\n>\n> -\tif (cfg.pixelFormat != pixelFormat || cfg.size != size) {\n> +\tif (cfg.pixelFormat != format.fourcc || cfg.size != format.size) {\n> \t\tLOG(UVC, Debug)\n> \t\t\t<< \"Adjusting configuration from \" << cfg.toString()\n> -\t\t\t<< \" to \" << cfg.size.toString() << \"-YUYV\";\n> +\t\t\t<< \" to \" << format.toString();\n> +\n> +\t\tcfg.pixelFormat = format.fourcc;\n> +\t\tcfg.size = format.size;\n> \t\tstatus = Adjusted;\n> \t}\n>\n> @@ -123,7 +138,8 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)\n> CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,\n> \tconst StreamRoles &roles)\n> {\n> -\tCameraConfiguration *config = new UVCCameraConfiguration();\n> +\tUVCCameraData *data = cameraData(camera);\n> +\tCameraConfiguration *config = new UVCCameraConfiguration(camera, data);\n>\n> \tif (roles.empty())\n> \t\treturn config;\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 relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C895A60BFA\n\tfor ;\n\tFri, 24 May 2019 09:55:32 +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 relay7-d.mail.gandi.net (Postfix) with ESMTPSA id 4EDD62000C;\n\tFri, 24 May 2019 07:55:32 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Fri, 24 May 2019 09:56:24 +0200","From":"Jacopo Mondi ","To":"Kieran Bingham ","Cc":"LibCamera Devel ","Message-ID":"<20190524075624.6lkxyyws5czawt63@uno.localdomain>","References":"<20190523135900.24029-1-kieran.bingham@ideasonboard.com>\n\t<20190523135900.24029-4-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"jevbmmym7jlmgzpc\"","Content-Disposition":"inline","In-Reply-To":"<20190523135900.24029-4-kieran.bingham@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] libcamera: pipeline: uvc: Use\n\tthe device to validate formats","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, 24 May 2019 07:55:33 -0000"}},{"id":1694,"web_url":"https://patchwork.libcamera.org/comment/1694/","msgid":"<87bb1411-f982-f8b8-5f58-9beb82513cd1@ideasonboard.com>","date":"2019-05-24T08:25:16","subject":"Re: [libcamera-devel] [PATCH v2 3/3] libcamera: pipeline: uvc: Use\n\tthe device to validate formats","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 24/05/2019 08:56, Jacopo Mondi wrote:\n> Hi Kieran,\n> \n> On Thu, May 23, 2019 at 02:59:00PM +0100, Kieran Bingham wrote:\n>> UVC pipelines are highly variable, and we can not define their\n>> configuration restrictions within the UVC pipeline handler directly.\n>>\n>> Update the UVCCameraConfiguration to store the UVCCameraData (storing\n>> a reference to the camera as a means of borrowing a reference to the\n>> camera data).\n>>\n>> We then validate the configuration by the tryFormat() operation on the\n>> UVC V4L2Device.\n>>\n>> Signed-off-by: Kieran Bingham \n>> ---\n>> src/libcamera/pipeline/uvcvideo.cpp | 36 +++++++++++++++++++++--------\n>> 1 file changed, 26 insertions(+), 10 deletions(-)\n>>\n>> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n>> index 45260f34c8f5..df321c6e64a6 100644\n>> --- a/src/libcamera/pipeline/uvcvideo.cpp\n>> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n>> @@ -42,9 +42,18 @@ public:\n>> class UVCCameraConfiguration : public CameraConfiguration\n>> {\n>> public:\n>> -\tUVCCameraConfiguration();\n>> +\tUVCCameraConfiguration(Camera *camera, UVCCameraData *data);\n> \n> Minor: do you need 'camera' in UVCCameraConfiguration or just 'data' ?\n> You can pass data to the UVCCameraConfiguration constructor at\n> generateConfiguration() time, and save storing one shared pointer.\n\nI added it due to my interpretation of the comment in\nRkISP1CameraConfiguration (which I have replicated below) as requiring a\nreference to the Camera to ensure that the CameraData is valid.\n\n\n> Apart from this minor thing:\n> Reviewed-by: Jacopo Mondi \n\nThank you,\n\nI think Laurent has some reservations about this ... as Niklas is\nworking on some different enumerations.\n\nBut I still need this series to be able to utilise libcamera on my\ndevelopment laptop, so of course I'd like it to go in ...\n\n\n> \n> Thanks\n> j\n> \n>>\n>> \tStatus validate() override;\n>> +\n>> +private:\n>> +\t/*\n>> +\t * The UVCCameraConfiguration instance is guaranteed to be valid as long\n>> +\t * as the corresponding Camera instance is valid. In order to borrow a\n>> +\t * reference to the camera data, store a new reference to the camera.\n>> +\t */\n\nHere: ^^^^^^^^^^^^^^^^^^^\n\n\n\n>> +\tstd::shared_ptr camera_;\n>> +\tconst UVCCameraData *data_;\n>> };\n>>\n>> class PipelineHandlerUVC : public PipelineHandler\n>> @@ -76,9 +85,12 @@ private:\n>> \t}\n>> };\n>>\n>> -UVCCameraConfiguration::UVCCameraConfiguration()\n>> +UVCCameraConfiguration::UVCCameraConfiguration(Camera *camera,\n>> +\t\t\t\t\t UVCCameraData *data)\n>> \t: CameraConfiguration()\n>> {\n>> +\tcamera_ = camera->shared_from_this();\n>> +\tdata_ = data;\n>> }\n>>\n>> CameraConfiguration::Status UVCCameraConfiguration::validate()\n>> @@ -96,17 +108,20 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()\n>>\n>> \tStreamConfiguration &cfg = config_[0];\n>>\n>> -\t/* \\todo: Validate the configuration against the device capabilities. */\n>> -\tconst unsigned int pixelFormat = cfg.pixelFormat;\n>> -\tconst Size size = cfg.size;\n>> +\tV4L2DeviceFormat format;\n>> +\tformat.fourcc = cfg.pixelFormat;\n>> +\tformat.size = cfg.size;\n>>\n>> -\tcfg.pixelFormat = V4L2_PIX_FMT_YUYV;\n>> -\tcfg.size = { 640, 480 };\n>> +\t/* Validate the format on the device. */\n>> +\tdata_->video_->tryFormat(&format);\n>>\n>> -\tif (cfg.pixelFormat != pixelFormat || cfg.size != size) {\n>> +\tif (cfg.pixelFormat != format.fourcc || cfg.size != format.size) {\n>> \t\tLOG(UVC, Debug)\n>> \t\t\t<< \"Adjusting configuration from \" << cfg.toString()\n>> -\t\t\t<< \" to \" << cfg.size.toString() << \"-YUYV\";\n>> +\t\t\t<< \" to \" << format.toString();\n>> +\n>> +\t\tcfg.pixelFormat = format.fourcc;\n>> +\t\tcfg.size = format.size;\n>> \t\tstatus = Adjusted;\n>> \t}\n>>\n>> @@ -123,7 +138,8 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)\n>> CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,\n>> \tconst StreamRoles &roles)\n>> {\n>> -\tCameraConfiguration *config = new UVCCameraConfiguration();\n>> +\tUVCCameraData *data = cameraData(camera);\n>> +\tCameraConfiguration *config = new UVCCameraConfiguration(camera, data);\n>>\n>> \tif (roles.empty())\n>> \t\treturn config;\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 perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2122F60BFA\n\tfor ;\n\tFri, 24 May 2019 10:25:20 +0200 (CEST)","from [192.168.0.20]\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 8EF132DE;\n\tFri, 24 May 2019 10:25:19 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1558686319;\n\tbh=KAKh3vskmMT6oC9umhgaGptu69NQ+c+HmS68QrxaQSs=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=eaY1C8o+Y/QuGLvm2NjvRDX4FarcTdtlVGtx6TVBj3equhc6FybHhCeKGFYun3jyg\n\th+cP40ivyenViHmzdI/bopQ2iNZkC8AYwHgZ7EU8cDKzK8iSZNJgwNa4nR6RpRfUfZ\n\tI9qiPGYP+YF1EeHmn/PeBNU6bYuXDjLKPLJBrK10=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Jacopo Mondi ","Cc":"LibCamera Devel ","References":"<20190523135900.24029-1-kieran.bingham@ideasonboard.com>\n\t<20190523135900.24029-4-kieran.bingham@ideasonboard.com>\n\t<20190524075624.6lkxyyws5czawt63@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":"<87bb1411-f982-f8b8-5f58-9beb82513cd1@ideasonboard.com>","Date":"Fri, 24 May 2019 09:25:16 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.6.1","MIME-Version":"1.0","In-Reply-To":"<20190524075624.6lkxyyws5czawt63@uno.localdomain>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] libcamera: pipeline: uvc: Use\n\tthe device to validate formats","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, 24 May 2019 08:25:20 -0000"}},{"id":1695,"web_url":"https://patchwork.libcamera.org/comment/1695/","msgid":"<20190524084532.u4c7advhqa7kl7ml@uno.localdomain>","date":"2019-05-24T08:45:32","subject":"Re: [libcamera-devel] [PATCH v2 3/3] libcamera: pipeline: uvc: Use\n\tthe device to validate formats","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran,\n\nOn Fri, May 24, 2019 at 09:25:16AM +0100, Kieran Bingham wrote:\n> Hi Jacopo,\n>\n> On 24/05/2019 08:56, Jacopo Mondi wrote:\n> > Hi Kieran,\n> >\n> > On Thu, May 23, 2019 at 02:59:00PM +0100, Kieran Bingham wrote:\n> >> UVC pipelines are highly variable, and we can not define their\n> >> configuration restrictions within the UVC pipeline handler directly.\n> >>\n> >> Update the UVCCameraConfiguration to store the UVCCameraData (storing\n> >> a reference to the camera as a means of borrowing a reference to the\n> >> camera data).\n> >>\n> >> We then validate the configuration by the tryFormat() operation on the\n> >> UVC V4L2Device.\n> >>\n> >> Signed-off-by: Kieran Bingham \n> >> ---\n> >> src/libcamera/pipeline/uvcvideo.cpp | 36 +++++++++++++++++++++--------\n> >> 1 file changed, 26 insertions(+), 10 deletions(-)\n> >>\n> >> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> >> index 45260f34c8f5..df321c6e64a6 100644\n> >> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> >> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> >> @@ -42,9 +42,18 @@ public:\n> >> class UVCCameraConfiguration : public CameraConfiguration\n> >> {\n> >> public:\n> >> -\tUVCCameraConfiguration();\n> >> +\tUVCCameraConfiguration(Camera *camera, UVCCameraData *data);\n> >\n> > Minor: do you need 'camera' in UVCCameraConfiguration or just 'data' ?\n> > You can pass data to the UVCCameraConfiguration constructor at\n> > generateConfiguration() time, and save storing one shared pointer.\n>\n> I added it due to my interpretation of the comment in\n> RkISP1CameraConfiguration (which I have replicated below) as requiring a\n> reference to the Camera to ensure that the CameraData is valid.\n\nCorrect, I've also been told offline just yesterday that this is\nmostly for reference counting on the Camera instance. Please ignore my\ncomment then!\n\n>\n>\n> > Apart from this minor thing:\n> > Reviewed-by: Jacopo Mondi \n>\n> Thank you,\n>\n> I think Laurent has some reservations about this ... as Niklas is\n> working on some different enumerations.\n>\n> But I still need this series to be able to utilise libcamera on my\n> development laptop, so of course I'd like it to go in ...\n\nLooking at the code I would have preferred too to see the UVC pipeline\nhandler matching the required configuration against the list of\nenumerated formats, but since we don't have it yet... :)\n\nAnyway, the patch itself is good to me, let's wait to hear from others\nif we want to wait for format enumeration support to land first.\n\n>\n>\n> >\n> > Thanks\n> > j\n> >\n> >>\n> >> \tStatus validate() override;\n> >> +\n> >> +private:\n> >> +\t/*\n> >> +\t * The UVCCameraConfiguration instance is guaranteed to be valid as long\n> >> +\t * as the corresponding Camera instance is valid. In order to borrow a\n> >> +\t * reference to the camera data, store a new reference to the camera.\n> >> +\t */\n>\n> Here: ^^^^^^^^^^^^^^^^^^^\n>\n>\n>\n> >> +\tstd::shared_ptr camera_;\n> >> +\tconst UVCCameraData *data_;\n> >> };\n> >>\n> >> class PipelineHandlerUVC : public PipelineHandler\n> >> @@ -76,9 +85,12 @@ private:\n> >> \t}\n> >> };\n> >>\n> >> -UVCCameraConfiguration::UVCCameraConfiguration()\n> >> +UVCCameraConfiguration::UVCCameraConfiguration(Camera *camera,\n> >> +\t\t\t\t\t UVCCameraData *data)\n> >> \t: CameraConfiguration()\n> >> {\n> >> +\tcamera_ = camera->shared_from_this();\n> >> +\tdata_ = data;\n> >> }\n> >>\n> >> CameraConfiguration::Status UVCCameraConfiguration::validate()\n> >> @@ -96,17 +108,20 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()\n> >>\n> >> \tStreamConfiguration &cfg = config_[0];\n> >>\n> >> -\t/* \\todo: Validate the configuration against the device capabilities. */\n> >> -\tconst unsigned int pixelFormat = cfg.pixelFormat;\n> >> -\tconst Size size = cfg.size;\n> >> +\tV4L2DeviceFormat format;\n> >> +\tformat.fourcc = cfg.pixelFormat;\n> >> +\tformat.size = cfg.size;\n> >>\n> >> -\tcfg.pixelFormat = V4L2_PIX_FMT_YUYV;\n> >> -\tcfg.size = { 640, 480 };\n> >> +\t/* Validate the format on the device. */\n> >> +\tdata_->video_->tryFormat(&format);\n> >>\n> >> -\tif (cfg.pixelFormat != pixelFormat || cfg.size != size) {\n> >> +\tif (cfg.pixelFormat != format.fourcc || cfg.size != format.size) {\n> >> \t\tLOG(UVC, Debug)\n> >> \t\t\t<< \"Adjusting configuration from \" << cfg.toString()\n> >> -\t\t\t<< \" to \" << cfg.size.toString() << \"-YUYV\";\n> >> +\t\t\t<< \" to \" << format.toString();\n> >> +\n> >> +\t\tcfg.pixelFormat = format.fourcc;\n> >> +\t\tcfg.size = format.size;\n> >> \t\tstatus = Adjusted;\n> >> \t}\n> >>\n> >> @@ -123,7 +138,8 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)\n> >> CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,\n> >> \tconst StreamRoles &roles)\n> >> {\n> >> -\tCameraConfiguration *config = new UVCCameraConfiguration();\n> >> +\tUVCCameraData *data = cameraData(camera);\n> >> +\tCameraConfiguration *config = new UVCCameraConfiguration(camera, data);\n> >>\n> >> \tif (roles.empty())\n> >> \t\treturn config;\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> --\n> Kieran","headers":{"Return-Path":"","Received":["from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1776160BFA\n\tfor ;\n\tFri, 24 May 2019 10:44:27 +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 relay10.mail.gandi.net (Postfix) with ESMTPSA id CCA3924000A;\n\tFri, 24 May 2019 08:44:24 +0000 (UTC)"],"Date":"Fri, 24 May 2019 10:45:32 +0200","From":"Jacopo Mondi ","To":"Kieran Bingham ","Cc":"LibCamera Devel ","Message-ID":"<20190524084532.u4c7advhqa7kl7ml@uno.localdomain>","References":"<20190523135900.24029-1-kieran.bingham@ideasonboard.com>\n\t<20190523135900.24029-4-kieran.bingham@ideasonboard.com>\n\t<20190524075624.6lkxyyws5czawt63@uno.localdomain>\n\t<87bb1411-f982-f8b8-5f58-9beb82513cd1@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"o7ocazkiijbytbtt\"","Content-Disposition":"inline","In-Reply-To":"<87bb1411-f982-f8b8-5f58-9beb82513cd1@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] libcamera: pipeline: uvc: Use\n\tthe device to validate formats","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, 24 May 2019 08:44:27 -0000"}}]