[{"id":15738,"web_url":"https://patchwork.libcamera.org/comment/15738/","msgid":"<3fa6662c-b338-ac07-8fb0-cedb74e2d46a@ideasonboard.com>","date":"2021-03-17T11:33:17","subject":"Re: [libcamera-devel] [RFC PATCH 1/6] libcamera: Add fraction.h","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Marian,\n\nOn 16/03/2021 15:52, Marian Cichy wrote:\n> Add a new class that represents a fraction. A structure like this is\n> also often used in linux camera drivers, e.g. v4l2_fract to represent a\n> frame interval or in applications like Gstreamer to represent frame\n> rates.\n> \n> Adding this class helps to interface frame intervals and frame\n> rates in video streams.\n\nThis indeed would be helpful in any area that handles time.\n\nI think this is normally called a rational though?\n\nWe're really building up a math library in here aren't we with our\nsizes, geometry, and number classes...\n\n> Signed-off-by: Marian Cichy \n> ---\n> include/libcamera/fraction.h | 34 ++++++++++++++++++++\n> src/libcamera/fraction.cpp | 60 ++++++++++++++++++++++++++++++++++++\n> src/libcamera/meson.build | 1 +\n> 3 files changed, 95 insertions(+)\n> create mode 100644 include/libcamera/fraction.h\n> create mode 100644 src/libcamera/fraction.cpp\n> \n> diff --git a/include/libcamera/fraction.h b/include/libcamera/fraction.h\n> new file mode 100644\n> index 00000000..aa6a1abb\n> --- /dev/null\n> +++ b/include/libcamera/fraction.h\n> @@ -0,0 +1,34 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Pengutronix, Marian Cichy \n> + *\n> + * fraction.h - A fraction consisting of two integers\n> + */\n> +\n> +#ifndef __LIBCAMERA_FRACTION_H__\n> +#define __LIBCAMERA_FRACTION_H__\n> +\n> +namespace libcamera {\n> +\n> +class Fraction {\n> +public:\n> +\tconstexpr Fraction() :\n> +\t\tnumerator(0), denominator(0)\n> +\t{\n> +\t}\n> +\n> +\tconstexpr Fraction(unsigned int num, unsigned int den) :\n> +\t\tnumerator(num), denominator(den)\n> +\t{\n> +\t}\n> +\n> +\tunsigned int numerator;\n> +\tunsigned int denominator;\n\nGenerically - Fractions/(Rational) numbers could be negative.\nBut given our use cases, I presume we wouldn't expect negative values ?\n\nWell ... not unless we start needing to do things like clock recovery\n.... I don't think we'll need to do anything like that ...\n\nWould you expect further math functions in here to be able to add /\nmultiple two fractions/rational numbers together?\n\nIn fact - I wonder if this would also tie into the geometry functions\nfor scaling or such?\n\n Size(640, 480) * Rational(1/2) == Size(320, 240);\n\nI'm not saying the extra functionality possible here is required for\nthis patch though - just exploring what the goals would be for this class.\n\n--\nKieran\n\n\n\n> +\n> +\tconst std::string toString() const;\n> +};\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_FRACTION_H__ */\n> diff --git a/src/libcamera/fraction.cpp b/src/libcamera/fraction.cpp\n> new file mode 100644\n> index 00000000..76c373aa\n> --- /dev/null\n> +++ b/src/libcamera/fraction.cpp\n> @@ -0,0 +1,60 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Pengutronix, Marian Cichy \n> + *\n> + * fraction.h - A fraction consisting of two integers\n> + */\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +/**\n> + * \\file fraction.h\n> + * \\brief A fraction consisting of two integers.\n> + */\n> +\n> +namespace libcamera {\n> +\n> +/**\n> + * \\class Fraction\n> + * \\brief Represents a fraction with a nominator and a denominator.\n> + */\n> +\n> +/**\n> + * \\fn Fraction::Fraction()\n> + * \\brief Construct a Fraction with value 0/0. This should be interpreted\n> + * as invalid or not-used Fraction.\n> + */\n> +\n> +/**\n> + * \\fn Fraction::Fraction(unsigned int num, unsigned int den)\n> + * \\brief Construct a Fraction with value n/d.\n> + */\n> +\n> +/**\n> + * \\var Fraction::numerator\n> + * \\brief The numerator of the fraction.\n> + */\n> +\n> +/**\n> + * \\var Fraction::denominator\n> + * \\brief The denominator of the fraction.\n> + */\n> +\n> +/**\n> + * \\brief Assemble and return a string describing the fraction\n> + * \\return A string describing the fraction.\n> + */\n> +const std::string Fraction::toString() const\n> +{\n> +\tstd::stringstream ss;\n> +\n> +\tss << numerator << \"/\" << denominator;\n> +\n> +\treturn ss.str();\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 815629db..7927481f 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -22,6 +22,7 @@ libcamera_sources = files([\n> 'file.cpp',\n> 'file_descriptor.cpp',\n> 'formats.cpp',\n> + 'fraction.cpp',\n> 'framebuffer_allocator.cpp',\n> 'geometry.cpp',\n> 'ipa_controls.cpp',\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7A91AC32E1\n\tfor ;\n\tWed, 17 Mar 2021 11:33:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B63EB68D60;\n\tWed, 17 Mar 2021 12:33:21 +0100 (CET)","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 F01F3605B1\n\tfor ;\n\tWed, 17 Mar 2021 12:33:20 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5E6AA45D;\n\tWed, 17 Mar 2021 12:33:20 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"iEIj3vGh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615980800;\n\tbh=0dSTro7jSSmC9pb9MWgRHikThrw2SECteIB+/BWm2jg=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=iEIj3vGhCAqbCIGcpzs5dZqoeflYL1iAuZxwbRvDwYgeNYsgDVQkYCOJugL4AnMnY\n\tMoHcBxzQv1sH9R6+1GcHy9347UGr0e54gH1aAoMZWndf9CTug50pg7k0X5vI+VrtxR\n\tonftpzFo3yqBdQISqDY1HjmVM6FbIo9c3/Ha7LGM=","To":"Marian Cichy ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210316155211.6679-1-m.cichy@pengutronix.de>\n\t<20210316155211.6679-2-m.cichy@pengutronix.de>","From":"Kieran Bingham ","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\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<3fa6662c-b338-ac07-8fb0-cedb74e2d46a@ideasonboard.com>","Date":"Wed, 17 Mar 2021 11:33:17 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20210316155211.6679-2-m.cichy@pengutronix.de>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [RFC PATCH 1/6] libcamera: Add fraction.h","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"graphics@pengutronix.de","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15739,"web_url":"https://patchwork.libcamera.org/comment/15739/","msgid":"","date":"2021-03-17T11:48:40","subject":"Re: [libcamera-devel] [RFC PATCH 1/6] libcamera: Add fraction.h","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 17/03/2021 11:33, Kieran Bingham wrote:\n> Hi Marian,\n> \n> On 16/03/2021 15:52, Marian Cichy wrote:\n>> Add a new class that represents a fraction. A structure like this is\n>> also often used in linux camera drivers, e.g. v4l2_fract to represent a\n>> frame interval or in applications like Gstreamer to represent frame\n>> rates.\n>>\n>> Adding this class helps to interface frame intervals and frame\n>> rates in video streams.\n\nAlso, and only for reference really, because it's fun*, and I've always\nbeen intrigued by the concept of Flicks in this area:\n\n\thttps://github.com/facebookarchive/Flicks\n\thttps://en.wikipedia.org/wiki/Flick_(time)\n\nwhere a flick is:\n> using flicks = std::chrono::duration>;\n\n> A flick (frame-tick) is a very small unit of time. It is 1/705600000 of a second, exactly.\n> \n> 1 flick = 1/705600000 second\n> \n> This unit of time is the smallest time unit which is LARGER than a\n> nanosecond, and can in integer quantities exactly represent a single\n> frame duration for 24 Hz, 25 Hz, 30 Hz, 48 Hz, 50 Hz, 60 Hz, 90 Hz,\n> 100 Hz, 120 Hz, and also 1/1000 divisions of each, as well as a\n> single sample duration for 8 kHz, 16 kHz, 22.05 kHz, 24 kHz, 32 kHz,\n> 44.1 kHz, 48 kHz, 88.2 kHz, 96 kHz, and 192kHz, as well as the NTSC\n> frame durations for 24 * (1000/1001) Hz, 30 * (1000/1001) Hz, 60 *\n> (1000/1001) Hz, and 120 * (1000/1001) Hz.\n\n*fun might not be well-defined in this use case.","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 16EEEBD80C\n\tfor ;\n\tWed, 17 Mar 2021 11:48:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 840C668D50;\n\tWed, 17 Mar 2021 12:48:45 +0100 (CET)","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 16AE9605B1\n\tfor ;\n\tWed, 17 Mar 2021 12:48:44 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 80D3345D;\n\tWed, 17 Mar 2021 12:48:43 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"l1i9dSrB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615981723;\n\tbh=8ew9HQdyvCUrlcOdMcJ5VV0wlkBQ4zA49yHRacZtfQY=;\n\th=Reply-To:Subject:From:To:Cc:References:Date:In-Reply-To:From;\n\tb=l1i9dSrBu9lv11W3mLkzm8nynkwFyL5D1TipisEIyYJNAMz7+7JDnWgSdGMa3ykI2\n\tCwVDQk0Q+Y/14SIrJP5ECwo4Q28feWBO7rE2+nkFndioJE/mD5S9W1xLMwRWfbJndI\n\t7AKlAVR6zzxNfcYS4r6JV0rsPpTI4GtUMe5U3Dv0=","From":"Kieran Bingham ","To":"Marian Cichy ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210316155211.6679-1-m.cichy@pengutronix.de>\n\t<20210316155211.6679-2-m.cichy@pengutronix.de>\n\t<3fa6662c-b338-ac07-8fb0-cedb74e2d46a@ideasonboard.com>","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\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"","Date":"Wed, 17 Mar 2021 11:48:40 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<3fa6662c-b338-ac07-8fb0-cedb74e2d46a@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [RFC PATCH 1/6] libcamera: Add fraction.h","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"graphics@pengutronix.de","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15740,"web_url":"https://patchwork.libcamera.org/comment/15740/","msgid":"<853a6e03-9fcd-92a7-e130-fdde6b66138c@pengutronix.de>","date":"2021-03-17T12:41:22","subject":"Re: [libcamera-devel] [RFC PATCH 1/6] libcamera: Add fraction.h","submitter":{"id":81,"url":"https://patchwork.libcamera.org/api/people/81/","name":"Marian Cichy","email":"mci@pengutronix.de"},"content":"On 3/17/21 12:33 PM, Kieran Bingham wrote:\n> Hi Marian,\n>\n> On 16/03/2021 15:52, Marian Cichy wrote:\n>> Add a new class that represents a fraction. A structure like this is\n>> also often used in linux camera drivers, e.g. v4l2_fract to represent a\n>> frame interval or in applications like Gstreamer to represent frame\n>> rates.\n>>\n>> Adding this class helps to interface frame intervals and frame\n>> rates in video streams.\n> This indeed would be helpful in any area that handles time.\n>\n> I think this is normally called a rational though?\n\nWell .. this is probably mathematical nit-picking, but a fraction is \nalways a construct that consists of two integers, numerator and \ndenominator. A rational number is any number that *can* be displayed as \na fraction. For example, \"5\" is in the set of rational numbers, because \nwe can display it as the fraction \"5/1\".\n\nAlso, since it is called v4l2_fract in V4L2 and GST_FRACTION in \nGstreamer, I went for consistency in the linux media universe.\n\n>\n> We're really building up a math library in here aren't we with our\n> sizes, geometry, and number classes...\n>\n>> Signed-off-by: Marian Cichy \n>> ---\n>> include/libcamera/fraction.h | 34 ++++++++++++++++++++\n>> src/libcamera/fraction.cpp | 60 ++++++++++++++++++++++++++++++++++++\n>> src/libcamera/meson.build | 1 +\n>> 3 files changed, 95 insertions(+)\n>> create mode 100644 include/libcamera/fraction.h\n>> create mode 100644 src/libcamera/fraction.cpp\n>>\n>> diff --git a/include/libcamera/fraction.h b/include/libcamera/fraction.h\n>> new file mode 100644\n>> index 00000000..aa6a1abb\n>> --- /dev/null\n>> +++ b/include/libcamera/fraction.h\n>> @@ -0,0 +1,34 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2021, Pengutronix, Marian Cichy \n>> + *\n>> + * fraction.h - A fraction consisting of two integers\n>> + */\n>> +\n>> +#ifndef __LIBCAMERA_FRACTION_H__\n>> +#define __LIBCAMERA_FRACTION_H__\n>> +\n>> +namespace libcamera {\n>> +\n>> +class Fraction {\n>> +public:\n>> +\tconstexpr Fraction() :\n>> +\t\tnumerator(0), denominator(0)\n>> +\t{\n>> +\t}\n>> +\n>> +\tconstexpr Fraction(unsigned int num, unsigned int den) :\n>> +\t\tnumerator(num), denominator(den)\n>> +\t{\n>> +\t}\n>> +\n>> +\tunsigned int numerator;\n>> +\tunsigned int denominator;\n> Generically - Fractions/(Rational) numbers could be negative.\n> But given our use cases, I presume we wouldn't expect negative values ?\n>\n> Well ... not unless we start needing to do things like clock recovery\n> .... I don't think we'll need to do anything like that ...\n>\n> Would you expect further math functions in here to be able to add /\n> multiple two fractions/rational numbers together?\n>\n> In fact - I wonder if this would also tie into the geometry functions\n> for scaling or such?\n>\n> Size(640, 480) * Rational(1/2) == Size(320, 240);\n>\n> I'm not saying the extra functionality possible here is required for\n> this patch though - just exploring what the goals would be for this class.\n>\n> --\n> Kieran\n>\n>\n>\n>> +\n>> +\tconst std::string toString() const;\n>> +};\n>> +\n>> +} /* namespace libcamera */\n>> +\n>> +#endif /* __LIBCAMERA_FRACTION_H__ */\n>> diff --git a/src/libcamera/fraction.cpp b/src/libcamera/fraction.cpp\n>> new file mode 100644\n>> index 00000000..76c373aa\n>> --- /dev/null\n>> +++ b/src/libcamera/fraction.cpp\n>> @@ -0,0 +1,60 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2021, Pengutronix, Marian Cichy \n>> + *\n>> + * fraction.h - A fraction consisting of two integers\n>> + */\n>> +\n>> +#include \n>> +#include \n>> +\n>> +#include \n>> +\n>> +/**\n>> + * \\file fraction.h\n>> + * \\brief A fraction consisting of two integers.\n>> + */\n>> +\n>> +namespace libcamera {\n>> +\n>> +/**\n>> + * \\class Fraction\n>> + * \\brief Represents a fraction with a nominator and a denominator.\n>> + */\n>> +\n>> +/**\n>> + * \\fn Fraction::Fraction()\n>> + * \\brief Construct a Fraction with value 0/0. This should be interpreted\n>> + * as invalid or not-used Fraction.\n>> + */\n>> +\n>> +/**\n>> + * \\fn Fraction::Fraction(unsigned int num, unsigned int den)\n>> + * \\brief Construct a Fraction with value n/d.\n>> + */\n>> +\n>> +/**\n>> + * \\var Fraction::numerator\n>> + * \\brief The numerator of the fraction.\n>> + */\n>> +\n>> +/**\n>> + * \\var Fraction::denominator\n>> + * \\brief The denominator of the fraction.\n>> + */\n>> +\n>> +/**\n>> + * \\brief Assemble and return a string describing the fraction\n>> + * \\return A string describing the fraction.\n>> + */\n>> +const std::string Fraction::toString() const\n>> +{\n>> +\tstd::stringstream ss;\n>> +\n>> +\tss << numerator << \"/\" << denominator;\n>> +\n>> +\treturn ss.str();\n>> +}\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n>> index 815629db..7927481f 100644\n>> --- a/src/libcamera/meson.build\n>> +++ b/src/libcamera/meson.build\n>> @@ -22,6 +22,7 @@ libcamera_sources = files([\n>> 'file.cpp',\n>> 'file_descriptor.cpp',\n>> 'formats.cpp',\n>> + 'fraction.cpp',\n>> 'framebuffer_allocator.cpp',\n>> 'geometry.cpp',\n>> 'ipa_controls.cpp',\n>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7EFD6BD80C\n\tfor ;\n\tWed, 17 Mar 2021 12:41:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BC4B568D60;\n\tWed, 17 Mar 2021 13:41:24 +0100 (CET)","from metis.ext.pengutronix.de (metis.ext.pengutronix.de\n\t[IPv6:2001:67c:670:201:290:27ff:fe1d:cc33])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 30D93605B1\n\tfor ;\n\tWed, 17 Mar 2021 13:41:23 +0100 (CET)","from erdnuss.hi.pengutronix.de\n\t([2001:67c:670:100:2e4d:54ff:fe9d:849c])\n\tby metis.ext.pengutronix.de with esmtp (Exim 4.92)\n\t(envelope-from )\n\tid 1lMVUI-0003BO-Jb; Wed, 17 Mar 2021 13:41:22 +0100"],"To":"kieran.bingham@ideasonboard.com, libcamera-devel@lists.libcamera.org","References":"<20210316155211.6679-1-m.cichy@pengutronix.de>\n\t<20210316155211.6679-2-m.cichy@pengutronix.de>\n\t<3fa6662c-b338-ac07-8fb0-cedb74e2d46a@ideasonboard.com>","From":"Marian Cichy ","Message-ID":"<853a6e03-9fcd-92a7-e130-fdde6b66138c@pengutronix.de>","Date":"Wed, 17 Mar 2021 13:41:22 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.8.0","MIME-Version":"1.0","In-Reply-To":"<3fa6662c-b338-ac07-8fb0-cedb74e2d46a@ideasonboard.com>","Content-Language":"en-US","X-SA-Exim-Connect-IP":"2001:67c:670:100:2e4d:54ff:fe9d:849c","X-SA-Exim-Mail-From":"mci@pengutronix.de","X-SA-Exim-Scanned":"No (on metis.ext.pengutronix.de);\n\tSAEximRunCond expanded to false","X-PTX-Original-Recipient":"libcamera-devel@lists.libcamera.org","Subject":"Re: [libcamera-devel] [RFC PATCH 1/6] libcamera: Add fraction.h","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"graphics@pengutronix.de","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15743,"web_url":"https://patchwork.libcamera.org/comment/15743/","msgid":"","date":"2021-03-17T13:48:55","subject":"Re: [libcamera-devel] [RFC PATCH 1/6] libcamera: Add fraction.h","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Marian,\n\nOn 17/03/2021 12:41, Marian Cichy wrote:\n> \n> \n> On 3/17/21 12:33 PM, Kieran Bingham wrote:\n>> Hi Marian,\n>>\n>> On 16/03/2021 15:52, Marian Cichy wrote:\n>>> Add a new class that represents a fraction. A structure like this is\n>>> also often used in linux camera drivers, e.g. v4l2_fract to represent a\n>>> frame interval or in applications like Gstreamer to represent frame\n>>> rates.\n>>>\n>>> Adding this class helps to interface frame intervals and frame\n>>> rates in video streams.\n>> This indeed would be helpful in any area that handles time.\n>>\n>> I think this is normally called a rational though?\n> \n> Well .. this is probably mathematical nit-picking, but a fraction is\n> always a construct that consists of two integers, numerator and\n> denominator. A rational number is any number that *can* be displayed as\n> a fraction. For example, \"5\" is in the set of rational numbers, because\n> we can display it as the fraction \"5/1\".\n\nIndeed, and wouldn't we use such a representation in the case of long\nexposures if we use this for frame rates?\n\nIsn't 5/1 the frame rate for a 5 second exposure?\n\nHowever I don't think 5/1 could easily be called a fraction...\n\n\n\n\n> Also, since it is called v4l2_fract in V4L2 and GST_FRACTION in\n> Gstreamer, I went for consistency in the linux media universe.\n\nAh, yes I see that reference later in the series now which I hadn't before.\n\n\n--\nKieran\n\n\n>>\n>> We're really building up a math library in here aren't we with our\n>> sizes, geometry, and number classes...\n>>\n>>> Signed-off-by: Marian Cichy \n>>> ---\n>>>   include/libcamera/fraction.h | 34 ++++++++++++++++++++\n>>>   src/libcamera/fraction.cpp   | 60 ++++++++++++++++++++++++++++++++++++\n>>>   src/libcamera/meson.build    |  1 +\n>>>   3 files changed, 95 insertions(+)\n>>>   create mode 100644 include/libcamera/fraction.h\n>>>   create mode 100644 src/libcamera/fraction.cpp\n>>>\n>>> diff --git a/include/libcamera/fraction.h b/include/libcamera/fraction.h\n>>> new file mode 100644\n>>> index 00000000..aa6a1abb\n>>> --- /dev/null\n>>> +++ b/include/libcamera/fraction.h\n>>> @@ -0,0 +1,34 @@\n>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>> +/*\n>>> + * Copyright (C) 2021, Pengutronix, Marian Cichy\n>>> \n>>> + *\n>>> + * fraction.h - A fraction consisting of two integers\n>>> + */\n>>> +\n>>> +#ifndef __LIBCAMERA_FRACTION_H__\n>>> +#define __LIBCAMERA_FRACTION_H__\n>>> +\n>>> +namespace libcamera {\n>>> +\n>>> +class Fraction {\n>>> +public:\n>>> +    constexpr Fraction() :\n>>> +        numerator(0), denominator(0)\n>>> +    {\n>>> +    }\n>>> +\n>>> +    constexpr Fraction(unsigned int num, unsigned int den) :\n>>> +        numerator(num), denominator(den)\n>>> +    {\n>>> +    }\n>>> +\n>>> +    unsigned int numerator;\n>>> +    unsigned int denominator;\n>> Generically - Fractions/(Rational) numbers could be negative.\n>> But given our use cases, I presume we wouldn't expect  negative values ?\n>>\n>> Well ... not unless we start needing to do things like clock recovery\n>> .... I don't think we'll need to do anything like that ...\n>>\n>> Would you expect further math functions in here to be able to add /\n>> multiple two fractions/rational numbers together?\n>>\n>> In fact - I wonder if this would also tie into the geometry functions\n>> for scaling or such?\n>>\n>>    Size(640, 480) * Rational(1/2) == Size(320, 240);\n>>\n>> I'm not saying the extra functionality possible here is required for\n>> this patch though - just exploring what the goals would be for this\n>> class.\n>>\n>> -- \n>> Kieran\n>>\n>>\n>>\n>>> +\n>>> +    const std::string toString() const;\n>>> +};\n>>> +\n>>> +} /* namespace libcamera */\n>>> +\n>>> +#endif /* __LIBCAMERA_FRACTION_H__ */\n>>> diff --git a/src/libcamera/fraction.cpp b/src/libcamera/fraction.cpp\n>>> new file mode 100644\n>>> index 00000000..76c373aa\n>>> --- /dev/null\n>>> +++ b/src/libcamera/fraction.cpp\n>>> @@ -0,0 +1,60 @@\n>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>> +/*\n>>> + * Copyright (C) 2021, Pengutronix, Marian Cichy\n>>> \n>>> + *\n>>> + * fraction.h - A fraction consisting of two integers\n>>> + */\n>>> +\n>>> +#include \n>>> +#include \n>>> +\n>>> +#include \n>>> +\n>>> +/**\n>>> + * \\file fraction.h\n>>> + * \\brief A fraction consisting of two integers.\n>>> + */\n>>> +\n>>> +namespace libcamera {\n>>> +\n>>> +/**\n>>> + * \\class Fraction\n>>> + * \\brief Represents a fraction with a nominator and a denominator.\n>>> + */\n>>> +\n>>> +/**\n>>> + * \\fn Fraction::Fraction()\n>>> + * \\brief Construct a Fraction with value 0/0. This should be\n>>> interpreted\n>>> + * as invalid or not-used Fraction.\n>>> + */\n>>> +\n>>> +/**\n>>> + * \\fn Fraction::Fraction(unsigned int num, unsigned int den)\n>>> + * \\brief Construct a Fraction with value n/d.\n>>> + */\n>>> +\n>>> +/**\n>>> + * \\var Fraction::numerator\n>>> + * \\brief The numerator of the fraction.\n>>> + */\n>>> +\n>>> +/**\n>>> + * \\var Fraction::denominator\n>>> + * \\brief The denominator of the fraction.\n>>> + */\n>>> +\n>>> +/**\n>>> + * \\brief Assemble and return a string describing the fraction\n>>> + * \\return A string describing the fraction.\n>>> + */\n>>> +const std::string Fraction::toString() const\n>>> +{\n>>> +    std::stringstream ss;\n>>> +\n>>> +    ss << numerator << \"/\" << denominator;\n>>> +\n>>> +    return ss.str();\n>>> +}\n>>> +\n>>> +} /* namespace libcamera */\n>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n>>> index 815629db..7927481f 100644\n>>> --- a/src/libcamera/meson.build\n>>> +++ b/src/libcamera/meson.build\n>>> @@ -22,6 +22,7 @@ libcamera_sources = files([\n>>>       'file.cpp',\n>>>       'file_descriptor.cpp',\n>>>       'formats.cpp',\n>>> +    'fraction.cpp',\n>>>       'framebuffer_allocator.cpp',\n>>>       'geometry.cpp',\n>>>       'ipa_controls.cpp',\n>>>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E136BC32E1\n\tfor ;\n\tWed, 17 Mar 2021 13:49:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2856A68D60;\n\tWed, 17 Mar 2021 14:49:01 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 12FA8605B1\n\tfor ;\n\tWed, 17 Mar 2021 14:48:59 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6E687596;\n\tWed, 17 Mar 2021 14:48:58 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Dd2B4shY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615988938;\n\tbh=TT8x16PQy3qt601KveGoqTQoLWkfKMAP8cUjAX+A1sw=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=Dd2B4shY8uw4k60CqfzAPaDmitvJ7zDKwmjDMIgxgY7PtgzDlB7cfVUiVJHTTYuZw\n\tQu1QzMF48t6ZX6I/i13vmu8UMjbZyGShbe+vH2yfM3mniRxzTZc8f57mT5bEt2EWZ8\n\t1q4iMg+ncUdU02Rp8aHkRqBurXoeG7+5k7KMVNIc=","To":"Marian Cichy , libcamera-devel@lists.libcamera.org","References":"<20210316155211.6679-1-m.cichy@pengutronix.de>\n\t<20210316155211.6679-2-m.cichy@pengutronix.de>\n\t<3fa6662c-b338-ac07-8fb0-cedb74e2d46a@ideasonboard.com>\n\t<853a6e03-9fcd-92a7-e130-fdde6b66138c@pengutronix.de>","From":"Kieran Bingham ","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\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"","Date":"Wed, 17 Mar 2021 13:48:55 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<853a6e03-9fcd-92a7-e130-fdde6b66138c@pengutronix.de>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [RFC PATCH 1/6] libcamera: Add fraction.h","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"graphics@pengutronix.de","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15748,"web_url":"https://patchwork.libcamera.org/comment/15748/","msgid":"","date":"2021-03-17T17:19:26","subject":"Re: [libcamera-devel] [RFC PATCH 1/6] libcamera: Add fraction.h","submitter":{"id":81,"url":"https://patchwork.libcamera.org/api/people/81/","name":"Marian Cichy","email":"mci@pengutronix.de"},"content":"On 3/17/21 2:48 PM, Kieran Bingham wrote:\n> Hi Marian,\n>\n> On 17/03/2021 12:41, Marian Cichy wrote:\n>>\n>> On 3/17/21 12:33 PM, Kieran Bingham wrote:\n>>> Hi Marian,\n>>>\n>>> On 16/03/2021 15:52, Marian Cichy wrote:\n>>>> Add a new class that represents a fraction. A structure like this is\n>>>> also often used in linux camera drivers, e.g. v4l2_fract to represent a\n>>>> frame interval or in applications like Gstreamer to represent frame\n>>>> rates.\n>>>>\n>>>> Adding this class helps to interface frame intervals and frame\n>>>> rates in video streams.\n>>> This indeed would be helpful in any area that handles time.\n>>>\n>>> I think this is normally called a rational though?\n>> Well .. this is probably mathematical nit-picking, but a fraction is\n>> always a construct that consists of two integers, numerator and\n>> denominator. A rational number is any number that *can* be displayed as\n>> a fraction. For example, \"5\" is in the set of rational numbers, because\n>> we can display it as the fraction \"5/1\".\n> Indeed, and wouldn't we use such a representation in the case of long\n> exposures if we use this for frame rates?\n\n\nBut as the frame rate is configurable to be, for example 30 or 29.97, it \nis only sometimes representable by single integers but always by \nfractions. Gstreamer also reports 30/1 for a frame rate of 30.\n\n\n>\n> Isn't 5/1 the frame rate for a 5 second exposure?\n\n\n5/1 is the frame rate of 0.2 seconds exposure\n\n5 frames per 1 second\n\n5/1 is the frame interval of 5 seconds exposure\n\n5 seconds interval for 1 frame\n\nI know, numbers are confusing :-(\n\n\n>\n> However I don't think 5/1 could easily be called a fraction...\n>\n> fractions'>\n>\n>\n>> Also, since it is called v4l2_fract in V4L2 and GST_FRACTION in\n>> Gstreamer, I went for consistency in the linux media universe.\n> Ah, yes I see that reference later in the series now which I hadn't before.\n>\n>\n> --\n> Kieran\n>\n>\n>>> We're really building up a math library in here aren't we with our\n>>> sizes, geometry, and number classes...\n>>>\n>>>> Signed-off-by: Marian Cichy \n>>>> ---\n>>>>   include/libcamera/fraction.h | 34 ++++++++++++++++++++\n>>>>   src/libcamera/fraction.cpp   | 60 ++++++++++++++++++++++++++++++++++++\n>>>>   src/libcamera/meson.build    |  1 +\n>>>>   3 files changed, 95 insertions(+)\n>>>>   create mode 100644 include/libcamera/fraction.h\n>>>>   create mode 100644 src/libcamera/fraction.cpp\n>>>>\n>>>> diff --git a/include/libcamera/fraction.h b/include/libcamera/fraction.h\n>>>> new file mode 100644\n>>>> index 00000000..aa6a1abb\n>>>> --- /dev/null\n>>>> +++ b/include/libcamera/fraction.h\n>>>> @@ -0,0 +1,34 @@\n>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>>> +/*\n>>>> + * Copyright (C) 2021, Pengutronix, Marian Cichy\n>>>> \n>>>> + *\n>>>> + * fraction.h - A fraction consisting of two integers\n>>>> + */\n>>>> +\n>>>> +#ifndef __LIBCAMERA_FRACTION_H__\n>>>> +#define __LIBCAMERA_FRACTION_H__\n>>>> +\n>>>> +namespace libcamera {\n>>>> +\n>>>> +class Fraction {\n>>>> +public:\n>>>> +    constexpr Fraction() :\n>>>> +        numerator(0), denominator(0)\n>>>> +    {\n>>>> +    }\n>>>> +\n>>>> +    constexpr Fraction(unsigned int num, unsigned int den) :\n>>>> +        numerator(num), denominator(den)\n>>>> +    {\n>>>> +    }\n>>>> +\n>>>> +    unsigned int numerator;\n>>>> +    unsigned int denominator;\n>>> Generically - Fractions/(Rational) numbers could be negative.\n>>> But given our use cases, I presume we wouldn't expect  negative values ?\n>>>\n>>> Well ... not unless we start needing to do things like clock recovery\n>>> .... I don't think we'll need to do anything like that ...\n>>>\n>>> Would you expect further math functions in here to be able to add /\n>>> multiple two fractions/rational numbers together?\n>>>\n>>> In fact - I wonder if this would also tie into the geometry functions\n>>> for scaling or such?\n>>>\n>>>    Size(640, 480) * Rational(1/2) == Size(320, 240);\n>>>\n>>> I'm not saying the extra functionality possible here is required for\n>>> this patch though - just exploring what the goals would be for this\n>>> class.\n>>>\n>>> -- \n>>> Kieran\n>>>\n>>>\n>>>\n>>>> +\n>>>> +    const std::string toString() const;\n>>>> +};\n>>>> +\n>>>> +} /* namespace libcamera */\n>>>> +\n>>>> +#endif /* __LIBCAMERA_FRACTION_H__ */\n>>>> diff --git a/src/libcamera/fraction.cpp b/src/libcamera/fraction.cpp\n>>>> new file mode 100644\n>>>> index 00000000..76c373aa\n>>>> --- /dev/null\n>>>> +++ b/src/libcamera/fraction.cpp\n>>>> @@ -0,0 +1,60 @@\n>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>>> +/*\n>>>> + * Copyright (C) 2021, Pengutronix, Marian Cichy\n>>>> \n>>>> + *\n>>>> + * fraction.h - A fraction consisting of two integers\n>>>> + */\n>>>> +\n>>>> +#include \n>>>> +#include \n>>>> +\n>>>> +#include \n>>>> +\n>>>> +/**\n>>>> + * \\file fraction.h\n>>>> + * \\brief A fraction consisting of two integers.\n>>>> + */\n>>>> +\n>>>> +namespace libcamera {\n>>>> +\n>>>> +/**\n>>>> + * \\class Fraction\n>>>> + * \\brief Represents a fraction with a nominator and a denominator.\n>>>> + */\n>>>> +\n>>>> +/**\n>>>> + * \\fn Fraction::Fraction()\n>>>> + * \\brief Construct a Fraction with value 0/0. This should be\n>>>> interpreted\n>>>> + * as invalid or not-used Fraction.\n>>>> + */\n>>>> +\n>>>> +/**\n>>>> + * \\fn Fraction::Fraction(unsigned int num, unsigned int den)\n>>>> + * \\brief Construct a Fraction with value n/d.\n>>>> + */\n>>>> +\n>>>> +/**\n>>>> + * \\var Fraction::numerator\n>>>> + * \\brief The numerator of the fraction.\n>>>> + */\n>>>> +\n>>>> +/**\n>>>> + * \\var Fraction::denominator\n>>>> + * \\brief The denominator of the fraction.\n>>>> + */\n>>>> +\n>>>> +/**\n>>>> + * \\brief Assemble and return a string describing the fraction\n>>>> + * \\return A string describing the fraction.\n>>>> + */\n>>>> +const std::string Fraction::toString() const\n>>>> +{\n>>>> +    std::stringstream ss;\n>>>> +\n>>>> +    ss << numerator << \"/\" << denominator;\n>>>> +\n>>>> +    return ss.str();\n>>>> +}\n>>>> +\n>>>> +} /* namespace libcamera */\n>>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n>>>> index 815629db..7927481f 100644\n>>>> --- a/src/libcamera/meson.build\n>>>> +++ b/src/libcamera/meson.build\n>>>> @@ -22,6 +22,7 @@ libcamera_sources = files([\n>>>>       'file.cpp',\n>>>>       'file_descriptor.cpp',\n>>>>       'formats.cpp',\n>>>> +    'fraction.cpp',\n>>>>       'framebuffer_allocator.cpp',\n>>>>       'geometry.cpp',\n>>>>       'ipa_controls.cpp',\n>>>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1D806BD80C\n\tfor ;\n\tWed, 17 Mar 2021 17:19:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8D9A168D60;\n\tWed, 17 Mar 2021 18:19:29 +0100 (CET)","from metis.ext.pengutronix.de (metis.ext.pengutronix.de\n\t[IPv6:2001:67c:670:201:290:27ff:fe1d:cc33])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0C1CE6084F\n\tfor ;\n\tWed, 17 Mar 2021 18:19:28 +0100 (CET)","from gallifrey.ext.pengutronix.de\n\t([2001:67c:670:201:5054:ff:fe8d:eefb] helo=[127.0.0.1])\n\tby metis.ext.pengutronix.de with esmtp (Exim 4.92)\n\t(envelope-from )\n\tid 1lMZpP-0000tz-CN; Wed, 17 Mar 2021 18:19:27 +0100"],"To":"kieran.bingham@ideasonboard.com, libcamera-devel@lists.libcamera.org","References":"<20210316155211.6679-1-m.cichy@pengutronix.de>\n\t<20210316155211.6679-2-m.cichy@pengutronix.de>\n\t<3fa6662c-b338-ac07-8fb0-cedb74e2d46a@ideasonboard.com>\n\t<853a6e03-9fcd-92a7-e130-fdde6b66138c@pengutronix.de>\n\t","From":"Marian Cichy ","Message-ID":"","Date":"Wed, 17 Mar 2021 18:19:26 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.5.0","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-US","X-SA-Exim-Connect-IP":"2001:67c:670:201:5054:ff:fe8d:eefb","X-SA-Exim-Mail-From":"mci@pengutronix.de","X-SA-Exim-Scanned":"No (on metis.ext.pengutronix.de);\n\tSAEximRunCond expanded to false","X-PTX-Original-Recipient":"libcamera-devel@lists.libcamera.org","Subject":"Re: [libcamera-devel] [RFC PATCH 1/6] libcamera: Add fraction.h","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"graphics@pengutronix.de","Content-Transfer-Encoding":"base64","Content-Type":"text/plain; charset=\"utf-8\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15766,"web_url":"https://patchwork.libcamera.org/comment/15766/","msgid":"","date":"2021-03-19T00:14:19","subject":"Re: [libcamera-devel] [RFC PATCH 1/6] libcamera: Add fraction.h","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Marian,\n\nOn Wed, Mar 17, 2021 at 06:19:26PM +0100, Marian Cichy wrote:\n> On 3/17/21 2:48 PM, Kieran Bingham wrote:\n> > On 17/03/2021 12:41, Marian Cichy wrote:\n> >> On 3/17/21 12:33 PM, Kieran Bingham wrote:\n> >>> On 16/03/2021 15:52, Marian Cichy wrote:\n> >>>> Add a new class that represents a fraction. A structure like this is\n> >>>> also often used in linux camera drivers, e.g. v4l2_fract to represent a\n> >>>> frame interval or in applications like Gstreamer to represent frame\n> >>>> rates.\n> >>>>\n> >>>> Adding this class helps to interface frame intervals and frame\n> >>>> rates in video streams.\n> >>> This indeed would be helpful in any area that handles time.\n> >>>\n> >>> I think this is normally called a rational though?\n> >> Well .. this is probably mathematical nit-picking, but a fraction is\n> >> always a construct that consists of two integers, numerator and\n> >> denominator. A rational number is any number that *can* be displayed as\n> >> a fraction. For example, \"5\" is in the set of rational numbers, because\n> >> we can display it as the fraction \"5/1\".\n> > Indeed, and wouldn't we use such a representation in the case of long\n> > exposures if we use this for frame rates?\n> \n> But as the frame rate is configurable to be, for example 30 or 29.97, it \n> is only sometimes representable by single integers but always by \n> fractions. Gstreamer also reports 30/1 for a frame rate of 30.\n\nThis would be true if the frame rate was really 30 or 29.97 fps, but\nthat's rarely the case in practice with cameras. For instance, there's\nan Intel camera HAL implementation that produces ~29.94 fps, and reports\nan exact 30 fps to Android as the Android camera framework wants at\nleast 29.97 fps for the corresponding use case.\n\nI'm not entirely opposed to using fractions, but we need to consider the\nimplications carefully, looking at the big picture. It's a non-trivial\ntopic, and lots of assumptions from the TV world don't apply as-is to\ncameras.\n\n> > Isn't 5/1 the frame rate for a 5 second exposure?\n> \n> 5/1 is the frame rate of 0.2 seconds exposure\n\nFor 0.2s of frame duration. The exposure time can be (and usually is)\nshorter.\n\n> 5 frames per 1 second\n> \n> 5/1 is the frame interval of 5 seconds exposure\n> \n> 5 seconds interval for 1 frame\n> \n> I know, numbers are confusing :-(\n> \n> > However I don't think 5/1 could easily be called a fraction...\n> >\n> > > fractions'>\n> >\n> >> Also, since it is called v4l2_fract in V4L2 and GST_FRACTION in\n> >> Gstreamer, I went for consistency in the linux media universe.\n> >\n> > Ah, yes I see that reference later in the series now which I hadn't before.\n> >\n> >>> We're really building up a math library in here aren't we with our\n> >>> sizes, geometry, and number classes...\n> >>>\n> >>>> Signed-off-by: Marian Cichy \n> >>>> ---\n> >>>>   include/libcamera/fraction.h | 34 ++++++++++++++++++++\n> >>>>   src/libcamera/fraction.cpp   | 60 ++++++++++++++++++++++++++++++++++++\n> >>>>   src/libcamera/meson.build    |  1 +\n> >>>>   3 files changed, 95 insertions(+)\n> >>>>   create mode 100644 include/libcamera/fraction.h\n> >>>>   create mode 100644 src/libcamera/fraction.cpp\n> >>>>\n> >>>> diff --git a/include/libcamera/fraction.h b/include/libcamera/fraction.h\n> >>>> new file mode 100644\n> >>>> index 00000000..aa6a1abb\n> >>>> --- /dev/null\n> >>>> +++ b/include/libcamera/fraction.h\n> >>>> @@ -0,0 +1,34 @@\n> >>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >>>> +/*\n> >>>> + * Copyright (C) 2021, Pengutronix, Marian Cichy\n> >>>> \n> >>>> + *\n> >>>> + * fraction.h - A fraction consisting of two integers\n> >>>> + */\n> >>>> +\n> >>>> +#ifndef __LIBCAMERA_FRACTION_H__\n> >>>> +#define __LIBCAMERA_FRACTION_H__\n> >>>> +\n> >>>> +namespace libcamera {\n> >>>> +\n> >>>> +class Fraction {\n> >>>> +public:\n> >>>> +    constexpr Fraction() :\n> >>>> +        numerator(0), denominator(0)\n> >>>> +    {\n> >>>> +    }\n> >>>> +\n> >>>> +    constexpr Fraction(unsigned int num, unsigned int den) :\n> >>>> +        numerator(num), denominator(den)\n> >>>> +    {\n> >>>> +    }\n> >>>> +\n> >>>> +    unsigned int numerator;\n> >>>> +    unsigned int denominator;\n> >>> Generically - Fractions/(Rational) numbers could be negative.\n> >>> But given our use cases, I presume we wouldn't expect  negative values ?\n> >>>\n> >>> Well ... not unless we start needing to do things like clock recovery\n> >>> .... I don't think we'll need to do anything like that ...\n> >>>\n> >>> Would you expect further math functions in here to be able to add /\n> >>> multiple two fractions/rational numbers together?\n> >>>\n> >>> In fact - I wonder if this would also tie into the geometry functions\n> >>> for scaling or such?\n> >>>\n> >>>    Size(640, 480) * Rational(1/2) == Size(320, 240);\n> >>>\n> >>> I'm not saying the extra functionality possible here is required for\n> >>> this patch though - just exploring what the goals would be for this\n> >>> class.\n> >>>\n> >>> -- \n> >>> Kieran\n> >>>\n> >>>\n> >>>\n> >>>> +\n> >>>> +    const std::string toString() const;\n> >>>> +};\n> >>>> +\n> >>>> +} /* namespace libcamera */\n> >>>> +\n> >>>> +#endif /* __LIBCAMERA_FRACTION_H__ */\n> >>>> diff --git a/src/libcamera/fraction.cpp b/src/libcamera/fraction.cpp\n> >>>> new file mode 100644\n> >>>> index 00000000..76c373aa\n> >>>> --- /dev/null\n> >>>> +++ b/src/libcamera/fraction.cpp\n> >>>> @@ -0,0 +1,60 @@\n> >>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >>>> +/*\n> >>>> + * Copyright (C) 2021, Pengutronix, Marian Cichy\n> >>>> \n> >>>> + *\n> >>>> + * fraction.h - A fraction consisting of two integers\n> >>>> + */\n> >>>> +\n> >>>> +#include \n> >>>> +#include \n> >>>> +\n> >>>> +#include \n> >>>> +\n> >>>> +/**\n> >>>> + * \\file fraction.h\n> >>>> + * \\brief A fraction consisting of two integers.\n> >>>> + */\n> >>>> +\n> >>>> +namespace libcamera {\n> >>>> +\n> >>>> +/**\n> >>>> + * \\class Fraction\n> >>>> + * \\brief Represents a fraction with a nominator and a denominator.\n> >>>> + */\n> >>>> +\n> >>>> +/**\n> >>>> + * \\fn Fraction::Fraction()\n> >>>> + * \\brief Construct a Fraction with value 0/0. This should be\n> >>>> interpreted\n> >>>> + * as invalid or not-used Fraction.\n> >>>> + */\n> >>>> +\n> >>>> +/**\n> >>>> + * \\fn Fraction::Fraction(unsigned int num, unsigned int den)\n> >>>> + * \\brief Construct a Fraction with value n/d.\n> >>>> + */\n> >>>> +\n> >>>> +/**\n> >>>> + * \\var Fraction::numerator\n> >>>> + * \\brief The numerator of the fraction.\n> >>>> + */\n> >>>> +\n> >>>> +/**\n> >>>> + * \\var Fraction::denominator\n> >>>> + * \\brief The denominator of the fraction.\n> >>>> + */\n> >>>> +\n> >>>> +/**\n> >>>> + * \\brief Assemble and return a string describing the fraction\n> >>>> + * \\return A string describing the fraction.\n> >>>> + */\n> >>>> +const std::string Fraction::toString() const\n> >>>> +{\n> >>>> +    std::stringstream ss;\n> >>>> +\n> >>>> +    ss << numerator << \"/\" << denominator;\n> >>>> +\n> >>>> +    return ss.str();\n> >>>> +}\n> >>>> +\n> >>>> +} /* namespace libcamera */\n> >>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> >>>> index 815629db..7927481f 100644\n> >>>> --- a/src/libcamera/meson.build\n> >>>> +++ b/src/libcamera/meson.build\n> >>>> @@ -22,6 +22,7 @@ libcamera_sources = files([\n> >>>>       'file.cpp',\n> >>>>       'file_descriptor.cpp',\n> >>>>       'formats.cpp',\n> >>>> +    'fraction.cpp',\n> >>>>       'framebuffer_allocator.cpp',\n> >>>>       'geometry.cpp',\n> >>>>       'ipa_controls.cpp',","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D9045BD80C\n\tfor ;\n\tFri, 19 Mar 2021 00:15:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 573ED68D51;\n\tFri, 19 Mar 2021 01:15:00 +0100 (CET)","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 3F2C76051D\n\tfor ;\n\tFri, 19 Mar 2021 01:14:58 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AAA974FD;\n\tFri, 19 Mar 2021 01:14:57 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ofAcDvuB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616112897;\n\tbh=wcdsQY/oUmgfoS1qGwuS6wG7PEntaZKO8Xo9sOGvv9o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ofAcDvuBSC3Uxcs+Iub3wIkM0NCZJKklBchvhNp8uufADLhqtwrq9dJHkEss30Vat\n\tFBCVOY1zsBDYG770eAQQ6oy3qKK13qujaV7HURDsypziRttkuXj+1kgbSAlgcq+K/5\n\tBDBy4rn6gX9q1nN1kCKXfkZCgGvk65sIyxvfvsH0=","Date":"Fri, 19 Mar 2021 02:14:19 +0200","From":"Laurent Pinchart ","To":"Marian Cichy ","Message-ID":"","References":"<20210316155211.6679-1-m.cichy@pengutronix.de>\n\t<20210316155211.6679-2-m.cichy@pengutronix.de>\n\t<3fa6662c-b338-ac07-8fb0-cedb74e2d46a@ideasonboard.com>\n\t<853a6e03-9fcd-92a7-e130-fdde6b66138c@pengutronix.de>\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH 1/6] libcamera: Add fraction.h","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org, graphics@pengutronix.de","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15768,"web_url":"https://patchwork.libcamera.org/comment/15768/","msgid":"","date":"2021-03-19T09:43:48","subject":"Re: [libcamera-devel] [RFC PATCH 1/6] libcamera: Add fraction.h","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi everyone\n\nThanks for this interesting proposal. I don't normally venture too many\nopinions on stuff that doesn't really affect the Pi, but I did have just a\ncouple of comments on this one, if that's all right!\n\nOn Fri, 19 Mar 2021 at 00:15, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Marian,\n>\n> On Wed, Mar 17, 2021 at 06:19:26PM +0100, Marian Cichy wrote:\n> > On 3/17/21 2:48 PM, Kieran Bingham wrote:\n> > > On 17/03/2021 12:41, Marian Cichy wrote:\n> > >> On 3/17/21 12:33 PM, Kieran Bingham wrote:\n> > >>> On 16/03/2021 15:52, Marian Cichy wrote:\n> > >>>> Add a new class that represents a fraction. A structure like this is\n> > >>>> also often used in linux camera drivers, e.g. v4l2_fract to\n> represent a\n> > >>>> frame interval or in applications like Gstreamer to represent frame\n> > >>>> rates.\n> > >>>>\n> > >>>> Adding this class helps to interface frame intervals and frame\n> > >>>> rates in video streams.\n> > >>> This indeed would be helpful in any area that handles time.\n> > >>>\n> > >>> I think this is normally called a rational though?\n>\n\n1. I think \"fraction\" is not an ideal name. The very fact that we've seen\nsome discussion about what, exactly, a \"fraction\" is kind of demonstrates\nthe problem.\n\nPersonally, I would be inclined to go with \"rational\". It has a cast iron\nand straightforward mathematical definition. (Numbers of the form p/q,\nwhere p and q are both integers, q != 0).\n\n2. Following the idea of using rationals, the numerator would need to\nbecome signed.\n\nIn particular, this makes rationals closed under the usual arithmetic\noperations (+ - * /). There may be no plans to overload them, but you could\nimagine, in a C++ world, someone wanting to do that and finding the lack of\nnegative numbers rather awkward! With rationals, you at least get to do\nwhatever calculation you want, and test if the result is \">= 0\" or not at\nthe end.\n\n> >> Well .. this is probably mathematical nit-picking, but a fraction is\n> > >> always a construct that consists of two integers, numerator and\n> > >> denominator. A rational number is any number that *can* be displayed\n> as\n> > >> a fraction. For example, \"5\" is in the set of rational numbers,\n> because\n> > >> we can display it as the fraction \"5/1\".\n> > > Indeed, and wouldn't we use such a representation in the case of long\n> > > exposures if we use this for frame rates?\n> >\n> > But as the frame rate is configurable to be, for example 30 or 29.97, it\n> > is only sometimes representable by single integers but always by\n> > fractions. Gstreamer also reports 30/1 for a frame rate of 30.\n>\n> This would be true if the frame rate was really 30 or 29.97 fps, but\n> that's rarely the case in practice with cameras. For instance, there's\n> an Intel camera HAL implementation that produces ~29.94 fps, and reports\n> an exact 30 fps to Android as the Android camera framework wants at\n> least 29.97 fps for the corresponding use case.\n>\n> I'm not entirely opposed to using fractions, but we need to consider the\n> implications carefully, looking at the big picture. It's a non-trivial\n> topic, and lots of assumptions from the TV world don't apply as-is to\n> cameras.\n>\n> > > Isn't 5/1 the frame rate for a 5 second exposure?\n> >\n> > 5/1 is the frame rate of 0.2 seconds exposure\n>\n> For 0.2s of frame duration. The exposure time can be (and usually is)\n> shorter.\n>\n> > 5 frames per 1 second\n> >\n> > 5/1 is the frame interval of 5 seconds exposure\n> >\n> > 5 seconds interval for 1 frame\n> >\n> > I know, numbers are confusing :-(\n> >\n> > > However I don't think 5/1 could easily be called a fraction...\n> > >\n> > > > > fractions'>\n> > >\n> > >> Also, since it is called v4l2_fract in V4L2 and GST_FRACTION in\n> > >> Gstreamer, I went for consistency in the linux media universe.\n> > >\n>\n\nI appreciate that \"rationals\" don't 100% match the v4l2_fract any more, but\nthe inability to represent some of those rationals where you need full\n32-bit positive denominators seems negligible to me. However, I'd be\ndelighted to be educated on that if anyone can think of any examples!\n\nThanks and best regards\nDavid\n\n\n> > > Ah, yes I see that reference later in the series now which I hadn't\n> before.\n> > >\n> > >>> We're really building up a math library in here aren't we with our\n> > >>> sizes, geometry, and number classes...\n> > >>>\n> > >>>> Signed-off-by: Marian Cichy \n> > >>>> ---\n> > >>>> include/libcamera/fraction.h | 34 ++++++++++++++++++++\n> > >>>> src/libcamera/fraction.cpp | 60\n> ++++++++++++++++++++++++++++++++++++\n> > >>>> src/libcamera/meson.build | 1 +\n> > >>>> 3 files changed, 95 insertions(+)\n> > >>>> create mode 100644 include/libcamera/fraction.h\n> > >>>> create mode 100644 src/libcamera/fraction.cpp\n> > >>>>\n> > >>>> diff --git a/include/libcamera/fraction.h\n> b/include/libcamera/fraction.h\n> > >>>> new file mode 100644\n> > >>>> index 00000000..aa6a1abb\n> > >>>> --- /dev/null\n> > >>>> +++ b/include/libcamera/fraction.h\n> > >>>> @@ -0,0 +1,34 @@\n> > >>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > >>>> +/*\n> > >>>> + * Copyright (C) 2021, Pengutronix, Marian Cichy\n> > >>>> \n> > >>>> + *\n> > >>>> + * fraction.h - A fraction consisting of two integers\n> > >>>> + */\n> > >>>> +\n> > >>>> +#ifndef __LIBCAMERA_FRACTION_H__\n> > >>>> +#define __LIBCAMERA_FRACTION_H__\n> > >>>> +\n> > >>>> +namespace libcamera {\n> > >>>> +\n> > >>>> +class Fraction {\n> > >>>> +public:\n> > >>>> + constexpr Fraction() :\n> > >>>> + numerator(0), denominator(0)\n> > >>>> + {\n> > >>>> + }\n> > >>>> +\n> > >>>> + constexpr Fraction(unsigned int num, unsigned int den) :\n> > >>>> + numerator(num), denominator(den)\n> > >>>> + {\n> > >>>> + }\n> > >>>> +\n> > >>>> + unsigned int numerator;\n> > >>>> + unsigned int denominator;\n> > >>> Generically - Fractions/(Rational) numbers could be negative.\n> > >>> But given our use cases, I presume we wouldn't expect negative\n> values ?\n> > >>>\n> > >>> Well ... not unless we start needing to do things like clock recovery\n> > >>> .... I don't think we'll need to do anything like that ...\n> > >>>\n> > >>> Would you expect further math functions in here to be able to add /\n> > >>> multiple two fractions/rational numbers together?\n> > >>>\n> > >>> In fact - I wonder if this would also tie into the geometry functions\n> > >>> for scaling or such?\n> > >>>\n> > >>> Size(640, 480) * Rational(1/2) == Size(320, 240);\n> > >>>\n> > >>> I'm not saying the extra functionality possible here is required for\n> > >>> this patch though - just exploring what the goals would be for this\n> > >>> class.\n> > >>>\n> > >>> --\n> > >>> Kieran\n> > >>>\n> > >>>\n> > >>>\n> > >>>> +\n> > >>>> + const std::string toString() const;\n> > >>>> +};\n> > >>>> +\n> > >>>> +} /* namespace libcamera */\n> > >>>> +\n> > >>>> +#endif /* __LIBCAMERA_FRACTION_H__ */\n> > >>>> diff --git a/src/libcamera/fraction.cpp b/src/libcamera/fraction.cpp\n> > >>>> new file mode 100644\n> > >>>> index 00000000..76c373aa\n> > >>>> --- /dev/null\n> > >>>> +++ b/src/libcamera/fraction.cpp\n> > >>>> @@ -0,0 +1,60 @@\n> > >>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > >>>> +/*\n> > >>>> + * Copyright (C) 2021, Pengutronix, Marian Cichy\n> > >>>> \n> > >>>> + *\n> > >>>> + * fraction.h - A fraction consisting of two integers\n> > >>>> + */\n> > >>>> +\n> > >>>> +#include \n> > >>>> +#include \n> > >>>> +\n> > >>>> +#include \n> > >>>> +\n> > >>>> +/**\n> > >>>> + * \\file fraction.h\n> > >>>> + * \\brief A fraction consisting of two integers.\n> > >>>> + */\n> > >>>> +\n> > >>>> +namespace libcamera {\n> > >>>> +\n> > >>>> +/**\n> > >>>> + * \\class Fraction\n> > >>>> + * \\brief Represents a fraction with a nominator and a denominator.\n> > >>>> + */\n> > >>>> +\n> > >>>> +/**\n> > >>>> + * \\fn Fraction::Fraction()\n> > >>>> + * \\brief Construct a Fraction with value 0/0. This should be\n> > >>>> interpreted\n> > >>>> + * as invalid or not-used Fraction.\n> > >>>> + */\n> > >>>> +\n> > >>>> +/**\n> > >>>> + * \\fn Fraction::Fraction(unsigned int num, unsigned int den)\n> > >>>> + * \\brief Construct a Fraction with value n/d.\n> > >>>> + */\n> > >>>> +\n> > >>>> +/**\n> > >>>> + * \\var Fraction::numerator\n> > >>>> + * \\brief The numerator of the fraction.\n> > >>>> + */\n> > >>>> +\n> > >>>> +/**\n> > >>>> + * \\var Fraction::denominator\n> > >>>> + * \\brief The denominator of the fraction.\n> > >>>> + */\n> > >>>> +\n> > >>>> +/**\n> > >>>> + * \\brief Assemble and return a string describing the fraction\n> > >>>> + * \\return A string describing the fraction.\n> > >>>> + */\n> > >>>> +const std::string Fraction::toString() const\n> > >>>> +{\n> > >>>> + std::stringstream ss;\n> > >>>> +\n> > >>>> + ss << numerator << \"/\" << denominator;\n> > >>>> +\n> > >>>> + return ss.str();\n> > >>>> +}\n> > >>>> +\n> > >>>> +} /* namespace libcamera */\n> > >>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > >>>> index 815629db..7927481f 100644\n> > >>>> --- a/src/libcamera/meson.build\n> > >>>> +++ b/src/libcamera/meson.build\n> > >>>> @@ -22,6 +22,7 @@ libcamera_sources = files([\n> > >>>> 'file.cpp',\n> > >>>> 'file_descriptor.cpp',\n> > >>>> 'formats.cpp',\n> > >>>> + 'fraction.cpp',\n> > >>>> 'framebuffer_allocator.cpp',\n> > >>>> 'geometry.cpp',\n> > >>>> 'ipa_controls.cpp',\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 26428BD80C\n\tfor ;\n\tFri, 19 Mar 2021 09:44:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3C9B968D65;\n\tFri, 19 Mar 2021 10:44:01 +0100 (CET)","from mail-ot1-x32c.google.com (mail-ot1-x32c.google.com\n\t[IPv6:2607:f8b0:4864:20::32c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E92CD6084F\n\tfor ;\n\tFri, 19 Mar 2021 10:43:59 +0100 (CET)","by mail-ot1-x32c.google.com with SMTP id\n\ty19-20020a0568301d93b02901b9f88a238eso7916677oti.11\n\tfor ;\n\tFri, 19 Mar 2021 02:43:59 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"Ce7RxXn0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=A6e++fmOqxUMdPrxbzfcYvu0993o6mdgUN9zOiInCmE=;\n\tb=Ce7RxXn0ADTW8LUcyqoAbqF26XeD/qypVy/RbuQlZeaR7X2oHylrvt/FhnPFf8L9lZ\n\t4OYQhI65j8Pqqaa/llOJz40xNWX/HLYcleUdalesvN69e5eFkhrJzoO5/Ed2XzqP1ijf\n\tH11exYt/eEmGFzXeijZwiaAWXM/f9joWHlVgPS5ndGujsxG+aLbZ1k2yR7UJqOtqJADH\n\tzXwE7Rtuk1eaiZhllpKjOLc6w3uu3jD6Z500dNZbEUPE3iQj7gfjUpbo0ggjXXnApWEy\n\t5Z9QmGL2iQ3Vq0Iln2UNinG88G3nYoRjxesCbRBofk83WegGwrsb3vLLqwNfihu7JiJ6\n\tB84A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=A6e++fmOqxUMdPrxbzfcYvu0993o6mdgUN9zOiInCmE=;\n\tb=e5wcau4MA+i+PsZhV21wwI6vKH3Xu+3nNYOdxV3hUzXG3HqXTur2GCvLI5Sk9yHUAb\n\t0yRCteMK06nHBggj8sxUmC6yFMy2XHHKU0RNCdyvG9/lrLzHnf9KY4Ry53q3CK6WJtsJ\n\t7nwatJfJqWFxC+P3r/9cjhW43xWaezCNfRB3XtDZs2VbCH3F+MFI+4Kg+S2Vbfg4pKMC\n\tGma2y8GkSjTFgyZtvaXyQih95L0ZNz4gLKqNKl3O3XuNXSmioKtaug159KPvfqmuOq6D\n\t0VetGDcWtxDh+X086q1fYaRt/q352MtNOqoMbqVAhhuzYkxMoyHvCawO9S5y+47Doh7F\n\t/JpA==","X-Gm-Message-State":"AOAM530jWcHMUrRiqUt5znEjvZe4EvtIJ6PCdPffPsIkDmRTuV6KyP33\n\t3LcAIAMzMQFCY5bcQ5h2R20JqnNQ3TfxQHCInkw/+g==","X-Google-Smtp-Source":"ABdhPJy/dnQE8xOAp7utVOs/4tn+3AxvYig9U6kujcXjIXG3uE61PiCCuDgxu9eNI3IRjBdTIUF5OmV4kgjxBbQv2N4=","X-Received":"by 2002:a9d:6ad6:: with SMTP id m22mr414703otq.160.1616147038583;\n\tFri, 19 Mar 2021 02:43:58 -0700 (PDT)","MIME-Version":"1.0","References":"<20210316155211.6679-1-m.cichy@pengutronix.de>\n\t<20210316155211.6679-2-m.cichy@pengutronix.de>\n\t<3fa6662c-b338-ac07-8fb0-cedb74e2d46a@ideasonboard.com>\n\t<853a6e03-9fcd-92a7-e130-fdde6b66138c@pengutronix.de>\n\t\n\t\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Fri, 19 Mar 2021 09:43:48 +0000","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [RFC PATCH 1/6] libcamera: Add fraction.h","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ,\n\tgraphics@pengutronix.de","Content-Type":"multipart/mixed;\n\tboundary=\"===============9178471733774031871==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15775,"web_url":"https://patchwork.libcamera.org/comment/15775/","msgid":"","date":"2021-03-21T00:04:12","subject":"Re: [libcamera-devel] [RFC PATCH 1/6] libcamera: Add fraction.h","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nOn Fri, Mar 19, 2021 at 09:43:48AM +0000, David Plowman wrote:\n> Hi everyone\n> \n> Thanks for this interesting proposal. I don't normally venture too many\n> opinions on stuff that doesn't really affect the Pi, but I did have just a\n> couple of comments on this one, if that's all right!\n\nIt's more than alright, you've provided very valuable feedback on a wide\nvariety of topics, and I'd be very happy to see you sharing your opinion\non anything related to libcamera :-)\n\n> On Fri, 19 Mar 2021 at 00:15, Laurent Pinchart wrote:\n> > On Wed, Mar 17, 2021 at 06:19:26PM +0100, Marian Cichy wrote:\n> > > On 3/17/21 2:48 PM, Kieran Bingham wrote:\n> > > > On 17/03/2021 12:41, Marian Cichy wrote:\n> > > >> On 3/17/21 12:33 PM, Kieran Bingham wrote:\n> > > >>> On 16/03/2021 15:52, Marian Cichy wrote:\n> > > >>>> Add a new class that represents a fraction. A structure like this is\n> > > >>>> also often used in linux camera drivers, e.g. v4l2_fract to represent a\n> > > >>>> frame interval or in applications like Gstreamer to represent frame\n> > > >>>> rates.\n> > > >>>>\n> > > >>>> Adding this class helps to interface frame intervals and frame\n> > > >>>> rates in video streams.\n> > > >>> This indeed would be helpful in any area that handles time.\n> > > >>>\n> > > >>> I think this is normally called a rational though?\n> \n> 1. I think \"fraction\" is not an ideal name. The very fact that we've seen\n> some discussion about what, exactly, a \"fraction\" is kind of demonstrates\n> the problem.\n> \n> Personally, I would be inclined to go with \"rational\". It has a cast iron\n> and straightforward mathematical definition. (Numbers of the form p/q,\n> where p and q are both integers, q != 0).\n> \n> 2. Following the idea of using rationals, the numerator would need to\n> become signed.\n> \n> In particular, this makes rationals closed under the usual arithmetic\n> operations (+ - * /). There may be no plans to overload them, but you could\n> imagine, in a C++ world, someone wanting to do that and finding the lack of\n> negative numbers rather awkward! With rationals, you at least get to do\n> whatever calculation you want, and test if the result is \">= 0\" or not at\n> the end.\n\nI share your opinion on this topic.\n\n> > >> Well .. this is probably mathematical nit-picking, but a fraction is\n> > > >> always a construct that consists of two integers, numerator and\n> > > >> denominator. A rational number is any number that *can* be displayed as\n> > > >> a fraction. For example, \"5\" is in the set of rational numbers, because\n> > > >> we can display it as the fraction \"5/1\".\n> > > > Indeed, and wouldn't we use such a representation in the case of long\n> > > > exposures if we use this for frame rates?\n> > >\n> > > But as the frame rate is configurable to be, for example 30 or 29.97, it\n> > > is only sometimes representable by single integers but always by\n> > > fractions. Gstreamer also reports 30/1 for a frame rate of 30.\n> >\n> > This would be true if the frame rate was really 30 or 29.97 fps, but\n> > that's rarely the case in practice with cameras. For instance, there's\n> > an Intel camera HAL implementation that produces ~29.94 fps, and reports\n> > an exact 30 fps to Android as the Android camera framework wants at\n> > least 29.97 fps for the corresponding use case.\n> >\n> > I'm not entirely opposed to using fractions, but we need to consider the\n> > implications carefully, looking at the big picture. It's a non-trivial\n> > topic, and lots of assumptions from the TV world don't apply as-is to\n> > cameras.\n> >\n> > > > Isn't 5/1 the frame rate for a 5 second exposure?\n> > >\n> > > 5/1 is the frame rate of 0.2 seconds exposure\n> >\n> > For 0.2s of frame duration. The exposure time can be (and usually is)\n> > shorter.\n> >\n> > > 5 frames per 1 second\n> > >\n> > > 5/1 is the frame interval of 5 seconds exposure\n> > >\n> > > 5 seconds interval for 1 frame\n> > >\n> > > I know, numbers are confusing :-(\n> > >\n> > > > However I don't think 5/1 could easily be called a fraction...\n> > > >\n> > > > > > > fractions'>\n> > > >\n> > > >> Also, since it is called v4l2_fract in V4L2 and GST_FRACTION in\n> > > >> Gstreamer, I went for consistency in the linux media universe.\n> > > >\n> \n> I appreciate that \"rationals\" don't 100% match the v4l2_fract any more, but\n> the inability to represent some of those rationals where you need full\n> 32-bit positive denominators seems negligible to me. However, I'd be\n> delighted to be educated on that if anyone can think of any examples!\n> \n> > > > Ah, yes I see that reference later in the series now which I hadn't before.\n> > > >\n> > > >>> We're really building up a math library in here aren't we with our\n> > > >>> sizes, geometry, and number classes...\n> > > >>>\n> > > >>>> Signed-off-by: Marian Cichy \n> > > >>>> ---\n> > > >>>> include/libcamera/fraction.h | 34 ++++++++++++++++++++\n> > > >>>> src/libcamera/fraction.cpp | 60 ++++++++++++++++++++++++++++++++++++\n> > > >>>> src/libcamera/meson.build | 1 +\n> > > >>>> 3 files changed, 95 insertions(+)\n> > > >>>> create mode 100644 include/libcamera/fraction.h\n> > > >>>> create mode 100644 src/libcamera/fraction.cpp\n> > > >>>>\n> > > >>>> diff --git a/include/libcamera/fraction.h\n> > b/include/libcamera/fraction.h\n> > > >>>> new file mode 100644\n> > > >>>> index 00000000..aa6a1abb\n> > > >>>> --- /dev/null\n> > > >>>> +++ b/include/libcamera/fraction.h\n> > > >>>> @@ -0,0 +1,34 @@\n> > > >>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > >>>> +/*\n> > > >>>> + * Copyright (C) 2021, Pengutronix, Marian Cichy\n> > > >>>> \n> > > >>>> + *\n> > > >>>> + * fraction.h - A fraction consisting of two integers\n> > > >>>> + */\n> > > >>>> +\n> > > >>>> +#ifndef __LIBCAMERA_FRACTION_H__\n> > > >>>> +#define __LIBCAMERA_FRACTION_H__\n> > > >>>> +\n> > > >>>> +namespace libcamera {\n> > > >>>> +\n> > > >>>> +class Fraction {\n> > > >>>> +public:\n> > > >>>> + constexpr Fraction() :\n> > > >>>> + numerator(0), denominator(0)\n> > > >>>> + {\n> > > >>>> + }\n> > > >>>> +\n> > > >>>> + constexpr Fraction(unsigned int num, unsigned int den) :\n> > > >>>> + numerator(num), denominator(den)\n> > > >>>> + {\n> > > >>>> + }\n> > > >>>> +\n> > > >>>> + unsigned int numerator;\n> > > >>>> + unsigned int denominator;\n> > > >>> Generically - Fractions/(Rational) numbers could be negative.\n> > > >>> But given our use cases, I presume we wouldn't expect negative values ?\n> > > >>>\n> > > >>> Well ... not unless we start needing to do things like clock recovery\n> > > >>> .... I don't think we'll need to do anything like that ...\n> > > >>>\n> > > >>> Would you expect further math functions in here to be able to add /\n> > > >>> multiple two fractions/rational numbers together?\n> > > >>>\n> > > >>> In fact - I wonder if this would also tie into the geometry functions\n> > > >>> for scaling or such?\n> > > >>>\n> > > >>> Size(640, 480) * Rational(1/2) == Size(320, 240);\n> > > >>>\n> > > >>> I'm not saying the extra functionality possible here is required for\n> > > >>> this patch though - just exploring what the goals would be for this\n> > > >>> class.\n> > > >>>\n> > > >>> --\n> > > >>> Kieran\n> > > >>>\n> > > >>>\n> > > >>>\n> > > >>>> +\n> > > >>>> + const std::string toString() const;\n> > > >>>> +};\n> > > >>>> +\n> > > >>>> +} /* namespace libcamera */\n> > > >>>> +\n> > > >>>> +#endif /* __LIBCAMERA_FRACTION_H__ */\n> > > >>>> diff --git a/src/libcamera/fraction.cpp b/src/libcamera/fraction.cpp\n> > > >>>> new file mode 100644\n> > > >>>> index 00000000..76c373aa\n> > > >>>> --- /dev/null\n> > > >>>> +++ b/src/libcamera/fraction.cpp\n> > > >>>> @@ -0,0 +1,60 @@\n> > > >>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > >>>> +/*\n> > > >>>> + * Copyright (C) 2021, Pengutronix, Marian Cichy\n> > > >>>> \n> > > >>>> + *\n> > > >>>> + * fraction.h - A fraction consisting of two integers\n> > > >>>> + */\n> > > >>>> +\n> > > >>>> +#include \n> > > >>>> +#include \n> > > >>>> +\n> > > >>>> +#include \n> > > >>>> +\n> > > >>>> +/**\n> > > >>>> + * \\file fraction.h\n> > > >>>> + * \\brief A fraction consisting of two integers.\n> > > >>>> + */\n> > > >>>> +\n> > > >>>> +namespace libcamera {\n> > > >>>> +\n> > > >>>> +/**\n> > > >>>> + * \\class Fraction\n> > > >>>> + * \\brief Represents a fraction with a nominator and a denominator.\n> > > >>>> + */\n> > > >>>> +\n> > > >>>> +/**\n> > > >>>> + * \\fn Fraction::Fraction()\n> > > >>>> + * \\brief Construct a Fraction with value 0/0. This should be\n> > > >>>> interpreted\n> > > >>>> + * as invalid or not-used Fraction.\n> > > >>>> + */\n> > > >>>> +\n> > > >>>> +/**\n> > > >>>> + * \\fn Fraction::Fraction(unsigned int num, unsigned int den)\n> > > >>>> + * \\brief Construct a Fraction with value n/d.\n> > > >>>> + */\n> > > >>>> +\n> > > >>>> +/**\n> > > >>>> + * \\var Fraction::numerator\n> > > >>>> + * \\brief The numerator of the fraction.\n> > > >>>> + */\n> > > >>>> +\n> > > >>>> +/**\n> > > >>>> + * \\var Fraction::denominator\n> > > >>>> + * \\brief The denominator of the fraction.\n> > > >>>> + */\n> > > >>>> +\n> > > >>>> +/**\n> > > >>>> + * \\brief Assemble and return a string describing the fraction\n> > > >>>> + * \\return A string describing the fraction.\n> > > >>>> + */\n> > > >>>> +const std::string Fraction::toString() const\n> > > >>>> +{\n> > > >>>> + std::stringstream ss;\n> > > >>>> +\n> > > >>>> + ss << numerator << \"/\" << denominator;\n> > > >>>> +\n> > > >>>> + return ss.str();\n> > > >>>> +}\n> > > >>>> +\n> > > >>>> +} /* namespace libcamera */\n> > > >>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > > >>>> index 815629db..7927481f 100644\n> > > >>>> --- a/src/libcamera/meson.build\n> > > >>>> +++ b/src/libcamera/meson.build\n> > > >>>> @@ -22,6 +22,7 @@ libcamera_sources = files([\n> > > >>>> 'file.cpp',\n> > > >>>> 'file_descriptor.cpp',\n> > > >>>> 'formats.cpp',\n> > > >>>> + 'fraction.cpp',\n> > > >>>> 'framebuffer_allocator.cpp',\n> > > >>>> 'geometry.cpp',\n> > > >>>> 'ipa_controls.cpp',","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C7384C32E1\n\tfor ;\n\tSun, 21 Mar 2021 00:04:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1584168D5E;\n\tSun, 21 Mar 2021 01:04:56 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0C39C602D8\n\tfor ;\n\tSun, 21 Mar 2021 01:04:53 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 774126EF;\n\tSun, 21 Mar 2021 01:04:52 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ISxwjg5m\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616285092;\n\tbh=ZwaEjHVACm4DaH6KO7BaSzFu/bqcTFKuc8xw2BQq2hk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ISxwjg5mbD1UwkRqxtFQxOihKN+StXAyZCdO/WLCF8i/sW3n81NisH+3qz+bQfogF\n\tvhjRqxt2Y3zqkmyunbflSoVGNQIX+fWSGzW/wJfWsdS/o8i9yDmDN8hKVL7HNZuCxb\n\t3566FDDAq0Vw27T4j87WaPTH0s9EAVdssHc2cZvQ=","Date":"Sun, 21 Mar 2021 02:04:12 +0200","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"","References":"<20210316155211.6679-1-m.cichy@pengutronix.de>\n\t<20210316155211.6679-2-m.cichy@pengutronix.de>\n\t<3fa6662c-b338-ac07-8fb0-cedb74e2d46a@ideasonboard.com>\n\t<853a6e03-9fcd-92a7-e130-fdde6b66138c@pengutronix.de>\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH 1/6] libcamera: Add fraction.h","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ,\n\tgraphics@pengutronix.de","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]