[{"id":656,"web_url":"https://patchwork.libcamera.org/comment/656/","msgid":"<2698a85d-df00-12d3-652f-04e21ea6b6c8@ideasonboard.com>","date":"2019-01-28T15:42:22","subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: v4l2-format: Add\n\tV4L2DeviceFormat","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 28/01/2019 15:11, Jacopo Mondi wrote:\n> Add a V4L2DeviceFormat class aimed to be used to provide format configuration\n> requests to a V4L2Device.\n> \n> Signed-off-by: Jacopo Mondi \n\nReviewed-by: Kieran Bingham \n\n> ---\n> src/libcamera/include/v4l2_device.h | 14 ++++++++\n> src/libcamera/v4l2_device.cpp | 56 +++++++++++++++++++++++++++++\n> 2 files changed, 70 insertions(+)\n> \n> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> index c67ebbf..c70959e 100644\n> --- a/src/libcamera/include/v4l2_device.h\n> +++ b/src/libcamera/include/v4l2_device.h\n> @@ -53,6 +53,20 @@ struct V4L2Capability final : v4l2_capability {\n> \t}\n> };\n> \n> +class V4L2DeviceFormat\n> +{\n> +public:\n> +\tuint32_t width;\n> +\tuint32_t height;\n> +\tuint32_t fourcc;\n\nI might have called this pixelFormat, but it's internal and it can\nchange if we later define a PixelFormat class or such.\n\nCurrently we will store a fourcc so this is accurate.\n\n> +\n> +\tstruct {\n> +\t\tuint32_t size;\n> +\t\tuint32_t bpl;\n> +\t} planesFmt[3];\n> +\tunsigned int planes;\n> +};\n> +\n> class MediaEntity;\n> class V4L2Device\n> {\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 408f9b9..d6143f2 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -80,6 +80,62 @@ LOG_DEFINE_CATEGORY(V4L2)\n> * \\return True if the device provides Streaming I/O IOCTLs\n> */\n> \n> +/**\n> + * \\class V4L2DeviceFormat\n> + * \\brief The V4L2 device image format and sizes\n> + *\n> + * Describes the image format and image sizes to be programmed on a V4L2\n> + * video device. The image format is defined by fourcc code as defined by\n> + * the V4L2 APIs with the V4L2_PIX_FMT_ macros, a visible width and height\n> + * and a variable number of planes (1 to 3) with variable sizes and line\n> + * strides.\n> + *\n> + * Formats defined as 'single planar' by the V4L2 APIs are represented with\n> + * V4L2DeviceFormat instances with a single plane\n> + * (V4L2DeviceFormat::planes = 1). Semi-planar and multiplanar formats use\n> + * 2 and 3 planes respectively.\n> + *\n> + * V4L2DeviceFormat defines the exchange format between components that\n> + * receive image configuration requests from applications and a V4L2Device.\n> + * The V4L2Device validates and applies the requested size and format to\n> + * the device driver.\n> + */\n> +\n> +/**\n> + * \\var V4L2DeviceFormat::width\n> + * \\brief The image width\n> + */\n> +\n> +/**\n> + * \\var V4L2DeviceFormat::height\n> + * \\brief The image height\n> + */\n> +\n> +/**\n> + * \\var V4L2DeviceFormat::fourcc> + * \\brief The pixel encoding scheme\n> + *\n> + * The fourcc code, as defined by the V4L2 APIs with the V4L2_PIX_FMT_ macros,\n> + * that identifies the image format pixel encoding scheme.\n> + */\n> +\n> +/**\n> + * \\var V4L2DeviceFormat::planesFmt\n> + * \\brief The per-plane size information\n> + *\n> + * Images are stored in memory in one or more data planes. Each data plane\n> + * has a specific size and line length, which could differ from the image\n> + * visible sizes to accommodate line or plane padding data.\n> + *\n> + * Only the first V4L2DeviceFormat::planes entries are considered valid.\n> + *\n> + */\n> +\n> +/**\n> + * \\var V4L2DeviceFormat::planes\n> + * \\brief The number of valid data planes\n> + */\n> +\n> /**\n> * \\class V4L2Device\n> * \\brief V4L2Device object and API\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 4061060DB6\n\tfor ;\n\tMon, 28 Jan 2019 16:42:26 +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 A2ADC85;\n\tMon, 28 Jan 2019 16:42:25 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548690145;\n\tbh=PMltcrqfwbVWXqFGlyKnLy3vBfYU7ZKGXZdEq1mDIlI=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=XlkPxLQX8BJ/7f6dFpyzh8I5CUB2YOUBgKdMKkNwtuhoK3UNxuehwSkFRykKCbZgX\n\tjVKMgZYFG+jERgSgUT6iFWtxC/i20FsASGnxjfv4u/5XQdq9Hq+N1nazQJtr2nAHV1\n\teLcJoD7prqR945hsePZTXRughAwvW/sRGEDspLuc=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","References":"<20190128151137.31075-1-jacopo@jmondi.org>\n\t<20190128151137.31075-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":"<2698a85d-df00-12d3-652f-04e21ea6b6c8@ideasonboard.com>","Date":"Mon, 28 Jan 2019 15:42:22 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.2.1","MIME-Version":"1.0","In-Reply-To":"<20190128151137.31075-2-jacopo@jmondi.org>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: v4l2-format: Add\n\tV4L2DeviceFormat","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, 28 Jan 2019 15:42:26 -0000"}},{"id":658,"web_url":"https://patchwork.libcamera.org/comment/658/","msgid":"<451742d7-af82-1f17-bf20-4a425ec2b9c2@ideasonboard.com>","date":"2019-01-28T15:57:45","subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: v4l2-format: Add\n\tV4L2DeviceFormat","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOne more thing...\n\nOn 28/01/2019 15:42, Kieran Bingham wrote:\n> Hi Jacopo,\n> \n> On 28/01/2019 15:11, Jacopo Mondi wrote:\n>> Add a V4L2DeviceFormat class aimed to be used to provide format configuration\n>> requests to a V4L2Device.\n>>\n>> Signed-off-by: Jacopo Mondi \n> \n> Reviewed-by: Kieran Bingham \n> \n>> ---\n>> src/libcamera/include/v4l2_device.h | 14 ++++++++\n>> src/libcamera/v4l2_device.cpp | 56 +++++++++++++++++++++++++++++\n>> 2 files changed, 70 insertions(+)\n>>\n>> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n>> index c67ebbf..c70959e 100644\n>> --- a/src/libcamera/include/v4l2_device.h\n>> +++ b/src/libcamera/include/v4l2_device.h\n>> @@ -53,6 +53,20 @@ struct V4L2Capability final : v4l2_capability {\n>> \t}\n>> };\n>> \n>> +class V4L2DeviceFormat\n>> +{\n>> +public:\n>> +\tuint32_t width;\n>> +\tuint32_t height;\n>> +\tuint32_t fourcc;\n> \n> I might have called this pixelFormat, but it's internal and it can\n> change if we later define a PixelFormat class or such.\n> \n> Currently we will store a fourcc so this is accurate.\n\n\nstruct v4l2_pix_format *pix stores the following extra information:\n\n\tfield (we might need to consider interlaced somewhere?)\n\tcolorspace,\n\tflags\n\tycbcr_enc, hsv_enc\n\tquantization\n\txfer_func\n\nShould we be handling any of that information here too?\n\nOr should we add it on an as-required basis?\n\n--\nKieran\n\n\n\n> \n>> +\n>> +\tstruct {\n>> +\t\tuint32_t size;\n>> +\t\tuint32_t bpl;\n>> +\t} planesFmt[3];\n>> +\tunsigned int planes;\n>> +};\n>> +\n>> class MediaEntity;\n>> class V4L2Device\n>> {\n>> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n>> index 408f9b9..d6143f2 100644\n>> --- a/src/libcamera/v4l2_device.cpp\n>> +++ b/src/libcamera/v4l2_device.cpp\n>> @@ -80,6 +80,62 @@ LOG_DEFINE_CATEGORY(V4L2)\n>> * \\return True if the device provides Streaming I/O IOCTLs\n>> */\n>> \n>> +/**\n>> + * \\class V4L2DeviceFormat\n>> + * \\brief The V4L2 device image format and sizes\n>> + *\n>> + * Describes the image format and image sizes to be programmed on a V4L2\n>> + * video device. The image format is defined by fourcc code as defined by\n>> + * the V4L2 APIs with the V4L2_PIX_FMT_ macros, a visible width and height\n>> + * and a variable number of planes (1 to 3) with variable sizes and line\n>> + * strides.\n>> + *\n>> + * Formats defined as 'single planar' by the V4L2 APIs are represented with\n>> + * V4L2DeviceFormat instances with a single plane\n>> + * (V4L2DeviceFormat::planes = 1). Semi-planar and multiplanar formats use\n>> + * 2 and 3 planes respectively.\n>> + *\n>> + * V4L2DeviceFormat defines the exchange format between components that\n>> + * receive image configuration requests from applications and a V4L2Device.\n>> + * The V4L2Device validates and applies the requested size and format to\n>> + * the device driver.\n>> + */\n>> +\n>> +/**\n>> + * \\var V4L2DeviceFormat::width\n>> + * \\brief The image width\n>> + */\n>> +\n>> +/**\n>> + * \\var V4L2DeviceFormat::height\n>> + * \\brief The image height\n>> + */\n>> +\n>> +/**\n>> + * \\var V4L2DeviceFormat::fourcc> + * \\brief The pixel encoding scheme\n>> + *\n>> + * The fourcc code, as defined by the V4L2 APIs with the V4L2_PIX_FMT_ macros,\n>> + * that identifies the image format pixel encoding scheme.\n>> + */\n>> +\n>> +/**\n>> + * \\var V4L2DeviceFormat::planesFmt\n>> + * \\brief The per-plane size information\n>> + *\n>> + * Images are stored in memory in one or more data planes. Each data plane\n>> + * has a specific size and line length, which could differ from the image\n>> + * visible sizes to accommodate line or plane padding data.\n>> + *\n>> + * Only the first V4L2DeviceFormat::planes entries are considered valid.\n>> + *\n>> + */\n>> +\n>> +/**\n>> + * \\var V4L2DeviceFormat::planes\n>> + * \\brief The number of valid data planes\n>> + */\n>> +\n>> /**\n>> * \\class V4L2Device\n>> * \\brief V4L2Device object and API\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 5887160DB7\n\tfor ;\n\tMon, 28 Jan 2019 16:57:49 +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 B871B85;\n\tMon, 28 Jan 2019 16:57:48 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548691068;\n\tbh=AEbs7mh9A6s2THwMfII2eMnCMbE+K8h+5fEjYKPjR+I=;\n\th=Reply-To:Subject:From:To:References:Date:In-Reply-To:From;\n\tb=EiUM1P5CBBfO0LiFA7SCxD/c0VF8AI6RAeHIm9vIb0QO+ejkDrmU9CT5yGf18RTyt\n\t/tndy5Le8FdUk3FgzqGMlCx080ba7m0hYWf+1hxeARBPYZPVAGW5nH11MKQ6DCaVCR\n\tpZyB+jufFPAlfXQOrB8w3bIc0FXKMSNuDgliV2ro=","Reply-To":"kieran.bingham@ideasonboard.com","From":"Kieran Bingham ","To":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","References":"<20190128151137.31075-1-jacopo@jmondi.org>\n\t<20190128151137.31075-2-jacopo@jmondi.org>\n\t<2698a85d-df00-12d3-652f-04e21ea6b6c8@ideasonboard.com>","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":"<451742d7-af82-1f17-bf20-4a425ec2b9c2@ideasonboard.com>","Date":"Mon, 28 Jan 2019 15:57:45 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.2.1","MIME-Version":"1.0","In-Reply-To":"<2698a85d-df00-12d3-652f-04e21ea6b6c8@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: v4l2-format: Add\n\tV4L2DeviceFormat","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, 28 Jan 2019 15:57:49 -0000"}},{"id":660,"web_url":"https://patchwork.libcamera.org/comment/660/","msgid":"<20190128162002.6bz3phpxnnqrkjda@uno.localdomain>","date":"2019-01-28T16:20:02","subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: v4l2-format: Add\n\tV4L2DeviceFormat","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran,\n\nOn Mon, Jan 28, 2019 at 03:57:45PM +0000, Kieran Bingham wrote:\n> Hi Jacopo,\n>\n> One more thing...\n>\n> On 28/01/2019 15:42, Kieran Bingham wrote:\n> > Hi Jacopo,\n> >\n> > On 28/01/2019 15:11, Jacopo Mondi wrote:\n> >> Add a V4L2DeviceFormat class aimed to be used to provide format configuration\n> >> requests to a V4L2Device.\n> >>\n> >> Signed-off-by: Jacopo Mondi \n> >\n> > Reviewed-by: Kieran Bingham \n> >\n> >> ---\n> >> src/libcamera/include/v4l2_device.h | 14 ++++++++\n> >> src/libcamera/v4l2_device.cpp | 56 +++++++++++++++++++++++++++++\n> >> 2 files changed, 70 insertions(+)\n> >>\n> >> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> >> index c67ebbf..c70959e 100644\n> >> --- a/src/libcamera/include/v4l2_device.h\n> >> +++ b/src/libcamera/include/v4l2_device.h\n> >> @@ -53,6 +53,20 @@ struct V4L2Capability final : v4l2_capability {\n> >> \t}\n> >> };\n> >>\n> >> +class V4L2DeviceFormat\n> >> +{\n> >> +public:\n> >> +\tuint32_t width;\n> >> +\tuint32_t height;\n> >> +\tuint32_t fourcc;\n> >\n> > I might have called this pixelFormat, but it's internal and it can\n> > change if we later define a PixelFormat class or such.\n> >\n> > Currently we will store a fourcc so this is accurate.\n>\n>\n> struct v4l2_pix_format *pix stores the following extra information:\n\nNot only v4l2_pix_format, but v4l2_pix_format_mplane and\nv4l2_mbus_framefmt too...\n\n>\n> \tfield (we might need to consider interlaced somewhere?)\n> \tcolorspace,\n> \tflags\n> \tycbcr_enc, hsv_enc\n> \tquantization\n> \txfer_func\n\nRight now, I cannot tell. I'm sure there are use cases where those\ninformations are relevant.\n\n>\n> Should we be handling any of that information here too?\n>\n> Or should we add it on an as-required basis?\n\nI think they just need to be copied back and forth from the ioctl\narguments, once the V4L2DeviceFormat has been expanded to include\nthese fields.\n\nI'll leave it for later, but if anyone has other opinions we can look\ninto that now.\n\nThanks\n j\n\n>\n> --\n> Kieran\n>\n>\n>\n> >\n> >> +\n> >> +\tstruct {\n> >> +\t\tuint32_t size;\n> >> +\t\tuint32_t bpl;\n> >> +\t} planesFmt[3];\n> >> +\tunsigned int planes;\n> >> +};\n> >> +\n> >> class MediaEntity;\n> >> class V4L2Device\n> >> {\n> >> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> >> index 408f9b9..d6143f2 100644\n> >> --- a/src/libcamera/v4l2_device.cpp\n> >> +++ b/src/libcamera/v4l2_device.cpp\n> >> @@ -80,6 +80,62 @@ LOG_DEFINE_CATEGORY(V4L2)\n> >> * \\return True if the device provides Streaming I/O IOCTLs\n> >> */\n> >>\n> >> +/**\n> >> + * \\class V4L2DeviceFormat\n> >> + * \\brief The V4L2 device image format and sizes\n> >> + *\n> >> + * Describes the image format and image sizes to be programmed on a V4L2\n> >> + * video device. The image format is defined by fourcc code as defined by\n> >> + * the V4L2 APIs with the V4L2_PIX_FMT_ macros, a visible width and height\n> >> + * and a variable number of planes (1 to 3) with variable sizes and line\n> >> + * strides.\n> >> + *\n> >> + * Formats defined as 'single planar' by the V4L2 APIs are represented with\n> >> + * V4L2DeviceFormat instances with a single plane\n> >> + * (V4L2DeviceFormat::planes = 1). Semi-planar and multiplanar formats use\n> >> + * 2 and 3 planes respectively.\n> >> + *\n> >> + * V4L2DeviceFormat defines the exchange format between components that\n> >> + * receive image configuration requests from applications and a V4L2Device.\n> >> + * The V4L2Device validates and applies the requested size and format to\n> >> + * the device driver.\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\var V4L2DeviceFormat::width\n> >> + * \\brief The image width\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\var V4L2DeviceFormat::height\n> >> + * \\brief The image height\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\var V4L2DeviceFormat::fourcc> + * \\brief The pixel encoding scheme\n> >> + *\n> >> + * The fourcc code, as defined by the V4L2 APIs with the V4L2_PIX_FMT_ macros,\n> >> + * that identifies the image format pixel encoding scheme.\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\var V4L2DeviceFormat::planesFmt\n> >> + * \\brief The per-plane size information\n> >> + *\n> >> + * Images are stored in memory in one or more data planes. Each data plane\n> >> + * has a specific size and line length, which could differ from the image\n> >> + * visible sizes to accommodate line or plane padding data.\n> >> + *\n> >> + * Only the first V4L2DeviceFormat::planes entries are considered valid.\n> >> + *\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\var V4L2DeviceFormat::planes\n> >> + * \\brief The number of valid data planes\n> >> + */\n> >> +\n> >> /**\n> >> * \\class V4L2Device\n> >> * \\brief V4L2Device object and API\n> >>\n> >\n>\n> --\n> Regards\n> --\n> Kieran","headers":{"Return-Path":"","Received":["from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5CB7B60DB7\n\tfor ;\n\tMon, 28 Jan 2019 17:19:47 +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 relay4-d.mail.gandi.net (Postfix) with ESMTPSA id CA1C9E0013;\n\tMon, 28 Jan 2019 16:19:46 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Mon, 28 Jan 2019 17:20:02 +0100","From":"Jacopo Mondi ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190128162002.6bz3phpxnnqrkjda@uno.localdomain>","References":"<20190128151137.31075-1-jacopo@jmondi.org>\n\t<20190128151137.31075-2-jacopo@jmondi.org>\n\t<2698a85d-df00-12d3-652f-04e21ea6b6c8@ideasonboard.com>\n\t<451742d7-af82-1f17-bf20-4a425ec2b9c2@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"2jcxtieefsrrpe6b\"","Content-Disposition":"inline","In-Reply-To":"<451742d7-af82-1f17-bf20-4a425ec2b9c2@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: v4l2-format: Add\n\tV4L2DeviceFormat","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, 28 Jan 2019 16:19:47 -0000"}},{"id":687,"web_url":"https://patchwork.libcamera.org/comment/687/","msgid":"<3dfbc5a3-6bea-69de-315b-20bd9bebb866@ideasonboard.com>","date":"2019-01-29T13:58:45","subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: v4l2-format: Add\n\tV4L2DeviceFormat","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 28/01/2019 16:20, Jacopo Mondi wrote:\n> Hi Kieran,\n> \n> On Mon, Jan 28, 2019 at 03:57:45PM +0000, Kieran Bingham wrote:\n>> Hi Jacopo,\n>>\n>> One more thing...\n>>\n>> On 28/01/2019 15:42, Kieran Bingham wrote:\n>>> Hi Jacopo,\n>>>\n>>> On 28/01/2019 15:11, Jacopo Mondi wrote:\n>>>> Add a V4L2DeviceFormat class aimed to be used to provide format configuration\n>>>> requests to a V4L2Device.\n>>>>\n>>>> Signed-off-by: Jacopo Mondi \n>>>\n>>> Reviewed-by: Kieran Bingham \n>>>\n>>>> ---\n>>>> src/libcamera/include/v4l2_device.h | 14 ++++++++\n>>>> src/libcamera/v4l2_device.cpp | 56 +++++++++++++++++++++++++++++\n>>>> 2 files changed, 70 insertions(+)\n>>>>\n>>>> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n>>>> index c67ebbf..c70959e 100644\n>>>> --- a/src/libcamera/include/v4l2_device.h\n>>>> +++ b/src/libcamera/include/v4l2_device.h\n>>>> @@ -53,6 +53,20 @@ struct V4L2Capability final : v4l2_capability {\n>>>> \t}\n>>>> };\n>>>>\n>>>> +class V4L2DeviceFormat\n>>>> +{\n>>>> +public:\n>>>> +\tuint32_t width;\n>>>> +\tuint32_t height;\n>>>> +\tuint32_t fourcc;\n>>>\n>>> I might have called this pixelFormat, but it's internal and it can\n>>> change if we later define a PixelFormat class or such.\n>>>\n>>> Currently we will store a fourcc so this is accurate.\n>>\n>>\n>> struct v4l2_pix_format *pix stores the following extra information:\n> \n> Not only v4l2_pix_format, but v4l2_pix_format_mplane and\n> v4l2_mbus_framefmt too...\n> \n>>\n>> \tfield (we might need to consider interlaced somewhere?)\n>> \tcolorspace,\n>> \tflags\n>> \tycbcr_enc, hsv_enc\n>> \tquantization\n>> \txfer_func\n> \n> Right now, I cannot tell. I'm sure there are use cases where those\n> informations are relevant.\n> \n>>\n>> Should we be handling any of that information here too?\n>>\n>> Or should we add it on an as-required basis?\n> \n> I think they just need to be copied back and forth from the ioctl\n> arguments, once the V4L2DeviceFormat has been expanded to include\n> these fields.\n> \n> I'll leave it for later, but if anyone has other opinions we can look\n> into that now.\n\nLeaving it until it is needed is fine with me.\n--\nKieran\n\n\n> \n> Thanks\n> j\n> \n>>\n>> --\n>> Kieran\n>>\n>>\n>>\n>>>\n>>>> +\n>>>> +\tstruct {\n>>>> +\t\tuint32_t size;\n>>>> +\t\tuint32_t bpl;\n>>>> +\t} planesFmt[3];\n>>>> +\tunsigned int planes;\n>>>> +};\n>>>> +\n>>>> class MediaEntity;\n>>>> class V4L2Device\n>>>> {\n>>>> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n>>>> index 408f9b9..d6143f2 100644\n>>>> --- a/src/libcamera/v4l2_device.cpp\n>>>> +++ b/src/libcamera/v4l2_device.cpp\n>>>> @@ -80,6 +80,62 @@ LOG_DEFINE_CATEGORY(V4L2)\n>>>> * \\return True if the device provides Streaming I/O IOCTLs\n>>>> */\n>>>>\n>>>> +/**\n>>>> + * \\class V4L2DeviceFormat\n>>>> + * \\brief The V4L2 device image format and sizes\n>>>> + *\n>>>> + * Describes the image format and image sizes to be programmed on a V4L2\n>>>> + * video device. The image format is defined by fourcc code as defined by\n>>>> + * the V4L2 APIs with the V4L2_PIX_FMT_ macros, a visible width and height\n>>>> + * and a variable number of planes (1 to 3) with variable sizes and line\n>>>> + * strides.\n>>>> + *\n>>>> + * Formats defined as 'single planar' by the V4L2 APIs are represented with\n>>>> + * V4L2DeviceFormat instances with a single plane\n>>>> + * (V4L2DeviceFormat::planes = 1). Semi-planar and multiplanar formats use\n>>>> + * 2 and 3 planes respectively.\n>>>> + *\n>>>> + * V4L2DeviceFormat defines the exchange format between components that\n>>>> + * receive image configuration requests from applications and a V4L2Device.\n>>>> + * The V4L2Device validates and applies the requested size and format to\n>>>> + * the device driver.\n>>>> + */\n>>>> +\n>>>> +/**\n>>>> + * \\var V4L2DeviceFormat::width\n>>>> + * \\brief The image width\n>>>> + */\n>>>> +\n>>>> +/**\n>>>> + * \\var V4L2DeviceFormat::height\n>>>> + * \\brief The image height\n>>>> + */\n>>>> +\n>>>> +/**\n>>>> + * \\var V4L2DeviceFormat::fourcc> + * \\brief The pixel encoding scheme\n>>>> + *\n>>>> + * The fourcc code, as defined by the V4L2 APIs with the V4L2_PIX_FMT_ macros,\n>>>> + * that identifies the image format pixel encoding scheme.\n>>>> + */\n>>>> +\n>>>> +/**\n>>>> + * \\var V4L2DeviceFormat::planesFmt\n>>>> + * \\brief The per-plane size information\n>>>> + *\n>>>> + * Images are stored in memory in one or more data planes. Each data plane\n>>>> + * has a specific size and line length, which could differ from the image\n>>>> + * visible sizes to accommodate line or plane padding data.\n>>>> + *\n>>>> + * Only the first V4L2DeviceFormat::planes entries are considered valid.\n>>>> + *\n>>>> + */\n>>>> +\n>>>> +/**\n>>>> + * \\var V4L2DeviceFormat::planes\n>>>> + * \\brief The number of valid data planes\n>>>> + */\n>>>> +\n>>>> /**\n>>>> * \\class V4L2Device\n>>>> * \\brief V4L2Device object and API\n>>>>\n>>>\n>>\n>> --\n>> Regards\n>> --\n>> Kieran","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 64FFD60DB6\n\tfor ;\n\tTue, 29 Jan 2019 14:58:48 +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 A2E4641;\n\tTue, 29 Jan 2019 14:58:47 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548770327;\n\tbh=jENHgxDepwz8TDtgAol6ziYs7nx+mWIYdybqcuMh1jE=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=Los/3ZKeKK7hxlaWan+qVFisLFiqeMBzqv+l4iArMw4U0iZvXBmmlpzREhsoEqddm\n\tDOUyTwi/V6rGgsAWKkvlNGxqHOGPvL6Bnw5oxL2hnB8rTjup+UmkFrAVYmXWQTcvb/\n\tkxeZV9aPfyLM6DFNgcarOQ75NR5fb5fi1IprkMaU=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20190128151137.31075-1-jacopo@jmondi.org>\n\t<20190128151137.31075-2-jacopo@jmondi.org>\n\t<2698a85d-df00-12d3-652f-04e21ea6b6c8@ideasonboard.com>\n\t<451742d7-af82-1f17-bf20-4a425ec2b9c2@ideasonboard.com>\n\t<20190128162002.6bz3phpxnnqrkjda@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":"<3dfbc5a3-6bea-69de-315b-20bd9bebb866@ideasonboard.com>","Date":"Tue, 29 Jan 2019 13:58:45 +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":"<20190128162002.6bz3phpxnnqrkjda@uno.localdomain>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: v4l2-format: Add\n\tV4L2DeviceFormat","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, 29 Jan 2019 13:58:48 -0000"}},{"id":694,"web_url":"https://patchwork.libcamera.org/comment/694/","msgid":"<20190130100601.GE4336@pendragon.ideasonboard.com>","date":"2019-01-30T10:06:01","subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: v4l2-format: Add\n\tV4L2DeviceFormat","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Jan 28, 2019 at 05:20:02PM +0100, Jacopo Mondi wrote:\n> On Mon, Jan 28, 2019 at 03:57:45PM +0000, Kieran Bingham wrote:\n> > On 28/01/2019 15:42, Kieran Bingham wrote:\n> >> On 28/01/2019 15:11, Jacopo Mondi wrote:\n> >>> Add a V4L2DeviceFormat class aimed to be used to provide format configuration\n> >>> requests to a V4L2Device.\n> >>>\n> >>> Signed-off-by: Jacopo Mondi \n> >>\n> >> Reviewed-by: Kieran Bingham \n> >>\n> >>> ---\n> >>> src/libcamera/include/v4l2_device.h | 14 ++++++++\n> >>> src/libcamera/v4l2_device.cpp | 56 +++++++++++++++++++++++++++++\n> >>> 2 files changed, 70 insertions(+)\n> >>>\n> >>> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> >>> index c67ebbf..c70959e 100644\n> >>> --- a/src/libcamera/include/v4l2_device.h\n> >>> +++ b/src/libcamera/include/v4l2_device.h\n> >>> @@ -53,6 +53,20 @@ struct V4L2Capability final : v4l2_capability {\n> >>> \t}\n> >>> };\n> >>>\n> >>> +class V4L2DeviceFormat\n> >>> +{\n> >>> +public:\n> >>> +\tuint32_t width;\n> >>> +\tuint32_t height;\n> >>> +\tuint32_t fourcc;\n> >>\n> >> I might have called this pixelFormat, but it's internal and it can\n> >> change if we later define a PixelFormat class or such.\n> >>\n> >> Currently we will store a fourcc so this is accurate.\n> >\n> > struct v4l2_pix_format *pix stores the following extra information:\n> \n> Not only v4l2_pix_format, but v4l2_pix_format_mplane and\n> v4l2_mbus_framefmt too...\n> \n> > \tfield (we might need to consider interlaced somewhere?)\n> > \tcolorspace,\n> > \tflags\n> > \tycbcr_enc, hsv_enc\n> > \tquantization\n> > \txfer_func\n> \n> Right now, I cannot tell. I'm sure there are use cases where those\n> informations are relevant.\n> \n> > Should we be handling any of that information here too?\n> >\n> > Or should we add it on an as-required basis?\n> \n> I think they just need to be copied back and forth from the ioctl\n> arguments, once the V4L2DeviceFormat has been expanded to include\n> these fields.\n> \n> I'll leave it for later, but if anyone has other opinions we can look\n> into that now.\n\nI'm fine with that. For the record, colorspace is a legacy field that\nhas been replaced with encoding, quantization and xfer function. We\nshould expose the new API through V4L2DeviceFormat when we'll add\nsupport for colorspaces.\n\n> >>> +\n> >>> +\tstruct {\n> >>> +\t\tuint32_t size;\n> >>> +\t\tuint32_t bpl;\n> >>> +\t} planesFmt[3];\n> >>> +\tunsigned int planes;\n> >>> +};\n> >>> +\n> >>> class MediaEntity;\n> >>> class V4L2Device\n> >>> {\n> >>> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> >>> index 408f9b9..d6143f2 100644\n> >>> --- a/src/libcamera/v4l2_device.cpp\n> >>> +++ b/src/libcamera/v4l2_device.cpp\n> >>> @@ -80,6 +80,62 @@ LOG_DEFINE_CATEGORY(V4L2)\n> >>> * \\return True if the device provides Streaming I/O IOCTLs\n> >>> */\n> >>>\n> >>> +/**\n> >>> + * \\class V4L2DeviceFormat\n> >>> + * \\brief The V4L2 device image format and sizes\n> >>> + *\n> >>> + * Describes the image format and image sizes to be programmed on a V4L2\n> >>> + * video device. The image format is defined by fourcc code as defined by\n> >>> + * the V4L2 APIs with the V4L2_PIX_FMT_ macros, a visible width and height\n> >>> + * and a variable number of planes (1 to 3) with variable sizes and line\n> >>> + * strides.\n> >>> + *\n> >>> + * Formats defined as 'single planar' by the V4L2 APIs are represented with\n> >>> + * V4L2DeviceFormat instances with a single plane\n> >>> + * (V4L2DeviceFormat::planes = 1). Semi-planar and multiplanar formats use\n> >>> + * 2 and 3 planes respectively.\n> >>> + *\n> >>> + * V4L2DeviceFormat defines the exchange format between components that\n> >>> + * receive image configuration requests from applications and a V4L2Device.\n> >>> + * The V4L2Device validates and applies the requested size and format to\n> >>> + * the device driver.\n> >>> + */\n> >>> +\n> >>> +/**\n> >>> + * \\var V4L2DeviceFormat::width\n> >>> + * \\brief The image width\n> >>> + */\n> >>> +\n> >>> +/**\n> >>> + * \\var V4L2DeviceFormat::height\n> >>> + * \\brief The image height\n> >>> + */\n> >>> +\n> >>> +/**\n> >>> + * \\var V4L2DeviceFormat::fourcc> + * \\brief The pixel encoding scheme\n> >>> + *\n> >>> + * The fourcc code, as defined by the V4L2 APIs with the V4L2_PIX_FMT_ macros,\n> >>> + * that identifies the image format pixel encoding scheme.\n> >>> + */\n> >>> +\n> >>> +/**\n> >>> + * \\var V4L2DeviceFormat::planesFmt\n> >>> + * \\brief The per-plane size information\n> >>> + *\n> >>> + * Images are stored in memory in one or more data planes. Each data plane\n> >>> + * has a specific size and line length, which could differ from the image\n> >>> + * visible sizes to accommodate line or plane padding data.\n> >>> + *\n> >>> + * Only the first V4L2DeviceFormat::planes entries are considered valid.\n> >>> + *\n> >>> + */\n> >>> +\n> >>> +/**\n> >>> + * \\var V4L2DeviceFormat::planes\n> >>> + * \\brief The number of valid data planes\n> >>> + */\n> >>> +\n> >>> /**\n> >>> * \\class V4L2Device\n> >>> * \\brief V4L2Device object and API","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 CFA3060C78\n\tfor ;\n\tWed, 30 Jan 2019 11:06:04 +0100 (CET)","from pendragon.ideasonboard.com (85-76-73-0-nat.elisa-mobile.fi\n\t[85.76.73.0])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 43A8F41;\n\tWed, 30 Jan 2019 11:06:03 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548842764;\n\tbh=V45kqgNYWZyZLEODan/f0Z5TG6EJfeZngSQ6RGCWxp8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=i55b9XVbStaxwOwCGGzERL7pOaFQJgSUtGwNoNg8QRWf/LM2YBMsr1ujmncnPBKZi\n\t8RZ1DBF443IZ6Iy+gZ7GOwszqVjNSdLxq7XIVVleKrv9Zev6cTd6tOqzLMsGV8Qf4w\n\tm37rgKf+fIjd9Uot2824dMgPqrHwgFAowVSjO3VU=","Date":"Wed, 30 Jan 2019 12:06:01 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20190130100601.GE4336@pendragon.ideasonboard.com>","References":"<20190128151137.31075-1-jacopo@jmondi.org>\n\t<20190128151137.31075-2-jacopo@jmondi.org>\n\t<2698a85d-df00-12d3-652f-04e21ea6b6c8@ideasonboard.com>\n\t<451742d7-af82-1f17-bf20-4a425ec2b9c2@ideasonboard.com>\n\t<20190128162002.6bz3phpxnnqrkjda@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190128162002.6bz3phpxnnqrkjda@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: v4l2-format: Add\n\tV4L2DeviceFormat","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, 30 Jan 2019 10:06:05 -0000"}},{"id":695,"web_url":"https://patchwork.libcamera.org/comment/695/","msgid":"<20190130103941.GF4336@pendragon.ideasonboard.com>","date":"2019-01-30T10:39:41","subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: v4l2-format: Add\n\tV4L2DeviceFormat","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 Mon, Jan 28, 2019 at 04:11:36PM +0100, Jacopo Mondi wrote:\n> Add a V4L2DeviceFormat class aimed to be used to provide format configuration\n> requests to a V4L2Device.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/include/v4l2_device.h | 14 ++++++++\n> src/libcamera/v4l2_device.cpp | 56 +++++++++++++++++++++++++++++\n> 2 files changed, 70 insertions(+)\n> \n> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> index c67ebbf..c70959e 100644\n> --- a/src/libcamera/include/v4l2_device.h\n> +++ b/src/libcamera/include/v4l2_device.h\n> @@ -53,6 +53,20 @@ struct V4L2Capability final : v4l2_capability {\n> \t}\n> };\n> \n> +class V4L2DeviceFormat\n\nShould this be named V4L2Format ?\n\n> +{\n> +public:\n> +\tuint32_t width;\n> +\tuint32_t height;\n\nI would have gone for unsigned int instead of uint32_t as the exact\ninteger type isn't really relevant, but uint32_t is fine too. You should\nhowever include stdint.h to ensure that v4l2_device.h stays\nself-contained.\n\n> +\tuint32_t fourcc;\n> +\n> +\tstruct {\n> +\t\tuint32_t size;\n> +\t\tuint32_t bpl;\n> +\t} planesFmt[3];\n> +\tunsigned int planes;\n\nI would have named the first field planes and the second numPlanes or\nplanesCount.\n\n> +};\n> +\n> class MediaEntity;\n\nShould we keep all forward declarations at the top of the file ?\nOtherwise we'll likely end up with duplicated forward declarations at\nsome point.\n\n> class V4L2Device\n> {\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 408f9b9..d6143f2 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -80,6 +80,62 @@ LOG_DEFINE_CATEGORY(V4L2)\n> * \\return True if the device provides Streaming I/O IOCTLs\n> */\n> \n> +/**\n> + * \\class V4L2DeviceFormat\n> + * \\brief The V4L2 device image format and sizes\n> + *\n> + * Describes the image format and image sizes to be programmed on a V4L2\n\n\"This class describes ...3\n\nIt would be fine for the \\brief text, but the documentation body should\nbe made of full sentences.\n\n> + * video device. The image format is defined by fourcc code as defined by\n\ns/fourcc code/a fourcc code/\n\nOr maybe \"a fourcc\" (or \"a 4CC\" or \"a FOURCC\" ?), as the last C in\nfourcc stands for code already.\n\n> + * the V4L2 APIs with the V4L2_PIX_FMT_ macros, a visible width and height\n\ns/V4L2_PIX_FMT_/V4L2_PIX_FMT_*/\n\n> + * and a variable number of planes (1 to 3) with variable sizes and line\n> + * strides.\n\nI wouldn't say variable as they don't really vary. Maybe \"and one to\nthree planes with configurable line stride and total size for each\nplane\" ?\n\n> + *\n> + * Formats defined as 'single planar' by the V4L2 APIs are represented with\n> + * V4L2DeviceFormat instances with a single plane\n> + * (V4L2DeviceFormat::planes = 1). Semi-planar and multiplanar formats use\n> + * 2 and 3 planes respectively.\n\nI would add one paragraph here to explain that line and plane padding\nmay not be separately configurable, depending on the device (devices\nthat don't support the MPLANE API have a fixed relationship between\npadding for different planes), and how this works in that case (do we\nignore padding information for planes > 0 ? do we require them to be\nspecified an in sync, and return an error otherwise ?).\n\n> + * V4L2DeviceFormat defines the exchange format between components that\n> + * receive image configuration requests from applications and a V4L2Device.\n> + * The V4L2Device validates and applies the requested size and format to\n> + * the device driver.\n\nShouldn't we define the class as a way to pass format information to\nV4L2Device, without caring about what's on the other side (and thus\nwithout mentioning it's an exchange format) ?\n\n> + */\n> +\n> +/**\n> + * \\var V4L2DeviceFormat::width\n> + * \\brief The image width\n\nThe image width in pixels\n\n> + */\n> +\n> +/**\n> + * \\var V4L2DeviceFormat::height\n> + * \\brief The image height\n\nIn pixels here too.\n\n> + */\n> +\n> +/**\n> + * \\var V4L2DeviceFormat::fourcc\n> + * \\brief The pixel encoding scheme\n> + *\n> + * The fourcc code, as defined by the V4L2 APIs with the V4L2_PIX_FMT_ macros,\n\ns/APIs/API/\ns/V4L2_PIX_FMT_/V4L2_PIX_FMT_*/\n\n> + * that identifies the image format pixel encoding scheme.\n> + */\n> +\n> +/**\n> + * \\var V4L2DeviceFormat::planesFmt\n> + * \\brief The per-plane size information\n\nI'd mention \"memory size\" instead of just \"size\" to make it clear this\ndoesn't refer to the size in pixels.\n\n> + *\n> + * Images are stored in memory in one or more data planes. Each data plane\n> + * has a specific size and line length, which could differ from the image\n\ns/size/memory size/ here too.\n\n> + * visible sizes to accommodate line or plane padding data.\n> + *\n> + * Only the first V4L2DeviceFormat::planes entries are considered valid.\n\nYou could also write \"\\ref planes\".\n\n> + *\n\nExtra blank line.\n\n> + */\n> +\n> +/**\n> + * \\var V4L2DeviceFormat::planes\n> + * \\brief The number of valid data planes\n> + */\n> +\n> /**\n> * \\class V4L2Device\n> * \\brief V4L2Device object and API","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 0082460C78\n\tfor ;\n\tWed, 30 Jan 2019 11:39:43 +0100 (CET)","from pendragon.ideasonboard.com (85-76-73-0-nat.elisa-mobile.fi\n\t[85.76.73.0])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7F69841;\n\tWed, 30 Jan 2019 11:39:42 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548844783;\n\tbh=IulgAzj3Mtz00u4V/feQRp1CQRoaY3Y4XMpOZXPnBAg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Npk/iRlzUCwG1ZOEkUpRBlszTo55u1O8eqe+iqW+Kbce5++GoYCfveazhvCbIi+P0\n\tPFLH0HsMBPh/ozpfNqnWZ4kVBJf23IbjqYPzIfQemRsw6H99gemxg11jZmBvzDjzqY\n\t9AmHUKArIOpWzuW4sDu7XC6qISV27x0+OiYTgiNg=","Date":"Wed, 30 Jan 2019 12:39:41 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190130103941.GF4336@pendragon.ideasonboard.com>","References":"<20190128151137.31075-1-jacopo@jmondi.org>\n\t<20190128151137.31075-2-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190128151137.31075-2-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: v4l2-format: Add\n\tV4L2DeviceFormat","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, 30 Jan 2019 10:39:44 -0000"}},{"id":736,"web_url":"https://patchwork.libcamera.org/comment/736/","msgid":"<20190201100712.tshsgme7gqud2nxd@uno.localdomain>","date":"2019-02-01T10:07:12","subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: v4l2-format: Add\n\tV4L2DeviceFormat","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Wed, Jan 30, 2019 at 12:39:41PM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Mon, Jan 28, 2019 at 04:11:36PM +0100, Jacopo Mondi wrote:\n> > Add a V4L2DeviceFormat class aimed to be used to provide format configuration\n> > requests to a V4L2Device.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/include/v4l2_device.h | 14 ++++++++\n> > src/libcamera/v4l2_device.cpp | 56 +++++++++++++++++++++++++++++\n> > 2 files changed, 70 insertions(+)\n> >\n> > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> > index c67ebbf..c70959e 100644\n> > --- a/src/libcamera/include/v4l2_device.h\n> > +++ b/src/libcamera/include/v4l2_device.h\n> > @@ -53,6 +53,20 @@ struct V4L2Capability final : v4l2_capability {\n> > \t}\n> > };\n> >\n> > +class V4L2DeviceFormat\n>\n> Should this be named V4L2Format ?\n>\n\nAs I expect a V4L2SubdeviceFormat, I would keep this as\nV4L2DeviceFormat.\n\n> > +{\n> > +public:\n> > +\tuint32_t width;\n> > +\tuint32_t height;\n>\n> I would have gone for unsigned int instead of uint32_t as the exact\n> integer type isn't really relevant, but uint32_t is fine too. You should\n\nI went for the lenght-specific type because the v4l2 structures has\nlength specified. If not that relevant I can change back to unsigned\nint maybe?\n\n> however include stdint.h to ensure that v4l2_device.h stays\n> self-contained.\n\nI recall I had to add it, then it's not here anymore...\n\n>\n> > +\tuint32_t fourcc;\n> > +\n> > +\tstruct {\n> > +\t\tuint32_t size;\n> > +\t\tuint32_t bpl;\n> > +\t} planesFmt[3];\n> > +\tunsigned int planes;\n>\n> I would have named the first field planes and the second numPlanes or\n> planesCount.\n>\n\nor planesNum?\n\n> > +};\n> > +\n> > class MediaEntity;\n>\n> Should we keep all forward declarations at the top of the file ?\n> Otherwise we'll likely end up with duplicated forward declarations at\n> some point.\n>\n\nI will change this library-wide.\n\n> > class V4L2Device\n> > {\n> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > index 408f9b9..d6143f2 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -80,6 +80,62 @@ LOG_DEFINE_CATEGORY(V4L2)\n> > * \\return True if the device provides Streaming I/O IOCTLs\n> > */\n> >\n> > +/**\n> > + * \\class V4L2DeviceFormat\n> > + * \\brief The V4L2 device image format and sizes\n> > + *\n> > + * Describes the image format and image sizes to be programmed on a V4L2\n>\n> \"This class describes ...3\n>\n> It would be fine for the \\brief text, but the documentation body should\n> be made of full sentences.\n>\n> > + * video device. The image format is defined by fourcc code as defined by\n>\n> s/fourcc code/a fourcc code/\n>\n> Or maybe \"a fourcc\" (or \"a 4CC\" or \"a FOURCC\" ?), as the last C in\n> fourcc stands for code already.\n>\n> > + * the V4L2 APIs with the V4L2_PIX_FMT_ macros, a visible width and height\n>\n> s/V4L2_PIX_FMT_/V4L2_PIX_FMT_*/\n>\n> > + * and a variable number of planes (1 to 3) with variable sizes and line\n> > + * strides.\n>\n> I wouldn't say variable as they don't really vary. Maybe \"and one to\n> three planes with configurable line stride and total size for each\n> plane\" ?\n>\n\nack\n\n> > + *\n> > + * Formats defined as 'single planar' by the V4L2 APIs are represented with\n> > + * V4L2DeviceFormat instances with a single plane\n> > + * (V4L2DeviceFormat::planes = 1). Semi-planar and multiplanar formats use\n> > + * 2 and 3 planes respectively.\n>\n> I would add one paragraph here to explain that line and plane padding\n> may not be separately configurable, depending on the device (devices\n> that don't support the MPLANE API have a fixed relationship between\n> padding for different planes), and how this works in that case (do we\n> ignore padding information for planes > 0 ? do we require them to be\n> specified an in sync, and return an error otherwise ?).\n>\n\nI'm not sure I got you here.\nWhy devices that do not support MPLANE should care about planes > 1 ?\n\n> > + * V4L2DeviceFormat defines the exchange format between components that\n> > + * receive image configuration requests from applications and a V4L2Device.\n> > + * The V4L2Device validates and applies the requested size and format to\n> > + * the device driver.\n>\n> Shouldn't we define the class as a way to pass format information to\n> V4L2Device, without caring about what's on the other side (and thus\n> without mentioning it's an exchange format) ?\n>\n> > + */\n> > +\n> > +/**\n> > + * \\var V4L2DeviceFormat::width\n> > + * \\brief The image width\n>\n> The image width in pixels\n>\n> > + */\n> > +\n> > +/**\n> > + * \\var V4L2DeviceFormat::height\n> > + * \\brief The image height\n>\n> In pixels here too.\n>\n> > + */\n> > +\n> > +/**\n> > + * \\var V4L2DeviceFormat::fourcc\n> > + * \\brief The pixel encoding scheme\n> > + *\n> > + * The fourcc code, as defined by the V4L2 APIs with the V4L2_PIX_FMT_ macros,\n>\n> s/APIs/API/\n> s/V4L2_PIX_FMT_/V4L2_PIX_FMT_*/\n>\n> > + * that identifies the image format pixel encoding scheme.\n> > + */\n> > +\n> > +/**\n> > + * \\var V4L2DeviceFormat::planesFmt\n> > + * \\brief The per-plane size information\n>\n> I'd mention \"memory size\" instead of just \"size\" to make it clear this\n> doesn't refer to the size in pixels.\n>\n> > + *\n> > + * Images are stored in memory in one or more data planes. Each data plane\n> > + * has a specific size and line length, which could differ from the image\n>\n> s/size/memory size/ here too.\n>\n> > + * visible sizes to accommodate line or plane padding data.\n> > + *\n> > + * Only the first V4L2DeviceFormat::planes entries are considered valid.\n>\n> You could also write \"\\ref planes\".\n>\n> > + *\n>\n> Extra blank line.\n>\n\nThanks, please clarify the question I didn't get and I'll send a\nfixups series on top of this.\n\nThanks\n j\n\n> > + */\n> > +\n> > +/**\n> > + * \\var V4L2DeviceFormat::planes\n> > + * \\brief The number of valid data planes\n> > + */\n> > +\n> > /**\n> > * \\class V4L2Device\n> > * \\brief V4L2Device object and API\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D3BCC60C78\n\tfor ;\n\tFri, 1 Feb 2019 11:06:55 +0100 (CET)","from uno.localdomain (unknown [91.183.179.147])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay12.mail.gandi.net (Postfix) with ESMTPSA id 743B6200014;\n\tFri, 1 Feb 2019 10:06:55 +0000 (UTC)"],"Date":"Fri, 1 Feb 2019 11:07:12 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190201100712.tshsgme7gqud2nxd@uno.localdomain>","References":"<20190128151137.31075-1-jacopo@jmondi.org>\n\t<20190128151137.31075-2-jacopo@jmondi.org>\n\t<20190130103941.GF4336@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"fzv42sxdlqwucr72\"","Content-Disposition":"inline","In-Reply-To":"<20190130103941.GF4336@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: v4l2-format: Add\n\tV4L2DeviceFormat","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, 01 Feb 2019 10:06:56 -0000"}},{"id":737,"web_url":"https://patchwork.libcamera.org/comment/737/","msgid":"<20190201101836.GA4341@pendragon.ideasonboard.com>","date":"2019-02-01T10:18:36","subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: v4l2-format: Add\n\tV4L2DeviceFormat","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Fri, Feb 01, 2019 at 11:07:12AM +0100, Jacopo Mondi wrote:\n> On Wed, Jan 30, 2019 at 12:39:41PM +0200, Laurent Pinchart wrote:\n> > On Mon, Jan 28, 2019 at 04:11:36PM +0100, Jacopo Mondi wrote:\n> >> Add a V4L2DeviceFormat class aimed to be used to provide format configuration\n> >> requests to a V4L2Device.\n> >>\n> >> Signed-off-by: Jacopo Mondi \n> >> ---\n> >> src/libcamera/include/v4l2_device.h | 14 ++++++++\n> >> src/libcamera/v4l2_device.cpp | 56 +++++++++++++++++++++++++++++\n> >> 2 files changed, 70 insertions(+)\n> >>\n> >> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> >> index c67ebbf..c70959e 100644\n> >> --- a/src/libcamera/include/v4l2_device.h\n> >> +++ b/src/libcamera/include/v4l2_device.h\n> >> @@ -53,6 +53,20 @@ struct V4L2Capability final : v4l2_capability {\n> >> \t}\n> >> };\n> >>\n> >> +class V4L2DeviceFormat\n> >\n> > Should this be named V4L2Format ?\n> \n> As I expect a V4L2SubdeviceFormat, I would keep this as\n> V4L2DeviceFormat.\n\nThe kernel API has v4l2_format and v4l2_subdev_format, but that's also\nbecause subdevs were introduced later. V4L2DeviceFormat makes sense too.\n\n> >> +{\n> >> +public:\n> >> +\tuint32_t width;\n> >> +\tuint32_t height;\n> >\n> > I would have gone for unsigned int instead of uint32_t as the exact\n> > integer type isn't really relevant, but uint32_t is fine too. You should\n> \n> I went for the lenght-specific type because the v4l2 structures has\n> length specified. If not that relevant I can change back to unsigned\n> int maybe?\n\nI'm fine either way.\n\n> > however include stdint.h to ensure that v4l2_device.h stays\n> > self-contained.\n> \n> I recall I had to add it, then it's not here anymore...\n> \n> >> +\tuint32_t fourcc;\n> >> +\n> >> +\tstruct {\n> >> +\t\tuint32_t size;\n> >> +\t\tuint32_t bpl;\n> >> +\t} planesFmt[3];\n> >> +\tunsigned int planes;\n> >\n> > I would have named the first field planes and the second numPlanes or\n> > planesCount.\n> \n> or planesNum?\n\nPossibly, although \"number of planes\" sounds better to me than \"planes\nnumber\".\n\n> >> +};\n> >> +\n> >> class MediaEntity;\n> >\n> > Should we keep all forward declarations at the top of the file ?\n> > Otherwise we'll likely end up with duplicated forward declarations at\n> > some point.\n> \n> I will change this library-wide.\n> \n> >> class V4L2Device\n> >> {\n> >> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> >> index 408f9b9..d6143f2 100644\n> >> --- a/src/libcamera/v4l2_device.cpp\n> >> +++ b/src/libcamera/v4l2_device.cpp\n> >> @@ -80,6 +80,62 @@ LOG_DEFINE_CATEGORY(V4L2)\n> >> * \\return True if the device provides Streaming I/O IOCTLs\n> >> */\n> >>\n> >> +/**\n> >> + * \\class V4L2DeviceFormat\n> >> + * \\brief The V4L2 device image format and sizes\n> >> + *\n> >> + * Describes the image format and image sizes to be programmed on a V4L2\n> >\n> > \"This class describes ...3\n> >\n> > It would be fine for the \\brief text, but the documentation body should\n> > be made of full sentences.\n> >\n> >> + * video device. The image format is defined by fourcc code as defined by\n> >\n> > s/fourcc code/a fourcc code/\n> >\n> > Or maybe \"a fourcc\" (or \"a 4CC\" or \"a FOURCC\" ?), as the last C in\n> > fourcc stands for code already.\n> >\n> >> + * the V4L2 APIs with the V4L2_PIX_FMT_ macros, a visible width and height\n> >\n> > s/V4L2_PIX_FMT_/V4L2_PIX_FMT_*/\n> >\n> >> + * and a variable number of planes (1 to 3) with variable sizes and line\n> >> + * strides.\n> >\n> > I wouldn't say variable as they don't really vary. Maybe \"and one to\n> > three planes with configurable line stride and total size for each\n> > plane\" ?\n> \n> ack\n> \n> >> + *\n> >> + * Formats defined as 'single planar' by the V4L2 APIs are represented with\n> >> + * V4L2DeviceFormat instances with a single plane\n> >> + * (V4L2DeviceFormat::planes = 1). Semi-planar and multiplanar formats use\n> >> + * 2 and 3 planes respectively.\n> >\n> > I would add one paragraph here to explain that line and plane padding\n> > may not be separately configurable, depending on the device (devices\n> > that don't support the MPLANE API have a fixed relationship between\n> > padding for different planes), and how this works in that case (do we\n> > ignore padding information for planes > 0 ? do we require them to be\n> > specified an in sync, and return an error otherwise ?).\n> \n> I'm not sure I got you here.\n> Why devices that do not support MPLANE should care about planes > 1 ?\n\nBecause the non-MPLANE API supports multi-planar formats (such as NV12\nfor instance). The different between the two APIs is that only the\nMPLANE API allows multi-planar formats to be handled with separate\nmemory regions for each plane.\n\n> >> + * V4L2DeviceFormat defines the exchange format between components that\n> >> + * receive image configuration requests from applications and a V4L2Device.\n> >> + * The V4L2Device validates and applies the requested size and format to\n> >> + * the device driver.\n> >\n> > Shouldn't we define the class as a way to pass format information to\n> > V4L2Device, without caring about what's on the other side (and thus\n> > without mentioning it's an exchange format) ?\n> >\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\var V4L2DeviceFormat::width\n> >> + * \\brief The image width\n> >\n> > The image width in pixels\n> >\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\var V4L2DeviceFormat::height\n> >> + * \\brief The image height\n> >\n> > In pixels here too.\n> >\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\var V4L2DeviceFormat::fourcc\n> >> + * \\brief The pixel encoding scheme\n> >> + *\n> >> + * The fourcc code, as defined by the V4L2 APIs with the V4L2_PIX_FMT_ macros,\n> >\n> > s/APIs/API/\n> > s/V4L2_PIX_FMT_/V4L2_PIX_FMT_*/\n> >\n> >> + * that identifies the image format pixel encoding scheme.\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\var V4L2DeviceFormat::planesFmt\n> >> + * \\brief The per-plane size information\n> >\n> > I'd mention \"memory size\" instead of just \"size\" to make it clear this\n> > doesn't refer to the size in pixels.\n> >\n> >> + *\n> >> + * Images are stored in memory in one or more data planes. Each data plane\n> >> + * has a specific size and line length, which could differ from the image\n> >\n> > s/size/memory size/ here too.\n> >\n> >> + * visible sizes to accommodate line or plane padding data.\n> >> + *\n> >> + * Only the first V4L2DeviceFormat::planes entries are considered valid.\n> >\n> > You could also write \"\\ref planes\".\n> >\n> >> + *\n> >\n> > Extra blank line.\n> \n> Thanks, please clarify the question I didn't get and I'll send a\n> fixups series on top of this.\n\nDone :-) Thank you.\n\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\var V4L2DeviceFormat::planes\n> >> + * \\brief The number of valid data planes\n> >> + */\n> >> +\n> >> /**\n> >> * \\class V4L2Device\n> >> * \\brief V4L2Device object and API","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 050B660C78\n\tfor ;\n\tFri, 1 Feb 2019 11:18:46 +0100 (CET)","from pendragon.ideasonboard.com (85-76-34-136-nat.elisa-mobile.fi\n\t[85.76.34.136])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 77BE941;\n\tFri, 1 Feb 2019 11:18:41 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1549016324;\n\tbh=QAJFXnhLV2JTli4w15cleDx2sOoCFC3IuYAjuQjiFck=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=chuiLS/UMR3hd0AChPFnoiEk3/J9Gp/X8mARQisoPqyCP45zuDB+ObDzDstAKJHa6\n\tL2/pYFwnPNO4Bxc9luxKONsF5gYsddMIcUaMt18SahM+JRrpYDSUOSK7sv9NS/I9hP\n\twKdBv8nUtkYmBi6tk1OxHCzAu6VNfDL/MNaJ3AMI=","Date":"Fri, 1 Feb 2019 12:18:36 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190201101836.GA4341@pendragon.ideasonboard.com>","References":"<20190128151137.31075-1-jacopo@jmondi.org>\n\t<20190128151137.31075-2-jacopo@jmondi.org>\n\t<20190130103941.GF4336@pendragon.ideasonboard.com>\n\t<20190201100712.tshsgme7gqud2nxd@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190201100712.tshsgme7gqud2nxd@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: v4l2-format: Add\n\tV4L2DeviceFormat","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, 01 Feb 2019 10:18:46 -0000"}}]