[{"id":515,"web_url":"https://patchwork.libcamera.org/comment/515/","msgid":"","date":"2019-01-23T10:19:55","subject":"Re: [libcamera-devel] [PATCH] libcamera: utils: Add arithmetic\n\toperators for struct timespec","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 23/01/2019 08:28, Laurent Pinchart wrote:\n> Implement additions and subtraction of struct timespec through\n> non-member operators to simplify timespec handling.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> \n> I wrote this code while working on EINTR handling for the event\n> dispatcher, and later refactored the implementation in a way that\n> doesn't make use of these operators anymore. I thought they could still\n> be useful as reference for later use. This patch is thus not meant to be\n> merged now, but can be picked up later if anyone needs it.\n\n\nThank you, I do foresee this being useful in the future when handling\nbuffer timestamps and such. Even if just for printing debug statements\nand measuring durations between frames.\n\nMost of the time the library itself shouldn't care too much about buffer\ntimestamps, but perhaps even that might change when we look at 3a.\n\nPerhaps because of this I'd almost be tempted to add these utils anyway\nif we wrapped a test that used them ? But then we're dragging unused\ncode around - so it can wait until it's necessary.\n\n--\nKieran\n\n\n> src/libcamera/include/utils.h | 6 +++++\n> src/libcamera/meson.build | 1 +\n> src/libcamera/utils.cpp | 46 +++++++++++++++++++++++++++++++++++\n> 3 files changed, 53 insertions(+)\n> create mode 100644 src/libcamera/utils.cpp\n> \n> diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h\n> index 73fa2e69b029..7df20976f36f 100644\n> --- a/src/libcamera/include/utils.h\n> +++ b/src/libcamera/include/utils.h\n> @@ -8,6 +8,7 @@\n> #define __LIBCAMERA_UTILS_H__\n> \n> #include \n> +#include \n> \n> #define ARRAY_SIZE(a)\t(sizeof(a) / sizeof(a[0]))\n> \n> @@ -24,6 +25,11 @@ std::unique_ptr make_unique(Args&&... args)\n> \n> } /* namespace utils */\n> \n> +struct timespec &operator+=(struct timespec &lhs, const struct timespec &rhs);\n> +struct timespec operator+(struct timespec lhs, const struct timespec &rhs);\n> +struct timespec &operator-=(struct timespec &lhs, const struct timespec &rhs);\n> +struct timespec operator-(struct timespec lhs, const struct timespec &rhs);\n> +\n> } /* namespace libcamera */\n> \n> #endif /* __LIBCAMERA_UTILS_H__ */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index f9f25c0ecf15..72d4a194e19a 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -11,6 +11,7 @@ libcamera_sources = files([\n> 'pipeline_handler.cpp',\n> 'signal.cpp',\n> 'timer.cpp',\n> + 'utils.cpp',\n> 'v4l2_device.cpp',\n> ])\n> \n> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp\n> new file mode 100644\n> index 000000000000..5014b27d8c7d\n> --- /dev/null\n> +++ b/src/libcamera/utils.cpp\n> @@ -0,0 +1,46 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2018, Google Inc.\n> + *\n> + * utils.cpp - Miscellaneous utility functions\n> + */\n> +\n> +#include \"utils.h\"\n> +\n> +namespace libcamera {\n> +\n> +struct timespec &operator+=(struct timespec &lhs, const struct timespec &rhs)\n> +{\n> +\tlhs.tv_sec += rhs.tv_sec;\n> +\tlhs.tv_nsec += rhs.tv_nsec;\n> +\tif (lhs.tv_nsec >= 1000000000) {\n> +\t\tlhs.tv_sec++;\n> +\t\tlhs.tv_nsec -= 1000000000;\n> +\t}\n> +\n> +\treturn lhs;\n> +}\n> +\n> +struct timespec operator+(struct timespec lhs, const struct timespec &rhs)\n> +{\n> +\treturn lhs += rhs;\n> +}\n> +\n> +struct timespec &operator-=(struct timespec &lhs, const struct timespec &rhs)\n> +{\n> +\tlhs.tv_sec -= rhs.tv_sec;\n> +\tlhs.tv_nsec -= rhs.tv_nsec;\n> +\tif (lhs.tv_nsec < 0) {\n> +\t\tlhs.tv_sec--;\n> +\t\tlhs.tv_nsec += 1000000000;\n> +\t}\n> +\n> +\treturn lhs;\n> +}\n> +\n> +struct timespec operator-(struct timespec lhs, const struct timespec &rhs)\n> +{\n> +\treturn lhs - rhs;\n> +}\n> +\n> +} /* namespace libcamera */\n>","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 9DD3360B1B\n\tfor ;\n\tWed, 23 Jan 2019 11:19:58 +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 0675B23D;\n\tWed, 23 Jan 2019 11:19:57 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548238798;\n\tbh=u6rQYRneUHMDseb/YQi5UTBaI3NOvOSDB0UA+NxcwRg=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=q4DLxEPfffhEVb+mhEOGUCumqIiBzk01g4slmaiVGcMEafetjlMkWMsEaMBsZs3qq\n\tC5W6iSrNwWavGzTB9vefClbTSKlAL5bywIjWn+l12kp1ZN/y0R39IoUj/5Q/uBrl3w\n\tpN4afU5t7WE4iz5AyGhAEQRE3OzXctD0LQE4vTWo=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20190123082830.10572-1-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"","Date":"Wed, 23 Jan 2019 10:19:55 +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":"<20190123082830.10572-1-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH] libcamera: utils: Add arithmetic\n\toperators for struct timespec","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, 23 Jan 2019 10:19:58 -0000"}},{"id":536,"web_url":"https://patchwork.libcamera.org/comment/536/","msgid":"","date":"2019-01-23T15:00:21","subject":"Re: [libcamera-devel] [PATCH] libcamera: utils: Add arithmetic\n\toperators for struct timespec","submitter":{"id":7,"url":"https://patchwork.libcamera.org/api/people/7/","name":"Shik Chen","email":"shik@chromium.org"},"content":"Hi Laurent,\n\nOn Wed, Jan 23, 2019 at 4:28 PM Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n>\n> Implement additions and subtraction of struct timespec through\n> non-member operators to simplify timespec handling.\n\nIIUC, operator overloading on types of other libs is banned by the style\nguide :Q\nhttps://google.github.io/styleguide/cppguide.html#Operator_Overloading\n\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n>\n> I wrote this code while working on EINTR handling for the event\n> dispatcher, and later refactored the implementation in a way that\n> doesn't make use of these operators anymore. I thought they could still\n> be useful as reference for later use. This patch is thus not meant to be\n> merged now, but can be picked up later if anyone needs it.\n>\n> src/libcamera/include/utils.h | 6 +++++\n> src/libcamera/meson.build | 1 +\n> src/libcamera/utils.cpp | 46 +++++++++++++++++++++++++++++++++++\n> 3 files changed, 53 insertions(+)\n> create mode 100644 src/libcamera/utils.cpp\n>\n> diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h\n> index 73fa2e69b029..7df20976f36f 100644\n> --- a/src/libcamera/include/utils.h\n> +++ b/src/libcamera/include/utils.h\n> @@ -8,6 +8,7 @@\n> #define __LIBCAMERA_UTILS_H__\n>\n> #include \n> +#include \n>\n> #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))\n>\n> @@ -24,6 +25,11 @@ std::unique_ptr make_unique(Args&&... args)\n>\n> } /* namespace utils */\n>\n> +struct timespec &operator+=(struct timespec &lhs, const struct timespec\n&rhs);\n> +struct timespec operator+(struct timespec lhs, const struct timespec\n&rhs);\n> +struct timespec &operator-=(struct timespec &lhs, const struct timespec\n&rhs);\n> +struct timespec operator-(struct timespec lhs, const struct timespec\n&rhs);\n> +\n> } /* namespace libcamera */\n>\n> #endif /* __LIBCAMERA_UTILS_H__ */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index f9f25c0ecf15..72d4a194e19a 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -11,6 +11,7 @@ libcamera_sources = files([\n> 'pipeline_handler.cpp',\n> 'signal.cpp',\n> 'timer.cpp',\n> + 'utils.cpp',\n> 'v4l2_device.cpp',\n> ])\n>\n> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp\n> new file mode 100644\n> index 000000000000..5014b27d8c7d\n> --- /dev/null\n> +++ b/src/libcamera/utils.cpp\n> @@ -0,0 +1,46 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2018, Google Inc.\n> + *\n> + * utils.cpp - Miscellaneous utility functions\n> + */\n> +\n> +#include \"utils.h\"\n> +\n> +namespace libcamera {\n> +\n> +struct timespec &operator+=(struct timespec &lhs, const struct timespec\n&rhs)\n> +{\n> + lhs.tv_sec += rhs.tv_sec;\n> + lhs.tv_nsec += rhs.tv_nsec;\n> + if (lhs.tv_nsec >= 1000000000) {\n> + lhs.tv_sec++;\n> + lhs.tv_nsec -= 1000000000;\n> + }\n> +\n> + return lhs;\n> +}\n> +\n> +struct timespec operator+(struct timespec lhs, const struct timespec\n&rhs)\n> +{\n> + return lhs += rhs;\n> +}\n> +\n> +struct timespec &operator-=(struct timespec &lhs, const struct timespec\n&rhs)\n> +{\n> + lhs.tv_sec -= rhs.tv_sec;\n> + lhs.tv_nsec -= rhs.tv_nsec;\n> + if (lhs.tv_nsec < 0) {\n> + lhs.tv_sec--;\n> + lhs.tv_nsec += 1000000000;\n> + }\n> +\n> + return lhs;\n> +}\n> +\n> +struct timespec operator-(struct timespec lhs, const struct timespec\n&rhs)\n> +{\n> + return lhs - rhs;\n> +}\n> +\n> +} /* namespace libcamera */\n> --\n> Regards,\n>\n> Laurent Pinchart\n\nSincerely,\nShik","headers":{"Return-Path":"","Received":["from mail-qk1-x742.google.com (mail-qk1-x742.google.com\n\t[IPv6:2607:f8b0:4864:20::742])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C522F60B2D\n\tfor ;\n\tWed, 23 Jan 2019 16:00:40 +0100 (CET)","by mail-qk1-x742.google.com with SMTP id o8so1229318qkk.11\n\tfor ;\n\tWed, 23 Jan 2019 07:00:40 -0800 (PST)","from mail-qt1-f176.google.com (mail-qt1-f176.google.com.\n\t[209.85.160.176]) by smtp.gmail.com with ESMTPSA id\n\tj38sm88108469qtj.72.2019.01.23.07.00.37\n\tfor \n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tWed, 23 Jan 2019 07:00:38 -0800 (PST)","by mail-qt1-f176.google.com with SMTP id l12so2628678qtf.8\n\tfor ;\n\tWed, 23 Jan 2019 07:00:37 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=blT5Pq9FD2UJmx7ZaH2sOxL/TaouISidrvuGnnO8xnU=;\n\tb=XucxihECl8JHfPH6ybVEDnd+e7Sf6nZBjNmsWHe+j553H0X/UAk0gFcc0EAk83lLZu\n\tnWr+uZW5atLxvAWQY84l9hBku3GvmzrB69ga25CKNu9R8vvRtovD4OcOMKbKY/t//DLy\n\t+0KxeXCuAwl/0p5IHEHapyRUP0r29EuWFp0Sc=","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=blT5Pq9FD2UJmx7ZaH2sOxL/TaouISidrvuGnnO8xnU=;\n\tb=niqVv+uw5jOENXbIeLz3p+dZrJFNyb1GiWAv4zOZtA4WA/mt/Rje1GgExIYc8CQ19m\n\te5gPGlyNxiSmqmq9XW6JDaCa37zkUYOUH40kSPx5LB41N0wQKfF59jh6+qgfJvux6Gob\n\tfi+MZpDkuwm6qk+5tP8VU3qZcUTM5c0lGqSTVLxrebW4YRti7e7DzZBaznAFf9E0n/mU\n\tOkDTziR3NlbFNPL49RRK6nPewdW3F3zDlfvGKY/lM2ur+M58rQnhT/O31ItbapirOtxs\n\tCakAQC1kJO6gMeS624hSQlO8JINfDhD3APgBmWGljcVn+Fq+3vM4A85DWZtTeXFO/yCP\n\t3gdQ==","X-Gm-Message-State":"AJcUukdTnEz7eSgEcR5DAhP8Zs3ZmtKo8BgBjQqYDIbpgqF5E92gefOL\n\tKWI/dH8TVvb4PWh2VnmdwIt1VNvM6VPZQQ==","X-Google-Smtp-Source":"ALg8bN7MC9vP3GhhNmpAlrjxoelc1ShqP4bD42IF5WbF19NBF905u03VaddBna8i+8SLIoalENwMBA==","X-Received":["by 2002:a37:9cce:: with SMTP id\n\tf197mr2080991qke.98.1548255639163; \n\tWed, 23 Jan 2019 07:00:39 -0800 (PST)","by 2002:ac8:76c3:: with SMTP id q3mr2595160qtr.48.1548255637498; \n\tWed, 23 Jan 2019 07:00:37 -0800 (PST)"],"MIME-Version":"1.0","References":"<20190123082830.10572-1-laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20190123082830.10572-1-laurent.pinchart@ideasonboard.com>","From":"Shik Chen ","Date":"Wed, 23 Jan 2019 23:00:21 +0800","X-Gmail-Original-Message-ID":"","Message-ID":"","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"multipart/alternative; boundary=\"0000000000004bdec40580215b1e\"","Subject":"Re: [libcamera-devel] [PATCH] libcamera: utils: Add arithmetic\n\toperators for struct timespec","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, 23 Jan 2019 15:00:41 -0000"}},{"id":540,"web_url":"https://patchwork.libcamera.org/comment/540/","msgid":"<20190123160423.GC31885@pendragon.ideasonboard.com>","date":"2019-01-23T16:04:23","subject":"Re: [libcamera-devel] [PATCH] libcamera: utils: Add arithmetic\n\toperators for struct timespec","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Shik,\n\nOn Wed, Jan 23, 2019 at 11:00:21PM +0800, Shik Chen wrote:\n> On Wed, Jan 23, 2019 at 4:28 PM Laurent Pinchart <\n> laurent.pinchart@ideasonboard.com> wrote:\n> >\n> > Implement additions and subtraction of struct timespec through\n> > non-member operators to simplify timespec handling.\n> \n> IIUC, operator overloading on types of other libs is banned by the style guide\n> :Q\n> https://google.github.io/styleguide/cppguide.html#Operator_Overloading\n\nBut it also states \"Don't go out of your way to avoid defining operator\noverloads. For example, prefer to define ==, =, and <<, rather than\nEquals(), CopyFrom(), and PrintTo().\". Would you recommend create\ntimespecAdd() and timespecSub() functions instead ? I think this is a\ngood case for operator overloading, as adding or subtracting timespec\nstructures will be rejected if utils.h isn't included (so no risk of\ndifferent behaviours unknown to the developer), and the + and -\noperators map very well to what we need to do.\n\nFor reference, my use case was\n\n\tstruct timespec start;\n\tclock_gettime(CLOCK_MONOTINIC, &start);\n\n\t...\n\n\tstruct timespec now;\n\tclock_gettime(CLOCK_MONOTINIC, &now);\n\tstruct timespec timeout = old_timeout + start - now;\n\nwhich could be written\n\n\tstruct timespec timeout = old_timeout;\n\ttimespecAdd(&timeout, start);\n\ttimespecSub(&timeout, now);\n\nbut is a bit less convenient.\n\nWhat do you advise ?\n\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> >\n> > I wrote this code while working on EINTR handling for the event\n> > dispatcher, and later refactored the implementation in a way that\n> > doesn't make use of these operators anymore. I thought they could still\n> > be useful as reference for later use. This patch is thus not meant to be\n> > merged now, but can be picked up later if anyone needs it.\n> >\n> >  src/libcamera/include/utils.h |  6 +++++\n> >  src/libcamera/meson.build     |  1 +\n> >  src/libcamera/utils.cpp       | 46 +++++++++++++++++++++++++++++++++++\n> >  3 files changed, 53 insertions(+)\n> >  create mode 100644 src/libcamera/utils.cpp\n> >\n> > diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h\n> > index 73fa2e69b029..7df20976f36f 100644\n> > --- a/src/libcamera/include/utils.h\n> > +++ b/src/libcamera/include/utils.h\n> > @@ -8,6 +8,7 @@\n> >  #define __LIBCAMERA_UTILS_H__\n> >\n> >  #include \n> > +#include \n> >\n> >  #define ARRAY_SIZE(a)  (sizeof(a) / sizeof(a[0]))\n> >\n> > @@ -24,6 +25,11 @@ std::unique_ptr make_unique(Args&&... args)\n> >\n> >  } /* namespace utils */\n> >\n> > +struct timespec &operator+=(struct timespec &lhs, const struct timespec &\n> rhs);\n> > +struct timespec operator+(struct timespec lhs, const struct timespec &rhs);\n> > +struct timespec &operator-=(struct timespec &lhs, const struct timespec &\n> rhs);\n> > +struct timespec operator-(struct timespec lhs, const struct timespec &rhs);\n> > +\n> >  } /* namespace libcamera */\n> >\n> >  #endif /* __LIBCAMERA_UTILS_H__ */\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index f9f25c0ecf15..72d4a194e19a 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -11,6 +11,7 @@ libcamera_sources = files([\n> >      'pipeline_handler.cpp',\n> >      'signal.cpp',\n> >      'timer.cpp',\n> > +    'utils.cpp',\n> >      'v4l2_device.cpp',\n> >  ])\n> >\n> > diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp\n> > new file mode 100644\n> > index 000000000000..5014b27d8c7d\n> > --- /dev/null\n> > +++ b/src/libcamera/utils.cpp\n> > @@ -0,0 +1,46 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2018, Google Inc.\n> > + *\n> > + * utils.cpp - Miscellaneous utility functions\n> > + */\n> > +\n> > +#include \"utils.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +struct timespec &operator+=(struct timespec &lhs, const struct timespec &\n> rhs)\n> > +{\n> > +       lhs.tv_sec += rhs.tv_sec;\n> > +       lhs.tv_nsec += rhs.tv_nsec;\n> > +       if (lhs.tv_nsec >= 1000000000) {\n> > +               lhs.tv_sec++;\n> > +               lhs.tv_nsec -= 1000000000;\n> > +       }\n> > +\n> > +       return lhs;\n> > +}\n> > +\n> > +struct timespec operator+(struct timespec lhs, const struct timespec &rhs)\n> > +{\n> > +       return lhs += rhs;\n> > +}\n> > +\n> > +struct timespec &operator-=(struct timespec &lhs, const struct timespec &\n> rhs)\n> > +{\n> > +       lhs.tv_sec -= rhs.tv_sec;\n> > +       lhs.tv_nsec -= rhs.tv_nsec;\n> > +       if (lhs.tv_nsec < 0) {\n> > +               lhs.tv_sec--;\n> > +               lhs.tv_nsec += 1000000000;\n> > +       }\n> > +\n> > +       return lhs;\n> > +}\n> > +\n> > +struct timespec operator-(struct timespec lhs, const struct timespec &rhs)\n> > +{\n> > +       return lhs - rhs;\n> > +}\n> > +\n> > +} /* namespace libcamera */","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 95F1B60B2D\n\tfor ;\n\tWed, 23 Jan 2019 17:04:24 +0100 (CET)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 316CF23D;\n\tWed, 23 Jan 2019 17:04:24 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548259464;\n\tbh=fUEafH3LkanlnIIZBo7cALmJPouMMsZ6KQbp1gWzpjE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YAc+Mj5r0we/XgKpKCZbHRic0klribQoEJnlX0UrfJJIMMA3J/zJH/vGWbaqJAwoj\n\ttqi8e0kdaPpfZvcZrBxzuzmPk9zLhEch0UVE5K5iAqFGcYMEdDM992m0RZajqp0kWO\n\tntIPg5NlC1ROE7j+q51lsdofcLHb+lIj3+GxBd40=","Date":"Wed, 23 Jan 2019 18:04:23 +0200","From":"Laurent Pinchart ","To":"Shik Chen ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190123160423.GC31885@pendragon.ideasonboard.com>","References":"<20190123082830.10572-1-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: utils: Add arithmetic\n\toperators for struct timespec","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, 23 Jan 2019 16:04:24 -0000"}},{"id":550,"web_url":"https://patchwork.libcamera.org/comment/550/","msgid":"","date":"2019-01-23T18:08:25","subject":"Re: [libcamera-devel] [PATCH] libcamera: utils: Add arithmetic\n\toperators for struct timespec","submitter":{"id":7,"url":"https://patchwork.libcamera.org/api/people/7/","name":"Shik Chen","email":"shik@chromium.org"},"content":"Hi Laurent,\n\nOn Thu, Jan 24, 2019 at 12:04 AM Laurent Pinchart\n wrote:\n>\n> Hi Shik,\n>\n> On Wed, Jan 23, 2019 at 11:00:21PM +0800, Shik Chen wrote:\n> > On Wed, Jan 23, 2019 at 4:28 PM Laurent Pinchart <\n> > laurent.pinchart@ideasonboard.com> wrote:\n> > >\n> > > Implement additions and subtraction of struct timespec through\n> > > non-member operators to simplify timespec handling.\n> >\n> > IIUC, operator overloading on types of other libs is banned by the style guide\n> > :Q\n> > https://google.github.io/styleguide/cppguide.html#Operator_Overloading\n>\n> But it also states \"Don't go out of your way to avoid defining operator\n> overloads. For example, prefer to define ==, =, and <<, rather than\n> Equals(), CopyFrom(), and PrintTo().\". Would you recommend create\n> timespecAdd() and timespecSub() functions instead ? I think this is a\n> good case for operator overloading, as adding or subtracting timespec\n> structures will be rejected if utils.h isn't included (so no risk of\n> different behaviours unknown to the developer), and the + and -\n> operators map very well to what we need to do.\n\nIMO the rule \"Define operators only on your own types\" has higher precedence.\nTwo libraries might have conflict definitions if they violate this rule.\n\n>\n> For reference, my use case was\n>\n> struct timespec start;\n> clock_gettime(CLOCK_MONOTINIC, &start);\n>\n> ...\n>\n> struct timespec now;\n> clock_gettime(CLOCK_MONOTINIC, &now);\n> struct timespec timeout = old_timeout + start - now;\n>\n> which could be written\n>\n> struct timespec timeout = old_timeout;\n> timespecAdd(&timeout, start);\n> timespecSub(&timeout, now);\n>\n> but is a bit less convenient.\n>\n> What do you advise ?\n\nIdeally most unavoidable C fashion code snippets should be wrapped in idiomatic\nC++ if possible. Can we replace it with standard C++ types such as\nstd::chrono::duration and std::chrono::time_point, which already provide\narithmetic operators? Using things in std::chrono is a little bit verbose and\ntemplate heavy though (it's too flexible...).\n\nAnother option is to create/borrow some time manipulating classes and only\nconvert to C structs when needed. A good example is absl time library [1] [2],\nwhich has a nicer API than std::chrono and can be easily converted to/from the\ntypes in std::chrono/timespec/timeval.\n\n[1] https://abseil.io/docs/cpp/guides/time\n[2] https://github.com/abseil/abseil-cpp/blob/master/absl/time/time.h\n\n>\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > >\n> > > I wrote this code while working on EINTR handling for the event\n> > > dispatcher, and later refactored the implementation in a way that\n> > > doesn't make use of these operators anymore. I thought they could still\n> > > be useful as reference for later use. This patch is thus not meant to be\n> > > merged now, but can be picked up later if anyone needs it.\n> > >\n> > > src/libcamera/include/utils.h | 6 +++++\n> > > src/libcamera/meson.build | 1 +\n> > > src/libcamera/utils.cpp | 46 +++++++++++++++++++++++++++++++++++\n> > > 3 files changed, 53 insertions(+)\n> > > create mode 100644 src/libcamera/utils.cpp\n> > >\n> > > diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h\n> > > index 73fa2e69b029..7df20976f36f 100644\n> > > --- a/src/libcamera/include/utils.h\n> > > +++ b/src/libcamera/include/utils.h\n> > > @@ -8,6 +8,7 @@\n> > > #define __LIBCAMERA_UTILS_H__\n> > >\n> > > #include \n> > > +#include \n> > >\n> > > #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))\n> > >\n> > > @@ -24,6 +25,11 @@ std::unique_ptr make_unique(Args&&... args)\n> > >\n> > > } /* namespace utils */\n> > >\n> > > +struct timespec &operator+=(struct timespec &lhs, const struct timespec &\n> > rhs);\n> > > +struct timespec operator+(struct timespec lhs, const struct timespec &rhs);\n> > > +struct timespec &operator-=(struct timespec &lhs, const struct timespec &\n> > rhs);\n> > > +struct timespec operator-(struct timespec lhs, const struct timespec &rhs);\n> > > +\n> > > } /* namespace libcamera */\n> > >\n> > > #endif /* __LIBCAMERA_UTILS_H__ */\n> > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > > index f9f25c0ecf15..72d4a194e19a 100644\n> > > --- a/src/libcamera/meson.build\n> > > +++ b/src/libcamera/meson.build\n> > > @@ -11,6 +11,7 @@ libcamera_sources = files([\n> > > 'pipeline_handler.cpp',\n> > > 'signal.cpp',\n> > > 'timer.cpp',\n> > > + 'utils.cpp',\n> > > 'v4l2_device.cpp',\n> > > ])\n> > >\n> > > diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp\n> > > new file mode 100644\n> > > index 000000000000..5014b27d8c7d\n> > > --- /dev/null\n> > > +++ b/src/libcamera/utils.cpp\n> > > @@ -0,0 +1,46 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2018, Google Inc.\n> > > + *\n> > > + * utils.cpp - Miscellaneous utility functions\n> > > + */\n> > > +\n> > > +#include \"utils.h\"\n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +struct timespec &operator+=(struct timespec &lhs, const struct timespec &\n> > rhs)\n> > > +{\n> > > + lhs.tv_sec += rhs.tv_sec;\n> > > + lhs.tv_nsec += rhs.tv_nsec;\n> > > + if (lhs.tv_nsec >= 1000000000) {\n> > > + lhs.tv_sec++;\n> > > + lhs.tv_nsec -= 1000000000;\n> > > + }\n> > > +\n> > > + return lhs;\n> > > +}\n> > > +\n> > > +struct timespec operator+(struct timespec lhs, const struct timespec &rhs)\n> > > +{\n> > > + return lhs += rhs;\n> > > +}\n> > > +\n> > > +struct timespec &operator-=(struct timespec &lhs, const struct timespec &\n> > rhs)\n> > > +{\n> > > + lhs.tv_sec -= rhs.tv_sec;\n> > > + lhs.tv_nsec -= rhs.tv_nsec;\n> > > + if (lhs.tv_nsec < 0) {\n> > > + lhs.tv_sec--;\n> > > + lhs.tv_nsec += 1000000000;\n> > > + }\n> > > +\n> > > + return lhs;\n> > > +}\n> > > +\n> > > +struct timespec operator-(struct timespec lhs, const struct timespec &rhs)\n> > > +{\n> > > + return lhs - rhs;\n> > > +}\n> > > +\n> > > +} /* namespace libcamera */\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n\nSincerely,\nShik","headers":{"Return-Path":"","Received":["from mail-qt1-x843.google.com (mail-qt1-x843.google.com\n\t[IPv6:2607:f8b0:4864:20::843])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CD39360C7D\n\tfor ;\n\tWed, 23 Jan 2019 19:08:44 +0100 (CET)","by mail-qt1-x843.google.com with SMTP id l12so3430357qtf.8\n\tfor ;\n\tWed, 23 Jan 2019 10:08:44 -0800 (PST)","from mail-qt1-f172.google.com (mail-qt1-f172.google.com.\n\t[209.85.160.172]) by smtp.gmail.com with ESMTPSA id\n\th35sm75370407qth.59.2019.01.23.10.08.42\n\tfor \n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tWed, 23 Jan 2019 10:08:42 -0800 (PST)","by mail-qt1-f172.google.com with SMTP id p17so3453607qtl.5\n\tfor ;\n\tWed, 23 Jan 2019 10:08:42 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=etjpjKaSfXVtYJRqthztszmmQV51RzJp9y7lOG8j5YQ=;\n\tb=GYRXpALbuAG46lI2vBrmq/63KnUUI5G3Q0NQ0DWz2cwejcfqrNDNyl9GfZ8fxEsnDu\n\tilpMiqo605EUDwUj/+f3Cg3kcglH6cL4n9cX2j4dQU4PzwObbAWjtP9PBxgnHmkuFqTP\n\tC6jAagA9QLjccKD9YjbS69BsDUnzqCEUXoA0Y=","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=etjpjKaSfXVtYJRqthztszmmQV51RzJp9y7lOG8j5YQ=;\n\tb=T+eD7KbR/Sti3DaEVvMsgChFhh4uaNLlJO3BvXjPcPj3xwM4kdZCq1cED/VVcHWOI1\n\tw80fkMlhSaH8RG5uif0yydsy7QEqRsekpWu1X9LjcaIdw25UXdP0K6n5/2zfZieILS6j\n\tJiFR4QdkexIup0WYZBI5cBGhoBHgsT6mh1jgYbwGn0cvvfWebRpeBJqvlJ03kX+/nbw1\n\tkytfbeXVrW+zKGdJ7n06Znyew/d9x/7TpgO5Rsg7+QU6iTLE8JmxIwhTpbbKRlB0t+Ek\n\tlmZQ/949UzaNVS5x/K6C1SGhmjJTtmPBB2HF1gAcEWD1sYjHKMKCqWAOTzLqGUL/CrAX\n\tQ6IQ==","X-Gm-Message-State":"AJcUukfRb1+6TavPUBmH/wHuYcYqqawXBv9zGRsY53AAWsnc5C/Pw+Tx\n\tlFlejstn2lDD25Blf9q6tZxRWsnNeaAYxQ==","X-Google-Smtp-Source":"ALg8bN6nUIYu+t+qUBuF/4JmF8A70zBTDdbn/0nfODYXV/tX2P98/WhU9l+aVhDTM4tGqwhl3XcQyw==","X-Received":["by 2002:ac8:3181:: with SMTP id h1mr3535942qte.14.1548266923251; \n\tWed, 23 Jan 2019 10:08:43 -0800 (PST)","by 2002:aed:3fa8:: with SMTP id\n\ts37mr3399045qth.208.1548266921858; \n\tWed, 23 Jan 2019 10:08:41 -0800 (PST)"],"MIME-Version":"1.0","References":"<20190123082830.10572-1-laurent.pinchart@ideasonboard.com>\n\t\n\t<20190123160423.GC31885@pendragon.ideasonboard.com>","In-Reply-To":"<20190123160423.GC31885@pendragon.ideasonboard.com>","From":"Shik Chen ","Date":"Thu, 24 Jan 2019 02:08:25 +0800","X-Gmail-Original-Message-ID":"","Message-ID":"","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] libcamera: utils: Add arithmetic\n\toperators for struct timespec","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, 23 Jan 2019 18:08:45 -0000"}},{"id":552,"web_url":"https://patchwork.libcamera.org/comment/552/","msgid":"<20190123183847.GB4675@pendragon.ideasonboard.com>","date":"2019-01-23T18:38:47","subject":"Re: [libcamera-devel] [PATCH] libcamera: utils: Add arithmetic\n\toperators for struct timespec","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Shik,\n\nOn Thu, Jan 24, 2019 at 02:08:25AM +0800, Shik Chen wrote:\n> On Thu, Jan 24, 2019 at 12:04 AM Laurent Pinchart\n> wrote:\n> > On Wed, Jan 23, 2019 at 11:00:21PM +0800, Shik Chen wrote:\n> >> On Wed, Jan 23, 2019 at 4:28 PM Laurent Pinchart <\n> >> laurent.pinchart@ideasonboard.com> wrote:\n> >>>\n> >>> Implement additions and subtraction of struct timespec through\n> >>> non-member operators to simplify timespec handling.\n> >>\n> >> IIUC, operator overloading on types of other libs is banned by the style guide\n> >> :Q\n> >> https://google.github.io/styleguide/cppguide.html#Operator_Overloading\n> >\n> > But it also states \"Don't go out of your way to avoid defining operator\n> > overloads. For example, prefer to define ==, =, and <<, rather than\n> > Equals(), CopyFrom(), and PrintTo().\". Would you recommend create\n> > timespecAdd() and timespecSub() functions instead ? I think this is a\n> > good case for operator overloading, as adding or subtracting timespec\n> > structures will be rejected if utils.h isn't included (so no risk of\n> > different behaviours unknown to the developer), and the + and -\n> > operators map very well to what we need to do.\n> \n> IMO the rule \"Define operators only on your own types\" has higher precedence.\n> Two libraries might have conflict definitions if they violate this rule.\n\nThat's true, but those operators are internal to the library, they're\nnot exposed to applications, so that shouldn't be a problem in this\ncase.\n\n> > For reference, my use case was\n> >\n> > struct timespec start;\n> > clock_gettime(CLOCK_MONOTINIC, &start);\n> >\n> > ...\n> >\n> > struct timespec now;\n> > clock_gettime(CLOCK_MONOTINIC, &now);\n> > struct timespec timeout = old_timeout + start - now;\n> >\n> > which could be written\n> >\n> > struct timespec timeout = old_timeout;\n> > timespecAdd(&timeout, start);\n> > timespecSub(&timeout, now);\n> >\n> > but is a bit less convenient.\n> >\n> > What do you advise ?\n> \n> Ideally most unavoidable C fashion code snippets should be wrapped in idiomatic\n> C++ if possible. Can we replace it with standard C++ types such as\n> std::chrono::duration and std::chrono::time_point, which already provide\n> arithmetic operators? Using things in std::chrono is a little bit verbose and\n> template heavy though (it's too flexible...).\n\nThat seems pretty painful to use indeed, and the guarantees on\nresolution are pretty vague.\n\n> Another option is to create/borrow some time manipulating classes and only\n> convert to C structs when needed. A good example is absl time library [1] [2],\n> which has a nicer API than std::chrono and can be easily converted to/from the\n> types in std::chrono/timespec/timeval.\n\nWe could use our own time class, and may do so in the future, or a class\nborrowed from another project, but for now, given that we don't need\nthese operators, I propose deferring the decision.\n\nPlease note that I'm using an uint64_t to store a duration in\nnanoseconds in the timer code, and only convert from/to timespec as\nrequired per the C library API and/or system calls. We could standardize\non uint64_t durations internally in libcamera. This would still leave\npotential arithmetic operations on timespec (or timeval in other\nplaces), but they would be confined and timespec/timeval wouldn't be\nused in APIs.\n\n> [1] https://abseil.io/docs/cpp/guides/time\n> [2] https://github.com/abseil/abseil-cpp/blob/master/absl/time/time.h\n> \n> >>> Signed-off-by: Laurent Pinchart \n> >>> ---\n> >>>\n> >>> I wrote this code while working on EINTR handling for the event\n> >>> dispatcher, and later refactored the implementation in a way that\n> >>> doesn't make use of these operators anymore. I thought they could still\n> >>> be useful as reference for later use. This patch is thus not meant to be\n> >>> merged now, but can be picked up later if anyone needs it.\n> >>>\n> >>> src/libcamera/include/utils.h | 6 +++++\n> >>> src/libcamera/meson.build | 1 +\n> >>> src/libcamera/utils.cpp | 46 +++++++++++++++++++++++++++++++++++\n> >>> 3 files changed, 53 insertions(+)\n> >>> create mode 100644 src/libcamera/utils.cpp\n> >>>\n> >>> diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h\n> >>> index 73fa2e69b029..7df20976f36f 100644\n> >>> --- a/src/libcamera/include/utils.h\n> >>> +++ b/src/libcamera/include/utils.h\n> >>> @@ -8,6 +8,7 @@\n> >>> #define __LIBCAMERA_UTILS_H__\n> >>>\n> >>> #include \n> >>> +#include \n> >>>\n> >>> #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))\n> >>>\n> >>> @@ -24,6 +25,11 @@ std::unique_ptr make_unique(Args&&... args)\n> >>>\n> >>> } /* namespace utils */\n> >>>\n> >>> +struct timespec &operator+=(struct timespec &lhs, const struct timespec &\n> >> rhs);\n> >>> +struct timespec operator+(struct timespec lhs, const struct timespec &rhs);\n> >>> +struct timespec &operator-=(struct timespec &lhs, const struct timespec &\n> >> rhs);\n> >>> +struct timespec operator-(struct timespec lhs, const struct timespec &rhs);\n> >>> +\n> >>> } /* namespace libcamera */\n> >>>\n> >>> #endif /* __LIBCAMERA_UTILS_H__ */\n> >>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> >>> index f9f25c0ecf15..72d4a194e19a 100644\n> >>> --- a/src/libcamera/meson.build\n> >>> +++ b/src/libcamera/meson.build\n> >>> @@ -11,6 +11,7 @@ libcamera_sources = files([\n> >>> 'pipeline_handler.cpp',\n> >>> 'signal.cpp',\n> >>> 'timer.cpp',\n> >>> + 'utils.cpp',\n> >>> 'v4l2_device.cpp',\n> >>> ])\n> >>>\n> >>> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp\n> >>> new file mode 100644\n> >>> index 000000000000..5014b27d8c7d\n> >>> --- /dev/null\n> >>> +++ b/src/libcamera/utils.cpp\n> >>> @@ -0,0 +1,46 @@\n> >>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >>> +/*\n> >>> + * Copyright (C) 2018, Google Inc.\n> >>> + *\n> >>> + * utils.cpp - Miscellaneous utility functions\n> >>> + */\n> >>> +\n> >>> +#include \"utils.h\"\n> >>> +\n> >>> +namespace libcamera {\n> >>> +\n> >>> +struct timespec &operator+=(struct timespec &lhs, const struct timespec &\n> >> rhs)\n> >>> +{\n> >>> + lhs.tv_sec += rhs.tv_sec;\n> >>> + lhs.tv_nsec += rhs.tv_nsec;\n> >>> + if (lhs.tv_nsec >= 1000000000) {\n> >>> + lhs.tv_sec++;\n> >>> + lhs.tv_nsec -= 1000000000;\n> >>> + }\n> >>> +\n> >>> + return lhs;\n> >>> +}\n> >>> +\n> >>> +struct timespec operator+(struct timespec lhs, const struct timespec &rhs)\n> >>> +{\n> >>> + return lhs += rhs;\n> >>> +}\n> >>> +\n> >>> +struct timespec &operator-=(struct timespec &lhs, const struct timespec &\n> >> rhs)\n> >>> +{\n> >>> + lhs.tv_sec -= rhs.tv_sec;\n> >>> + lhs.tv_nsec -= rhs.tv_nsec;\n> >>> + if (lhs.tv_nsec < 0) {\n> >>> + lhs.tv_sec--;\n> >>> + lhs.tv_nsec += 1000000000;\n> >>> + }\n> >>> +\n> >>> + return lhs;\n> >>> +}\n> >>> +\n> >>> +struct timespec operator-(struct timespec lhs, const struct timespec &rhs)\n> >>> +{\n> >>> + return lhs - rhs;\n> >>> +}\n> >>> +\n> >>> +} /* namespace libcamera */","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 C2D2A60C7D\n\tfor ;\n\tWed, 23 Jan 2019 19:38:49 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F0F2523D;\n\tWed, 23 Jan 2019 19:38:48 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548268729;\n\tbh=W8+j3m50d0bZ1uHG/kYHm9zHUMEEZrS222uemeloRT4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=v53ZQwXrv6KaXNniepvzkHSsDl5uwx5A3pju4E8C4fGIpeVLl/4Nv8sksLbbTAxiM\n\tET0KEUNi7kX00UgyZErbTJYIpmJIrYKC1VZkMkcTqMr9KcNxBBRhF+WBKbFDbOOeBQ\n\ta7e2levHpjwn2SKS88qfNN5ZiQ95dyaz89AioDTo=","Date":"Wed, 23 Jan 2019 20:38:47 +0200","From":"Laurent Pinchart ","To":"Shik Chen ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190123183847.GB4675@pendragon.ideasonboard.com>","References":"<20190123082830.10572-1-laurent.pinchart@ideasonboard.com>\n\t\n\t<20190123160423.GC31885@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: utils: Add arithmetic\n\toperators for struct timespec","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, 23 Jan 2019 18:38:49 -0000"}}]