[{"id":1671,"web_url":"https://patchwork.libcamera.org/comment/1671/","msgid":"<93abd31c-04d5-e02c-7aed-9c76a7fa61bd@ideasonboard.com>","date":"2019-05-23T08:53:10","subject":"Re: [libcamera-devel] [PATCH 1/2] cam: capture: Break out capture\n\tto a new class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Niklas,\n\nOn 23/05/2019 01:55, Niklas Söderlund wrote:\n> Reduce the complexity of main.cpp by compartmentalize the capture logic\n\n.. compartmentalising the capture logic (you can /s/z/ if you're american)\n\n> in its own class. There is no functional change.\n\n/in its/in to its/\n\n> \n\nThis looks good to me, and as far as I can tell it is just some code\nmove, so no real blocker.\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  src/cam/capture.cpp | 244 ++++++++++++++++++++++++++++++++++++++++++++\n>  src/cam/capture.h   |  42 ++++++++\n>  src/cam/main.cpp    | 236 +-----------------------------------------\n>  src/cam/main.h      |  19 ++++\n>  src/cam/meson.build |   1 +\n>  5 files changed, 310 insertions(+), 232 deletions(-)\n>  create mode 100644 src/cam/capture.cpp\n>  create mode 100644 src/cam/capture.h\n>  create mode 100644 src/cam/main.h\n> \n> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n> new file mode 100644\n> index 0000000000000000..91f65e8cf23c888d\n> --- /dev/null\n> +++ b/src/cam/capture.cpp\n> @@ -0,0 +1,244 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * capture.cpp - Cam capture\n> + */\n> +\n> +#include <climits>\n> +#include <iomanip>\n> +#include <iostream>\n> +#include <sstream>\n> +\n> +#include \"capture.h\"\n> +#include \"main.h\"\n> +\n> +using namespace libcamera;\n> +\n> +Capture::Capture()\n> +\t: camera_(nullptr), writer_(nullptr)\n> +{\n> +}\n> +\n> +int Capture::run(libcamera::Camera *camera, EventLoop *loop,\n> +\t\t const OptionsParser::Options &options)\n> +{\n> +\tint ret;\n> +\n> +\tif (!camera) {\n> +\t\tstd::cout << \"Can't capture without a camera\" << std::endl;\n> +\t\treturn -ENODEV;\n> +\t}\n> +\n> +\tcamera_ = camera;\n> +\n> +\tret = prepareConfig(options);\n> +\tif (ret) {\n> +\t\tstd::cout << \"Failed to prepare camera configuration\" << std::endl;\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tif (options.isSet(OptFile)) {\n> +\t\tif (!options[OptFile].toString().empty())\n> +\t\t\twriter_ = new BufferWriter(options[OptFile]);\n> +\t\telse\n> +\t\t\twriter_ = new BufferWriter();\n> +\t}\n> +\n> +\tret = capture(loop);\n> +\n> +\tif (options.isSet(OptFile))\n> +\t\tdelete writer_;\n> +\n> +\treturn ret;\n> +}\n> +\n> +int Capture::prepareConfig(const OptionsParser::Options &options)\n> +{\n> +\tStreamRoles roles;\n> +\n> +\t/* If no configuration is provided assume a single video stream. */\n> +\tif (!options.isSet(OptStream)) {\n> +\t\tconfig_ = camera_->generateConfiguration({ StreamRole::VideoRecording });\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tconst std::vector<OptionValue> &streamOptions =\n> +\t\toptions[OptStream].toArray();\n> +\n> +\t/* Use roles and get a default configuration. */\n> +\tfor (auto const &value : streamOptions) {\n> +\t\tKeyValueParser::Options conf = value.toKeyValues();\n> +\n> +\t\tif (!conf.isSet(\"role\")) {\n> +\t\t\troles.push_back(StreamRole::VideoRecording);\n> +\t\t} else if (conf[\"role\"].toString() == \"viewfinder\") {\n> +\t\t\troles.push_back(StreamRole::Viewfinder);\n> +\t\t} else if (conf[\"role\"].toString() == \"video\") {\n> +\t\t\troles.push_back(StreamRole::VideoRecording);\n> +\t\t} else if (conf[\"role\"].toString() == \"still\") {\n> +\t\t\troles.push_back(StreamRole::StillCapture);\n> +\t\t} else {\n> +\t\t\tstd::cerr << \"Unknown stream role \"\n> +\t\t\t\t  << conf[\"role\"].toString() << std::endl;\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\t}\n> +\n> +\tconfig_ = camera_->generateConfiguration(roles);\n> +\tif (!config_ || config_->size() != roles.size()) {\n> +\t\tstd::cerr << \"Failed to get default stream configuration\"\n> +\t\t\t  << std::endl;\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\t/* Apply configuration explicitly requested. */\n> +\tunsigned int i = 0;\n> +\tfor (auto const &value : streamOptions) {\n> +\t\tKeyValueParser::Options conf = value.toKeyValues();\n> +\t\tStreamConfiguration &cfg = config_->at(i++);\n> +\n> +\t\tif (conf.isSet(\"width\"))\n> +\t\t\tcfg.size.width = conf[\"width\"];\n> +\n> +\t\tif (conf.isSet(\"height\"))\n> +\t\t\tcfg.size.height = conf[\"height\"];\n> +\n> +\t\t/* TODO: Translate 4CC string to ID. */\n> +\t\tif (conf.isSet(\"pixelformat\"))\n> +\t\t\tcfg.pixelFormat = conf[\"pixelformat\"];\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +int Capture::capture(EventLoop *loop)\n> +{\n> +\tint ret;\n> +\n> +\tret = camera_->configure(config_.get());\n> +\tif (ret < 0) {\n> +\t\tstd::cout << \"Failed to configure camera\" << std::endl;\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tstreamName_.clear();\n> +\n> +\tfor (unsigned int index = 0; index < config_->size(); ++index) {\n> +\t\tStreamConfiguration &cfg = config_->at(index);\n> +\t\tstreamName_[cfg.stream()] = \"stream\" + std::to_string(index);\n> +\t}\n> +\n> +\tret = camera_->allocateBuffers();\n> +\tif (ret) {\n> +\t\tstd::cerr << \"Failed to allocate buffers\" << std::endl;\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tcamera_->requestCompleted.connect(this, &Capture::requestComplete);\n> +\n> +\t/* Identify the stream with the least number of buffers. */\n> +\tunsigned int nbuffers = UINT_MAX;\n> +\tfor (StreamConfiguration &cfg : *config_) {\n> +\t\tStream *stream = cfg.stream();\n> +\t\tnbuffers = std::min(nbuffers, stream->bufferPool().count());\n> +\t}\n> +\n> +\t/*\n> +\t * TODO: make cam tool smarter to support still capture by for\n> +\t * example pushing a button. For now run all streams all the time.\n> +\t */\n> +\n> +\tstd::vector<Request *> requests;\n> +\tfor (unsigned int i = 0; i < nbuffers; i++) {\n> +\t\tRequest *request = camera_->createRequest();\n> +\t\tif (!request) {\n> +\t\t\tstd::cerr << \"Can't create request\" << std::endl;\n> +\t\t\tret = -ENOMEM;\n> +\t\t\tgoto out;\n> +\t\t}\n> +\n> +\t\tstd::map<Stream *, Buffer *> map;\n> +\t\tfor (StreamConfiguration &cfg : *config_) {\n> +\t\t\tStream *stream = cfg.stream();\n> +\t\t\tmap[stream] = &stream->bufferPool().buffers()[i];\n> +\t\t}\n> +\n> +\t\tret = request->setBuffers(map);\n> +\t\tif (ret < 0) {\n> +\t\t\tstd::cerr << \"Can't set buffers for request\" << std::endl;\n> +\t\t\tgoto out;\n> +\t\t}\n> +\n> +\t\trequests.push_back(request);\n> +\t}\n> +\n> +\tret = camera_->start();\n> +\tif (ret) {\n> +\t\tstd::cout << \"Failed to start capture\" << std::endl;\n> +\t\tgoto out;\n> +\t}\n> +\n> +\tfor (Request *request : requests) {\n> +\t\tret = camera_->queueRequest(request);\n> +\t\tif (ret < 0) {\n> +\t\t\tstd::cerr << \"Can't queue request\" << std::endl;\n> +\t\t\tgoto out;\n> +\t\t}\n> +\t}\n> +\n> +\tstd::cout << \"Capture until user interrupts by SIGINT\" << std::endl;\n> +\tret = loop->exec();\n> +\n> +\tret = camera_->stop();\n> +\tif (ret)\n> +\t\tstd::cout << \"Failed to stop capture\" << std::endl;\n> +out:\n> +\tcamera_->freeBuffers();\n> +\n> +\treturn ret;\n> +}\n> +\n> +void Capture::requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers)\n> +{\n> +\tstatic uint64_t now, last = 0;\n> +\tdouble fps = 0.0;\n> +\n> +\tif (request->status() == Request::RequestCancelled)\n> +\t\treturn;\n> +\n> +\tstruct timespec time;\n> +\tclock_gettime(CLOCK_MONOTONIC, &time);\n> +\tnow = time.tv_sec * 1000 + time.tv_nsec / 1000000;\n> +\tfps = now - last;\n> +\tfps = last && fps ? 1000.0 / fps : 0.0;\n> +\tlast = now;\n> +\n> +\tstd::stringstream info;\n> +\tinfo << \"fps: \" << std::fixed << std::setprecision(2) << fps;\n> +\n> +\tfor (auto it = buffers.begin(); it != buffers.end(); ++it) {\n> +\t\tStream *stream = it->first;\n> +\t\tBuffer *buffer = it->second;\n> +\t\tconst std::string &name = streamName_[stream];\n> +\n> +\t\tinfo << \" \" << name\n> +\t\t     << \" (\" << buffer->index() << \")\"\n> +\t\t     << \" seq: \" << std::setw(6) << std::setfill('0') << buffer->sequence()\n> +\t\t     << \" bytesused: \" << buffer->bytesused();\n> +\n> +\t\tif (writer_)\n> +\t\t\twriter_->write(buffer, name);\n> +\t}\n> +\n> +\tstd::cout << info.str() << std::endl;\n> +\n> +\trequest = camera_->createRequest();\n> +\tif (!request) {\n> +\t\tstd::cerr << \"Can't create request\" << std::endl;\n> +\t\treturn;\n> +\t}\n> +\n> +\trequest->setBuffers(buffers);\n> +\tcamera_->queueRequest(request);\n> +}\n> diff --git a/src/cam/capture.h b/src/cam/capture.h\n> new file mode 100644\n> index 0000000000000000..728b1d22b159b046\n> --- /dev/null\n> +++ b/src/cam/capture.h\n> @@ -0,0 +1,42 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * capture.h - Cam capture\n> + */\n> +#ifndef __CAM_CAPTURE_H__\n> +#define __CAM_CAPTURE_H__\n> +\n> +#include <memory>\n> +\n> +#include <libcamera/camera.h>\n> +#include <libcamera/request.h>\n> +#include <libcamera/stream.h>\n> +\n> +#include \"buffer_writer.h\"\n> +#include \"event_loop.h\"\n> +#include \"options.h\"\n> +\n> +class Capture\n> +{\n> +public:\n> +\tCapture();\n> +\n> +\tint run(libcamera::Camera *camera, EventLoop *loop,\n> +\t\tconst OptionsParser::Options &options);\n> +private:\n> +\tint prepareConfig(const OptionsParser::Options &options);\n> +\n> +\tint capture(EventLoop *loop);\n> +\n> +\tvoid requestComplete(libcamera::Request *request,\n> +\t\t\t     const std::map<libcamera::Stream *, libcamera::Buffer *> &buffers);\n> +\n> +\tlibcamera::Camera *camera_;\n> +\tstd::unique_ptr<libcamera::CameraConfiguration> config_;\n> +\n> +\tstd::map<libcamera::Stream *, std::string> streamName_;\n> +\tBufferWriter *writer_;\n> +};\n> +\n> +#endif /* __CAM_CAPTURE_H__ */\n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index 5ecd7e0e38d768bb..fe7d4f90dbf14ffd 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -5,37 +5,22 @@\n>   * main.cpp - cam - The libcamera swiss army knife\n>   */\n>  \n> -#include <algorithm>\n> -#include <iomanip>\n>  #include <iostream>\n> -#include <limits.h>\n> -#include <map>\n>  #include <signal.h>\n> -#include <sstream>\n>  #include <string.h>\n>  \n>  #include <libcamera/libcamera.h>\n>  \n> -#include \"buffer_writer.h\"\n> +#include \"capture.h\"\n>  #include \"event_loop.h\"\n> +#include \"main.h\"\n>  #include \"options.h\"\n>  \n>  using namespace libcamera;\n>  \n>  OptionsParser::Options options;\n>  std::shared_ptr<Camera> camera;\n> -std::map<Stream *, std::string> streamInfo;\n>  EventLoop *loop;\n> -BufferWriter *writer;\n> -\n> -enum {\n> -\tOptCamera = 'c',\n> -\tOptCapture = 'C',\n> -\tOptFile = 'F',\n> -\tOptHelp = 'h',\n> -\tOptList = 'l',\n> -\tOptStream = 's',\n> -};\n>  \n>  void signalHandler(int signal)\n>  {\n> @@ -85,201 +70,6 @@ static int parseOptions(int argc, char *argv[])\n>  \treturn 0;\n>  }\n>  \n> -static std::unique_ptr<CameraConfiguration> prepareCameraConfig()\n> -{\n> -\tStreamRoles roles;\n> -\n> -\t/* If no configuration is provided assume a single video stream. */\n> -\tif (!options.isSet(OptStream))\n> -\t\treturn camera->generateConfiguration({ StreamRole::VideoRecording });\n> -\n> -\tconst std::vector<OptionValue> &streamOptions =\n> -\t\toptions[OptStream].toArray();\n> -\n> -\t/* Use roles and get a default configuration. */\n> -\tfor (auto const &value : streamOptions) {\n> -\t\tKeyValueParser::Options conf = value.toKeyValues();\n> -\n> -\t\tif (!conf.isSet(\"role\")) {\n> -\t\t\troles.push_back(StreamRole::VideoRecording);\n> -\t\t} else if (conf[\"role\"].toString() == \"viewfinder\") {\n> -\t\t\troles.push_back(StreamRole::Viewfinder);\n> -\t\t} else if (conf[\"role\"].toString() == \"video\") {\n> -\t\t\troles.push_back(StreamRole::VideoRecording);\n> -\t\t} else if (conf[\"role\"].toString() == \"still\") {\n> -\t\t\troles.push_back(StreamRole::StillCapture);\n> -\t\t} else {\n> -\t\t\tstd::cerr << \"Unknown stream role \"\n> -\t\t\t\t  << conf[\"role\"].toString() << std::endl;\n> -\t\t\treturn nullptr;\n> -\t\t}\n> -\t}\n> -\n> -\tstd::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles);\n> -\tif (!config || config->size() != roles.size()) {\n> -\t\tstd::cerr << \"Failed to get default stream configuration\"\n> -\t\t\t  << std::endl;\n> -\t\treturn nullptr;\n> -\t}\n> -\n> -\t/* Apply configuration explicitly requested. */\n> -\tunsigned int i = 0;\n> -\tfor (auto const &value : streamOptions) {\n> -\t\tKeyValueParser::Options conf = value.toKeyValues();\n> -\t\tStreamConfiguration &cfg = config->at(i++);\n> -\n> -\t\tif (conf.isSet(\"width\"))\n> -\t\t\tcfg.size.width = conf[\"width\"];\n> -\n> -\t\tif (conf.isSet(\"height\"))\n> -\t\t\tcfg.size.height = conf[\"height\"];\n> -\n> -\t\t/* TODO: Translate 4CC string to ID. */\n> -\t\tif (conf.isSet(\"pixelformat\"))\n> -\t\t\tcfg.pixelFormat = conf[\"pixelformat\"];\n> -\t}\n> -\n> -\treturn config;\n> -}\n> -\n> -static void requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers)\n> -{\n> -\tstatic uint64_t now, last = 0;\n> -\tdouble fps = 0.0;\n> -\n> -\tif (request->status() == Request::RequestCancelled)\n> -\t\treturn;\n> -\n> -\tstruct timespec time;\n> -\tclock_gettime(CLOCK_MONOTONIC, &time);\n> -\tnow = time.tv_sec * 1000 + time.tv_nsec / 1000000;\n> -\tfps = now - last;\n> -\tfps = last && fps ? 1000.0 / fps : 0.0;\n> -\tlast = now;\n> -\n> -\tstd::stringstream info;\n> -\tinfo << \"fps: \" << std::fixed << std::setprecision(2) << fps;\n> -\n> -\tfor (auto it = buffers.begin(); it != buffers.end(); ++it) {\n> -\t\tStream *stream = it->first;\n> -\t\tBuffer *buffer = it->second;\n> -\t\tconst std::string &name = streamInfo[stream];\n> -\n> -\t\tinfo << \" \" << name\n> -\t\t     << \" (\" << buffer->index() << \")\"\n> -\t\t     << \" seq: \" << std::setw(6) << std::setfill('0') << buffer->sequence()\n> -\t\t     << \" bytesused: \" << buffer->bytesused();\n> -\n> -\t\tif (writer)\n> -\t\t\twriter->write(buffer, name);\n> -\t}\n> -\n> -\tstd::cout << info.str() << std::endl;\n> -\n> -\trequest = camera->createRequest();\n> -\tif (!request) {\n> -\t\tstd::cerr << \"Can't create request\" << std::endl;\n> -\t\treturn;\n> -\t}\n> -\n> -\trequest->setBuffers(buffers);\n> -\tcamera->queueRequest(request);\n> -}\n> -\n> -static int capture()\n> -{\n> -\tint ret;\n> -\n> -\tstd::unique_ptr<CameraConfiguration> config = prepareCameraConfig();\n> -\tif (!config) {\n> -\t\tstd::cout << \"Failed to prepare camera configuration\" << std::endl;\n> -\t\treturn -EINVAL;\n> -\t}\n> -\n> -\tret = camera->configure(config.get());\n> -\tif (ret < 0) {\n> -\t\tstd::cout << \"Failed to configure camera\" << std::endl;\n> -\t\treturn ret;\n> -\t}\n> -\n> -\tstreamInfo.clear();\n> -\n> -\tfor (unsigned int index = 0; index < config->size(); ++index) {\n> -\t\tStreamConfiguration &cfg = config->at(index);\n> -\t\tstreamInfo[cfg.stream()] = \"stream\" + std::to_string(index);\n> -\t}\n> -\n> -\tret = camera->allocateBuffers();\n> -\tif (ret) {\n> -\t\tstd::cerr << \"Failed to allocate buffers\"\n> -\t\t\t  << std::endl;\n> -\t\treturn ret;\n> -\t}\n> -\n> -\tcamera->requestCompleted.connect(requestComplete);\n> -\n> -\t/* Identify the stream with the least number of buffers. */\n> -\tunsigned int nbuffers = UINT_MAX;\n> -\tfor (StreamConfiguration &cfg : *config) {\n> -\t\tStream *stream = cfg.stream();\n> -\t\tnbuffers = std::min(nbuffers, stream->bufferPool().count());\n> -\t}\n> -\n> -\t/*\n> -\t * TODO: make cam tool smarter to support still capture by for\n> -\t * example pushing a button. For now run all streams all the time.\n> -\t */\n> -\n> -\tstd::vector<Request *> requests;\n> -\tfor (unsigned int i = 0; i < nbuffers; i++) {\n> -\t\tRequest *request = camera->createRequest();\n> -\t\tif (!request) {\n> -\t\t\tstd::cerr << \"Can't create request\" << std::endl;\n> -\t\t\tret = -ENOMEM;\n> -\t\t\tgoto out;\n> -\t\t}\n> -\n> -\t\tstd::map<Stream *, Buffer *> map;\n> -\t\tfor (StreamConfiguration &cfg : *config) {\n> -\t\t\tStream *stream = cfg.stream();\n> -\t\t\tmap[stream] = &stream->bufferPool().buffers()[i];\n> -\t\t}\n> -\n> -\t\tret = request->setBuffers(map);\n> -\t\tif (ret < 0) {\n> -\t\t\tstd::cerr << \"Can't set buffers for request\" << std::endl;\n> -\t\t\tgoto out;\n> -\t\t}\n> -\n> -\t\trequests.push_back(request);\n> -\t}\n> -\n> -\tret = camera->start();\n> -\tif (ret) {\n> -\t\tstd::cout << \"Failed to start capture\" << std::endl;\n> -\t\tgoto out;\n> -\t}\n> -\n> -\tfor (Request *request : requests) {\n> -\t\tret = camera->queueRequest(request);\n> -\t\tif (ret < 0) {\n> -\t\t\tstd::cerr << \"Can't queue request\" << std::endl;\n> -\t\t\tgoto out;\n> -\t\t}\n> -\t}\n> -\n> -\tstd::cout << \"Capture until user interrupts by SIGINT\" << std::endl;\n> -\tret = loop->exec();\n> -\n> -\tret = camera->stop();\n> -\tif (ret)\n> -\t\tstd::cout << \"Failed to stop capture\" << std::endl;\n> -out:\n> -\tcamera->freeBuffers();\n> -\n> -\treturn ret;\n> -}\n> -\n>  int main(int argc, char **argv)\n>  {\n>  \tint ret;\n> @@ -327,26 +117,8 @@ int main(int argc, char **argv)\n>  \t}\n>  \n>  \tif (options.isSet(OptCapture)) {\n> -\t\tif (!camera) {\n> -\t\t\tstd::cout << \"Can't capture without a camera\"\n> -\t\t\t\t  << std::endl;\n> -\t\t\tret = EXIT_FAILURE;\n> -\t\t\tgoto out;\n> -\t\t}\n> -\n> -\t\tif (options.isSet(OptFile)) {\n> -\t\t\tif (!options[OptFile].toString().empty())\n> -\t\t\t\twriter = new BufferWriter(options[OptFile]);\n> -\t\t\telse\n> -\t\t\t\twriter = new BufferWriter();\n> -\t\t}\n> -\n> -\t\tcapture();\n> -\n> -\t\tif (options.isSet(OptFile)) {\n> -\t\t\tdelete writer;\n> -\t\t\twriter = nullptr;\n> -\t\t}\n> +\t\tCapture capture;\n> +\t\tret = capture.run(camera.get(), loop, options);\n>  \t}\n>  \n>  \tif (camera) {\n> diff --git a/src/cam/main.h b/src/cam/main.h\n> new file mode 100644\n> index 0000000000000000..a48bde620dc957f0\n> --- /dev/null\n> +++ b/src/cam/main.h\n> @@ -0,0 +1,19 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * main.h - Cam application\n> + */\n> +#ifndef __CAM_MAIN_H__\n> +#define __CAM_MAIN_H__\n> +\n> +enum {\n> +\tOptCamera = 'c',\n> +\tOptCapture = 'C',\n> +\tOptFile = 'F',\n> +\tOptHelp = 'h',\n> +\tOptList = 'l',\n> +\tOptStream = 's',\n> +};\n> +\n> +#endif /* __CAM_CAPTURE_H__ */\n> diff --git a/src/cam/meson.build b/src/cam/meson.build\n> index 851295091d0d5132..6d27b57393584fac 100644\n> --- a/src/cam/meson.build\n> +++ b/src/cam/meson.build\n> @@ -1,5 +1,6 @@\n>  cam_sources = files([\n>      'buffer_writer.cpp',\n> +    'capture.cpp',\n>      'event_loop.cpp',\n>      'main.cpp',\n>      'options.cpp',\n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 04D7E60C02\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 23 May 2019 10:53:13 +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 8B003583;\n\tThu, 23 May 2019 10:53:12 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1558601592;\n\tbh=wZ/7PPO35cZWdQ7EEZPwJhsm9F0EmuNbKZ0B1FC4JlQ=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=TbzGArm269bpRimB9wLwzD2A//7DtGaLGqDhOPtd6bcG08nvC++enTun7VN+koVVi\n\tR7yfJ6whLx3SZhvWNOmKJnZnnT2aveymz2/HSAQdrFUKLqv4jZeNteoyD94+OCALZu\n\tb4Km+jGlPBcA0StnJFSJus92aIeRQsLSoehL5X64=","Reply-To":"kieran.bingham@ideasonboard.com","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20190523005534.9631-1-niklas.soderlund@ragnatech.se>\n\t<20190523005534.9631-2-niklas.soderlund@ragnatech.se>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<93abd31c-04d5-e02c-7aed-9c76a7fa61bd@ideasonboard.com>","Date":"Thu, 23 May 2019 09:53:10 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.6.1","MIME-Version":"1.0","In-Reply-To":"<20190523005534.9631-2-niklas.soderlund@ragnatech.se>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 1/2] cam: capture: Break out capture\n\tto a new class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 23 May 2019 08:53:13 -0000"}},{"id":1672,"web_url":"https://patchwork.libcamera.org/comment/1672/","msgid":"<20190523092935.GH4745@pendragon.ideasonboard.com>","date":"2019-05-23T09:29:35","subject":"Re: [libcamera-devel] [PATCH 1/2] cam: capture: Break out capture\n\tto a new class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Thu, May 23, 2019 at 02:55:33AM +0200, Niklas Söderlund wrote:\n> Reduce the complexity of main.cpp by compartmentalize the capture logic\n> in its own class. There is no functional change.\n\nI'll refer to Kieran's excellent British review here, except that I\nthink \"in to\" should be \"into\" :-)\n\n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  src/cam/capture.cpp | 244 ++++++++++++++++++++++++++++++++++++++++++++\n>  src/cam/capture.h   |  42 ++++++++\n>  src/cam/main.cpp    | 236 +-----------------------------------------\n>  src/cam/main.h      |  19 ++++\n>  src/cam/meson.build |   1 +\n>  5 files changed, 310 insertions(+), 232 deletions(-)\n>  create mode 100644 src/cam/capture.cpp\n>  create mode 100644 src/cam/capture.h\n>  create mode 100644 src/cam/main.h\n> \n> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n> new file mode 100644\n> index 0000000000000000..91f65e8cf23c888d\n> --- /dev/null\n> +++ b/src/cam/capture.cpp\n> @@ -0,0 +1,244 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * capture.cpp - Cam capture\n> + */\n> +\n> +#include <climits>\n> +#include <iomanip>\n> +#include <iostream>\n> +#include <sstream>\n> +\n> +#include \"capture.h\"\n> +#include \"main.h\"\n> +\n> +using namespace libcamera;\n> +\n> +Capture::Capture()\n> +\t: camera_(nullptr), writer_(nullptr)\n> +{\n> +}\n> +\n> +int Capture::run(libcamera::Camera *camera, EventLoop *loop,\n\nNo need for the libcamera:: namespace qualifier given the using\ndirective above.\n\n> +\t\t const OptionsParser::Options &options)\n> +{\n> +\tint ret;\n> +\n> +\tif (!camera) {\n> +\t\tstd::cout << \"Can't capture without a camera\" << std::endl;\n> +\t\treturn -ENODEV;\n> +\t}\n> +\n> +\tcamera_ = camera;\n> +\n> +\tret = prepareConfig(options);\n> +\tif (ret) {\n> +\t\tstd::cout << \"Failed to prepare camera configuration\" << std::endl;\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tif (options.isSet(OptFile)) {\n> +\t\tif (!options[OptFile].toString().empty())\n> +\t\t\twriter_ = new BufferWriter(options[OptFile]);\n> +\t\telse\n> +\t\t\twriter_ = new BufferWriter();\n> +\t}\n> +\n\nShould we move the camera->config() and buffer allocation code here, out\nof Capture::capture() ? They don't really belong to the capture loop\nitself, and I think the change would get rid of the out: label in that\nfunction.\n\n> +\tret = capture(loop);\n> +\n> +\tif (options.isSet(OptFile))\n> +\t\tdelete writer_;\n\nI would add a writer_ = nullptr here just in case we want to run\nmultiple capture sessions.\n\n> +\n> +\treturn ret;\n> +}\n> +\n> +int Capture::prepareConfig(const OptionsParser::Options &options)\n> +{\n> +\tStreamRoles roles;\n> +\n> +\t/* If no configuration is provided assume a single video stream. */\n> +\tif (!options.isSet(OptStream)) {\n> +\t\tconfig_ = camera_->generateConfiguration({ StreamRole::VideoRecording });\n\nShould we guard against generateConfiguration() returning nullptr ?\nObviously something would be very wrong, but not crashing would still be\nnice.\n\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tconst std::vector<OptionValue> &streamOptions =\n> +\t\toptions[OptStream].toArray();\n> +\n> +\t/* Use roles and get a default configuration. */\n> +\tfor (auto const &value : streamOptions) {\n> +\t\tKeyValueParser::Options conf = value.toKeyValues();\n> +\n> +\t\tif (!conf.isSet(\"role\")) {\n> +\t\t\troles.push_back(StreamRole::VideoRecording);\n> +\t\t} else if (conf[\"role\"].toString() == \"viewfinder\") {\n> +\t\t\troles.push_back(StreamRole::Viewfinder);\n> +\t\t} else if (conf[\"role\"].toString() == \"video\") {\n> +\t\t\troles.push_back(StreamRole::VideoRecording);\n> +\t\t} else if (conf[\"role\"].toString() == \"still\") {\n> +\t\t\troles.push_back(StreamRole::StillCapture);\n> +\t\t} else {\n> +\t\t\tstd::cerr << \"Unknown stream role \"\n> +\t\t\t\t  << conf[\"role\"].toString() << std::endl;\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\t}\n> +\n> +\tconfig_ = camera_->generateConfiguration(roles);\n> +\tif (!config_ || config_->size() != roles.size()) {\n> +\t\tstd::cerr << \"Failed to get default stream configuration\"\n> +\t\t\t  << std::endl;\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\t/* Apply configuration explicitly requested. */\n> +\tunsigned int i = 0;\n> +\tfor (auto const &value : streamOptions) {\n> +\t\tKeyValueParser::Options conf = value.toKeyValues();\n> +\t\tStreamConfiguration &cfg = config_->at(i++);\n> +\n> +\t\tif (conf.isSet(\"width\"))\n> +\t\t\tcfg.size.width = conf[\"width\"];\n> +\n> +\t\tif (conf.isSet(\"height\"))\n> +\t\t\tcfg.size.height = conf[\"height\"];\n> +\n> +\t\t/* TODO: Translate 4CC string to ID. */\n> +\t\tif (conf.isSet(\"pixelformat\"))\n> +\t\t\tcfg.pixelFormat = conf[\"pixelformat\"];\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +int Capture::capture(EventLoop *loop)\n> +{\n> +\tint ret;\n> +\n> +\tret = camera_->configure(config_.get());\n> +\tif (ret < 0) {\n> +\t\tstd::cout << \"Failed to configure camera\" << std::endl;\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tstreamName_.clear();\n> +\n> +\tfor (unsigned int index = 0; index < config_->size(); ++index) {\n> +\t\tStreamConfiguration &cfg = config_->at(index);\n> +\t\tstreamName_[cfg.stream()] = \"stream\" + std::to_string(index);\n> +\t}\n> +\n> +\tret = camera_->allocateBuffers();\n> +\tif (ret) {\n> +\t\tstd::cerr << \"Failed to allocate buffers\" << std::endl;\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tcamera_->requestCompleted.connect(this, &Capture::requestComplete);\n> +\n> +\t/* Identify the stream with the least number of buffers. */\n> +\tunsigned int nbuffers = UINT_MAX;\n> +\tfor (StreamConfiguration &cfg : *config_) {\n> +\t\tStream *stream = cfg.stream();\n> +\t\tnbuffers = std::min(nbuffers, stream->bufferPool().count());\n> +\t}\n> +\n> +\t/*\n> +\t * TODO: make cam tool smarter to support still capture by for\n> +\t * example pushing a button. For now run all streams all the time.\n> +\t */\n> +\n> +\tstd::vector<Request *> requests;\n> +\tfor (unsigned int i = 0; i < nbuffers; i++) {\n> +\t\tRequest *request = camera_->createRequest();\n> +\t\tif (!request) {\n> +\t\t\tstd::cerr << \"Can't create request\" << std::endl;\n> +\t\t\tret = -ENOMEM;\n> +\t\t\tgoto out;\n> +\t\t}\n> +\n> +\t\tstd::map<Stream *, Buffer *> map;\n> +\t\tfor (StreamConfiguration &cfg : *config_) {\n> +\t\t\tStream *stream = cfg.stream();\n> +\t\t\tmap[stream] = &stream->bufferPool().buffers()[i];\n> +\t\t}\n> +\n> +\t\tret = request->setBuffers(map);\n> +\t\tif (ret < 0) {\n> +\t\t\tstd::cerr << \"Can't set buffers for request\" << std::endl;\n> +\t\t\tgoto out;\n> +\t\t}\n> +\n> +\t\trequests.push_back(request);\n> +\t}\n> +\n> +\tret = camera_->start();\n> +\tif (ret) {\n> +\t\tstd::cout << \"Failed to start capture\" << std::endl;\n> +\t\tgoto out;\n> +\t}\n> +\n> +\tfor (Request *request : requests) {\n> +\t\tret = camera_->queueRequest(request);\n> +\t\tif (ret < 0) {\n> +\t\t\tstd::cerr << \"Can't queue request\" << std::endl;\n> +\t\t\tgoto out;\n> +\t\t}\n> +\t}\n> +\n> +\tstd::cout << \"Capture until user interrupts by SIGINT\" << std::endl;\n> +\tret = loop->exec();\n> +\n> +\tret = camera_->stop();\n> +\tif (ret)\n> +\t\tstd::cout << \"Failed to stop capture\" << std::endl;\n> +out:\n> +\tcamera_->freeBuffers();\n\nMaybe a config_.reset() here to make it explicit that we don't need the\nconfig anymore ? Otherwise this will result in a reference to the camera\nbeing stored internally (at least for some pipeline handlers), and if\nthe Capture class doesn't get deleted before the camera manager is\nstopped, there will be a warning.\n\n> +\n> +\treturn ret;\n> +}\n> +\n> +void Capture::requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers)\n> +{\n> +\tstatic uint64_t now, last = 0;\n\nYou can now move these to member variables :-)\n\n> +\tdouble fps = 0.0;\n> +\n> +\tif (request->status() == Request::RequestCancelled)\n> +\t\treturn;\n> +\n> +\tstruct timespec time;\n> +\tclock_gettime(CLOCK_MONOTONIC, &time);\n> +\tnow = time.tv_sec * 1000 + time.tv_nsec / 1000000;\n> +\tfps = now - last;\n> +\tfps = last && fps ? 1000.0 / fps : 0.0;\n> +\tlast = now;\n> +\n> +\tstd::stringstream info;\n> +\tinfo << \"fps: \" << std::fixed << std::setprecision(2) << fps;\n> +\n> +\tfor (auto it = buffers.begin(); it != buffers.end(); ++it) {\n> +\t\tStream *stream = it->first;\n> +\t\tBuffer *buffer = it->second;\n> +\t\tconst std::string &name = streamName_[stream];\n> +\n> +\t\tinfo << \" \" << name\n> +\t\t     << \" (\" << buffer->index() << \")\"\n> +\t\t     << \" seq: \" << std::setw(6) << std::setfill('0') << buffer->sequence()\n> +\t\t     << \" bytesused: \" << buffer->bytesused();\n> +\n> +\t\tif (writer_)\n> +\t\t\twriter_->write(buffer, name);\n> +\t}\n> +\n> +\tstd::cout << info.str() << std::endl;\n> +\n> +\trequest = camera_->createRequest();\n> +\tif (!request) {\n> +\t\tstd::cerr << \"Can't create request\" << std::endl;\n> +\t\treturn;\n> +\t}\n> +\n> +\trequest->setBuffers(buffers);\n> +\tcamera_->queueRequest(request);\n> +}\n> diff --git a/src/cam/capture.h b/src/cam/capture.h\n> new file mode 100644\n> index 0000000000000000..728b1d22b159b046\n> --- /dev/null\n> +++ b/src/cam/capture.h\n> @@ -0,0 +1,42 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * capture.h - Cam capture\n> + */\n> +#ifndef __CAM_CAPTURE_H__\n> +#define __CAM_CAPTURE_H__\n> +\n> +#include <memory>\n> +\n> +#include <libcamera/camera.h>\n> +#include <libcamera/request.h>\n> +#include <libcamera/stream.h>\n> +\n> +#include \"buffer_writer.h\"\n> +#include \"event_loop.h\"\n> +#include \"options.h\"\n> +\n> +class Capture\n> +{\n> +public:\n> +\tCapture();\n> +\n> +\tint run(libcamera::Camera *camera, EventLoop *loop,\n> +\t\tconst OptionsParser::Options &options);\n\nI like how the options are passed as a const reference.\n\n> +private:\n> +\tint prepareConfig(const OptionsParser::Options &options);\n> +\n> +\tint capture(EventLoop *loop);\n> +\n> +\tvoid requestComplete(libcamera::Request *request,\n> +\t\t\t     const std::map<libcamera::Stream *, libcamera::Buffer *> &buffers);\n> +\n> +\tlibcamera::Camera *camera_;\n> +\tstd::unique_ptr<libcamera::CameraConfiguration> config_;\n> +\n> +\tstd::map<libcamera::Stream *, std::string> streamName_;\n> +\tBufferWriter *writer_;\n> +};\n> +\n> +#endif /* __CAM_CAPTURE_H__ */\n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index 5ecd7e0e38d768bb..fe7d4f90dbf14ffd 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -5,37 +5,22 @@\n>   * main.cpp - cam - The libcamera swiss army knife\n>   */\n>  \n> -#include <algorithm>\n> -#include <iomanip>\n>  #include <iostream>\n> -#include <limits.h>\n> -#include <map>\n>  #include <signal.h>\n> -#include <sstream>\n>  #include <string.h>\n>  \n>  #include <libcamera/libcamera.h>\n>  \n> -#include \"buffer_writer.h\"\n> +#include \"capture.h\"\n>  #include \"event_loop.h\"\n> +#include \"main.h\"\n>  #include \"options.h\"\n>  \n>  using namespace libcamera;\n>  \n>  OptionsParser::Options options;\n>  std::shared_ptr<Camera> camera;\n> -std::map<Stream *, std::string> streamInfo;\n>  EventLoop *loop;\n> -BufferWriter *writer;\n> -\n> -enum {\n> -\tOptCamera = 'c',\n> -\tOptCapture = 'C',\n> -\tOptFile = 'F',\n> -\tOptHelp = 'h',\n> -\tOptList = 'l',\n> -\tOptStream = 's',\n> -};\n>  \n>  void signalHandler(int signal)\n>  {\n> @@ -85,201 +70,6 @@ static int parseOptions(int argc, char *argv[])\n>  \treturn 0;\n>  }\n>  \n> -static std::unique_ptr<CameraConfiguration> prepareCameraConfig()\n> -{\n> -\tStreamRoles roles;\n> -\n> -\t/* If no configuration is provided assume a single video stream. */\n> -\tif (!options.isSet(OptStream))\n> -\t\treturn camera->generateConfiguration({ StreamRole::VideoRecording });\n> -\n> -\tconst std::vector<OptionValue> &streamOptions =\n> -\t\toptions[OptStream].toArray();\n> -\n> -\t/* Use roles and get a default configuration. */\n> -\tfor (auto const &value : streamOptions) {\n> -\t\tKeyValueParser::Options conf = value.toKeyValues();\n> -\n> -\t\tif (!conf.isSet(\"role\")) {\n> -\t\t\troles.push_back(StreamRole::VideoRecording);\n> -\t\t} else if (conf[\"role\"].toString() == \"viewfinder\") {\n> -\t\t\troles.push_back(StreamRole::Viewfinder);\n> -\t\t} else if (conf[\"role\"].toString() == \"video\") {\n> -\t\t\troles.push_back(StreamRole::VideoRecording);\n> -\t\t} else if (conf[\"role\"].toString() == \"still\") {\n> -\t\t\troles.push_back(StreamRole::StillCapture);\n> -\t\t} else {\n> -\t\t\tstd::cerr << \"Unknown stream role \"\n> -\t\t\t\t  << conf[\"role\"].toString() << std::endl;\n> -\t\t\treturn nullptr;\n> -\t\t}\n> -\t}\n> -\n> -\tstd::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles);\n> -\tif (!config || config->size() != roles.size()) {\n> -\t\tstd::cerr << \"Failed to get default stream configuration\"\n> -\t\t\t  << std::endl;\n> -\t\treturn nullptr;\n> -\t}\n> -\n> -\t/* Apply configuration explicitly requested. */\n> -\tunsigned int i = 0;\n> -\tfor (auto const &value : streamOptions) {\n> -\t\tKeyValueParser::Options conf = value.toKeyValues();\n> -\t\tStreamConfiguration &cfg = config->at(i++);\n> -\n> -\t\tif (conf.isSet(\"width\"))\n> -\t\t\tcfg.size.width = conf[\"width\"];\n> -\n> -\t\tif (conf.isSet(\"height\"))\n> -\t\t\tcfg.size.height = conf[\"height\"];\n> -\n> -\t\t/* TODO: Translate 4CC string to ID. */\n> -\t\tif (conf.isSet(\"pixelformat\"))\n> -\t\t\tcfg.pixelFormat = conf[\"pixelformat\"];\n> -\t}\n> -\n> -\treturn config;\n> -}\n> -\n> -static void requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers)\n> -{\n> -\tstatic uint64_t now, last = 0;\n> -\tdouble fps = 0.0;\n> -\n> -\tif (request->status() == Request::RequestCancelled)\n> -\t\treturn;\n> -\n> -\tstruct timespec time;\n> -\tclock_gettime(CLOCK_MONOTONIC, &time);\n> -\tnow = time.tv_sec * 1000 + time.tv_nsec / 1000000;\n> -\tfps = now - last;\n> -\tfps = last && fps ? 1000.0 / fps : 0.0;\n> -\tlast = now;\n> -\n> -\tstd::stringstream info;\n> -\tinfo << \"fps: \" << std::fixed << std::setprecision(2) << fps;\n> -\n> -\tfor (auto it = buffers.begin(); it != buffers.end(); ++it) {\n> -\t\tStream *stream = it->first;\n> -\t\tBuffer *buffer = it->second;\n> -\t\tconst std::string &name = streamInfo[stream];\n> -\n> -\t\tinfo << \" \" << name\n> -\t\t     << \" (\" << buffer->index() << \")\"\n> -\t\t     << \" seq: \" << std::setw(6) << std::setfill('0') << buffer->sequence()\n> -\t\t     << \" bytesused: \" << buffer->bytesused();\n> -\n> -\t\tif (writer)\n> -\t\t\twriter->write(buffer, name);\n> -\t}\n> -\n> -\tstd::cout << info.str() << std::endl;\n> -\n> -\trequest = camera->createRequest();\n> -\tif (!request) {\n> -\t\tstd::cerr << \"Can't create request\" << std::endl;\n> -\t\treturn;\n> -\t}\n> -\n> -\trequest->setBuffers(buffers);\n> -\tcamera->queueRequest(request);\n> -}\n> -\n> -static int capture()\n> -{\n> -\tint ret;\n> -\n> -\tstd::unique_ptr<CameraConfiguration> config = prepareCameraConfig();\n> -\tif (!config) {\n> -\t\tstd::cout << \"Failed to prepare camera configuration\" << std::endl;\n> -\t\treturn -EINVAL;\n> -\t}\n> -\n> -\tret = camera->configure(config.get());\n> -\tif (ret < 0) {\n> -\t\tstd::cout << \"Failed to configure camera\" << std::endl;\n> -\t\treturn ret;\n> -\t}\n> -\n> -\tstreamInfo.clear();\n> -\n> -\tfor (unsigned int index = 0; index < config->size(); ++index) {\n> -\t\tStreamConfiguration &cfg = config->at(index);\n> -\t\tstreamInfo[cfg.stream()] = \"stream\" + std::to_string(index);\n> -\t}\n> -\n> -\tret = camera->allocateBuffers();\n> -\tif (ret) {\n> -\t\tstd::cerr << \"Failed to allocate buffers\"\n> -\t\t\t  << std::endl;\n> -\t\treturn ret;\n> -\t}\n> -\n> -\tcamera->requestCompleted.connect(requestComplete);\n> -\n> -\t/* Identify the stream with the least number of buffers. */\n> -\tunsigned int nbuffers = UINT_MAX;\n> -\tfor (StreamConfiguration &cfg : *config) {\n> -\t\tStream *stream = cfg.stream();\n> -\t\tnbuffers = std::min(nbuffers, stream->bufferPool().count());\n> -\t}\n> -\n> -\t/*\n> -\t * TODO: make cam tool smarter to support still capture by for\n> -\t * example pushing a button. For now run all streams all the time.\n> -\t */\n> -\n> -\tstd::vector<Request *> requests;\n> -\tfor (unsigned int i = 0; i < nbuffers; i++) {\n> -\t\tRequest *request = camera->createRequest();\n> -\t\tif (!request) {\n> -\t\t\tstd::cerr << \"Can't create request\" << std::endl;\n> -\t\t\tret = -ENOMEM;\n> -\t\t\tgoto out;\n> -\t\t}\n> -\n> -\t\tstd::map<Stream *, Buffer *> map;\n> -\t\tfor (StreamConfiguration &cfg : *config) {\n> -\t\t\tStream *stream = cfg.stream();\n> -\t\t\tmap[stream] = &stream->bufferPool().buffers()[i];\n> -\t\t}\n> -\n> -\t\tret = request->setBuffers(map);\n> -\t\tif (ret < 0) {\n> -\t\t\tstd::cerr << \"Can't set buffers for request\" << std::endl;\n> -\t\t\tgoto out;\n> -\t\t}\n> -\n> -\t\trequests.push_back(request);\n> -\t}\n> -\n> -\tret = camera->start();\n> -\tif (ret) {\n> -\t\tstd::cout << \"Failed to start capture\" << std::endl;\n> -\t\tgoto out;\n> -\t}\n> -\n> -\tfor (Request *request : requests) {\n> -\t\tret = camera->queueRequest(request);\n> -\t\tif (ret < 0) {\n> -\t\t\tstd::cerr << \"Can't queue request\" << std::endl;\n> -\t\t\tgoto out;\n> -\t\t}\n> -\t}\n> -\n> -\tstd::cout << \"Capture until user interrupts by SIGINT\" << std::endl;\n> -\tret = loop->exec();\n> -\n> -\tret = camera->stop();\n> -\tif (ret)\n> -\t\tstd::cout << \"Failed to stop capture\" << std::endl;\n> -out:\n> -\tcamera->freeBuffers();\n> -\n> -\treturn ret;\n> -}\n> -\n>  int main(int argc, char **argv)\n>  {\n>  \tint ret;\n> @@ -327,26 +117,8 @@ int main(int argc, char **argv)\n>  \t}\n>  \n>  \tif (options.isSet(OptCapture)) {\n> -\t\tif (!camera) {\n> -\t\t\tstd::cout << \"Can't capture without a camera\"\n> -\t\t\t\t  << std::endl;\n> -\t\t\tret = EXIT_FAILURE;\n> -\t\t\tgoto out;\n> -\t\t}\n> -\n> -\t\tif (options.isSet(OptFile)) {\n> -\t\t\tif (!options[OptFile].toString().empty())\n> -\t\t\t\twriter = new BufferWriter(options[OptFile]);\n> -\t\t\telse\n> -\t\t\t\twriter = new BufferWriter();\n> -\t\t}\n> -\n> -\t\tcapture();\n> -\n> -\t\tif (options.isSet(OptFile)) {\n> -\t\t\tdelete writer;\n> -\t\t\twriter = nullptr;\n> -\t\t}\n> +\t\tCapture capture;\n> +\t\tret = capture.run(camera.get(), loop, options);\n\nShould we move the event loop to the Capture class ? We may add more\nusers of the loop here, but in that case I think loop->exec() should\nstay in main() (which would require splitting Capture::run() into a\nstart and stop).\n\n>  \t}\n>  \n>  \tif (camera) {\n> diff --git a/src/cam/main.h b/src/cam/main.h\n> new file mode 100644\n> index 0000000000000000..a48bde620dc957f0\n> --- /dev/null\n> +++ b/src/cam/main.h\n> @@ -0,0 +1,19 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * main.h - Cam application\n> + */\n> +#ifndef __CAM_MAIN_H__\n> +#define __CAM_MAIN_H__\n> +\n> +enum {\n> +\tOptCamera = 'c',\n> +\tOptCapture = 'C',\n> +\tOptFile = 'F',\n> +\tOptHelp = 'h',\n> +\tOptList = 'l',\n> +\tOptStream = 's',\n> +};\n> +\n> +#endif /* __CAM_CAPTURE_H__ */\n\n__CAM_MAIN_H__\n\n> diff --git a/src/cam/meson.build b/src/cam/meson.build\n> index 851295091d0d5132..6d27b57393584fac 100644\n> --- a/src/cam/meson.build\n> +++ b/src/cam/meson.build\n> @@ -1,5 +1,6 @@\n>  cam_sources = files([\n>      'buffer_writer.cpp',\n> +    'capture.cpp',\n>      'event_loop.cpp',\n>      'main.cpp',\n>      'options.cpp',","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6AA6060003\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 23 May 2019 11:29:56 +0200 (CEST)","from pendragon.ideasonboard.com (85-76-106-214-nat.elisa-mobile.fi\n\t[85.76.106.214])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5953E334;\n\tThu, 23 May 2019 11:29:54 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1558603796;\n\tbh=WunNqEusQkUM5I7eGOtnayFTtuP/G8jrCYxJK+QmZew=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=EYk8J5k6Yj8VsSb47g4k8iR9ODE4OodWMeyTb/2YtCXybjSN9h/S/YC19qFHWVgEe\n\t8g2s5XM53h6+TFiwqxqHo9MucRwrFuv2OpHjSCJ2XlLMAPqVIeQ9b2dqt/4VbY7uIG\n\tg4KPKkVbOz9CHt8yxD6a6I6w/LCLhjDPQZlH+kqM=","Date":"Thu, 23 May 2019 12:29:35 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190523092935.GH4745@pendragon.ideasonboard.com>","References":"<20190523005534.9631-1-niklas.soderlund@ragnatech.se>\n\t<20190523005534.9631-2-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190523005534.9631-2-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 1/2] cam: capture: Break out capture\n\tto a new class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 23 May 2019 09:29:56 -0000"}},{"id":1673,"web_url":"https://patchwork.libcamera.org/comment/1673/","msgid":"<dd630abd-7309-c629-5856-0c20c5ae7a30@ideasonboard.com>","date":"2019-05-23T09:32:16","subject":"Re: [libcamera-devel] [PATCH 1/2] cam: capture: Break out capture\n\tto a new class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 23/05/2019 10:29, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Thu, May 23, 2019 at 02:55:33AM +0200, Niklas Söderlund wrote:\n>> Reduce the complexity of main.cpp by compartmentalize the capture logic\n>> in its own class. There is no functional change.\n> \n> I'll refer to Kieran's excellent British review here, except that I\n> think \"in to\" should be \"into\" :-)\n\nAhem - yes, I think 'into' is more correct than 'in to' in this instance.\n\n--\nKB\n\n> \n>> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n>> ---\n>>  src/cam/capture.cpp | 244 ++++++++++++++++++++++++++++++++++++++++++++\n>>  src/cam/capture.h   |  42 ++++++++\n>>  src/cam/main.cpp    | 236 +-----------------------------------------\n>>  src/cam/main.h      |  19 ++++\n>>  src/cam/meson.build |   1 +\n>>  5 files changed, 310 insertions(+), 232 deletions(-)\n>>  create mode 100644 src/cam/capture.cpp\n>>  create mode 100644 src/cam/capture.h\n>>  create mode 100644 src/cam/main.h\n>>\n>> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n>> new file mode 100644\n>> index 0000000000000000..91f65e8cf23c888d\n>> --- /dev/null\n>> +++ b/src/cam/capture.cpp\n>> @@ -0,0 +1,244 @@\n>> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n>> +/*\n>> + * Copyright (C) 2019, Google Inc.\n>> + *\n>> + * capture.cpp - Cam capture\n>> + */\n>> +\n>> +#include <climits>\n>> +#include <iomanip>\n>> +#include <iostream>\n>> +#include <sstream>\n>> +\n>> +#include \"capture.h\"\n>> +#include \"main.h\"\n>> +\n>> +using namespace libcamera;\n>> +\n>> +Capture::Capture()\n>> +\t: camera_(nullptr), writer_(nullptr)\n>> +{\n>> +}\n>> +\n>> +int Capture::run(libcamera::Camera *camera, EventLoop *loop,\n> \n> No need for the libcamera:: namespace qualifier given the using\n> directive above.\n> \n>> +\t\t const OptionsParser::Options &options)\n>> +{\n>> +\tint ret;\n>> +\n>> +\tif (!camera) {\n>> +\t\tstd::cout << \"Can't capture without a camera\" << std::endl;\n>> +\t\treturn -ENODEV;\n>> +\t}\n>> +\n>> +\tcamera_ = camera;\n>> +\n>> +\tret = prepareConfig(options);\n>> +\tif (ret) {\n>> +\t\tstd::cout << \"Failed to prepare camera configuration\" << std::endl;\n>> +\t\treturn -EINVAL;\n>> +\t}\n>> +\n>> +\tif (options.isSet(OptFile)) {\n>> +\t\tif (!options[OptFile].toString().empty())\n>> +\t\t\twriter_ = new BufferWriter(options[OptFile]);\n>> +\t\telse\n>> +\t\t\twriter_ = new BufferWriter();\n>> +\t}\n>> +\n> \n> Should we move the camera->config() and buffer allocation code here, out\n> of Capture::capture() ? They don't really belong to the capture loop\n> itself, and I think the change would get rid of the out: label in that\n> function.\n> \n>> +\tret = capture(loop);\n>> +\n>> +\tif (options.isSet(OptFile))\n>> +\t\tdelete writer_;\n> \n> I would add a writer_ = nullptr here just in case we want to run\n> multiple capture sessions.\n> \n>> +\n>> +\treturn ret;\n>> +}\n>> +\n>> +int Capture::prepareConfig(const OptionsParser::Options &options)\n>> +{\n>> +\tStreamRoles roles;\n>> +\n>> +\t/* If no configuration is provided assume a single video stream. */\n>> +\tif (!options.isSet(OptStream)) {\n>> +\t\tconfig_ = camera_->generateConfiguration({ StreamRole::VideoRecording });\n> \n> Should we guard against generateConfiguration() returning nullptr ?\n> Obviously something would be very wrong, but not crashing would still be\n> nice.\n> \n>> +\t\treturn 0;\n>> +\t}\n>> +\n>> +\tconst std::vector<OptionValue> &streamOptions =\n>> +\t\toptions[OptStream].toArray();\n>> +\n>> +\t/* Use roles and get a default configuration. */\n>> +\tfor (auto const &value : streamOptions) {\n>> +\t\tKeyValueParser::Options conf = value.toKeyValues();\n>> +\n>> +\t\tif (!conf.isSet(\"role\")) {\n>> +\t\t\troles.push_back(StreamRole::VideoRecording);\n>> +\t\t} else if (conf[\"role\"].toString() == \"viewfinder\") {\n>> +\t\t\troles.push_back(StreamRole::Viewfinder);\n>> +\t\t} else if (conf[\"role\"].toString() == \"video\") {\n>> +\t\t\troles.push_back(StreamRole::VideoRecording);\n>> +\t\t} else if (conf[\"role\"].toString() == \"still\") {\n>> +\t\t\troles.push_back(StreamRole::StillCapture);\n>> +\t\t} else {\n>> +\t\t\tstd::cerr << \"Unknown stream role \"\n>> +\t\t\t\t  << conf[\"role\"].toString() << std::endl;\n>> +\t\t\treturn -EINVAL;\n>> +\t\t}\n>> +\t}\n>> +\n>> +\tconfig_ = camera_->generateConfiguration(roles);\n>> +\tif (!config_ || config_->size() != roles.size()) {\n>> +\t\tstd::cerr << \"Failed to get default stream configuration\"\n>> +\t\t\t  << std::endl;\n>> +\t\treturn -EINVAL;\n>> +\t}\n>> +\n>> +\t/* Apply configuration explicitly requested. */\n>> +\tunsigned int i = 0;\n>> +\tfor (auto const &value : streamOptions) {\n>> +\t\tKeyValueParser::Options conf = value.toKeyValues();\n>> +\t\tStreamConfiguration &cfg = config_->at(i++);\n>> +\n>> +\t\tif (conf.isSet(\"width\"))\n>> +\t\t\tcfg.size.width = conf[\"width\"];\n>> +\n>> +\t\tif (conf.isSet(\"height\"))\n>> +\t\t\tcfg.size.height = conf[\"height\"];\n>> +\n>> +\t\t/* TODO: Translate 4CC string to ID. */\n>> +\t\tif (conf.isSet(\"pixelformat\"))\n>> +\t\t\tcfg.pixelFormat = conf[\"pixelformat\"];\n>> +\t}\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +int Capture::capture(EventLoop *loop)\n>> +{\n>> +\tint ret;\n>> +\n>> +\tret = camera_->configure(config_.get());\n>> +\tif (ret < 0) {\n>> +\t\tstd::cout << \"Failed to configure camera\" << std::endl;\n>> +\t\treturn ret;\n>> +\t}\n>> +\n>> +\tstreamName_.clear();\n>> +\n>> +\tfor (unsigned int index = 0; index < config_->size(); ++index) {\n>> +\t\tStreamConfiguration &cfg = config_->at(index);\n>> +\t\tstreamName_[cfg.stream()] = \"stream\" + std::to_string(index);\n>> +\t}\n>> +\n>> +\tret = camera_->allocateBuffers();\n>> +\tif (ret) {\n>> +\t\tstd::cerr << \"Failed to allocate buffers\" << std::endl;\n>> +\t\treturn ret;\n>> +\t}\n>> +\n>> +\tcamera_->requestCompleted.connect(this, &Capture::requestComplete);\n>> +\n>> +\t/* Identify the stream with the least number of buffers. */\n>> +\tunsigned int nbuffers = UINT_MAX;\n>> +\tfor (StreamConfiguration &cfg : *config_) {\n>> +\t\tStream *stream = cfg.stream();\n>> +\t\tnbuffers = std::min(nbuffers, stream->bufferPool().count());\n>> +\t}\n>> +\n>> +\t/*\n>> +\t * TODO: make cam tool smarter to support still capture by for\n>> +\t * example pushing a button. For now run all streams all the time.\n>> +\t */\n>> +\n>> +\tstd::vector<Request *> requests;\n>> +\tfor (unsigned int i = 0; i < nbuffers; i++) {\n>> +\t\tRequest *request = camera_->createRequest();\n>> +\t\tif (!request) {\n>> +\t\t\tstd::cerr << \"Can't create request\" << std::endl;\n>> +\t\t\tret = -ENOMEM;\n>> +\t\t\tgoto out;\n>> +\t\t}\n>> +\n>> +\t\tstd::map<Stream *, Buffer *> map;\n>> +\t\tfor (StreamConfiguration &cfg : *config_) {\n>> +\t\t\tStream *stream = cfg.stream();\n>> +\t\t\tmap[stream] = &stream->bufferPool().buffers()[i];\n>> +\t\t}\n>> +\n>> +\t\tret = request->setBuffers(map);\n>> +\t\tif (ret < 0) {\n>> +\t\t\tstd::cerr << \"Can't set buffers for request\" << std::endl;\n>> +\t\t\tgoto out;\n>> +\t\t}\n>> +\n>> +\t\trequests.push_back(request);\n>> +\t}\n>> +\n>> +\tret = camera_->start();\n>> +\tif (ret) {\n>> +\t\tstd::cout << \"Failed to start capture\" << std::endl;\n>> +\t\tgoto out;\n>> +\t}\n>> +\n>> +\tfor (Request *request : requests) {\n>> +\t\tret = camera_->queueRequest(request);\n>> +\t\tif (ret < 0) {\n>> +\t\t\tstd::cerr << \"Can't queue request\" << std::endl;\n>> +\t\t\tgoto out;\n>> +\t\t}\n>> +\t}\n>> +\n>> +\tstd::cout << \"Capture until user interrupts by SIGINT\" << std::endl;\n>> +\tret = loop->exec();\n>> +\n>> +\tret = camera_->stop();\n>> +\tif (ret)\n>> +\t\tstd::cout << \"Failed to stop capture\" << std::endl;\n>> +out:\n>> +\tcamera_->freeBuffers();\n> \n> Maybe a config_.reset() here to make it explicit that we don't need the\n> config anymore ? Otherwise this will result in a reference to the camera\n> being stored internally (at least for some pipeline handlers), and if\n> the Capture class doesn't get deleted before the camera manager is\n> stopped, there will be a warning.\n> \n>> +\n>> +\treturn ret;\n>> +}\n>> +\n>> +void Capture::requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers)\n>> +{\n>> +\tstatic uint64_t now, last = 0;\n> \n> You can now move these to member variables :-)\n> \n>> +\tdouble fps = 0.0;\n>> +\n>> +\tif (request->status() == Request::RequestCancelled)\n>> +\t\treturn;\n>> +\n>> +\tstruct timespec time;\n>> +\tclock_gettime(CLOCK_MONOTONIC, &time);\n>> +\tnow = time.tv_sec * 1000 + time.tv_nsec / 1000000;\n>> +\tfps = now - last;\n>> +\tfps = last && fps ? 1000.0 / fps : 0.0;\n>> +\tlast = now;\n>> +\n>> +\tstd::stringstream info;\n>> +\tinfo << \"fps: \" << std::fixed << std::setprecision(2) << fps;\n>> +\n>> +\tfor (auto it = buffers.begin(); it != buffers.end(); ++it) {\n>> +\t\tStream *stream = it->first;\n>> +\t\tBuffer *buffer = it->second;\n>> +\t\tconst std::string &name = streamName_[stream];\n>> +\n>> +\t\tinfo << \" \" << name\n>> +\t\t     << \" (\" << buffer->index() << \")\"\n>> +\t\t     << \" seq: \" << std::setw(6) << std::setfill('0') << buffer->sequence()\n>> +\t\t     << \" bytesused: \" << buffer->bytesused();\n>> +\n>> +\t\tif (writer_)\n>> +\t\t\twriter_->write(buffer, name);\n>> +\t}\n>> +\n>> +\tstd::cout << info.str() << std::endl;\n>> +\n>> +\trequest = camera_->createRequest();\n>> +\tif (!request) {\n>> +\t\tstd::cerr << \"Can't create request\" << std::endl;\n>> +\t\treturn;\n>> +\t}\n>> +\n>> +\trequest->setBuffers(buffers);\n>> +\tcamera_->queueRequest(request);\n>> +}\n>> diff --git a/src/cam/capture.h b/src/cam/capture.h\n>> new file mode 100644\n>> index 0000000000000000..728b1d22b159b046\n>> --- /dev/null\n>> +++ b/src/cam/capture.h\n>> @@ -0,0 +1,42 @@\n>> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n>> +/*\n>> + * Copyright (C) 2019, Google Inc.\n>> + *\n>> + * capture.h - Cam capture\n>> + */\n>> +#ifndef __CAM_CAPTURE_H__\n>> +#define __CAM_CAPTURE_H__\n>> +\n>> +#include <memory>\n>> +\n>> +#include <libcamera/camera.h>\n>> +#include <libcamera/request.h>\n>> +#include <libcamera/stream.h>\n>> +\n>> +#include \"buffer_writer.h\"\n>> +#include \"event_loop.h\"\n>> +#include \"options.h\"\n>> +\n>> +class Capture\n>> +{\n>> +public:\n>> +\tCapture();\n>> +\n>> +\tint run(libcamera::Camera *camera, EventLoop *loop,\n>> +\t\tconst OptionsParser::Options &options);\n> \n> I like how the options are passed as a const reference.\n> \n>> +private:\n>> +\tint prepareConfig(const OptionsParser::Options &options);\n>> +\n>> +\tint capture(EventLoop *loop);\n>> +\n>> +\tvoid requestComplete(libcamera::Request *request,\n>> +\t\t\t     const std::map<libcamera::Stream *, libcamera::Buffer *> &buffers);\n>> +\n>> +\tlibcamera::Camera *camera_;\n>> +\tstd::unique_ptr<libcamera::CameraConfiguration> config_;\n>> +\n>> +\tstd::map<libcamera::Stream *, std::string> streamName_;\n>> +\tBufferWriter *writer_;\n>> +};\n>> +\n>> +#endif /* __CAM_CAPTURE_H__ */\n>> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n>> index 5ecd7e0e38d768bb..fe7d4f90dbf14ffd 100644\n>> --- a/src/cam/main.cpp\n>> +++ b/src/cam/main.cpp\n>> @@ -5,37 +5,22 @@\n>>   * main.cpp - cam - The libcamera swiss army knife\n>>   */\n>>  \n>> -#include <algorithm>\n>> -#include <iomanip>\n>>  #include <iostream>\n>> -#include <limits.h>\n>> -#include <map>\n>>  #include <signal.h>\n>> -#include <sstream>\n>>  #include <string.h>\n>>  \n>>  #include <libcamera/libcamera.h>\n>>  \n>> -#include \"buffer_writer.h\"\n>> +#include \"capture.h\"\n>>  #include \"event_loop.h\"\n>> +#include \"main.h\"\n>>  #include \"options.h\"\n>>  \n>>  using namespace libcamera;\n>>  \n>>  OptionsParser::Options options;\n>>  std::shared_ptr<Camera> camera;\n>> -std::map<Stream *, std::string> streamInfo;\n>>  EventLoop *loop;\n>> -BufferWriter *writer;\n>> -\n>> -enum {\n>> -\tOptCamera = 'c',\n>> -\tOptCapture = 'C',\n>> -\tOptFile = 'F',\n>> -\tOptHelp = 'h',\n>> -\tOptList = 'l',\n>> -\tOptStream = 's',\n>> -};\n>>  \n>>  void signalHandler(int signal)\n>>  {\n>> @@ -85,201 +70,6 @@ static int parseOptions(int argc, char *argv[])\n>>  \treturn 0;\n>>  }\n>>  \n>> -static std::unique_ptr<CameraConfiguration> prepareCameraConfig()\n>> -{\n>> -\tStreamRoles roles;\n>> -\n>> -\t/* If no configuration is provided assume a single video stream. */\n>> -\tif (!options.isSet(OptStream))\n>> -\t\treturn camera->generateConfiguration({ StreamRole::VideoRecording });\n>> -\n>> -\tconst std::vector<OptionValue> &streamOptions =\n>> -\t\toptions[OptStream].toArray();\n>> -\n>> -\t/* Use roles and get a default configuration. */\n>> -\tfor (auto const &value : streamOptions) {\n>> -\t\tKeyValueParser::Options conf = value.toKeyValues();\n>> -\n>> -\t\tif (!conf.isSet(\"role\")) {\n>> -\t\t\troles.push_back(StreamRole::VideoRecording);\n>> -\t\t} else if (conf[\"role\"].toString() == \"viewfinder\") {\n>> -\t\t\troles.push_back(StreamRole::Viewfinder);\n>> -\t\t} else if (conf[\"role\"].toString() == \"video\") {\n>> -\t\t\troles.push_back(StreamRole::VideoRecording);\n>> -\t\t} else if (conf[\"role\"].toString() == \"still\") {\n>> -\t\t\troles.push_back(StreamRole::StillCapture);\n>> -\t\t} else {\n>> -\t\t\tstd::cerr << \"Unknown stream role \"\n>> -\t\t\t\t  << conf[\"role\"].toString() << std::endl;\n>> -\t\t\treturn nullptr;\n>> -\t\t}\n>> -\t}\n>> -\n>> -\tstd::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles);\n>> -\tif (!config || config->size() != roles.size()) {\n>> -\t\tstd::cerr << \"Failed to get default stream configuration\"\n>> -\t\t\t  << std::endl;\n>> -\t\treturn nullptr;\n>> -\t}\n>> -\n>> -\t/* Apply configuration explicitly requested. */\n>> -\tunsigned int i = 0;\n>> -\tfor (auto const &value : streamOptions) {\n>> -\t\tKeyValueParser::Options conf = value.toKeyValues();\n>> -\t\tStreamConfiguration &cfg = config->at(i++);\n>> -\n>> -\t\tif (conf.isSet(\"width\"))\n>> -\t\t\tcfg.size.width = conf[\"width\"];\n>> -\n>> -\t\tif (conf.isSet(\"height\"))\n>> -\t\t\tcfg.size.height = conf[\"height\"];\n>> -\n>> -\t\t/* TODO: Translate 4CC string to ID. */\n>> -\t\tif (conf.isSet(\"pixelformat\"))\n>> -\t\t\tcfg.pixelFormat = conf[\"pixelformat\"];\n>> -\t}\n>> -\n>> -\treturn config;\n>> -}\n>> -\n>> -static void requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers)\n>> -{\n>> -\tstatic uint64_t now, last = 0;\n>> -\tdouble fps = 0.0;\n>> -\n>> -\tif (request->status() == Request::RequestCancelled)\n>> -\t\treturn;\n>> -\n>> -\tstruct timespec time;\n>> -\tclock_gettime(CLOCK_MONOTONIC, &time);\n>> -\tnow = time.tv_sec * 1000 + time.tv_nsec / 1000000;\n>> -\tfps = now - last;\n>> -\tfps = last && fps ? 1000.0 / fps : 0.0;\n>> -\tlast = now;\n>> -\n>> -\tstd::stringstream info;\n>> -\tinfo << \"fps: \" << std::fixed << std::setprecision(2) << fps;\n>> -\n>> -\tfor (auto it = buffers.begin(); it != buffers.end(); ++it) {\n>> -\t\tStream *stream = it->first;\n>> -\t\tBuffer *buffer = it->second;\n>> -\t\tconst std::string &name = streamInfo[stream];\n>> -\n>> -\t\tinfo << \" \" << name\n>> -\t\t     << \" (\" << buffer->index() << \")\"\n>> -\t\t     << \" seq: \" << std::setw(6) << std::setfill('0') << buffer->sequence()\n>> -\t\t     << \" bytesused: \" << buffer->bytesused();\n>> -\n>> -\t\tif (writer)\n>> -\t\t\twriter->write(buffer, name);\n>> -\t}\n>> -\n>> -\tstd::cout << info.str() << std::endl;\n>> -\n>> -\trequest = camera->createRequest();\n>> -\tif (!request) {\n>> -\t\tstd::cerr << \"Can't create request\" << std::endl;\n>> -\t\treturn;\n>> -\t}\n>> -\n>> -\trequest->setBuffers(buffers);\n>> -\tcamera->queueRequest(request);\n>> -}\n>> -\n>> -static int capture()\n>> -{\n>> -\tint ret;\n>> -\n>> -\tstd::unique_ptr<CameraConfiguration> config = prepareCameraConfig();\n>> -\tif (!config) {\n>> -\t\tstd::cout << \"Failed to prepare camera configuration\" << std::endl;\n>> -\t\treturn -EINVAL;\n>> -\t}\n>> -\n>> -\tret = camera->configure(config.get());\n>> -\tif (ret < 0) {\n>> -\t\tstd::cout << \"Failed to configure camera\" << std::endl;\n>> -\t\treturn ret;\n>> -\t}\n>> -\n>> -\tstreamInfo.clear();\n>> -\n>> -\tfor (unsigned int index = 0; index < config->size(); ++index) {\n>> -\t\tStreamConfiguration &cfg = config->at(index);\n>> -\t\tstreamInfo[cfg.stream()] = \"stream\" + std::to_string(index);\n>> -\t}\n>> -\n>> -\tret = camera->allocateBuffers();\n>> -\tif (ret) {\n>> -\t\tstd::cerr << \"Failed to allocate buffers\"\n>> -\t\t\t  << std::endl;\n>> -\t\treturn ret;\n>> -\t}\n>> -\n>> -\tcamera->requestCompleted.connect(requestComplete);\n>> -\n>> -\t/* Identify the stream with the least number of buffers. */\n>> -\tunsigned int nbuffers = UINT_MAX;\n>> -\tfor (StreamConfiguration &cfg : *config) {\n>> -\t\tStream *stream = cfg.stream();\n>> -\t\tnbuffers = std::min(nbuffers, stream->bufferPool().count());\n>> -\t}\n>> -\n>> -\t/*\n>> -\t * TODO: make cam tool smarter to support still capture by for\n>> -\t * example pushing a button. For now run all streams all the time.\n>> -\t */\n>> -\n>> -\tstd::vector<Request *> requests;\n>> -\tfor (unsigned int i = 0; i < nbuffers; i++) {\n>> -\t\tRequest *request = camera->createRequest();\n>> -\t\tif (!request) {\n>> -\t\t\tstd::cerr << \"Can't create request\" << std::endl;\n>> -\t\t\tret = -ENOMEM;\n>> -\t\t\tgoto out;\n>> -\t\t}\n>> -\n>> -\t\tstd::map<Stream *, Buffer *> map;\n>> -\t\tfor (StreamConfiguration &cfg : *config) {\n>> -\t\t\tStream *stream = cfg.stream();\n>> -\t\t\tmap[stream] = &stream->bufferPool().buffers()[i];\n>> -\t\t}\n>> -\n>> -\t\tret = request->setBuffers(map);\n>> -\t\tif (ret < 0) {\n>> -\t\t\tstd::cerr << \"Can't set buffers for request\" << std::endl;\n>> -\t\t\tgoto out;\n>> -\t\t}\n>> -\n>> -\t\trequests.push_back(request);\n>> -\t}\n>> -\n>> -\tret = camera->start();\n>> -\tif (ret) {\n>> -\t\tstd::cout << \"Failed to start capture\" << std::endl;\n>> -\t\tgoto out;\n>> -\t}\n>> -\n>> -\tfor (Request *request : requests) {\n>> -\t\tret = camera->queueRequest(request);\n>> -\t\tif (ret < 0) {\n>> -\t\t\tstd::cerr << \"Can't queue request\" << std::endl;\n>> -\t\t\tgoto out;\n>> -\t\t}\n>> -\t}\n>> -\n>> -\tstd::cout << \"Capture until user interrupts by SIGINT\" << std::endl;\n>> -\tret = loop->exec();\n>> -\n>> -\tret = camera->stop();\n>> -\tif (ret)\n>> -\t\tstd::cout << \"Failed to stop capture\" << std::endl;\n>> -out:\n>> -\tcamera->freeBuffers();\n>> -\n>> -\treturn ret;\n>> -}\n>> -\n>>  int main(int argc, char **argv)\n>>  {\n>>  \tint ret;\n>> @@ -327,26 +117,8 @@ int main(int argc, char **argv)\n>>  \t}\n>>  \n>>  \tif (options.isSet(OptCapture)) {\n>> -\t\tif (!camera) {\n>> -\t\t\tstd::cout << \"Can't capture without a camera\"\n>> -\t\t\t\t  << std::endl;\n>> -\t\t\tret = EXIT_FAILURE;\n>> -\t\t\tgoto out;\n>> -\t\t}\n>> -\n>> -\t\tif (options.isSet(OptFile)) {\n>> -\t\t\tif (!options[OptFile].toString().empty())\n>> -\t\t\t\twriter = new BufferWriter(options[OptFile]);\n>> -\t\t\telse\n>> -\t\t\t\twriter = new BufferWriter();\n>> -\t\t}\n>> -\n>> -\t\tcapture();\n>> -\n>> -\t\tif (options.isSet(OptFile)) {\n>> -\t\t\tdelete writer;\n>> -\t\t\twriter = nullptr;\n>> -\t\t}\n>> +\t\tCapture capture;\n>> +\t\tret = capture.run(camera.get(), loop, options);\n> \n> Should we move the event loop to the Capture class ? We may add more\n> users of the loop here, but in that case I think loop->exec() should\n> stay in main() (which would require splitting Capture::run() into a\n> start and stop).\n> \n>>  \t}\n>>  \n>>  \tif (camera) {\n>> diff --git a/src/cam/main.h b/src/cam/main.h\n>> new file mode 100644\n>> index 0000000000000000..a48bde620dc957f0\n>> --- /dev/null\n>> +++ b/src/cam/main.h\n>> @@ -0,0 +1,19 @@\n>> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n>> +/*\n>> + * Copyright (C) 2019, Google Inc.\n>> + *\n>> + * main.h - Cam application\n>> + */\n>> +#ifndef __CAM_MAIN_H__\n>> +#define __CAM_MAIN_H__\n>> +\n>> +enum {\n>> +\tOptCamera = 'c',\n>> +\tOptCapture = 'C',\n>> +\tOptFile = 'F',\n>> +\tOptHelp = 'h',\n>> +\tOptList = 'l',\n>> +\tOptStream = 's',\n>> +};\n>> +\n>> +#endif /* __CAM_CAPTURE_H__ */\n> \n> __CAM_MAIN_H__\n> \n>> diff --git a/src/cam/meson.build b/src/cam/meson.build\n>> index 851295091d0d5132..6d27b57393584fac 100644\n>> --- a/src/cam/meson.build\n>> +++ b/src/cam/meson.build\n>> @@ -1,5 +1,6 @@\n>>  cam_sources = files([\n>>      'buffer_writer.cpp',\n>> +    'capture.cpp',\n>>      'event_loop.cpp',\n>>      'main.cpp',\n>>      'options.cpp',\n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DB1E660003\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 23 May 2019 11:32:21 +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 EBDE9334;\n\tThu, 23 May 2019 11:32:18 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1558603939;\n\tbh=cT+R/gDEivaScB7sMU0DnAQ2QyH0iLHkIRYjgZG47QQ=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=PImFjCKhjpY9lOyLeIhOsVo3agZLAoFbHXMcj9U+/paknNLZqFfbe4FSjxY5AruGh\n\t/fXLgyUfIhDKONtd3rtTDH5D+DhQh6/k93AnPZAncJGCO9DfbTJp+LLcZfn4+oalCj\n\tyFOgiqGaeGw7eahj6B3CsEPM15fIRGBw720/5tDM=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>, =?utf-8?q?Niklas?=\n\t=?utf-8?q?_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20190523005534.9631-1-niklas.soderlund@ragnatech.se>\n\t<20190523005534.9631-2-niklas.soderlund@ragnatech.se>\n\t<20190523092935.GH4745@pendragon.ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<dd630abd-7309-c629-5856-0c20c5ae7a30@ideasonboard.com>","Date":"Thu, 23 May 2019 10:32:16 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.6.1","MIME-Version":"1.0","In-Reply-To":"<20190523092935.GH4745@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 1/2] cam: capture: Break out capture\n\tto a new class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 23 May 2019 09:32:22 -0000"}}]