[{"id":19266,"web_url":"https://patchwork.libcamera.org/comment/19266/","msgid":"<20210902062638.GC968527@pyrite.rasen.tech>","date":"2021-09-02T06:26:38","subject":"Re: [libcamera-devel] [RFC PATCH v1] libcamera: isp: Add ISP class","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Siyuan,\n\nThank you for the patch.\n\nOn Wed, Sep 01, 2021 at 02:59:08PM +0100, Siyuan Fan wrote:\n> From: Fan Siyuan \n> \n> The ISP class is a abstract base class of software ISP. It includes image\n> format configuration, ISP algorithm parameters parser, pixel processing\n> image export and thread configuration.\n> \n> This new class will be used as the basic class for ISPCPU and ISPGPU.\n> \n> Signed-off-by: Fan Siyuan \n> ---\n> \n> ISP Parameters Tuning is a important part, so I've designed a parameters\n> parser in order that users can call any algorithm combination by passing\n> any number of tuple. Each tuple format is including algorithm name string\n> and algorithm param, such as tuple for black level correct. \n> Currently this function is only a demo. I'm thus sending it as an RFC.\n\nI don't quite see how you imagine parse() to be used. Do you \"configure\"\nthe function that you want to run the ISP with first, and then tell it\nto process it? What's wrong with passing the list of command-parameter\npairs into the processing function directly?\n\nI think that the processing function should also return statistics. For\na CPU ISP it's not unreasonable for the control and processing to be\ntogether, but remember that this interface must be usable for a GPU ISP\nas well, which would have separate control and processing.\n\n\nPaul\n\n> ---\n> include/libcamera/internal/isp.h | 67 ++++++++++++++++++++++++++++++++\n> 1 file changed, 67 insertions(+)\n> create mode 100644 include/libcamera/internal/isp.h\n> \n> diff --git a/include/libcamera/internal/isp.h b/include/libcamera/internal/isp.h\n> new file mode 100644\n> index 00000000..caab7050\n> --- /dev/null\n> +++ b/include/libcamera/internal/isp.h\n> @@ -0,0 +1,67 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Siyuan Fan \n> + *\n> + * isp.h - The software ISP abstract base class\n> + */\n> +#ifndef __LIBCAMERA_INTERNAL_ISP_H__\n> +#define __LIBCAMERA_INTERNAL_ISP_H__\n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \"libcamera/base/object.h\"\n> +#include \"libcamera/base/signal.h\"\n> +\n> +namespace libcamera{\n> +\n> +class ISP : public Object\n> +{\n> +public:\n> + ISP() {}\n> +\n> + virtual ~ISP() {}\n> +\n> + template\n> + void for_each_in_tuple(const std::tuple & tuple, F func, std::index_sequence) {\n> + \n> + return (void(func(std::get(tuple))), ...);\n> + }\n> +\n> + template\n> + void for_each_in_tuple(const std::tuple & tuple, F func) {\n> + for_each_in_tuple(tuple, func, std::make_index_sequence());\n> + }\n> +\n> + template\n> + void parse(const T &head, const Args &... rest)\n> + {\n> + for_each_in_tuple(head, [&](const auto &x) {\n> + // parse parameters for diff algorithm\n> + });\n> + }\n> +\n> + virtual int configure(PixelFormat inputFormat, PixelFormat outputFormat, Size inputSize, Size outputSize) = 0;\n> +\n> + virtual void processing(FrameBuffer *srcBuffer, FrameBuffer *dstBuffer,\n> + int width, int height) = 0;\n> +\n> + virtual int exportBuffers(std::vector> *buffers,\n> + unsigned int count, int width, int height) = 0;\n> +\n> + virtual void start() = 0;\n> +\n> + virtual void stop() = 0;\n> +\n> + Signal ispCompleted;\n> +};\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_INTERNAL_ISP_H__ */\n> -- \n> 2.20.1\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 618DDBD87D\n\tfor ;\n\tThu, 2 Sep 2021 06:26:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8E5AA6916A;\n\tThu, 2 Sep 2021 08:26:47 +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 5D10660503\n\tfor ;\n\tThu, 2 Sep 2021 08:26:46 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DCCCB45E;\n\tThu, 2 Sep 2021 08:26:44 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"r0RglOA1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1630564005;\n\tbh=AHlWlMkboZABVgjG3hRkeF8qJ7gDEmFoVHbVT/b0350=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=r0RglOA1pRECCu9TF0I/mL7OrN5mRWSwHaRxHoJiH5h34WDs/7v1WiO7wxzPgHycY\n\tiKJZ4DEhPwPMqGJWbKEwrlWAKpyO8vGKg0okpHfvBcjbTyDTEyE8mAFvc4JmIdndaT\n\tQKreV5pFRYmvKg2Ytvudc8tvGgrehMEe+rEyOq2M=","Date":"Thu, 2 Sep 2021 15:26:38 +0900","From":"paul.elder@ideasonboard.com","To":"Siyuan Fan ","Message-ID":"<20210902062638.GC968527@pyrite.rasen.tech>","References":"","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH v1] libcamera: isp: Add ISP class","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]