[{"id":13112,"web_url":"https://patchwork.libcamera.org/comment/13112/","msgid":"<20201008191009.GD3939@pendragon.ideasonboard.com>","date":"2020-10-08T19:10:09","subject":"Re: [libcamera-devel] [RFC PATCH 1/3] android: post_processor:\n\tIntroduce a PostProcessor interface","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Thu, Oct 08, 2020 at 07:40:36PM +0530, Umang Jain wrote:\n> Introduce a PostProcessor interface for the streams that require any\n> kind of processing for their consumption by the HAL layer. The\n> PostProcessor interface can be configured via configure() and the actual\n> processing can be initiated using process().\n> \n> The interface is similar to the Encoder interface. The PostProcessor is\n> meant to replace the Encoder interface and introduce a more generic\n> post-processing layer which can be extended to have multiple post\n> processors for various stream configurations. As of now, we only have\n> one post processor (JPEG), hence the subsequent commit will port its\n> function to this interface.\n> \n> Signed-off-by: Umang Jain \n> ---\n> src/android/post_processor.h | 30 ++++++++++++++++++++++++++++++\n> 1 file changed, 30 insertions(+)\n> create mode 100644 src/android/post_processor.h\n> \n> diff --git a/src/android/post_processor.h b/src/android/post_processor.h\n> new file mode 100644\n> index 0000000..fa676c9\n> --- /dev/null\n> +++ b/src/android/post_processor.h\n> @@ -0,0 +1,30 @@\n> +/* SPDX-License-Identifier: GPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * post_processor.h - CameraStream Post Processing Interface\n> + */\n> +#ifndef __ANDROID_POST_PROCESSOR_H__\n> +#define __ANDROID_POST_PROCESSOR_H__\n> +\n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +\n> +class CameraMetadata;\n> +\n> +class PostProcessor\n> +{\n> +public:\n> +\tvirtual ~PostProcessor() {};\n\nNo need for a semicolumn at the end of the line.\n\n> +\n> +\tvirtual int configure(const libcamera::StreamConfiguration &cfg) = 0;\n> +\tvirtual int process(const libcamera::FrameBuffer *source,\n\nIf we have multiple post-processors that run on the same source frame\nbuffer, it would be more efficient to map the frame buffer in the caller\nand passed a mapped frame buffer instead. On the other hand, some\npost-processor may be hardware-based, and creating a CPU mapping would\nthen be a waste. I think we can keep the API as-is for now, but it will\nlikely need to be reworked.\n\n> +\t\t\t const libcamera::Span &destination,\n> +\t\t\t CameraMetadata *metadata,\n> +\t\t\t ...) = 0;\n\nVariadic arguments are problematic if you want this interface to be\ngeneric. The whole point of this base class is to offer an API to users\nthat doesn't require handling any implementation-specific detail. If the\ncaller needs to know what extra arguments to pass to a particular\npost-processor, it defeats the point completely.\n\nI'll comment more about this on the next patch, but I think you can\nsimply drop the variadic arguments.\n\n> +};\n> +\n> +#endif /* __ANDROID_POST_PROCESSOR_H__ */","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 65F12BEEDF\n\tfor ;\n\tThu, 8 Oct 2020 19:10:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E09D9605D2;\n\tThu, 8 Oct 2020 21:10:54 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E62C9605B5\n\tfor ;\n\tThu, 8 Oct 2020 21:10:52 +0200 (CEST)","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 4BF1B59E;\n\tThu, 8 Oct 2020 21:10:52 +0200 (CEST)"],"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=\"E9AGkR4n\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1602184252;\n\tbh=kx2yrBE5y+R0UVtV8H0bwoKIoAOWIDusycTOKphMom8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=E9AGkR4n9sw6zmg6u0zQVQITvEckkvlivKhpx4FKZ4cEHlJ811mpw7SA6jci39v/C\n\tNs6VWINOvdWSAK8WXKcSOVtTunjGbqYsce+7H7BlA+jgn+aa63bBj3aktUHIFazmRG\n\tGoaaYWtUuMlvMs2/koWRDfOKcRJU+D418DL+87PY=","Date":"Thu, 8 Oct 2020 22:10:09 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"<20201008191009.GD3939@pendragon.ideasonboard.com>","References":"<20201008141038.83425-1-email@uajain.com>\n\t<20201008141038.83425-2-email@uajain.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201008141038.83425-2-email@uajain.com>","Subject":"Re: [libcamera-devel] [RFC PATCH 1/3] android: post_processor:\n\tIntroduce a PostProcessor interface","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13122,"web_url":"https://patchwork.libcamera.org/comment/13122/","msgid":"","date":"2020-10-09T08:49:25","subject":"Re: [libcamera-devel] [RFC PATCH 1/3] android: post_processor:\n\tIntroduce a PostProcessor interface","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 08/10/2020 20:10, Laurent Pinchart wrote:\n> Hi Umang,\n> \n> Thank you for the patch.\n> \n> On Thu, Oct 08, 2020 at 07:40:36PM +0530, Umang Jain wrote:\n>> Introduce a PostProcessor interface for the streams that require any\n>> kind of processing for their consumption by the HAL layer. The\n>> PostProcessor interface can be configured via configure() and the actual\n>> processing can be initiated using process().\n>>\n>> The interface is similar to the Encoder interface. The PostProcessor is\n>> meant to replace the Encoder interface and introduce a more generic\n>> post-processing layer which can be extended to have multiple post\n>> processors for various stream configurations. As of now, we only have\n>> one post processor (JPEG), hence the subsequent commit will port its\n>> function to this interface.\n>>\n>> Signed-off-by: Umang Jain \n>> ---\n>> src/android/post_processor.h | 30 ++++++++++++++++++++++++++++++\n>> 1 file changed, 30 insertions(+)\n>> create mode 100644 src/android/post_processor.h\n>>\n>> diff --git a/src/android/post_processor.h b/src/android/post_processor.h\n>> new file mode 100644\n>> index 0000000..fa676c9\n>> --- /dev/null\n>> +++ b/src/android/post_processor.h\n>> @@ -0,0 +1,30 @@\n>> +/* SPDX-License-Identifier: GPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2020, Google Inc.\n>> + *\n>> + * post_processor.h - CameraStream Post Processing Interface\n>> + */\n>> +#ifndef __ANDROID_POST_PROCESSOR_H__\n>> +#define __ANDROID_POST_PROCESSOR_H__\n>> +\n>> +#include \n>> +\n>> +#include \n>> +#include \n>> +#include \n>> +\n>> +class CameraMetadata;\n>> +\n>> +class PostProcessor\n>> +{\n>> +public:\n>> +\tvirtual ~PostProcessor() {};\n> \n> No need for a semicolumn at the end of the line.\n> \n>> +\n>> +\tvirtual int configure(const libcamera::StreamConfiguration &cfg) = 0;\n>> +\tvirtual int process(const libcamera::FrameBuffer *source,\n> \n> If we have multiple post-processors that run on the same source frame\n> buffer, it would be more efficient to map the frame buffer in the caller\n> and passed a mapped frame buffer instead. On the other hand, some\n> post-processor may be hardware-based, and creating a CPU mapping would\n> then be a waste. I think we can keep the API as-is for now, but it will\n> likely need to be reworked.\n\nOriginally I had wanted to keep the mapping within the FrameBuffer - but\neveryone seemed to dislike that.\n\nKeeping the mapping with the FrameBuffer (and having the required\nsupport there) would mean that a mapping could be done once, on the\nfirst instance it was needed, or if it was not needed, then not at all.\nAnd it would then be unmapped when the FrameBuffer was released.\n\nIf the FrameBuffer is re-used... the mapping persists, and no need to\nunmap/remap.\n\n...\n\nKieran\n\n\n\n> \n>> +\t\t\t const libcamera::Span &destination,\n>> +\t\t\t CameraMetadata *metadata,\n>> +\t\t\t ...) = 0;\n> \n> Variadic arguments are problematic if you want this interface to be\n> generic. The whole point of this base class is to offer an API to users\n> that doesn't require handling any implementation-specific detail. If the\n> caller needs to know what extra arguments to pass to a particular\n> post-processor, it defeats the point completely.\n> \n> I'll comment more about this on the next patch, but I think you can\n> simply drop the variadic arguments.\n> \n>> +};\n>> +\n>> +#endif /* __ANDROID_POST_PROCESSOR_H__ */\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 0F6FDBEEDF\n\tfor ;\n\tFri, 9 Oct 2020 08:49:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8A577605D1;\n\tFri, 9 Oct 2020 10:49:29 +0200 (CEST)","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 4FB1B60357\n\tfor ;\n\tFri, 9 Oct 2020 10:49:28 +0200 (CEST)","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 AD671539;\n\tFri, 9 Oct 2020 10:49:27 +0200 (CEST)"],"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=\"QK1+aniM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1602233367;\n\tbh=+Ku8uowvReesSoTTHmzd2VBUiPjvw9u5i+qqwx46h64=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=QK1+aniMK78EdT2G7vt6/yAODViynU7rWTeennOgUcIt4FgDO01XsGc0ckGggIzOJ\n\tZix8XTjH5nXPKgoNmXMOH/MUbiYw73kdz5x5TcH+YtxrW/U1WrSvNHIGHz3EOeJfZs\n\ts5H0mz3eWqSRxSqwX9p4Yor+PMEmMhOzU6n5o2q8=","To":"Laurent Pinchart ,\n\tUmang Jain ","References":"<20201008141038.83425-1-email@uajain.com>\n\t<20201008141038.83425-2-email@uajain.com>\n\t<20201008191009.GD3939@pendragon.ideasonboard.com>","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":"Fri, 9 Oct 2020 09:49:25 +0100","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":"<20201008191009.GD3939@pendragon.ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [RFC PATCH 1/3] android: post_processor:\n\tIntroduce a PostProcessor interface","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":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13150,"web_url":"https://patchwork.libcamera.org/comment/13150/","msgid":"<20201009214919.GF25040@pendragon.ideasonboard.com>","date":"2020-10-09T21:49:19","subject":"Re: [libcamera-devel] [RFC PATCH 1/3] android: post_processor:\n\tIntroduce a PostProcessor interface","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Fri, Oct 09, 2020 at 09:49:25AM +0100, Kieran Bingham wrote:\n> On 08/10/2020 20:10, Laurent Pinchart wrote:\n> > On Thu, Oct 08, 2020 at 07:40:36PM +0530, Umang Jain wrote:\n> >> Introduce a PostProcessor interface for the streams that require any\n> >> kind of processing for their consumption by the HAL layer. The\n> >> PostProcessor interface can be configured via configure() and the actual\n> >> processing can be initiated using process().\n> >>\n> >> The interface is similar to the Encoder interface. The PostProcessor is\n> >> meant to replace the Encoder interface and introduce a more generic\n> >> post-processing layer which can be extended to have multiple post\n> >> processors for various stream configurations. As of now, we only have\n> >> one post processor (JPEG), hence the subsequent commit will port its\n> >> function to this interface.\n> >>\n> >> Signed-off-by: Umang Jain \n> >> ---\n> >> src/android/post_processor.h | 30 ++++++++++++++++++++++++++++++\n> >> 1 file changed, 30 insertions(+)\n> >> create mode 100644 src/android/post_processor.h\n> >>\n> >> diff --git a/src/android/post_processor.h b/src/android/post_processor.h\n> >> new file mode 100644\n> >> index 0000000..fa676c9\n> >> --- /dev/null\n> >> +++ b/src/android/post_processor.h\n> >> @@ -0,0 +1,30 @@\n> >> +/* SPDX-License-Identifier: GPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2020, Google Inc.\n> >> + *\n> >> + * post_processor.h - CameraStream Post Processing Interface\n> >> + */\n> >> +#ifndef __ANDROID_POST_PROCESSOR_H__\n> >> +#define __ANDROID_POST_PROCESSOR_H__\n> >> +\n> >> +#include \n> >> +\n> >> +#include \n> >> +#include \n> >> +#include \n> >> +\n> >> +class CameraMetadata;\n> >> +\n> >> +class PostProcessor\n> >> +{\n> >> +public:\n> >> +\tvirtual ~PostProcessor() {};\n> > \n> > No need for a semicolumn at the end of the line.\n> > \n> >> +\n> >> +\tvirtual int configure(const libcamera::StreamConfiguration &cfg) = 0;\n> >> +\tvirtual int process(const libcamera::FrameBuffer *source,\n> > \n> > If we have multiple post-processors that run on the same source frame\n> > buffer, it would be more efficient to map the frame buffer in the caller\n> > and passed a mapped frame buffer instead. On the other hand, some\n> > post-processor may be hardware-based, and creating a CPU mapping would\n> > then be a waste. I think we can keep the API as-is for now, but it will\n> > likely need to be reworked.\n> \n> Originally I had wanted to keep the mapping within the FrameBuffer - but\n> everyone seemed to dislike that.\n> \n> Keeping the mapping with the FrameBuffer (and having the required\n> support there) would mean that a mapping could be done once, on the\n> first instance it was needed, or if it was not needed, then not at all.\n> And it would then be unmapped when the FrameBuffer was released.\n> \n> If the FrameBuffer is re-used... the mapping persists, and no need to\n> unmap/remap.\n\nI've thought exactly the same thing while writing the previous e-mail.\nBundling the FrameBuffer and the mapping in a single class is a good\nidea. I'm still not entirely convinced we should add that featuer to the\nFrameBuffer class itself though, albeit if you wanted to convince me it\nshould be done, you would be one step closer to the goal :-)\n\nWould it make sense (not as part of this series) to refactor the\nMappedFrammeBuffer class to bundle the FrameBuffer and the mapping, and\nexperiment with that in the HAL ?\n\n> >> +\t\t\t const libcamera::Span &destination,\n> >> +\t\t\t CameraMetadata *metadata,\n> >> +\t\t\t ...) = 0;\n> > \n> > Variadic arguments are problematic if you want this interface to be\n> > generic. The whole point of this base class is to offer an API to users\n> > that doesn't require handling any implementation-specific detail. If the\n> > caller needs to know what extra arguments to pass to a particular\n> > post-processor, it defeats the point completely.\n> > \n> > I'll comment more about this on the next patch, but I think you can\n> > simply drop the variadic arguments.\n> > \n> >> +};\n> >> +\n> >> +#endif /* __ANDROID_POST_PROCESSOR_H__ */","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 72ABEBEEDF\n\tfor ;\n\tFri, 9 Oct 2020 21:50:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D6E0260725;\n\tFri, 9 Oct 2020 23:50:04 +0200 (CEST)","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 55A0160358\n\tfor ;\n\tFri, 9 Oct 2020 23:50:03 +0200 (CEST)","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 B40DB329;\n\tFri, 9 Oct 2020 23:50:02 +0200 (CEST)"],"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=\"Zg7+A0c2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1602280202;\n\tbh=7/6mECgVXW0iLne3d5YiJmbZXOBkXI5Feaz2PVZdPVg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Zg7+A0c2oe70zJaZa800hnToKQFujML4zROAhDeD05FpriI6OzJTOzLtlx8Q88RcH\n\tghopn2btPQ8/G/XN6A1GHw3Q3+Z8qZ+EuBKT/H6xPD5irHNzY9wU9G4vUxzW+QfWYd\n\tOQfE8PpF9wXnNYV5yjggMdiMH1naY90WJPxC+ThU=","Date":"Sat, 10 Oct 2020 00:49:19 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"<20201009214919.GF25040@pendragon.ideasonboard.com>","References":"<20201008141038.83425-1-email@uajain.com>\n\t<20201008141038.83425-2-email@uajain.com>\n\t<20201008191009.GD3939@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH 1/3] android: post_processor:\n\tIntroduce a PostProcessor interface","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]