[{"id":3638,"web_url":"https://patchwork.libcamera.org/comment/3638/","msgid":"<20200207224030.GF4726@pendragon.ideasonboard.com>","date":"2020-02-07T22:40:30","subject":"Re: [libcamera-devel] [PATCH v2 4/7] libcamera: sensor: Add OV5670\n\tcamera sensor","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Thu, Feb 06, 2020 at 07:52:44PM +0100, Jacopo Mondi wrote:\n> Add OV5670CameraSensor class to handle Omnivision OV5670 image sensor\n> and register it to the camera sensor factory.\n\ns/to the/with the/\n\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/camera_sensor.cpp | 1 +\n> src/libcamera/meson.build | 1 +\n> src/libcamera/sensor/meson.build | 3 +++\n> src/libcamera/sensor/ov5670.cpp | 43 ++++++++++++++++++++++++++++++++\n> src/libcamera/sensor/ov5670.h | 24 ++++++++++++++++++\n> 5 files changed, 72 insertions(+)\n> create mode 100644 src/libcamera/sensor/meson.build\n> create mode 100644 src/libcamera/sensor/ov5670.cpp\n> create mode 100644 src/libcamera/sensor/ov5670.h\n> \n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index fc8452b607a0..06d10295a80e 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -6,6 +6,7 @@\n> */\n> \n> #include \"camera_sensor.h\"\n> +#include \"sensor/ov5670.h\"\n\nI don't think this is needed.\n\n> \n> #include \n> #include \n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index ecc5b5fe4023..7dd7358b174e 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -58,6 +58,7 @@ includes = [\n> \n> subdir('pipeline')\n> subdir('proxy')\n> +subdir('sensor')\n> \n> libudev = dependency('libudev', required : false)\n> \n> diff --git a/src/libcamera/sensor/meson.build b/src/libcamera/sensor/meson.build\n> new file mode 100644\n> index 000000000000..7af70370cf5c\n> --- /dev/null\n> +++ b/src/libcamera/sensor/meson.build\n> @@ -0,0 +1,3 @@\n> +libcamera_sources += files([\n> + 'ov5670.cpp',\n> +])\n> diff --git a/src/libcamera/sensor/ov5670.cpp b/src/libcamera/sensor/ov5670.cpp\n> new file mode 100644\n> index 000000000000..de6011875a2d\n> --- /dev/null\n> +++ b/src/libcamera/sensor/ov5670.cpp\n> @@ -0,0 +1,43 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n\nHappy new year :-)\n\n> + *\n> + * ov5670.cpp - OV5670 camera sensor\n> + */\n> +\n> +#include \"ov5670.h\"\n> +#include \"camera_sensor.h\"\n> +\n> +/**\n> + * \\file ov5670.h\n> + * \\brief Omnivision OV5670 image sensor handler\n\nAs for the previous patch, let's not use the name \"handler\".\n\n> + */\n> +\n> +namespace libcamera {\n> +\n> +/**\n> + * \\class OV5670CameraSensor\n\nWould OV5670 be a long-enough class name ?\n\nThe same way we don't document pipeline handlers, I think you can add\nsrc/libcamera/sensor/ to the doxygen exclude list.\n\n> + * \\brief Camera sensor handler for Omnivision OV5670 image sensor\n> + */\n> +\n> +/**\n> + * \\brief Retrieve the name of the sensor entity supported by the handler\n> + * \\return The supported sensor entity name\n> + */\n> +const char *OV5670CameraSensor::entityName()\n> +{\n> +\treturn \"ov5670\";\n> +}\n> +\n> +/**\n> + * \\brief Construct the ov5670 sensor handler\n> + * \\param[in] entity The media entity representing the sensor\n> + */\n> +OV5670CameraSensor::OV5670CameraSensor(const MediaEntity *entity)\n> +\t: CameraSensor(entity)\n> +{\n> +}\n> +\n> +REGISTER_CAMERA_SENSOR(OV5670);\n> +\n> +}; /* namespace libcamera */\n> diff --git a/src/libcamera/sensor/ov5670.h b/src/libcamera/sensor/ov5670.h\n> new file mode 100644\n> index 000000000000..4acf02a8b06b\n> --- /dev/null\n> +++ b/src/libcamera/sensor/ov5670.h\n> @@ -0,0 +1,24 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * ov5670.h - OV5670 camera sensor\n> + */\n> +#ifndef __LIBCAMERA_SENSOR_OV5670_H__\n> +#define __LIBCAMERA_SENSOR_OV5670_H__\n> +\n> +#include \"camera_sensor.h\"\n> +\n> +namespace libcamera {\n> +\n> +class OV5670CameraSensor final : public CameraSensor\n> +{\n> +public:\n> +\tstatic const char *entityName();\n> +\n> +\tOV5670CameraSensor(const MediaEntity *entity);\n> +};\n\nI think you can move this to ov5670.cpp and remove ov5670.h.\n\nThis patch will become pretty small, I'd squash it with 6/7.\n\n> +\n> +}; /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_SENSOR_OV5670_H__ */","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 78B9E60445\n\tfor ;\n\tFri, 7 Feb 2020 23:40:46 +0100 (CET)","from pendragon.ideasonboard.com\n\t(117.145-247-81.adsl-dyn.isp.belgacom.be [81.247.145.117])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E34729F3;\n\tFri, 7 Feb 2020 23:40:45 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1581115246;\n\tbh=5lHX0rumuGtIqaDrIaYqBWlcKjhh8sXHWxkR9JaUQB0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=fm1bZ8npmDonnQ1ltpcfWbVjj0lZMRAVOrCeurqjQ5Hbs5vALnQpAcg/bXI9Bx6Mi\n\tIO+LH/xhHWaxuUOPROUnkX2sgCC9EOepa7z6a7Q9KVmTs6V3JK/JHO2CbdLYHYRVKR\n\t01w9CQvEkxkS+cNlU9L14uq1LQ6pvra2f6DPYbIU=","Date":"Sat, 8 Feb 2020 00:40:30 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200207224030.GF4726@pendragon.ideasonboard.com>","References":"<20200206185247.202233-1-jacopo@jmondi.org>\n\t<20200206185247.202233-5-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200206185247.202233-5-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 4/7] libcamera: sensor: Add OV5670\n\tcamera sensor","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","X-List-Received-Date":"Fri, 07 Feb 2020 22:40:46 -0000"}}]