[{"id":11448,"web_url":"https://patchwork.libcamera.org/comment/11448/","msgid":"","date":"2020-07-21T10:45:20","subject":"Re: [libcamera-devel] [PATCH v4 1/9] libcamera: pipeline:\n\traspberrypi: Move RPiStream into a separate file","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Naush,\n\nOn 20/07/2020 10:13, Naushir Patuck wrote:\n> Put RPiStream into the RPi namespace and add a new log category (RPISTREAM).\n> There are no functional changes in this commit.\n> \n> Signed-off-by: Naushir Patuck \n> Reviewed-by: Niklas Söderlund \n\nGreat, I love seeing this become more modular, I think that will help\nnavigating code and maintenance.\n\nReviewed-by: Kieran Bingham \n\n> ---\n> .../pipeline/raspberrypi/meson.build | 1 +\n> .../pipeline/raspberrypi/raspberrypi.cpp | 193 ++----------------\n> .../pipeline/raspberrypi/rpi_stream.cpp | 116 +++++++++++\n> .../pipeline/raspberrypi/rpi_stream.h | 98 +++++++++\n> 4 files changed, 234 insertions(+), 174 deletions(-)\n> create mode 100644 src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> create mode 100644 src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/meson.build b/src/libcamera/pipeline/raspberrypi/meson.build\n> index ae0aed3b..7c5b6ff7 100644\n> --- a/src/libcamera/pipeline/raspberrypi/meson.build\n> +++ b/src/libcamera/pipeline/raspberrypi/meson.build\n> @@ -3,5 +3,6 @@\n> libcamera_sources += files([\n> 'dma_heaps.cpp',\n> 'raspberrypi.cpp',\n> + 'rpi_stream.cpp',\n> 'staggered_ctrl.cpp',\n> ])\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index bf1c7714..6630ef57 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -19,7 +19,6 @@\n> #include \n> #include \n> #include \n> -#include \n> \n> #include \n> \n> @@ -33,6 +32,7 @@\n> #include \"libcamera/internal/v4l2_videodevice.h\"\n> \n> #include \"dma_heaps.h\"\n> +#include \"rpi_stream.h\"\n> #include \"staggered_ctrl.h\"\n> \n> namespace libcamera {\n> @@ -123,165 +123,10 @@ V4L2DeviceFormat findBestMode(V4L2PixFmtMap &formatsMap, const Size &req)\n> \treturn bestMode;\n> }\n> \n> -} /* namespace */\n> -\n> -/*\n> - * Device stream abstraction for either an internal or external stream.\n> - * Used for both Unicam and the ISP.\n> - */\n> -class RPiStream : public Stream\n> -{\n> -public:\n> -\tRPiStream()\n> -\t{\n> -\t}\n> -\n> -\tRPiStream(const char *name, MediaEntity *dev, bool importOnly = false)\n> -\t\t: external_(false), importOnly_(importOnly), name_(name),\n> -\t\t dev_(std::make_unique(dev))\n> -\t{\n> -\t}\n> -\n> -\tV4L2VideoDevice *dev() const\n> -\t{\n> -\t\treturn dev_.get();\n> -\t}\n> -\n> -\tvoid setExternal(bool external)\n> -\t{\n> -\t\texternal_ = external;\n> -\t}\n> -\n> -\tbool isExternal() const\n> -\t{\n> -\t\t/*\n> -\t\t * Import streams cannot be external.\n> -\t\t *\n> -\t\t * RAW capture is a special case where we simply copy the RAW\n> -\t\t * buffer out of the request. All other buffer handling happens\n> -\t\t * as if the stream is internal.\n> -\t\t */\n> -\t\treturn external_ && !importOnly_;\n> -\t}\n> -\n> -\tbool isImporter() const\n> -\t{\n> -\t\treturn importOnly_;\n> -\t}\n> -\n> -\tvoid reset()\n> -\t{\n> -\t\texternal_ = false;\n> -\t\tinternalBuffers_.clear();\n> -\t}\n> -\n> -\tstd::string name() const\n> -\t{\n> -\t\treturn name_;\n> -\t}\n> -\n> -\tvoid setExternalBuffers(std::vector> *buffers)\n> -\t{\n> -\t\texternalBuffers_ = buffers;\n> -\t}\n> -\n> -\tconst std::vector> *getBuffers() const\n> -\t{\n> -\t\treturn external_ ? externalBuffers_ : &internalBuffers_;\n> -\t}\n> -\n> -\tvoid releaseBuffers()\n> -\t{\n> -\t\tdev_->releaseBuffers();\n> -\t\tif (!external_ && !importOnly_)\n> -\t\t\tinternalBuffers_.clear();\n> -\t}\n> -\n> -\tint importBuffers(unsigned int count)\n> -\t{\n> -\t\treturn dev_->importBuffers(count);\n> -\t}\n> -\n> -\tint allocateBuffers(unsigned int count)\n> -\t{\n> -\t\treturn dev_->allocateBuffers(count, &internalBuffers_);\n> -\t}\n> -\n> -\tint queueBuffers()\n> -\t{\n> -\t\tif (external_)\n> -\t\t\treturn 0;\n> -\n> -\t\tfor (auto &b : internalBuffers_) {\n> -\t\t\tint ret = dev_->queueBuffer(b.get());\n> -\t\t\tif (ret) {\n> -\t\t\t\tLOG(RPI, Error) << \"Failed to queue buffers for \"\n> -\t\t\t\t\t\t<< name_;\n> -\t\t\t\treturn ret;\n> -\t\t\t}\n> -\t\t}\n> -\n> -\t\treturn 0;\n> -\t}\n> -\n> -\tbool findFrameBuffer(FrameBuffer *buffer) const\n> -\t{\n> -\t\tauto start = external_ ? externalBuffers_->begin() : internalBuffers_.begin();\n> -\t\tauto end = external_ ? externalBuffers_->end() : internalBuffers_.end();\n> -\n> -\t\tif (importOnly_)\n> -\t\t\treturn false;\n> -\n> -\t\tif (std::find_if(start, end,\n> -\t\t\t\t [buffer](std::unique_ptr const &ref) { return ref.get() == buffer; }) != end)\n> -\t\t\treturn true;\n> -\n> -\t\treturn false;\n> -\t}\n> -\n> -private:\n> -\t/*\n> -\t * Indicates that this stream is active externally, i.e. the buffers\n> -\t * are provided by the application.\n> -\t */\n> -\tbool external_;\n> -\t/* Indicates that this stream only imports buffers, e.g. ISP input. */\n> -\tbool importOnly_;\n> -\t/* Stream name identifier. */\n> -\tstd::string name_;\n> -\t/* The actual device stream. */\n> -\tstd::unique_ptr dev_;\n> -\t/* Internally allocated framebuffers associated with this device stream. */\n> -\tstd::vector> internalBuffers_;\n> -\t/* Externally allocated framebuffers associated with this device stream. */\n> -\tstd::vector> *externalBuffers_;\n> -};\n> -\n> -/*\n> - * The following class is just a convenient (and typesafe) array of device\n> - * streams indexed with an enum class.\n> - */\n> enum class Unicam : unsigned int { Image, Embedded };\n> enum class Isp : unsigned int { Input, Output0, Output1, Stats };\n> \n> -template\n> -class RPiDevice : public std::array\n> -{\n> -private:\n> -\tconstexpr auto index(E e) const noexcept\n> -\t{\n> -\t\treturn static_cast>(e);\n> -\t}\n> -public:\n> -\tRPiStream &operator[](E e)\n> -\t{\n> -\t\treturn std::array::operator[](index(e));\n> -\t}\n> -\tconst RPiStream &operator[](E e) const\n> -\t{\n> -\t\treturn std::array::operator[](index(e));\n> -\t}\n> -};\n> +} /* namespace */\n> \n> class RPiCameraData : public CameraData\n> {\n> @@ -305,15 +150,15 @@ public:\n> \tvoid ispOutputDequeue(FrameBuffer *buffer);\n> \n> \tvoid clearIncompleteRequests();\n> -\tvoid handleStreamBuffer(FrameBuffer *buffer, const RPiStream *stream);\n> +\tvoid handleStreamBuffer(FrameBuffer *buffer, const RPi::RPiStream *stream);\n> \tvoid handleState();\n> \n> \tCameraSensor *sensor_;\n> \t/* Array of Unicam and ISP device streams and associated buffers/streams. */\n> -\tRPiDevice unicam_;\n> -\tRPiDevice isp_;\n> +\tRPi::RPiDevice unicam_;\n> +\tRPi::RPiDevice isp_;\n> \t/* The vector below is just for convenience when iterating over all streams. */\n> -\tstd::vector streams_;\n> +\tstd::vector streams_;\n> \t/* Buffers passed to the IPA. */\n> \tstd::vector ipaBuffers_;\n> \n> @@ -762,7 +607,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> int PipelineHandlerRPi::exportFrameBuffers(Camera *camera, Stream *stream,\n> \t\t\t\t\t std::vector> *buffers)\n> {\n> -\tRPiStream *s = static_cast(stream);\n> +\tRPi::RPiStream *s = static_cast(stream);\n> \tunsigned int count = stream->configuration().bufferCount;\n> \tint ret = s->dev()->exportBuffers(count, buffers);\n> \n> @@ -908,14 +753,14 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n> \tstd::unique_ptr data = std::make_unique(this);\n> \n> \t/* Locate and open the unicam video streams. */\n> -\tdata->unicam_[Unicam::Embedded] = RPiStream(\"Unicam Embedded\", unicam_->getEntityByName(\"unicam-embedded\"));\n> -\tdata->unicam_[Unicam::Image] = RPiStream(\"Unicam Image\", unicam_->getEntityByName(\"unicam-image\"));\n> +\tdata->unicam_[Unicam::Embedded] = RPi::RPiStream(\"Unicam Embedded\", unicam_->getEntityByName(\"unicam-embedded\"));\n> +\tdata->unicam_[Unicam::Image] = RPi::RPiStream(\"Unicam Image\", unicam_->getEntityByName(\"unicam-image\"));\n> \n> \t/* Tag the ISP input stream as an import stream. */\n> -\tdata->isp_[Isp::Input] = RPiStream(\"ISP Input\", isp_->getEntityByName(\"bcm2835-isp0-output0\"), true);\n> -\tdata->isp_[Isp::Output0] = RPiStream(\"ISP Output0\", isp_->getEntityByName(\"bcm2835-isp0-capture1\"));\n> -\tdata->isp_[Isp::Output1] = RPiStream(\"ISP Output1\", isp_->getEntityByName(\"bcm2835-isp0-capture2\"));\n> -\tdata->isp_[Isp::Stats] = RPiStream(\"ISP Stats\", isp_->getEntityByName(\"bcm2835-isp0-capture3\"));\n> +\tdata->isp_[Isp::Input] = RPi::RPiStream(\"ISP Input\", isp_->getEntityByName(\"bcm2835-isp0-output0\"), true);\n> +\tdata->isp_[Isp::Output0] = RPi::RPiStream(\"ISP Output0\", isp_->getEntityByName(\"bcm2835-isp0-capture1\"));\n> +\tdata->isp_[Isp::Output1] = RPi::RPiStream(\"ISP Output1\", isp_->getEntityByName(\"bcm2835-isp0-capture2\"));\n> +\tdata->isp_[Isp::Stats] = RPi::RPiStream(\"ISP Stats\", isp_->getEntityByName(\"bcm2835-isp0-capture3\"));\n> \n> \t/* This is just for convenience so that we can easily iterate over all streams. */\n> \tfor (auto &stream : data->unicam_)\n> @@ -1005,7 +850,7 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> \t */\n> \tunsigned int maxBuffers = 0;\n> \tfor (const Stream *s : camera->streams())\n> -\t\tif (static_cast(s)->isExternal())\n> +\t\tif (static_cast(s)->isExternal())\n> \t\t\tmaxBuffers = std::max(maxBuffers, s->configuration().bufferCount);\n> \n> \tfor (auto const stream : data->streams_) {\n> @@ -1255,12 +1100,12 @@ done:\n> \n> void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n> {\n> -\tconst RPiStream *stream = nullptr;\n> +\tconst RPi::RPiStream *stream = nullptr;\n> \n> \tif (state_ == State::Stopped)\n> \t\treturn;\n> \n> -\tfor (RPiStream const &s : unicam_) {\n> +\tfor (RPi::RPiStream const &s : unicam_) {\n> \t\tif (s.findFrameBuffer(buffer)) {\n> \t\t\tstream = &s;\n> \t\t\tbreak;\n> @@ -1316,12 +1161,12 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer)\n> \n> void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)\n> {\n> -\tconst RPiStream *stream = nullptr;\n> +\tconst RPi::RPiStream *stream = nullptr;\n> \n> \tif (state_ == State::Stopped)\n> \t\treturn;\n> \n> -\tfor (RPiStream const &s : isp_) {\n> +\tfor (RPi::RPiStream const &s : isp_) {\n> \t\tif (s.findFrameBuffer(buffer)) {\n> \t\t\tstream = &s;\n> \t\t\tbreak;\n> @@ -1402,7 +1247,7 @@ void RPiCameraData::clearIncompleteRequests()\n> \t}\n> }\n> \n> -void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, const RPiStream *stream)\n> +void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, const RPi::RPiStream *stream)\n> {\n> \tif (stream->isExternal()) {\n> \t\tif (!dropFrame_) {\n> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> new file mode 100644\n> index 00000000..2edb8b59\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> @@ -0,0 +1,116 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Raspberry Pi (Trading) Ltd.\n> + *\n> + * rpi_stream.cpp - Raspberry Pi device stream abstraction class.\n> + */\n> +#include \"rpi_stream.h\"\n> +\n> +#include \"libcamera/internal/log.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(RPISTREAM)\n> +\n> +namespace RPi {\n> +\n> +V4L2VideoDevice *RPiStream::dev() const\n> +{\n> +\treturn dev_.get();\n> +}\n> +\n> +void RPiStream::setExternal(bool external)\n> +{\n> +\texternal_ = external;\n> +}\n> +\n> +bool RPiStream::isExternal() const\n> +{\n> +\t/*\n> +\t * Import streams cannot be external.\n> +\t *\n> +\t * RAW capture is a special case where we simply copy the RAW\n> +\t * buffer out of the request. All other buffer handling happens\n> +\t * as if the stream is internal.\n> +\t */\n> +\treturn external_ && !importOnly_;\n> +}\n> +\n> +bool RPiStream::isImporter() const\n> +{\n> +\treturn importOnly_;\n> +}\n> +\n> +void RPiStream::reset()\n> +{\n> +\texternal_ = false;\n> +\tinternalBuffers_.clear();\n> +}\n> +\n> +std::string RPiStream::name() const\n> +{\n> +\treturn name_;\n> +}\n> +\n> +void RPiStream::setExternalBuffers(std::vector> *buffers)\n> +{\n> +\texternalBuffers_ = buffers;\n> +}\n> +\n> +const std::vector> *RPiStream::getBuffers() const\n> +{\n> +\treturn external_ ? externalBuffers_ : &internalBuffers_;\n> +}\n> +\n> +void RPiStream::releaseBuffers()\n> +{\n> +\tdev_->releaseBuffers();\n> +\tif (!external_ && !importOnly_)\n> +\t\tinternalBuffers_.clear();\n> +}\n> +\n> +int RPiStream::importBuffers(unsigned int count)\n> +{\n> +\treturn dev_->importBuffers(count);\n> +}\n> +\n> +int RPiStream::allocateBuffers(unsigned int count)\n> +{\n> +\treturn dev_->allocateBuffers(count, &internalBuffers_);\n> +}\n> +\n> +int RPiStream::queueBuffers()\n> +{\n> +\tif (external_)\n> +\t\treturn 0;\n> +\n> +\tfor (auto &b : internalBuffers_) {\n> +\t\tint ret = dev_->queueBuffer(b.get());\n> +\t\tif (ret) {\n> +\t\t\tLOG(RPISTREAM, Error) << \"Failed to queue buffers for \"\n> +\t\t\t\t\t << name_;\n> +\t\t\treturn ret;\n> +\t\t}\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +bool RPiStream::findFrameBuffer(FrameBuffer *buffer) const\n> +{\n> +\tauto start = external_ ? externalBuffers_->begin() : internalBuffers_.begin();\n> +\tauto end = external_ ? externalBuffers_->end() : internalBuffers_.end();\n> +\n> +\tif (importOnly_)\n> +\t\treturn false;\n> +\n> +\tif (std::find_if(start, end,\n> +\t\t\t [buffer](std::unique_ptr const &ref) { return ref.get() == buffer; }) != end)\n> +\t\treturn true;\n> +\n> +\treturn false;\n> +}\n> +\n> +} /* namespace RPi */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> new file mode 100644\n> index 00000000..3957e342\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> @@ -0,0 +1,98 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Raspberry Pi (Trading) Ltd.\n> + *\n> + * rpi_stream.h - Raspberry Pi device stream abstraction class.\n> + */\n> +#ifndef __LIBCAMERA_PIPELINE_RPI_STREAM_H__\n> +#define __LIBCAMERA_PIPELINE_RPI_STREAM_H__\n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/v4l2_videodevice.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace RPi {\n> +\n> +/*\n> + * Device stream abstraction for either an internal or external stream.\n> + * Used for both Unicam and the ISP.\n> + */\n> +class RPiStream : public Stream\n> +{\n> +public:\n> +\tRPiStream()\n> +\t{\n> +\t}\n> +\n> +\tRPiStream(const char *name, MediaEntity *dev, bool importOnly = false)\n> +\t\t: external_(false), importOnly_(importOnly), name_(name),\n> +\t\t dev_(std::make_unique(dev))\n> +\t{\n> +\t}\n> +\n> +\tV4L2VideoDevice *dev() const;\n> +\tvoid setExternal(bool external);\n> +\tbool isExternal() const;\n> +\tbool isImporter() const;\n> +\tvoid reset();\n> +\tstd::string name() const;\n> +\tvoid setExternalBuffers(std::vector> *buffers);\n> +\tconst std::vector> *getBuffers() const;\n> +\tvoid releaseBuffers();\n> +\tint importBuffers(unsigned int count);\n> +\tint allocateBuffers(unsigned int count);\n> +\tint queueBuffers();\n> +\tbool findFrameBuffer(FrameBuffer *buffer) const;\n> +\n> +private:\n> +\t/*\n> +\t * Indicates that this stream is active externally, i.e. the buffers\n> +\t * are provided by the application.\n> +\t */\n> +\tbool external_;\n> +\t/* Indicates that this stream only imports buffers, e.g. ISP input. */\n> +\tbool importOnly_;\n> +\t/* Stream name identifier. */\n> +\tstd::string name_;\n> +\t/* The actual device stream. */\n> +\tstd::unique_ptr dev_;\n> +\t/* Internally allocated framebuffers associated with this device stream. */\n> +\tstd::vector> internalBuffers_;\n> +\t/* Externally allocated framebuffers associated with this device stream. */\n> +\tstd::vector> *externalBuffers_;\n> +};\n> +\n> +/*\n> + * The following class is just a convenient (and typesafe) array of device\n> + * streams indexed with an enum class.\n> + */\n> +template\n> +class RPiDevice : public std::array\n> +{\n> +private:\n> +\tconstexpr auto index(E e) const noexcept\n> +\t{\n> +\t\treturn static_cast>(e);\n> +\t}\n> +public:\n> +\tRPiStream &operator[](E e)\n> +\t{\n> +\t\treturn std::array::operator[](index(e));\n> +\t}\n> +\tconst RPiStream &operator[](E e) const\n> +\t{\n> +\t\treturn std::array::operator[](index(e));\n> +\t}\n> +};\n> +\n> +} /* namespace RPi */\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_PIPELINE_RPI_STREAM_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 55C9FC0109\n\tfor ;\n\tTue, 21 Jul 2020 10:45:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D757A6087E;\n\tTue, 21 Jul 2020 12:45:24 +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 E3C2E60496\n\tfor ;\n\tTue, 21 Jul 2020 12:45:23 +0200 (CEST)","from [192.168.0.20]\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 027EC51A;\n\tTue, 21 Jul 2020 12:45:22 +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=\"wZSt9CBX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1595328323;\n\tbh=whLVFiRy5s+qGHymHAYX34/iYEVmWNPyK7Q8Ra7dcBw=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=wZSt9CBXZ9ZDdGrXRSGoB3gzi/k9ALGPRUuQ/hwtoGZU+loX40QwMLNXcT/65aYNj\n\trWNfoRtpiu6MADyFkX1uvfcdWJzO+wVzb/ZTFW++OdmO8QWprMI2M3fVylseKCMEJk\n\tPaIH9I+gacaOCc7xvfzHJ6l1NaHoZz+TiscK+55E=","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200720091311.805092-1-naush@raspberrypi.com>\n\t<20200720091311.805092-2-naush@raspberrypi.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":"Tue, 21 Jul 2020 11:45:20 +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":"<20200720091311.805092-2-naush@raspberrypi.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v4 1/9] libcamera: pipeline:\n\traspberrypi: Move RPiStream into a separate file","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","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11487,"web_url":"https://patchwork.libcamera.org/comment/11487/","msgid":"<389c5310-24f4-e3a2-ee28-21c6e004424f@ideasonboard.com>","date":"2020-07-22T12:29:22","subject":"Re: [libcamera-devel] [PATCH v4 1/9] libcamera: pipeline:\n\traspberrypi: Move RPiStream into a separate file","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Naush,\n\nOn 21/07/2020 11:45, Kieran Bingham wrote:\n> Hi Naush,\n> \n> On 20/07/2020 10:13, Naushir Patuck wrote:\n>> Put RPiStream into the RPi namespace and add a new log category (RPISTREAM).\n>> There are no functional changes in this commit.\n>>\n>> Signed-off-by: Naushir Patuck \n>> Reviewed-by: Niklas Söderlund \n> \n> Great, I love seeing this become more modular, I think that will help\n> navigating code and maintenance.\n> \n> Reviewed-by: Kieran Bingham \n> \n>> ---\n>> .../pipeline/raspberrypi/meson.build | 1 +\n>> .../pipeline/raspberrypi/raspberrypi.cpp | 193 ++----------------\n>> .../pipeline/raspberrypi/rpi_stream.cpp | 116 +++++++++++\n>> .../pipeline/raspberrypi/rpi_stream.h | 98 +++++++++\n>> 4 files changed, 234 insertions(+), 174 deletions(-)\n>> create mode 100644 src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n>> create mode 100644 src/libcamera/pipeline/raspberrypi/rpi_stream.h\n>>\n>> diff --git a/src/libcamera/pipeline/raspberrypi/meson.build b/src/libcamera/pipeline/raspberrypi/meson.build\n>> index ae0aed3b..7c5b6ff7 100644\n>> --- a/src/libcamera/pipeline/raspberrypi/meson.build\n>> +++ b/src/libcamera/pipeline/raspberrypi/meson.build\n>> @@ -3,5 +3,6 @@\n>> libcamera_sources += files([\n>> 'dma_heaps.cpp',\n>> 'raspberrypi.cpp',\n>> + 'rpi_stream.cpp',\n>> 'staggered_ctrl.cpp',\n>> ])\n>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> index bf1c7714..6630ef57 100644\n>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> @@ -19,7 +19,6 @@\n>> #include \n>> #include \n>> #include \n>> -#include \n>> \n>> #include \n>> \n>> @@ -33,6 +32,7 @@\n>> #include \"libcamera/internal/v4l2_videodevice.h\"\n>> \n>> #include \"dma_heaps.h\"\n>> +#include \"rpi_stream.h\"\n>> #include \"staggered_ctrl.h\"\n>> \n>> namespace libcamera {\n>> @@ -123,165 +123,10 @@ V4L2DeviceFormat findBestMode(V4L2PixFmtMap &formatsMap, const Size &req)\n>> \treturn bestMode;\n>> }\n>> \n>> -} /* namespace */\n>> -\n>> -/*\n>> - * Device stream abstraction for either an internal or external stream.\n>> - * Used for both Unicam and the ISP.\n>> - */\n>> -class RPiStream : public Stream\n>> -{\n>> -public:\n>> -\tRPiStream()\n>> -\t{\n>> -\t}\n>> -\n>> -\tRPiStream(const char *name, MediaEntity *dev, bool importOnly = false)\n>> -\t\t: external_(false), importOnly_(importOnly), name_(name),\n>> -\t\t dev_(std::make_unique(dev))\n>> -\t{\n>> -\t}\n>> -\n>> -\tV4L2VideoDevice *dev() const\n>> -\t{\n>> -\t\treturn dev_.get();\n>> -\t}\n>> -\n>> -\tvoid setExternal(bool external)\n>> -\t{\n>> -\t\texternal_ = external;\n>> -\t}\n>> -\n>> -\tbool isExternal() const\n>> -\t{\n>> -\t\t/*\n>> -\t\t * Import streams cannot be external.\n>> -\t\t *\n>> -\t\t * RAW capture is a special case where we simply copy the RAW\n>> -\t\t * buffer out of the request. All other buffer handling happens\n>> -\t\t * as if the stream is internal.\n>> -\t\t */\n>> -\t\treturn external_ && !importOnly_;\n>> -\t}\n>> -\n>> -\tbool isImporter() const\n>> -\t{\n>> -\t\treturn importOnly_;\n>> -\t}\n>> -\n>> -\tvoid reset()\n>> -\t{\n>> -\t\texternal_ = false;\n>> -\t\tinternalBuffers_.clear();\n>> -\t}\n>> -\n>> -\tstd::string name() const\n>> -\t{\n>> -\t\treturn name_;\n>> -\t}\n>> -\n>> -\tvoid setExternalBuffers(std::vector> *buffers)\n>> -\t{\n>> -\t\texternalBuffers_ = buffers;\n>> -\t}\n>> -\n>> -\tconst std::vector> *getBuffers() const\n>> -\t{\n>> -\t\treturn external_ ? externalBuffers_ : &internalBuffers_;\n>> -\t}\n>> -\n>> -\tvoid releaseBuffers()\n>> -\t{\n>> -\t\tdev_->releaseBuffers();\n>> -\t\tif (!external_ && !importOnly_)\n>> -\t\t\tinternalBuffers_.clear();\n>> -\t}\n>> -\n>> -\tint importBuffers(unsigned int count)\n>> -\t{\n>> -\t\treturn dev_->importBuffers(count);\n>> -\t}\n>> -\n>> -\tint allocateBuffers(unsigned int count)\n>> -\t{\n>> -\t\treturn dev_->allocateBuffers(count, &internalBuffers_);\n>> -\t}\n>> -\n>> -\tint queueBuffers()\n>> -\t{\n>> -\t\tif (external_)\n>> -\t\t\treturn 0;\n>> -\n>> -\t\tfor (auto &b : internalBuffers_) {\n>> -\t\t\tint ret = dev_->queueBuffer(b.get());\n>> -\t\t\tif (ret) {\n>> -\t\t\t\tLOG(RPI, Error) << \"Failed to queue buffers for \"\n>> -\t\t\t\t\t\t<< name_;\n>> -\t\t\t\treturn ret;\n>> -\t\t\t}\n>> -\t\t}\n>> -\n>> -\t\treturn 0;\n>> -\t}\n>> -\n>> -\tbool findFrameBuffer(FrameBuffer *buffer) const\n>> -\t{\n>> -\t\tauto start = external_ ? externalBuffers_->begin() : internalBuffers_.begin();\n>> -\t\tauto end = external_ ? externalBuffers_->end() : internalBuffers_.end();\n>> -\n>> -\t\tif (importOnly_)\n>> -\t\t\treturn false;\n>> -\n>> -\t\tif (std::find_if(start, end,\n>> -\t\t\t\t [buffer](std::unique_ptr const &ref) { return ref.get() == buffer; }) != end)\n>> -\t\t\treturn true;\n>> -\n>> -\t\treturn false;\n>> -\t}\n>> -\n>> -private:\n>> -\t/*\n>> -\t * Indicates that this stream is active externally, i.e. the buffers\n>> -\t * are provided by the application.\n>> -\t */\n>> -\tbool external_;\n>> -\t/* Indicates that this stream only imports buffers, e.g. ISP input. */\n>> -\tbool importOnly_;\n>> -\t/* Stream name identifier. */\n>> -\tstd::string name_;\n>> -\t/* The actual device stream. */\n>> -\tstd::unique_ptr dev_;\n>> -\t/* Internally allocated framebuffers associated with this device stream. */\n>> -\tstd::vector> internalBuffers_;\n>> -\t/* Externally allocated framebuffers associated with this device stream. */\n>> -\tstd::vector> *externalBuffers_;\n>> -};\n>> -\n>> -/*\n>> - * The following class is just a convenient (and typesafe) array of device\n>> - * streams indexed with an enum class.\n>> - */\n>> enum class Unicam : unsigned int { Image, Embedded };\n>> enum class Isp : unsigned int { Input, Output0, Output1, Stats };\n>> \n>> -template\n>> -class RPiDevice : public std::array\n>> -{\n>> -private:\n>> -\tconstexpr auto index(E e) const noexcept\n>> -\t{\n>> -\t\treturn static_cast>(e);\n>> -\t}\n>> -public:\n>> -\tRPiStream &operator[](E e)\n>> -\t{\n>> -\t\treturn std::array::operator[](index(e));\n>> -\t}\n>> -\tconst RPiStream &operator[](E e) const\n>> -\t{\n>> -\t\treturn std::array::operator[](index(e));\n>> -\t}\n>> -};\n>> +} /* namespace */\n>> \n>> class RPiCameraData : public CameraData\n>> {\n>> @@ -305,15 +150,15 @@ public:\n>> \tvoid ispOutputDequeue(FrameBuffer *buffer);\n>> \n>> \tvoid clearIncompleteRequests();\n>> -\tvoid handleStreamBuffer(FrameBuffer *buffer, const RPiStream *stream);\n>> +\tvoid handleStreamBuffer(FrameBuffer *buffer, const RPi::RPiStream *stream);\n>> \tvoid handleState();\n>> \n>> \tCameraSensor *sensor_;\n>> \t/* Array of Unicam and ISP device streams and associated buffers/streams. */\n>> -\tRPiDevice unicam_;\n>> -\tRPiDevice isp_;\n>> +\tRPi::RPiDevice unicam_;\n>> +\tRPi::RPiDevice isp_;\n>> \t/* The vector below is just for convenience when iterating over all streams. */\n>> -\tstd::vector streams_;\n>> +\tstd::vector streams_;\n>> \t/* Buffers passed to the IPA. */\n>> \tstd::vector ipaBuffers_;\n>> \n>> @@ -762,7 +607,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n>> int PipelineHandlerRPi::exportFrameBuffers(Camera *camera, Stream *stream,\n>> \t\t\t\t\t std::vector> *buffers)\n>> {\n>> -\tRPiStream *s = static_cast(stream);\n>> +\tRPi::RPiStream *s = static_cast(stream);\n>> \tunsigned int count = stream->configuration().bufferCount;\n>> \tint ret = s->dev()->exportBuffers(count, buffers);\n>> \n>> @@ -908,14 +753,14 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n>> \tstd::unique_ptr data = std::make_unique(this);\n>> \n>> \t/* Locate and open the unicam video streams. */\n>> -\tdata->unicam_[Unicam::Embedded] = RPiStream(\"Unicam Embedded\", unicam_->getEntityByName(\"unicam-embedded\"));\n>> -\tdata->unicam_[Unicam::Image] = RPiStream(\"Unicam Image\", unicam_->getEntityByName(\"unicam-image\"));\n>> +\tdata->unicam_[Unicam::Embedded] = RPi::RPiStream(\"Unicam Embedded\", unicam_->getEntityByName(\"unicam-embedded\"));\n>> +\tdata->unicam_[Unicam::Image] = RPi::RPiStream(\"Unicam Image\", unicam_->getEntityByName(\"unicam-image\"));\n>> \n>> \t/* Tag the ISP input stream as an import stream. */\n>> -\tdata->isp_[Isp::Input] = RPiStream(\"ISP Input\", isp_->getEntityByName(\"bcm2835-isp0-output0\"), true);\n>> -\tdata->isp_[Isp::Output0] = RPiStream(\"ISP Output0\", isp_->getEntityByName(\"bcm2835-isp0-capture1\"));\n>> -\tdata->isp_[Isp::Output1] = RPiStream(\"ISP Output1\", isp_->getEntityByName(\"bcm2835-isp0-capture2\"));\n>> -\tdata->isp_[Isp::Stats] = RPiStream(\"ISP Stats\", isp_->getEntityByName(\"bcm2835-isp0-capture3\"));\n>> +\tdata->isp_[Isp::Input] = RPi::RPiStream(\"ISP Input\", isp_->getEntityByName(\"bcm2835-isp0-output0\"), true);\n>> +\tdata->isp_[Isp::Output0] = RPi::RPiStream(\"ISP Output0\", isp_->getEntityByName(\"bcm2835-isp0-capture1\"));\n>> +\tdata->isp_[Isp::Output1] = RPi::RPiStream(\"ISP Output1\", isp_->getEntityByName(\"bcm2835-isp0-capture2\"));\n>> +\tdata->isp_[Isp::Stats] = RPi::RPiStream(\"ISP Stats\", isp_->getEntityByName(\"bcm2835-isp0-capture3\"));\n>> \n>> \t/* This is just for convenience so that we can easily iterate over all streams. */\n>> \tfor (auto &stream : data->unicam_)\n>> @@ -1005,7 +850,7 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n>> \t */\n>> \tunsigned int maxBuffers = 0;\n>> \tfor (const Stream *s : camera->streams())\n>> -\t\tif (static_cast(s)->isExternal())\n>> +\t\tif (static_cast(s)->isExternal())\n>> \t\t\tmaxBuffers = std::max(maxBuffers, s->configuration().bufferCount);\n>> \n>> \tfor (auto const stream : data->streams_) {\n>> @@ -1255,12 +1100,12 @@ done:\n>> \n>> void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n>> {\n>> -\tconst RPiStream *stream = nullptr;\n>> +\tconst RPi::RPiStream *stream = nullptr;\n>> \n>> \tif (state_ == State::Stopped)\n>> \t\treturn;\n>> \n>> -\tfor (RPiStream const &s : unicam_) {\n>> +\tfor (RPi::RPiStream const &s : unicam_) {\n>> \t\tif (s.findFrameBuffer(buffer)) {\n>> \t\t\tstream = &s;\n>> \t\t\tbreak;\n>> @@ -1316,12 +1161,12 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer)\n>> \n>> void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)\n>> {\n>> -\tconst RPiStream *stream = nullptr;\n>> +\tconst RPi::RPiStream *stream = nullptr;\n>> \n>> \tif (state_ == State::Stopped)\n>> \t\treturn;\n>> \n>> -\tfor (RPiStream const &s : isp_) {\n>> +\tfor (RPi::RPiStream const &s : isp_) {\n>> \t\tif (s.findFrameBuffer(buffer)) {\n>> \t\t\tstream = &s;\n>> \t\t\tbreak;\n>> @@ -1402,7 +1247,7 @@ void RPiCameraData::clearIncompleteRequests()\n>> \t}\n>> }\n>> \n>> -void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, const RPiStream *stream)\n>> +void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, const RPi::RPiStream *stream)\n>> {\n>> \tif (stream->isExternal()) {\n>> \t\tif (!dropFrame_) {\n>> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n>> new file mode 100644\n>> index 00000000..2edb8b59\n>> --- /dev/null\n>> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n>> @@ -0,0 +1,116 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2020, Raspberry Pi (Trading) Ltd.\n>> + *\n>> + * rpi_stream.cpp - Raspberry Pi device stream abstraction class.\n>> + */\n>> +#include \"rpi_stream.h\"\n>> +\n>> +#include \"libcamera/internal/log.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +LOG_DEFINE_CATEGORY(RPISTREAM)\n>> +\n>> +namespace RPi {\n>> +\n>> +V4L2VideoDevice *RPiStream::dev() const\n>> +{\n>> +\treturn dev_.get();\n>> +}\n>> +\n>> +void RPiStream::setExternal(bool external)\n>> +{\n>> +\texternal_ = external;\n>> +}\n>> +\n>> +bool RPiStream::isExternal() const\n>> +{\n>> +\t/*\n>> +\t * Import streams cannot be external.\n>> +\t *\n>> +\t * RAW capture is a special case where we simply copy the RAW\n>> +\t * buffer out of the request. All other buffer handling happens\n>> +\t * as if the stream is internal.\n>> +\t */\n>> +\treturn external_ && !importOnly_;\n>> +}\n>> +\n>> +bool RPiStream::isImporter() const\n>> +{\n>> +\treturn importOnly_;\n>> +}\n>> +\n>> +void RPiStream::reset()\n>> +{\n>> +\texternal_ = false;\n>> +\tinternalBuffers_.clear();\n>> +}\n>> +\n>> +std::string RPiStream::name() const\n>> +{\n>> +\treturn name_;\n>> +}\n>> +\n>> +void RPiStream::setExternalBuffers(std::vector> *buffers)\n>> +{\n>> +\texternalBuffers_ = buffers;\n>> +}\n>> +\n>> +const std::vector> *RPiStream::getBuffers() const\n>> +{\n>> +\treturn external_ ? externalBuffers_ : &internalBuffers_;\n>> +}\n>> +\n>> +void RPiStream::releaseBuffers()\n>> +{\n>> +\tdev_->releaseBuffers();\n>> +\tif (!external_ && !importOnly_)\n>> +\t\tinternalBuffers_.clear();\n>> +}\n>> +\n>> +int RPiStream::importBuffers(unsigned int count)\n>> +{\n>> +\treturn dev_->importBuffers(count);\n>> +}\n>> +\n>> +int RPiStream::allocateBuffers(unsigned int count)\n>> +{\n>> +\treturn dev_->allocateBuffers(count, &internalBuffers_);\n>> +}\n>> +\n>> +int RPiStream::queueBuffers()\n>> +{\n>> +\tif (external_)\n>> +\t\treturn 0;\n>> +\n>> +\tfor (auto &b : internalBuffers_) {\n>> +\t\tint ret = dev_->queueBuffer(b.get());\n>> +\t\tif (ret) {\n>> +\t\t\tLOG(RPISTREAM, Error) << \"Failed to queue buffers for \"\n>> +\t\t\t\t\t << name_;\n>> +\t\t\treturn ret;\n>> +\t\t}\n>> +\t}\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +bool RPiStream::findFrameBuffer(FrameBuffer *buffer) const\n>> +{\n>> +\tauto start = external_ ? externalBuffers_->begin() : internalBuffers_.begin();\n>> +\tauto end = external_ ? externalBuffers_->end() : internalBuffers_.end();\n>> +\n>> +\tif (importOnly_)\n>> +\t\treturn false;\n>> +\n>> +\tif (std::find_if(start, end,\n>> +\t\t\t [buffer](std::unique_ptr const &ref) { return ref.get() == buffer; }) != end)\n>> +\t\treturn true;\n>> +\n>> +\treturn false;\n>> +}\n>> +\n>> +} /* namespace RPi */\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n>> new file mode 100644\n>> index 00000000..3957e342\n>> --- /dev/null\n>> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n>> @@ -0,0 +1,98 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2020, Raspberry Pi (Trading) Ltd.\n>> + *\n>> + * rpi_stream.h - Raspberry Pi device stream abstraction class.\n>> + */\n>> +#ifndef __LIBCAMERA_PIPELINE_RPI_STREAM_H__\n>> +#define __LIBCAMERA_PIPELINE_RPI_STREAM_H__\n>> +\n>> +#include \n>> +#include \n>> +#include \n>> +\n>> +#include \n>> +\n>> +#include \"libcamera/internal/v4l2_videodevice.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +namespace RPi {\n>> +\n>> +/*\n>> + * Device stream abstraction for either an internal or external stream.\n>> + * Used for both Unicam and the ISP.\n>> + */\n>> +class RPiStream : public Stream\n>> +{\n>> +public:\n>> +\tRPiStream()\n>> +\t{\n>> +\t}\n>> +\n>> +\tRPiStream(const char *name, MediaEntity *dev, bool importOnly = false)\n>> +\t\t: external_(false), importOnly_(importOnly), name_(name),\n>> +\t\t dev_(std::make_unique(dev))\n>> +\t{\n>> +\t}\n>> +\n\n\nAs mentioned by the time I got to 6/9 (which I looked at last for some\nreason), this class became hard to parse as there are so many members so\nclose together\n\n\nAlthough maybe it was because I was looking at a patch diff, but this\nseems smaller, I guess the diff adds an extra line for the +/- entries ;-)\n\nBut perhaps breaking this up a bit below with:\n\n\n\n>> +\tV4L2VideoDevice *dev() const;\n\n>> +\tvoid setExternal(bool external);\n>> +\tbool isExternal() const;\n>> +\tbool isImporter() const;\n\n>> +\tvoid reset();\n>> +\tstd::string name() const;\n\n>> +\tvoid setExternalBuffers(std::vector> *buffers);\n>> +\tconst std::vector> *getBuffers() const;\n>> +\tvoid releaseBuffers();\n\n>> +\tint importBuffers(unsigned int count);\n>> +\tint allocateBuffers(unsigned int count);\n>> +\tint queueBuffers();\n>> +\tbool findFrameBuffer(FrameBuffer *buffer) const;\n\nI've punched newlines above (I hope that comes through in the mail) ...\nbut really all I was wondering is if there is a way to group related\nfunctions into their own blocks.\n\n\n>> +\n>> +private:\n>> +\t/*\n>> +\t * Indicates that this stream is active externally, i.e. the buffers\n>> +\t * are provided by the application.\n>> +\t */\n>> +\tbool external_;\n\n>> +\t/* Indicates that this stream only imports buffers, e.g. ISP input. */\n>> +\tbool importOnly_;\n\n>> +\t/* Stream name identifier. */\n>> +\tstd::string name_;\n\n>> +\t/* The actual device stream. */\n>> +\tstd::unique_ptr dev_;\n\n>> +\t/* Internally allocated framebuffers associated with this device stream. */\n>> +\tstd::vector> internalBuffers_;\n\n>> +\t/* Externally allocated framebuffers associated with this device stream. */\n>> +\tstd::vector> *externalBuffers_;\n>> +};\n\nWhen there's a comment line above an entry, for me it really helps to\nhave a newline before it.\n\n\nAnyway, nothing critical here, just where I think some whitespace would\nhave helped my weary eyes yesterday ;-)\n\n\n\n>> +\n>> +/*\n>> + * The following class is just a convenient (and typesafe) array of device\n>> + * streams indexed with an enum class.\n>> + */\n>> +template\n>> +class RPiDevice : public std::array\n>> +{\n>> +private:\n>> +\tconstexpr auto index(E e) const noexcept\n>> +\t{\n>> +\t\treturn static_cast>(e);\n>> +\t}\n>> +public:\n>> +\tRPiStream &operator[](E e)\n>> +\t{\n>> +\t\treturn std::array::operator[](index(e));\n>> +\t}\n>> +\tconst RPiStream &operator[](E e) const\n>> +\t{\n>> +\t\treturn std::array::operator[](index(e));\n>> +\t}\n>> +};\n>> +\n>> +} /* namespace RPi */\n>> +\n>> +} /* namespace libcamera */\n>> +\n>> +#endif /* __LIBCAMERA_PIPELINE_RPI_STREAM_H__ */\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 44BD9BDB1B\n\tfor ;\n\tWed, 22 Jul 2020 12:29:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CA6FD6093F;\n\tWed, 22 Jul 2020 14:29:30 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5A0636053C\n\tfor ;\n\tWed, 22 Jul 2020 14:29:29 +0200 (CEST)","from [192.168.0.20]\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 482AA329;\n\tWed, 22 Jul 2020 14:29:25 +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=\"fKBNeg9V\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1595420965;\n\tbh=TwSW3mUDTiLH7GkwvZw2FJ/hwZRMbR6nfAdPWYZf+s8=;\n\th=Reply-To:Subject:From:To:References:Date:In-Reply-To:From;\n\tb=fKBNeg9V6fRXXo+3dJiW9WREbJi6vY/NclzDnOax0XZLXIf0fFBXnWCKwEogCxmOL\n\tJU1q+1+/UO9I7p4BGQIHdWpI7b1KfbdB/nvV98Y9qSJZMP4RWUxmm0dcla4mTUbI/H\n\tIdsPwLQMLh2IO5AMLdhtb/XUOL3dy2lYjc07ee/w=","From":"Kieran Bingham ","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200720091311.805092-1-naush@raspberrypi.com>\n\t<20200720091311.805092-2-naush@raspberrypi.com>\n\t","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":"<389c5310-24f4-e3a2-ee28-21c6e004424f@ideasonboard.com>","Date":"Wed, 22 Jul 2020 13:29:22 +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":"","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v4 1/9] libcamera: pipeline:\n\traspberrypi: Move RPiStream into a separate file","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","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11488,"web_url":"https://patchwork.libcamera.org/comment/11488/","msgid":"","date":"2020-07-22T12:45:31","subject":"Re: [libcamera-devel] [PATCH v4 1/9] libcamera: pipeline:\n\traspberrypi: Move RPiStream into a separate file","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Kieran,\n\nThank you for the review. I'll make all the suggested changes and\nsubmit a new version soon.\n\nRegards,\nNaush\n\nOn Wed, 22 Jul 2020 at 13:29, Kieran Bingham\n wrote:\n>\n> Hi Naush,\n>\n> On 21/07/2020 11:45, Kieran Bingham wrote:\n> > Hi Naush,\n> >\n> > On 20/07/2020 10:13, Naushir Patuck wrote:\n> >> Put RPiStream into the RPi namespace and add a new log category (RPISTREAM).\n> >> There are no functional changes in this commit.\n> >>\n> >> Signed-off-by: Naushir Patuck \n> >> Reviewed-by: Niklas Söderlund \n> >\n> > Great, I love seeing this become more modular, I think that will help\n> > navigating code and maintenance.\n> >\n> > Reviewed-by: Kieran Bingham \n> >\n> >> ---\n> >> .../pipeline/raspberrypi/meson.build | 1 +\n> >> .../pipeline/raspberrypi/raspberrypi.cpp | 193 ++----------------\n> >> .../pipeline/raspberrypi/rpi_stream.cpp | 116 +++++++++++\n> >> .../pipeline/raspberrypi/rpi_stream.h | 98 +++++++++\n> >> 4 files changed, 234 insertions(+), 174 deletions(-)\n> >> create mode 100644 src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> >> create mode 100644 src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> >>\n> >> diff --git a/src/libcamera/pipeline/raspberrypi/meson.build b/src/libcamera/pipeline/raspberrypi/meson.build\n> >> index ae0aed3b..7c5b6ff7 100644\n> >> --- a/src/libcamera/pipeline/raspberrypi/meson.build\n> >> +++ b/src/libcamera/pipeline/raspberrypi/meson.build\n> >> @@ -3,5 +3,6 @@\n> >> libcamera_sources += files([\n> >> 'dma_heaps.cpp',\n> >> 'raspberrypi.cpp',\n> >> + 'rpi_stream.cpp',\n> >> 'staggered_ctrl.cpp',\n> >> ])\n> >> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> index bf1c7714..6630ef57 100644\n> >> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> @@ -19,7 +19,6 @@\n> >> #include \n> >> #include \n> >> #include \n> >> -#include \n> >>\n> >> #include \n> >>\n> >> @@ -33,6 +32,7 @@\n> >> #include \"libcamera/internal/v4l2_videodevice.h\"\n> >>\n> >> #include \"dma_heaps.h\"\n> >> +#include \"rpi_stream.h\"\n> >> #include \"staggered_ctrl.h\"\n> >>\n> >> namespace libcamera {\n> >> @@ -123,165 +123,10 @@ V4L2DeviceFormat findBestMode(V4L2PixFmtMap &formatsMap, const Size &req)\n> >> return bestMode;\n> >> }\n> >>\n> >> -} /* namespace */\n> >> -\n> >> -/*\n> >> - * Device stream abstraction for either an internal or external stream.\n> >> - * Used for both Unicam and the ISP.\n> >> - */\n> >> -class RPiStream : public Stream\n> >> -{\n> >> -public:\n> >> - RPiStream()\n> >> - {\n> >> - }\n> >> -\n> >> - RPiStream(const char *name, MediaEntity *dev, bool importOnly = false)\n> >> - : external_(false), importOnly_(importOnly), name_(name),\n> >> - dev_(std::make_unique(dev))\n> >> - {\n> >> - }\n> >> -\n> >> - V4L2VideoDevice *dev() const\n> >> - {\n> >> - return dev_.get();\n> >> - }\n> >> -\n> >> - void setExternal(bool external)\n> >> - {\n> >> - external_ = external;\n> >> - }\n> >> -\n> >> - bool isExternal() const\n> >> - {\n> >> - /*\n> >> - * Import streams cannot be external.\n> >> - *\n> >> - * RAW capture is a special case where we simply copy the RAW\n> >> - * buffer out of the request. All other buffer handling happens\n> >> - * as if the stream is internal.\n> >> - */\n> >> - return external_ && !importOnly_;\n> >> - }\n> >> -\n> >> - bool isImporter() const\n> >> - {\n> >> - return importOnly_;\n> >> - }\n> >> -\n> >> - void reset()\n> >> - {\n> >> - external_ = false;\n> >> - internalBuffers_.clear();\n> >> - }\n> >> -\n> >> - std::string name() const\n> >> - {\n> >> - return name_;\n> >> - }\n> >> -\n> >> - void setExternalBuffers(std::vector> *buffers)\n> >> - {\n> >> - externalBuffers_ = buffers;\n> >> - }\n> >> -\n> >> - const std::vector> *getBuffers() const\n> >> - {\n> >> - return external_ ? externalBuffers_ : &internalBuffers_;\n> >> - }\n> >> -\n> >> - void releaseBuffers()\n> >> - {\n> >> - dev_->releaseBuffers();\n> >> - if (!external_ && !importOnly_)\n> >> - internalBuffers_.clear();\n> >> - }\n> >> -\n> >> - int importBuffers(unsigned int count)\n> >> - {\n> >> - return dev_->importBuffers(count);\n> >> - }\n> >> -\n> >> - int allocateBuffers(unsigned int count)\n> >> - {\n> >> - return dev_->allocateBuffers(count, &internalBuffers_);\n> >> - }\n> >> -\n> >> - int queueBuffers()\n> >> - {\n> >> - if (external_)\n> >> - return 0;\n> >> -\n> >> - for (auto &b : internalBuffers_) {\n> >> - int ret = dev_->queueBuffer(b.get());\n> >> - if (ret) {\n> >> - LOG(RPI, Error) << \"Failed to queue buffers for \"\n> >> - << name_;\n> >> - return ret;\n> >> - }\n> >> - }\n> >> -\n> >> - return 0;\n> >> - }\n> >> -\n> >> - bool findFrameBuffer(FrameBuffer *buffer) const\n> >> - {\n> >> - auto start = external_ ? externalBuffers_->begin() : internalBuffers_.begin();\n> >> - auto end = external_ ? externalBuffers_->end() : internalBuffers_.end();\n> >> -\n> >> - if (importOnly_)\n> >> - return false;\n> >> -\n> >> - if (std::find_if(start, end,\n> >> - [buffer](std::unique_ptr const &ref) { return ref.get() == buffer; }) != end)\n> >> - return true;\n> >> -\n> >> - return false;\n> >> - }\n> >> -\n> >> -private:\n> >> - /*\n> >> - * Indicates that this stream is active externally, i.e. the buffers\n> >> - * are provided by the application.\n> >> - */\n> >> - bool external_;\n> >> - /* Indicates that this stream only imports buffers, e.g. ISP input. */\n> >> - bool importOnly_;\n> >> - /* Stream name identifier. */\n> >> - std::string name_;\n> >> - /* The actual device stream. */\n> >> - std::unique_ptr dev_;\n> >> - /* Internally allocated framebuffers associated with this device stream. */\n> >> - std::vector> internalBuffers_;\n> >> - /* Externally allocated framebuffers associated with this device stream. */\n> >> - std::vector> *externalBuffers_;\n> >> -};\n> >> -\n> >> -/*\n> >> - * The following class is just a convenient (and typesafe) array of device\n> >> - * streams indexed with an enum class.\n> >> - */\n> >> enum class Unicam : unsigned int { Image, Embedded };\n> >> enum class Isp : unsigned int { Input, Output0, Output1, Stats };\n> >>\n> >> -template\n> >> -class RPiDevice : public std::array\n> >> -{\n> >> -private:\n> >> - constexpr auto index(E e) const noexcept\n> >> - {\n> >> - return static_cast>(e);\n> >> - }\n> >> -public:\n> >> - RPiStream &operator[](E e)\n> >> - {\n> >> - return std::array::operator[](index(e));\n> >> - }\n> >> - const RPiStream &operator[](E e) const\n> >> - {\n> >> - return std::array::operator[](index(e));\n> >> - }\n> >> -};\n> >> +} /* namespace */\n> >>\n> >> class RPiCameraData : public CameraData\n> >> {\n> >> @@ -305,15 +150,15 @@ public:\n> >> void ispOutputDequeue(FrameBuffer *buffer);\n> >>\n> >> void clearIncompleteRequests();\n> >> - void handleStreamBuffer(FrameBuffer *buffer, const RPiStream *stream);\n> >> + void handleStreamBuffer(FrameBuffer *buffer, const RPi::RPiStream *stream);\n> >> void handleState();\n> >>\n> >> CameraSensor *sensor_;\n> >> /* Array of Unicam and ISP device streams and associated buffers/streams. */\n> >> - RPiDevice unicam_;\n> >> - RPiDevice isp_;\n> >> + RPi::RPiDevice unicam_;\n> >> + RPi::RPiDevice isp_;\n> >> /* The vector below is just for convenience when iterating over all streams. */\n> >> - std::vector streams_;\n> >> + std::vector streams_;\n> >> /* Buffers passed to the IPA. */\n> >> std::vector ipaBuffers_;\n> >>\n> >> @@ -762,7 +607,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> >> int PipelineHandlerRPi::exportFrameBuffers(Camera *camera, Stream *stream,\n> >> std::vector> *buffers)\n> >> {\n> >> - RPiStream *s = static_cast(stream);\n> >> + RPi::RPiStream *s = static_cast(stream);\n> >> unsigned int count = stream->configuration().bufferCount;\n> >> int ret = s->dev()->exportBuffers(count, buffers);\n> >>\n> >> @@ -908,14 +753,14 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n> >> std::unique_ptr data = std::make_unique(this);\n> >>\n> >> /* Locate and open the unicam video streams. */\n> >> - data->unicam_[Unicam::Embedded] = RPiStream(\"Unicam Embedded\", unicam_->getEntityByName(\"unicam-embedded\"));\n> >> - data->unicam_[Unicam::Image] = RPiStream(\"Unicam Image\", unicam_->getEntityByName(\"unicam-image\"));\n> >> + data->unicam_[Unicam::Embedded] = RPi::RPiStream(\"Unicam Embedded\", unicam_->getEntityByName(\"unicam-embedded\"));\n> >> + data->unicam_[Unicam::Image] = RPi::RPiStream(\"Unicam Image\", unicam_->getEntityByName(\"unicam-image\"));\n> >>\n> >> /* Tag the ISP input stream as an import stream. */\n> >> - data->isp_[Isp::Input] = RPiStream(\"ISP Input\", isp_->getEntityByName(\"bcm2835-isp0-output0\"), true);\n> >> - data->isp_[Isp::Output0] = RPiStream(\"ISP Output0\", isp_->getEntityByName(\"bcm2835-isp0-capture1\"));\n> >> - data->isp_[Isp::Output1] = RPiStream(\"ISP Output1\", isp_->getEntityByName(\"bcm2835-isp0-capture2\"));\n> >> - data->isp_[Isp::Stats] = RPiStream(\"ISP Stats\", isp_->getEntityByName(\"bcm2835-isp0-capture3\"));\n> >> + data->isp_[Isp::Input] = RPi::RPiStream(\"ISP Input\", isp_->getEntityByName(\"bcm2835-isp0-output0\"), true);\n> >> + data->isp_[Isp::Output0] = RPi::RPiStream(\"ISP Output0\", isp_->getEntityByName(\"bcm2835-isp0-capture1\"));\n> >> + data->isp_[Isp::Output1] = RPi::RPiStream(\"ISP Output1\", isp_->getEntityByName(\"bcm2835-isp0-capture2\"));\n> >> + data->isp_[Isp::Stats] = RPi::RPiStream(\"ISP Stats\", isp_->getEntityByName(\"bcm2835-isp0-capture3\"));\n> >>\n> >> /* This is just for convenience so that we can easily iterate over all streams. */\n> >> for (auto &stream : data->unicam_)\n> >> @@ -1005,7 +850,7 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> >> */\n> >> unsigned int maxBuffers = 0;\n> >> for (const Stream *s : camera->streams())\n> >> - if (static_cast(s)->isExternal())\n> >> + if (static_cast(s)->isExternal())\n> >> maxBuffers = std::max(maxBuffers, s->configuration().bufferCount);\n> >>\n> >> for (auto const stream : data->streams_) {\n> >> @@ -1255,12 +1100,12 @@ done:\n> >>\n> >> void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n> >> {\n> >> - const RPiStream *stream = nullptr;\n> >> + const RPi::RPiStream *stream = nullptr;\n> >>\n> >> if (state_ == State::Stopped)\n> >> return;\n> >>\n> >> - for (RPiStream const &s : unicam_) {\n> >> + for (RPi::RPiStream const &s : unicam_) {\n> >> if (s.findFrameBuffer(buffer)) {\n> >> stream = &s;\n> >> break;\n> >> @@ -1316,12 +1161,12 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer)\n> >>\n> >> void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)\n> >> {\n> >> - const RPiStream *stream = nullptr;\n> >> + const RPi::RPiStream *stream = nullptr;\n> >>\n> >> if (state_ == State::Stopped)\n> >> return;\n> >>\n> >> - for (RPiStream const &s : isp_) {\n> >> + for (RPi::RPiStream const &s : isp_) {\n> >> if (s.findFrameBuffer(buffer)) {\n> >> stream = &s;\n> >> break;\n> >> @@ -1402,7 +1247,7 @@ void RPiCameraData::clearIncompleteRequests()\n> >> }\n> >> }\n> >>\n> >> -void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, const RPiStream *stream)\n> >> +void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, const RPi::RPiStream *stream)\n> >> {\n> >> if (stream->isExternal()) {\n> >> if (!dropFrame_) {\n> >> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> >> new file mode 100644\n> >> index 00000000..2edb8b59\n> >> --- /dev/null\n> >> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> >> @@ -0,0 +1,116 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2020, Raspberry Pi (Trading) Ltd.\n> >> + *\n> >> + * rpi_stream.cpp - Raspberry Pi device stream abstraction class.\n> >> + */\n> >> +#include \"rpi_stream.h\"\n> >> +\n> >> +#include \"libcamera/internal/log.h\"\n> >> +\n> >> +namespace libcamera {\n> >> +\n> >> +LOG_DEFINE_CATEGORY(RPISTREAM)\n> >> +\n> >> +namespace RPi {\n> >> +\n> >> +V4L2VideoDevice *RPiStream::dev() const\n> >> +{\n> >> + return dev_.get();\n> >> +}\n> >> +\n> >> +void RPiStream::setExternal(bool external)\n> >> +{\n> >> + external_ = external;\n> >> +}\n> >> +\n> >> +bool RPiStream::isExternal() const\n> >> +{\n> >> + /*\n> >> + * Import streams cannot be external.\n> >> + *\n> >> + * RAW capture is a special case where we simply copy the RAW\n> >> + * buffer out of the request. All other buffer handling happens\n> >> + * as if the stream is internal.\n> >> + */\n> >> + return external_ && !importOnly_;\n> >> +}\n> >> +\n> >> +bool RPiStream::isImporter() const\n> >> +{\n> >> + return importOnly_;\n> >> +}\n> >> +\n> >> +void RPiStream::reset()\n> >> +{\n> >> + external_ = false;\n> >> + internalBuffers_.clear();\n> >> +}\n> >> +\n> >> +std::string RPiStream::name() const\n> >> +{\n> >> + return name_;\n> >> +}\n> >> +\n> >> +void RPiStream::setExternalBuffers(std::vector> *buffers)\n> >> +{\n> >> + externalBuffers_ = buffers;\n> >> +}\n> >> +\n> >> +const std::vector> *RPiStream::getBuffers() const\n> >> +{\n> >> + return external_ ? externalBuffers_ : &internalBuffers_;\n> >> +}\n> >> +\n> >> +void RPiStream::releaseBuffers()\n> >> +{\n> >> + dev_->releaseBuffers();\n> >> + if (!external_ && !importOnly_)\n> >> + internalBuffers_.clear();\n> >> +}\n> >> +\n> >> +int RPiStream::importBuffers(unsigned int count)\n> >> +{\n> >> + return dev_->importBuffers(count);\n> >> +}\n> >> +\n> >> +int RPiStream::allocateBuffers(unsigned int count)\n> >> +{\n> >> + return dev_->allocateBuffers(count, &internalBuffers_);\n> >> +}\n> >> +\n> >> +int RPiStream::queueBuffers()\n> >> +{\n> >> + if (external_)\n> >> + return 0;\n> >> +\n> >> + for (auto &b : internalBuffers_) {\n> >> + int ret = dev_->queueBuffer(b.get());\n> >> + if (ret) {\n> >> + LOG(RPISTREAM, Error) << \"Failed to queue buffers for \"\n> >> + << name_;\n> >> + return ret;\n> >> + }\n> >> + }\n> >> +\n> >> + return 0;\n> >> +}\n> >> +\n> >> +bool RPiStream::findFrameBuffer(FrameBuffer *buffer) const\n> >> +{\n> >> + auto start = external_ ? externalBuffers_->begin() : internalBuffers_.begin();\n> >> + auto end = external_ ? externalBuffers_->end() : internalBuffers_.end();\n> >> +\n> >> + if (importOnly_)\n> >> + return false;\n> >> +\n> >> + if (std::find_if(start, end,\n> >> + [buffer](std::unique_ptr const &ref) { return ref.get() == buffer; }) != end)\n> >> + return true;\n> >> +\n> >> + return false;\n> >> +}\n> >> +\n> >> +} /* namespace RPi */\n> >> +\n> >> +} /* namespace libcamera */\n> >> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> >> new file mode 100644\n> >> index 00000000..3957e342\n> >> --- /dev/null\n> >> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> >> @@ -0,0 +1,98 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2020, Raspberry Pi (Trading) Ltd.\n> >> + *\n> >> + * rpi_stream.h - Raspberry Pi device stream abstraction class.\n> >> + */\n> >> +#ifndef __LIBCAMERA_PIPELINE_RPI_STREAM_H__\n> >> +#define __LIBCAMERA_PIPELINE_RPI_STREAM_H__\n> >> +\n> >> +#include \n> >> +#include \n> >> +#include \n> >> +\n> >> +#include \n> >> +\n> >> +#include \"libcamera/internal/v4l2_videodevice.h\"\n> >> +\n> >> +namespace libcamera {\n> >> +\n> >> +namespace RPi {\n> >> +\n> >> +/*\n> >> + * Device stream abstraction for either an internal or external stream.\n> >> + * Used for both Unicam and the ISP.\n> >> + */\n> >> +class RPiStream : public Stream\n> >> +{\n> >> +public:\n> >> + RPiStream()\n> >> + {\n> >> + }\n> >> +\n> >> + RPiStream(const char *name, MediaEntity *dev, bool importOnly = false)\n> >> + : external_(false), importOnly_(importOnly), name_(name),\n> >> + dev_(std::make_unique(dev))\n> >> + {\n> >> + }\n> >> +\n>\n>\n> As mentioned by the time I got to 6/9 (which I looked at last for some\n> reason), this class became hard to parse as there are so many members so\n> close together\n>\n>\n> Although maybe it was because I was looking at a patch diff, but this\n> seems smaller, I guess the diff adds an extra line for the +/- entries ;-)\n>\n> But perhaps breaking this up a bit below with:\n>\n>\n>\n> >> + V4L2VideoDevice *dev() const;\n>\n> >> + void setExternal(bool external);\n> >> + bool isExternal() const;\n> >> + bool isImporter() const;\n>\n> >> + void reset();\n> >> + std::string name() const;\n>\n> >> + void setExternalBuffers(std::vector> *buffers);\n> >> + const std::vector> *getBuffers() const;\n> >> + void releaseBuffers();\n>\n> >> + int importBuffers(unsigned int count);\n> >> + int allocateBuffers(unsigned int count);\n> >> + int queueBuffers();\n> >> + bool findFrameBuffer(FrameBuffer *buffer) const;\n>\n> I've punched newlines above (I hope that comes through in the mail) ...\n> but really all I was wondering is if there is a way to group related\n> functions into their own blocks.\n>\n>\n> >> +\n> >> +private:\n> >> + /*\n> >> + * Indicates that this stream is active externally, i.e. the buffers\n> >> + * are provided by the application.\n> >> + */\n> >> + bool external_;\n>\n> >> + /* Indicates that this stream only imports buffers, e.g. ISP input. */\n> >> + bool importOnly_;\n>\n> >> + /* Stream name identifier. */\n> >> + std::string name_;\n>\n> >> + /* The actual device stream. */\n> >> + std::unique_ptr dev_;\n>\n> >> + /* Internally allocated framebuffers associated with this device stream. */\n> >> + std::vector> internalBuffers_;\n>\n> >> + /* Externally allocated framebuffers associated with this device stream. */\n> >> + std::vector> *externalBuffers_;\n> >> +};\n>\n> When there's a comment line above an entry, for me it really helps to\n> have a newline before it.\n>\n>\n> Anyway, nothing critical here, just where I think some whitespace would\n> have helped my weary eyes yesterday ;-)\n>\n>\n>\n> >> +\n> >> +/*\n> >> + * The following class is just a convenient (and typesafe) array of device\n> >> + * streams indexed with an enum class.\n> >> + */\n> >> +template\n> >> +class RPiDevice : public std::array\n> >> +{\n> >> +private:\n> >> + constexpr auto index(E e) const noexcept\n> >> + {\n> >> + return static_cast>(e);\n> >> + }\n> >> +public:\n> >> + RPiStream &operator[](E e)\n> >> + {\n> >> + return std::array::operator[](index(e));\n> >> + }\n> >> + const RPiStream &operator[](E e) const\n> >> + {\n> >> + return std::array::operator[](index(e));\n> >> + }\n> >> +};\n> >> +\n> >> +} /* namespace RPi */\n> >> +\n> >> +} /* namespace libcamera */\n> >> +\n> >> +#endif /* __LIBCAMERA_PIPELINE_RPI_STREAM_H__ */\n> >>\n> >\n>\n> --\n> Regards\n> --\n> Kieran","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 E2575C2E68\n\tfor ;\n\tWed, 22 Jul 2020 12:45:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 543AA60983;\n\tWed, 22 Jul 2020 14:45:50 +0200 (CEST)","from mail-lf1-x134.google.com (mail-lf1-x134.google.com\n\t[IPv6:2a00:1450:4864:20::134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7A2D56053C\n\tfor ;\n\tWed, 22 Jul 2020 14:45:48 +0200 (CEST)","by mail-lf1-x134.google.com with SMTP id i80so1216583lfi.13\n\tfor ;\n\tWed, 22 Jul 2020 05:45:48 -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=\"PsMDuuJh\"; 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:content-transfer-encoding;\n\tbh=LkMuQtacm2BXG74FlBF9kPqxGIu6/pJislJUAABQzvw=;\n\tb=PsMDuuJhwWmx5lrwGJiPvR/IHYJw0PlsCqyBMovbc0nFe7tgL/BhuqnZvx2+YugCqt\n\th1QwjmKe+7w7xn87eFZGSfdyzURRdKL414+ikK11bI1Wy690BmLRcPE7/qkAUG98hNhm\n\t8awSUE4cFcSzRL4y5WExDlx0IwB+PamfEfqZ1PJ9edAO2XPBmTlk/GL72V8sC6hiQETp\n\tbZ/YM29ndak5ggbJ8LWlNKyoX78l+/GymPUh1wXgcxjyIXuwDQX7IIEatudrrbjHccDR\n\tW7MhLDbdDR98WeQGnioHBDC51VJSzXyWErmox2kVhFZAi4IkVk1WbJAUX7EZAuImD8Uu\n\tYd4Q==","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:content-transfer-encoding;\n\tbh=LkMuQtacm2BXG74FlBF9kPqxGIu6/pJislJUAABQzvw=;\n\tb=snLoyvI8vzwniqTr7gwTRAAy1IvZL2conKyOxSao7WwwqNezupZGKZZ8MXR/dw/tZg\n\tmSIVE+AYUkaodfi5f32YqHU3x3z7rmLTDTVZH/3QhwXJO2Rh0gj0+b/02w2LoM+z8zYt\n\t6vI7VwzkS5KxsF0YlK46U/9lA6hA3a8HmZQKB6MiGFD0ik/0h42ch/NMJtiuf0wmX/6O\n\tO9FLo2akHsdY2K5x+x5PAP4FP8RolVR/ficyZ+RDrkkzslEoOrvB48DK55COxXgd8gXd\n\tQSWMtn5rHUIHXtrrVRdXeF0Gs8hjzUrdwxlglkH5lYskPjMKFgcTwOT3uIsGUElpFAAV\n\txBYQ==","X-Gm-Message-State":"AOAM532OpQmIRuKeJeh4WDPHBpJe7wOph8uzm7GQJwCan6MvUuBtmk/e\n\t1RqdVl6zLAAKVs1JGzkkeVVdlHgzfeMvj71SVzbbqw==","X-Google-Smtp-Source":"ABdhPJz/G9Sw9QmhJaIUk+IBcbwcD/c3+jBrIrD7MaZJDPxAaiJGp4bp3pLm5qXSHs+Rp0uKPh4/zB6GN1Iw5rsNcuw=","X-Received":"by 2002:a19:f20a:: with SMTP id\n\tq10mr16450296lfh.89.1595421947280; \n\tWed, 22 Jul 2020 05:45:47 -0700 (PDT)","MIME-Version":"1.0","References":"<20200720091311.805092-1-naush@raspberrypi.com>\n\t<20200720091311.805092-2-naush@raspberrypi.com>\n\t\n\t<389c5310-24f4-e3a2-ee28-21c6e004424f@ideasonboard.com>","In-Reply-To":"<389c5310-24f4-e3a2-ee28-21c6e004424f@ideasonboard.com>","From":"Naushir Patuck ","Date":"Wed, 22 Jul 2020 13:45:31 +0100","Message-ID":"","To":"Kieran Bingham ","Subject":"Re: [libcamera-devel] [PATCH v4 1/9] libcamera: pipeline:\n\traspberrypi: Move RPiStream into a separate file","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11493,"web_url":"https://patchwork.libcamera.org/comment/11493/","msgid":"<20200722141713.GA29813@pendragon.ideasonboard.com>","date":"2020-07-22T14:17:13","subject":"Re: [libcamera-devel] [PATCH v4 1/9] libcamera: pipeline:\n\traspberrypi: Move RPiStream into a separate file","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Wed, Jul 22, 2020 at 01:45:31PM +0100, Naushir Patuck wrote:\n> Hi Kieran,\n> \n> Thank you for the review. I'll make all the suggested changes and\n> submit a new version soon.\n> \n> Regards,\n> Naush\n> \n> On Wed, 22 Jul 2020 at 13:29, Kieran Bingham wrote:\n> > On 21/07/2020 11:45, Kieran Bingham wrote:\n> >> On 20/07/2020 10:13, Naushir Patuck wrote:\n> >>> Put RPiStream into the RPi namespace and add a new log category (RPISTREAM).\n> >>> There are no functional changes in this commit.\n> >>>\n> >>> Signed-off-by: Naushir Patuck \n> >>> Reviewed-by: Niklas Söderlund \n> >>\n> >> Great, I love seeing this become more modular, I think that will help\n> >> navigating code and maintenance.\n> >>\n> >> Reviewed-by: Kieran Bingham \n> >>\n> >>> ---\n> >>> .../pipeline/raspberrypi/meson.build | 1 +\n> >>> .../pipeline/raspberrypi/raspberrypi.cpp | 193 ++----------------\n> >>> .../pipeline/raspberrypi/rpi_stream.cpp | 116 +++++++++++\n> >>> .../pipeline/raspberrypi/rpi_stream.h | 98 +++++++++\n> >>> 4 files changed, 234 insertions(+), 174 deletions(-)\n> >>> create mode 100644 src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> >>> create mode 100644 src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> >>>\n> >>> diff --git a/src/libcamera/pipeline/raspberrypi/meson.build b/src/libcamera/pipeline/raspberrypi/meson.build\n> >>> index ae0aed3b..7c5b6ff7 100644\n> >>> --- a/src/libcamera/pipeline/raspberrypi/meson.build\n> >>> +++ b/src/libcamera/pipeline/raspberrypi/meson.build\n> >>> @@ -3,5 +3,6 @@\n> >>> libcamera_sources += files([\n> >>> 'dma_heaps.cpp',\n> >>> 'raspberrypi.cpp',\n> >>> + 'rpi_stream.cpp',\n> >>> 'staggered_ctrl.cpp',\n> >>> ])\n> >>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>> index bf1c7714..6630ef57 100644\n> >>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>> @@ -19,7 +19,6 @@\n> >>> #include \n> >>> #include \n> >>> #include \n> >>> -#include \n> >>>\n> >>> #include \n> >>>\n> >>> @@ -33,6 +32,7 @@\n> >>> #include \"libcamera/internal/v4l2_videodevice.h\"\n> >>>\n> >>> #include \"dma_heaps.h\"\n> >>> +#include \"rpi_stream.h\"\n> >>> #include \"staggered_ctrl.h\"\n> >>>\n> >>> namespace libcamera {\n> >>> @@ -123,165 +123,10 @@ V4L2DeviceFormat findBestMode(V4L2PixFmtMap &formatsMap, const Size &req)\n> >>> return bestMode;\n> >>> }\n> >>>\n> >>> -} /* namespace */\n> >>> -\n> >>> -/*\n> >>> - * Device stream abstraction for either an internal or external stream.\n> >>> - * Used for both Unicam and the ISP.\n> >>> - */\n> >>> -class RPiStream : public Stream\n> >>> -{\n> >>> -public:\n> >>> - RPiStream()\n> >>> - {\n> >>> - }\n> >>> -\n> >>> - RPiStream(const char *name, MediaEntity *dev, bool importOnly = false)\n> >>> - : external_(false), importOnly_(importOnly), name_(name),\n> >>> - dev_(std::make_unique(dev))\n> >>> - {\n> >>> - }\n> >>> -\n> >>> - V4L2VideoDevice *dev() const\n> >>> - {\n> >>> - return dev_.get();\n> >>> - }\n> >>> -\n> >>> - void setExternal(bool external)\n> >>> - {\n> >>> - external_ = external;\n> >>> - }\n> >>> -\n> >>> - bool isExternal() const\n> >>> - {\n> >>> - /*\n> >>> - * Import streams cannot be external.\n> >>> - *\n> >>> - * RAW capture is a special case where we simply copy the RAW\n> >>> - * buffer out of the request. All other buffer handling happens\n> >>> - * as if the stream is internal.\n> >>> - */\n> >>> - return external_ && !importOnly_;\n> >>> - }\n> >>> -\n> >>> - bool isImporter() const\n> >>> - {\n> >>> - return importOnly_;\n> >>> - }\n> >>> -\n> >>> - void reset()\n> >>> - {\n> >>> - external_ = false;\n> >>> - internalBuffers_.clear();\n> >>> - }\n> >>> -\n> >>> - std::string name() const\n> >>> - {\n> >>> - return name_;\n> >>> - }\n> >>> -\n> >>> - void setExternalBuffers(std::vector> *buffers)\n> >>> - {\n> >>> - externalBuffers_ = buffers;\n> >>> - }\n> >>> -\n> >>> - const std::vector> *getBuffers() const\n> >>> - {\n> >>> - return external_ ? externalBuffers_ : &internalBuffers_;\n> >>> - }\n> >>> -\n> >>> - void releaseBuffers()\n> >>> - {\n> >>> - dev_->releaseBuffers();\n> >>> - if (!external_ && !importOnly_)\n> >>> - internalBuffers_.clear();\n> >>> - }\n> >>> -\n> >>> - int importBuffers(unsigned int count)\n> >>> - {\n> >>> - return dev_->importBuffers(count);\n> >>> - }\n> >>> -\n> >>> - int allocateBuffers(unsigned int count)\n> >>> - {\n> >>> - return dev_->allocateBuffers(count, &internalBuffers_);\n> >>> - }\n> >>> -\n> >>> - int queueBuffers()\n> >>> - {\n> >>> - if (external_)\n> >>> - return 0;\n> >>> -\n> >>> - for (auto &b : internalBuffers_) {\n> >>> - int ret = dev_->queueBuffer(b.get());\n> >>> - if (ret) {\n> >>> - LOG(RPI, Error) << \"Failed to queue buffers for \"\n> >>> - << name_;\n> >>> - return ret;\n> >>> - }\n> >>> - }\n> >>> -\n> >>> - return 0;\n> >>> - }\n> >>> -\n> >>> - bool findFrameBuffer(FrameBuffer *buffer) const\n> >>> - {\n> >>> - auto start = external_ ? externalBuffers_->begin() : internalBuffers_.begin();\n> >>> - auto end = external_ ? externalBuffers_->end() : internalBuffers_.end();\n> >>> -\n> >>> - if (importOnly_)\n> >>> - return false;\n> >>> -\n> >>> - if (std::find_if(start, end,\n> >>> - [buffer](std::unique_ptr const &ref) { return ref.get() == buffer; }) != end)\n> >>> - return true;\n> >>> -\n> >>> - return false;\n> >>> - }\n> >>> -\n> >>> -private:\n> >>> - /*\n> >>> - * Indicates that this stream is active externally, i.e. the buffers\n> >>> - * are provided by the application.\n> >>> - */\n> >>> - bool external_;\n> >>> - /* Indicates that this stream only imports buffers, e.g. ISP input. */\n> >>> - bool importOnly_;\n> >>> - /* Stream name identifier. */\n> >>> - std::string name_;\n> >>> - /* The actual device stream. */\n> >>> - std::unique_ptr dev_;\n> >>> - /* Internally allocated framebuffers associated with this device stream. */\n> >>> - std::vector> internalBuffers_;\n> >>> - /* Externally allocated framebuffers associated with this device stream. */\n> >>> - std::vector> *externalBuffers_;\n> >>> -};\n> >>> -\n> >>> -/*\n> >>> - * The following class is just a convenient (and typesafe) array of device\n> >>> - * streams indexed with an enum class.\n> >>> - */\n> >>> enum class Unicam : unsigned int { Image, Embedded };\n> >>> enum class Isp : unsigned int { Input, Output0, Output1, Stats };\n> >>>\n> >>> -template\n> >>> -class RPiDevice : public std::array\n> >>> -{\n> >>> -private:\n> >>> - constexpr auto index(E e) const noexcept\n> >>> - {\n> >>> - return static_cast>(e);\n> >>> - }\n> >>> -public:\n> >>> - RPiStream &operator[](E e)\n> >>> - {\n> >>> - return std::array::operator[](index(e));\n> >>> - }\n> >>> - const RPiStream &operator[](E e) const\n> >>> - {\n> >>> - return std::array::operator[](index(e));\n> >>> - }\n> >>> -};\n> >>> +} /* namespace */\n> >>>\n> >>> class RPiCameraData : public CameraData\n> >>> {\n> >>> @@ -305,15 +150,15 @@ public:\n> >>> void ispOutputDequeue(FrameBuffer *buffer);\n> >>>\n> >>> void clearIncompleteRequests();\n> >>> - void handleStreamBuffer(FrameBuffer *buffer, const RPiStream *stream);\n> >>> + void handleStreamBuffer(FrameBuffer *buffer, const RPi::RPiStream *stream);\n> >>> void handleState();\n> >>>\n> >>> CameraSensor *sensor_;\n> >>> /* Array of Unicam and ISP device streams and associated buffers/streams. */\n> >>> - RPiDevice unicam_;\n> >>> - RPiDevice isp_;\n> >>> + RPi::RPiDevice unicam_;\n> >>> + RPi::RPiDevice isp_;\n> >>> /* The vector below is just for convenience when iterating over all streams. */\n> >>> - std::vector streams_;\n> >>> + std::vector streams_;\n> >>> /* Buffers passed to the IPA. */\n> >>> std::vector ipaBuffers_;\n> >>>\n> >>> @@ -762,7 +607,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> >>> int PipelineHandlerRPi::exportFrameBuffers(Camera *camera, Stream *stream,\n> >>> std::vector> *buffers)\n> >>> {\n> >>> - RPiStream *s = static_cast(stream);\n> >>> + RPi::RPiStream *s = static_cast(stream);\n> >>> unsigned int count = stream->configuration().bufferCount;\n> >>> int ret = s->dev()->exportBuffers(count, buffers);\n> >>>\n> >>> @@ -908,14 +753,14 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n> >>> std::unique_ptr data = std::make_unique(this);\n> >>>\n> >>> /* Locate and open the unicam video streams. */\n> >>> - data->unicam_[Unicam::Embedded] = RPiStream(\"Unicam Embedded\", unicam_->getEntityByName(\"unicam-embedded\"));\n> >>> - data->unicam_[Unicam::Image] = RPiStream(\"Unicam Image\", unicam_->getEntityByName(\"unicam-image\"));\n> >>> + data->unicam_[Unicam::Embedded] = RPi::RPiStream(\"Unicam Embedded\", unicam_->getEntityByName(\"unicam-embedded\"));\n> >>> + data->unicam_[Unicam::Image] = RPi::RPiStream(\"Unicam Image\", unicam_->getEntityByName(\"unicam-image\"));\n> >>>\n> >>> /* Tag the ISP input stream as an import stream. */\n> >>> - data->isp_[Isp::Input] = RPiStream(\"ISP Input\", isp_->getEntityByName(\"bcm2835-isp0-output0\"), true);\n> >>> - data->isp_[Isp::Output0] = RPiStream(\"ISP Output0\", isp_->getEntityByName(\"bcm2835-isp0-capture1\"));\n> >>> - data->isp_[Isp::Output1] = RPiStream(\"ISP Output1\", isp_->getEntityByName(\"bcm2835-isp0-capture2\"));\n> >>> - data->isp_[Isp::Stats] = RPiStream(\"ISP Stats\", isp_->getEntityByName(\"bcm2835-isp0-capture3\"));\n> >>> + data->isp_[Isp::Input] = RPi::RPiStream(\"ISP Input\", isp_->getEntityByName(\"bcm2835-isp0-output0\"), true);\n> >>> + data->isp_[Isp::Output0] = RPi::RPiStream(\"ISP Output0\", isp_->getEntityByName(\"bcm2835-isp0-capture1\"));\n> >>> + data->isp_[Isp::Output1] = RPi::RPiStream(\"ISP Output1\", isp_->getEntityByName(\"bcm2835-isp0-capture2\"));\n> >>> + data->isp_[Isp::Stats] = RPi::RPiStream(\"ISP Stats\", isp_->getEntityByName(\"bcm2835-isp0-capture3\"));\n> >>>\n> >>> /* This is just for convenience so that we can easily iterate over all streams. */\n> >>> for (auto &stream : data->unicam_)\n> >>> @@ -1005,7 +850,7 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> >>> */\n> >>> unsigned int maxBuffers = 0;\n> >>> for (const Stream *s : camera->streams())\n> >>> - if (static_cast(s)->isExternal())\n> >>> + if (static_cast(s)->isExternal())\n> >>> maxBuffers = std::max(maxBuffers, s->configuration().bufferCount);\n> >>>\n> >>> for (auto const stream : data->streams_) {\n> >>> @@ -1255,12 +1100,12 @@ done:\n> >>>\n> >>> void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n> >>> {\n> >>> - const RPiStream *stream = nullptr;\n> >>> + const RPi::RPiStream *stream = nullptr;\n> >>>\n> >>> if (state_ == State::Stopped)\n> >>> return;\n> >>>\n> >>> - for (RPiStream const &s : unicam_) {\n> >>> + for (RPi::RPiStream const &s : unicam_) {\n> >>> if (s.findFrameBuffer(buffer)) {\n> >>> stream = &s;\n> >>> break;\n> >>> @@ -1316,12 +1161,12 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer)\n> >>>\n> >>> void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)\n> >>> {\n> >>> - const RPiStream *stream = nullptr;\n> >>> + const RPi::RPiStream *stream = nullptr;\n> >>>\n> >>> if (state_ == State::Stopped)\n> >>> return;\n> >>>\n> >>> - for (RPiStream const &s : isp_) {\n> >>> + for (RPi::RPiStream const &s : isp_) {\n> >>> if (s.findFrameBuffer(buffer)) {\n> >>> stream = &s;\n> >>> break;\n> >>> @@ -1402,7 +1247,7 @@ void RPiCameraData::clearIncompleteRequests()\n> >>> }\n> >>> }\n> >>>\n> >>> -void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, const RPiStream *stream)\n> >>> +void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, const RPi::RPiStream *stream)\n> >>> {\n> >>> if (stream->isExternal()) {\n> >>> if (!dropFrame_) {\n> >>> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> >>> new file mode 100644\n> >>> index 00000000..2edb8b59\n> >>> --- /dev/null\n> >>> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> >>> @@ -0,0 +1,116 @@\n> >>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >>> +/*\n> >>> + * Copyright (C) 2020, Raspberry Pi (Trading) Ltd.\n> >>> + *\n> >>> + * rpi_stream.cpp - Raspberry Pi device stream abstraction class.\n> >>> + */\n> >>> +#include \"rpi_stream.h\"\n> >>> +\n> >>> +#include \"libcamera/internal/log.h\"\n> >>> +\n> >>> +namespace libcamera {\n> >>> +\n> >>> +LOG_DEFINE_CATEGORY(RPISTREAM)\n> >>> +\n> >>> +namespace RPi {\n> >>> +\n> >>> +V4L2VideoDevice *RPiStream::dev() const\n> >>> +{\n> >>> + return dev_.get();\n> >>> +}\n> >>> +\n> >>> +void RPiStream::setExternal(bool external)\n> >>> +{\n> >>> + external_ = external;\n> >>> +}\n> >>> +\n> >>> +bool RPiStream::isExternal() const\n> >>> +{\n> >>> + /*\n> >>> + * Import streams cannot be external.\n> >>> + *\n> >>> + * RAW capture is a special case where we simply copy the RAW\n> >>> + * buffer out of the request. All other buffer handling happens\n> >>> + * as if the stream is internal.\n> >>> + */\n> >>> + return external_ && !importOnly_;\n> >>> +}\n> >>> +\n> >>> +bool RPiStream::isImporter() const\n> >>> +{\n> >>> + return importOnly_;\n> >>> +}\n> >>> +\n> >>> +void RPiStream::reset()\n> >>> +{\n> >>> + external_ = false;\n> >>> + internalBuffers_.clear();\n> >>> +}\n> >>> +\n> >>> +std::string RPiStream::name() const\n> >>> +{\n> >>> + return name_;\n> >>> +}\n> >>> +\n> >>> +void RPiStream::setExternalBuffers(std::vector> *buffers)\n> >>> +{\n> >>> + externalBuffers_ = buffers;\n> >>> +}\n> >>> +\n> >>> +const std::vector> *RPiStream::getBuffers() const\n> >>> +{\n> >>> + return external_ ? externalBuffers_ : &internalBuffers_;\n> >>> +}\n> >>> +\n> >>> +void RPiStream::releaseBuffers()\n> >>> +{\n> >>> + dev_->releaseBuffers();\n> >>> + if (!external_ && !importOnly_)\n> >>> + internalBuffers_.clear();\n> >>> +}\n> >>> +\n> >>> +int RPiStream::importBuffers(unsigned int count)\n> >>> +{\n> >>> + return dev_->importBuffers(count);\n> >>> +}\n> >>> +\n> >>> +int RPiStream::allocateBuffers(unsigned int count)\n> >>> +{\n> >>> + return dev_->allocateBuffers(count, &internalBuffers_);\n> >>> +}\n> >>> +\n> >>> +int RPiStream::queueBuffers()\n> >>> +{\n> >>> + if (external_)\n> >>> + return 0;\n> >>> +\n> >>> + for (auto &b : internalBuffers_) {\n> >>> + int ret = dev_->queueBuffer(b.get());\n> >>> + if (ret) {\n> >>> + LOG(RPISTREAM, Error) << \"Failed to queue buffers for \"\n> >>> + << name_;\n> >>> + return ret;\n> >>> + }\n> >>> + }\n> >>> +\n> >>> + return 0;\n> >>> +}\n> >>> +\n> >>> +bool RPiStream::findFrameBuffer(FrameBuffer *buffer) const\n> >>> +{\n> >>> + auto start = external_ ? externalBuffers_->begin() : internalBuffers_.begin();\n> >>> + auto end = external_ ? externalBuffers_->end() : internalBuffers_.end();\n> >>> +\n> >>> + if (importOnly_)\n> >>> + return false;\n> >>> +\n> >>> + if (std::find_if(start, end,\n> >>> + [buffer](std::unique_ptr const &ref) { return ref.get() == buffer; }) != end)\n> >>> + return true;\n> >>> +\n> >>> + return false;\n> >>> +}\n> >>> +\n> >>> +} /* namespace RPi */\n> >>> +\n> >>> +} /* namespace libcamera */\n> >>> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> >>> new file mode 100644\n> >>> index 00000000..3957e342\n> >>> --- /dev/null\n> >>> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> >>> @@ -0,0 +1,98 @@\n> >>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >>> +/*\n> >>> + * Copyright (C) 2020, Raspberry Pi (Trading) Ltd.\n> >>> + *\n> >>> + * rpi_stream.h - Raspberry Pi device stream abstraction class.\n> >>> + */\n> >>> +#ifndef __LIBCAMERA_PIPELINE_RPI_STREAM_H__\n> >>> +#define __LIBCAMERA_PIPELINE_RPI_STREAM_H__\n> >>> +\n> >>> +#include \n> >>> +#include \n> >>> +#include \n> >>> +\n> >>> +#include \n> >>> +\n> >>> +#include \"libcamera/internal/v4l2_videodevice.h\"\n> >>> +\n> >>> +namespace libcamera {\n> >>> +\n> >>> +namespace RPi {\n> >>> +\n> >>> +/*\n> >>> + * Device stream abstraction for either an internal or external stream.\n> >>> + * Used for both Unicam and the ISP.\n> >>> + */\n> >>> +class RPiStream : public Stream\n\nDrive-by comment, as this class is in the RPi namespace, you could name\nis Stream. Same for the RPiDevice class below. Up to you (and of course\nnot for this patch).\n\n> >>> +{\n> >>> +public:\n> >>> + RPiStream()\n> >>> + {\n> >>> + }\n> >>> +\n> >>> + RPiStream(const char *name, MediaEntity *dev, bool importOnly = false)\n> >>> + : external_(false), importOnly_(importOnly), name_(name),\n> >>> + dev_(std::make_unique(dev))\n> >>> + {\n> >>> + }\n> >>> +\n> >\n> > As mentioned by the time I got to 6/9 (which I looked at last for some\n> > reason), this class became hard to parse as there are so many members so\n> > close together\n> >\n> >\n> > Although maybe it was because I was looking at a patch diff, but this\n> > seems smaller, I guess the diff adds an extra line for the +/- entries ;-)\n> >\n> > But perhaps breaking this up a bit below with:\n> >\n> >>> + V4L2VideoDevice *dev() const;\n> >\n> >>> + void setExternal(bool external);\n> >>> + bool isExternal() const;\n> >>> + bool isImporter() const;\n> >\n> >>> + void reset();\n> >>> + std::string name() const;\n> >\n> >>> + void setExternalBuffers(std::vector> *buffers);\n> >>> + const std::vector> *getBuffers() const;\n> >>> + void releaseBuffers();\n> >\n> >>> + int importBuffers(unsigned int count);\n> >>> + int allocateBuffers(unsigned int count);\n> >>> + int queueBuffers();\n> >>> + bool findFrameBuffer(FrameBuffer *buffer) const;\n> >\n> > I've punched newlines above (I hope that comes through in the mail) ...\n> > but really all I was wondering is if there is a way to group related\n> > functions into their own blocks.\n\nFor what it's worth, I also like separating groups of related members\nwith blank lines.\n\nReviewed-by: Laurent Pinchart \n\n> >>> +\n> >>> +private:\n> >>> + /*\n> >>> + * Indicates that this stream is active externally, i.e. the buffers\n> >>> + * are provided by the application.\n> >>> + */\n> >>> + bool external_;\n> >\n> >>> + /* Indicates that this stream only imports buffers, e.g. ISP input. */\n> >>> + bool importOnly_;\n> >\n> >>> + /* Stream name identifier. */\n> >>> + std::string name_;\n> >\n> >>> + /* The actual device stream. */\n> >>> + std::unique_ptr dev_;\n> >\n> >>> + /* Internally allocated framebuffers associated with this device stream. */\n> >>> + std::vector> internalBuffers_;\n> >\n> >>> + /* Externally allocated framebuffers associated with this device stream. */\n> >>> + std::vector> *externalBuffers_;\n> >>> +};\n> >\n> > When there's a comment line above an entry, for me it really helps to\n> > have a newline before it.\n> >\n> > Anyway, nothing critical here, just where I think some whitespace would\n> > have helped my weary eyes yesterday ;-)\n> >\n> >>> +\n> >>> +/*\n> >>> + * The following class is just a convenient (and typesafe) array of device\n> >>> + * streams indexed with an enum class.\n> >>> + */\n> >>> +template\n> >>> +class RPiDevice : public std::array\n> >>> +{\n> >>> +private:\n> >>> + constexpr auto index(E e) const noexcept\n> >>> + {\n> >>> + return static_cast>(e);\n> >>> + }\n> >>> +public:\n> >>> + RPiStream &operator[](E e)\n> >>> + {\n> >>> + return std::array::operator[](index(e));\n> >>> + }\n> >>> + const RPiStream &operator[](E e) const\n> >>> + {\n> >>> + return std::array::operator[](index(e));\n> >>> + }\n> >>> +};\n> >>> +\n> >>> +} /* namespace RPi */\n> >>> +\n> >>> +} /* namespace libcamera */\n> >>> +\n> >>> +#endif /* __LIBCAMERA_PIPELINE_RPI_STREAM_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 016BDBDB1B\n\tfor ;\n\tWed, 22 Jul 2020 14:17:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 685C660983;\n\tWed, 22 Jul 2020 16:17:21 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 683086053C\n\tfor ;\n\tWed, 22 Jul 2020 16:17:19 +0200 (CEST)","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 DBFE2329;\n\tWed, 22 Jul 2020 16:17:18 +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=\"UXP7X8Nk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1595427439;\n\tbh=S/NoTUHVeUzN5lyilbkFoRl/OkKvYVZJasQQ7pqaxb8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UXP7X8Nk9jY68a+6Zfo8LkZUDnA0ohCzpQe+t00C9pWC7Tpv0aftUeVK9mNCJZ5E4\n\tPjTZGYwPxPTlxi6I60hvSJt6U8SqDDiM9b9nEoUc/za5t+nv+aUO27gxqJzr57W44C\n\tHbe29IUTqeuhmZ67M1yT5ctFz2PLep2pBBxb6QII=","Date":"Wed, 22 Jul 2020 17:17:13 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"<20200722141713.GA29813@pendragon.ideasonboard.com>","References":"<20200720091311.805092-1-naush@raspberrypi.com>\n\t<20200720091311.805092-2-naush@raspberrypi.com>\n\t\n\t<389c5310-24f4-e3a2-ee28-21c6e004424f@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v4 1/9] libcamera: pipeline:\n\traspberrypi: Move RPiStream into a separate file","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]