[{"id":10895,"web_url":"https://patchwork.libcamera.org/comment/10895/","msgid":"<20200627095945.2ngwfxixmrlc6kth@uno.localdomain>","date":"2020-06-27T09:59:45","subject":"Re: [libcamera-devel] [PATCH 05/13] libcamera: ipu3: imgu: Move the\n\tImgUDevice class to separate files","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"On Sat, Jun 27, 2020 at 05:00:35AM +0200, Niklas Söderlund wrote:\n> In preparation of refactoring the IPU3 pipeline handler breakout the\n> ImgUDevice into its own .cpp and .h file, no functional change.\n>\n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  src/libcamera/pipeline/ipu3/imgu.cpp    | 351 +++++++++++++++++++++\n>  src/libcamera/pipeline/ipu3/imgu.h      |  86 +++++\n>  src/libcamera/pipeline/ipu3/ipu3.cpp    | 399 +-----------------------\n>  src/libcamera/pipeline/ipu3/meson.build |   1 +\n>  4 files changed, 439 insertions(+), 398 deletions(-)\n>  create mode 100644 src/libcamera/pipeline/ipu3/imgu.cpp\n>  create mode 100644 src/libcamera/pipeline/ipu3/imgu.h\n>\n> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp\n> new file mode 100644\n> index 0000000000000000..26a95979d0c7735a\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp\n> @@ -0,0 +1,351 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * imgu.cpp - Intel IPU3 ImgU\n> + */\n> +\n> +#include \"imgu.h\"\n> +\n> +#include <linux/media-bus-format.h>\n> +\n> +#include <libcamera/stream.h>\n> +\n> +#include <libcamera/formats.h>\n> +\n\nToo many empty lines ?\n\n> +#include \"libcamera/internal/log.h\"\n> +#include \"libcamera/internal/media_device.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(IPU3)\n> +\n> +/**\n> + * \\brief Initialize components of the ImgU instance\n> + * \\param[in] mediaDevice The ImgU instance media device\n> + * \\param[in] index The ImgU instance index\n> + *\n> + * Create and open the V4L2 devices and subdevices of the ImgU instance\n> + * with \\a index.\n> + *\n> + * In case of errors the created V4L2VideoDevice and V4L2Subdevice instances\n> + * are destroyed at pipeline handler delete time.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int ImgUDevice::init(MediaDevice *media, unsigned int index)\n> +{\n> +\tint ret;\n> +\n> +\tindex_ = index;\n> +\tname_ = \"ipu3-imgu \" + std::to_string(index_);\n> +\tmedia_ = media;\n> +\n> +\t/*\n> +\t * The media entities presence in the media device has been verified\n> +\t * by the match() function: no need to check for newly created\n> +\t * video devices and subdevice validity here.\n> +\t */\n> +\timgu_ = V4L2Subdevice::fromEntityName(media, name_);\n> +\tret = imgu_->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tinput_ = V4L2VideoDevice::fromEntityName(media, name_ + \" input\");\n> +\tret = input_->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\toutput_.dev = V4L2VideoDevice::fromEntityName(media, name_ + \" output\");\n> +\tret = output_.dev->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\toutput_.pad = PAD_OUTPUT;\n> +\toutput_.name = \"output\";\n> +\n> +\tviewfinder_.dev = V4L2VideoDevice::fromEntityName(media,\n> +\t\t\t\t\t\t\t  name_ + \" viewfinder\");\n> +\tret = viewfinder_.dev->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tviewfinder_.pad = PAD_VF;\n> +\tviewfinder_.name = \"viewfinder\";\n> +\n> +\tstat_.dev = V4L2VideoDevice::fromEntityName(media, name_ + \" 3a stat\");\n> +\tret = stat_.dev->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tstat_.pad = PAD_STAT;\n> +\tstat_.name = \"stat\";\n> +\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\brief Configure the ImgU unit input\n> + * \\param[in] size The ImgU input frame size\n> + * \\param[in] inputFormat The format to be applied to ImgU input\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int ImgUDevice::configureInput(const Size &size,\n> +\t\t\t       V4L2DeviceFormat *inputFormat)\n> +{\n> +\t/* Configure the ImgU input video device with the requested sizes. */\n> +\tint ret = input_->setFormat(inputFormat);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tLOG(IPU3, Debug) << \"ImgU input format = \" << inputFormat->toString();\n> +\n> +\t/*\n> +\t * \\todo The IPU3 driver implementation shall be changed to use the\n> +\t * input sizes as 'ImgU Input' subdevice sizes, and use the desired\n> +\t * GDC output sizes to configure the crop/compose rectangles.\n> +\t *\n> +\t * The current IPU3 driver implementation uses GDC sizes as the\n> +\t * 'ImgU Input' subdevice sizes, and the input video device sizes\n> +\t * to configure the crop/compose rectangles, contradicting the\n> +\t * V4L2 specification.\n> +\t */\n> +\tRectangle rect = {\n> +\t\t.x = 0,\n> +\t\t.y = 0,\n> +\t\t.width = inputFormat->size.width,\n> +\t\t.height = inputFormat->size.height,\n> +\t};\n> +\tret = imgu_->setSelection(PAD_INPUT, V4L2_SEL_TGT_CROP, &rect);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tret = imgu_->setSelection(PAD_INPUT, V4L2_SEL_TGT_COMPOSE, &rect);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tLOG(IPU3, Debug) << \"ImgU input feeder and BDS rectangle = \"\n> +\t\t\t << rect.toString();\n> +\n> +\tV4L2SubdeviceFormat imguFormat = {};\n> +\timguFormat.mbus_code = MEDIA_BUS_FMT_FIXED;\n> +\timguFormat.size = size;\n> +\n> +\tret = imgu_->setFormat(PAD_INPUT, &imguFormat);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tLOG(IPU3, Debug) << \"ImgU GDC format = \" << imguFormat.toString();\n> +\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\brief Configure the ImgU unit \\a id video output\n> + * \\param[in] output The ImgU output device to configure\n> + * \\param[in] cfg The requested configuration\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int ImgUDevice::configureOutput(ImgUOutput *output,\n> +\t\t\t\tconst StreamConfiguration &cfg,\n> +\t\t\t\tV4L2DeviceFormat *outputFormat)\n> +{\n> +\tV4L2VideoDevice *dev = output->dev;\n> +\tunsigned int pad = output->pad;\n> +\n> +\tV4L2SubdeviceFormat imguFormat = {};\n> +\timguFormat.mbus_code = MEDIA_BUS_FMT_FIXED;\n> +\timguFormat.size = cfg.size;\n> +\n> +\tint ret = imgu_->setFormat(pad, &imguFormat);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\t/* No need to apply format to the stat node. */\n> +\tif (output == &stat_)\n> +\t\treturn 0;\n> +\n> +\t*outputFormat = {};\n> +\toutputFormat->fourcc = dev->toV4L2PixelFormat(formats::NV12);\n> +\toutputFormat->size = cfg.size;\n> +\toutputFormat->planesCount = 2;\n> +\n> +\tret = dev->setFormat(outputFormat);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tLOG(IPU3, Debug) << \"ImgU \" << output->name << \" format = \"\n> +\t\t\t << outputFormat->toString();\n> +\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\brief Allocate buffers for all the ImgU video devices\n> + */\n> +int ImgUDevice::allocateBuffers(unsigned int bufferCount)\n> +{\n> +\t/* Share buffers between CIO2 output and ImgU input. */\n> +\tint ret = input_->importBuffers(bufferCount);\n> +\tif (ret) {\n> +\t\tLOG(IPU3, Error) << \"Failed to import ImgU input buffers\";\n> +\t\treturn ret;\n> +\t}\n> +\n> +\t/*\n> +\t * The kernel fails to start if buffers are not either imported or\n> +\t * allocated for the statisitcs video device. As statistics buffers are\n> +\t * not yet used by the pipeline import buffers to save memory.\n> +\t *\n> +\t * \\todo To be revised when we'll actually use the stat node.\n> +\t */\n> +\tret = stat_.dev->importBuffers(bufferCount);\n> +\tif (ret < 0) {\n> +\t\tLOG(IPU3, Error) << \"Failed to allocate ImgU stat buffers\";\n> +\t\tgoto error;\n> +\t}\n> +\n> +\tret = output_.dev->importBuffers(bufferCount);\n> +\tif (ret < 0) {\n> +\t\tLOG(IPU3, Error) << \"Failed to import ImgU output buffers\";\n> +\t\tgoto error;\n> +\t}\n> +\n> +\tret = viewfinder_.dev->importBuffers(bufferCount);\n> +\tif (ret < 0) {\n> +\t\tLOG(IPU3, Error) << \"Failed to import ImgU viewfinder buffers\";\n> +\t\tgoto error;\n> +\t}\n> +\n> +\treturn 0;\n> +\n> +error:\n> +\tfreeBuffers();\n> +\n> +\treturn ret;\n> +}\n> +\n> +/**\n> + * \\brief Release buffers for all the ImgU video devices\n> + */\n> +void ImgUDevice::freeBuffers()\n> +{\n> +\tint ret;\n> +\n> +\tret = output_.dev->releaseBuffers();\n> +\tif (ret)\n> +\t\tLOG(IPU3, Error) << \"Failed to release ImgU output buffers\";\n> +\n> +\tret = stat_.dev->releaseBuffers();\n> +\tif (ret)\n> +\t\tLOG(IPU3, Error) << \"Failed to release ImgU stat buffers\";\n> +\n> +\tret = viewfinder_.dev->releaseBuffers();\n> +\tif (ret)\n> +\t\tLOG(IPU3, Error) << \"Failed to release ImgU viewfinder buffers\";\n> +\n> +\tret = input_->releaseBuffers();\n> +\tif (ret)\n> +\t\tLOG(IPU3, Error) << \"Failed to release ImgU input buffers\";\n> +}\n> +\n> +int ImgUDevice::start()\n> +{\n> +\tint ret;\n> +\n> +\t/* Start the ImgU video devices. */\n> +\tret = output_.dev->streamOn();\n> +\tif (ret) {\n> +\t\tLOG(IPU3, Error) << \"Failed to start ImgU output\";\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tret = viewfinder_.dev->streamOn();\n> +\tif (ret) {\n> +\t\tLOG(IPU3, Error) << \"Failed to start ImgU viewfinder\";\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tret = stat_.dev->streamOn();\n> +\tif (ret) {\n> +\t\tLOG(IPU3, Error) << \"Failed to start ImgU stat\";\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tret = input_->streamOn();\n> +\tif (ret) {\n> +\t\tLOG(IPU3, Error) << \"Failed to start ImgU input\";\n> +\t\treturn ret;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +int ImgUDevice::stop()\n> +{\n> +\tint ret;\n> +\n> +\tret = output_.dev->streamOff();\n> +\tret |= viewfinder_.dev->streamOff();\n> +\tret |= stat_.dev->streamOff();\n> +\tret |= input_->streamOff();\n> +\n> +\treturn ret;\n> +}\n> +\n> +/**\n> + * \\brief Enable or disable a single link on the ImgU instance\n> + *\n> + * This method assumes the media device associated with the ImgU instance\n> + * is open.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int ImgUDevice::linkSetup(const std::string &source, unsigned int sourcePad,\n> +\t\t\t  const std::string &sink, unsigned int sinkPad,\n> +\t\t\t  bool enable)\n> +{\n> +\tMediaLink *link = media_->link(source, sourcePad, sink, sinkPad);\n> +\tif (!link) {\n> +\t\tLOG(IPU3, Error)\n> +\t\t\t<< \"Failed to get link: '\" << source << \"':\"\n> +\t\t\t<< sourcePad << \" -> '\" << sink << \"':\" << sinkPad;\n> +\t\treturn -ENODEV;\n> +\t}\n> +\n> +\treturn link->setEnabled(enable);\n> +}\n> +\n> +/**\n> + * \\brief Enable or disable all media links in the ImgU instance to prepare\n> + * for capture operations\n> + *\n> + * \\todo This method will probably be removed or changed once links will be\n> + * enabled or disabled selectively.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int ImgUDevice::enableLinks(bool enable)\n> +{\n> +\tstd::string viewfinderName = name_ + \" viewfinder\";\n> +\tstd::string outputName = name_ + \" output\";\n> +\tstd::string statName = name_ + \" 3a stat\";\n> +\tstd::string inputName = name_ + \" input\";\n> +\tint ret;\n> +\n> +\tret = linkSetup(inputName, 0, name_, PAD_INPUT, enable);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tret = linkSetup(name_, PAD_OUTPUT, outputName, 0, enable);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tret = linkSetup(name_, PAD_VF, viewfinderName, 0, enable);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\treturn linkSetup(name_, PAD_STAT, statName, 0, enable);\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h\n> new file mode 100644\n> index 0000000000000000..9bb1b88e2ca8ea2d\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/ipu3/imgu.h\n> @@ -0,0 +1,86 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * imgu.h - Intel IPU3 ImgU\n> + */\n> +#ifndef __LIBCAMERA_PIPELINE_IPU3_IMGU_H__\n> +#define __LIBCAMERA_PIPELINE_IPU3_IMGU_H__\n> +\n> +#include <memory>\n> +#include <string>\n> +#include <vector>\n> +\n> +#include \"libcamera/internal/v4l2_subdevice.h\"\n> +#include \"libcamera/internal/v4l2_videodevice.h\"\n> +\n> +namespace libcamera {\n> +\n> +class FrameBuffer;\n> +class MediaDevice;\n> +struct Size;\n> +struct StreamConfiguration;\n> +\n> +class ImgUDevice\n> +{\n> +public:\n> +\tstatic constexpr unsigned int PAD_INPUT = 0;\n> +\tstatic constexpr unsigned int PAD_OUTPUT = 2;\n> +\tstatic constexpr unsigned int PAD_VF = 3;\n> +\tstatic constexpr unsigned int PAD_STAT = 4;\n> +\n> +\t/* ImgU output descriptor: group data specific to an ImgU output. */\n> +\tstruct ImgUOutput {\n> +\t\tV4L2VideoDevice *dev;\n> +\t\tunsigned int pad;\n> +\t\tstd::string name;\n> +\t};\n> +\n> +\tImgUDevice()\n> +\t\t: imgu_(nullptr), input_(nullptr)\n> +\t{\n> +\t\toutput_.dev = nullptr;\n> +\t\tviewfinder_.dev = nullptr;\n> +\t\tstat_.dev = nullptr;\n> +\t}\n> +\n> +\t~ImgUDevice()\n> +\t{\n> +\t\tdelete imgu_;\n> +\t\tdelete input_;\n> +\t\tdelete output_.dev;\n> +\t\tdelete viewfinder_.dev;\n> +\t\tdelete stat_.dev;\n> +\t}\n> +\n> +\tint init(MediaDevice *media, unsigned int index);\n> +\tint configureInput(const Size &size, V4L2DeviceFormat *inputFormat);\n> +\tint configureOutput(ImgUOutput *output, const StreamConfiguration &cfg,\n> +\t\t\t    V4L2DeviceFormat *outputFormat);\n> +\n> +\tint allocateBuffers(unsigned int bufferCount);\n> +\tvoid freeBuffers();\n> +\n> +\tint start();\n> +\tint stop();\n> +\n> +\tint linkSetup(const std::string &source, unsigned int sourcePad,\n> +\t\t      const std::string &sink, unsigned int sinkPad,\n> +\t\t      bool enable);\n> +\tint enableLinks(bool enable);\n> +\n> +\tunsigned int index_;\n> +\tstd::string name_;\n> +\tMediaDevice *media_;\n> +\n> +\tV4L2Subdevice *imgu_;\n> +\tV4L2VideoDevice *input_;\n> +\tImgUOutput output_;\n> +\tImgUOutput viewfinder_;\n> +\tImgUOutput stat_;\n> +\t/* \\todo Add param video device for 3A tuning */\n> +};\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_PIPELINE_IPU3_IMGU_H__ */\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 36413a824ebbf564..b41a789e8dc2a7b2 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -11,8 +11,6 @@\n>  #include <queue>\n>  #include <vector>\n>\n> -#include <linux/media-bus-format.h>\n> -\n>  #include <libcamera/camera.h>\n>  #include <libcamera/formats.h>\n>  #include <libcamera/request.h>\n> @@ -25,77 +23,14 @@\n>  #include \"libcamera/internal/pipeline_handler.h\"\n>  #include \"libcamera/internal/utils.h\"\n>  #include \"libcamera/internal/v4l2_controls.h\"\n> -#include \"libcamera/internal/v4l2_subdevice.h\"\n> -#include \"libcamera/internal/v4l2_videodevice.h\"\n>\n>  #include \"cio2.h\"\n> +#include \"imgu.h\"\n>\n>  namespace libcamera {\n>\n>  LOG_DEFINE_CATEGORY(IPU3)\n>\n> -class ImgUDevice\n> -{\n> -public:\n> -\tstatic constexpr unsigned int PAD_INPUT = 0;\n> -\tstatic constexpr unsigned int PAD_OUTPUT = 2;\n> -\tstatic constexpr unsigned int PAD_VF = 3;\n> -\tstatic constexpr unsigned int PAD_STAT = 4;\n> -\n> -\t/* ImgU output descriptor: group data specific to an ImgU output. */\n> -\tstruct ImgUOutput {\n> -\t\tV4L2VideoDevice *dev;\n> -\t\tunsigned int pad;\n> -\t\tstd::string name;\n> -\t};\n> -\n> -\tImgUDevice()\n> -\t\t: imgu_(nullptr), input_(nullptr)\n> -\t{\n> -\t\toutput_.dev = nullptr;\n> -\t\tviewfinder_.dev = nullptr;\n> -\t\tstat_.dev = nullptr;\n> -\t}\n> -\n> -\t~ImgUDevice()\n> -\t{\n> -\t\tdelete imgu_;\n> -\t\tdelete input_;\n> -\t\tdelete output_.dev;\n> -\t\tdelete viewfinder_.dev;\n> -\t\tdelete stat_.dev;\n> -\t}\n> -\n> -\tint init(MediaDevice *media, unsigned int index);\n> -\tint configureInput(const Size &size,\n> -\t\t\t   V4L2DeviceFormat *inputFormat);\n> -\tint configureOutput(ImgUOutput *output,\n> -\t\t\t    const StreamConfiguration &cfg,\n> -\t\t\t    V4L2DeviceFormat *outputFormat);\n> -\n> -\tint allocateBuffers(unsigned int bufferCount);\n> -\tvoid freeBuffers();\n> -\n> -\tint start();\n> -\tint stop();\n> -\n> -\tint linkSetup(const std::string &source, unsigned int sourcePad,\n> -\t\t      const std::string &sink, unsigned int sinkPad,\n> -\t\t      bool enable);\n> -\tint enableLinks(bool enable);\n> -\n> -\tunsigned int index_;\n> -\tstd::string name_;\n> -\tMediaDevice *media_;\n> -\n> -\tV4L2Subdevice *imgu_;\n> -\tV4L2VideoDevice *input_;\n> -\tImgUOutput output_;\n> -\tImgUOutput viewfinder_;\n> -\tImgUOutput stat_;\n> -\t/* \\todo Add param video device for 3A tuning */\n> -};\n> -\n>  class IPU3Stream : public Stream\n>  {\n>  public:\n> @@ -952,338 +887,6 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n>  \timgu_->input_->queueBuffer(buffer);\n>  }\n>\n> -/* -----------------------------------------------------------------------------\n> - * ImgU Device\n> - */\n> -\n> -/**\n> - * \\brief Initialize components of the ImgU instance\n> - * \\param[in] mediaDevice The ImgU instance media device\n> - * \\param[in] index The ImgU instance index\n> - *\n> - * Create and open the V4L2 devices and subdevices of the ImgU instance\n> - * with \\a index.\n> - *\n> - * In case of errors the created V4L2VideoDevice and V4L2Subdevice instances\n> - * are destroyed at pipeline handler delete time.\n> - *\n> - * \\return 0 on success or a negative error code otherwise\n> - */\n> -int ImgUDevice::init(MediaDevice *media, unsigned int index)\n> -{\n> -\tint ret;\n> -\n> -\tindex_ = index;\n> -\tname_ = \"ipu3-imgu \" + std::to_string(index_);\n> -\tmedia_ = media;\n> -\n> -\t/*\n> -\t * The media entities presence in the media device has been verified\n> -\t * by the match() function: no need to check for newly created\n> -\t * video devices and subdevice validity here.\n> -\t */\n> -\timgu_ = V4L2Subdevice::fromEntityName(media, name_);\n> -\tret = imgu_->open();\n> -\tif (ret)\n> -\t\treturn ret;\n> -\n> -\tinput_ = V4L2VideoDevice::fromEntityName(media, name_ + \" input\");\n> -\tret = input_->open();\n> -\tif (ret)\n> -\t\treturn ret;\n> -\n> -\toutput_.dev = V4L2VideoDevice::fromEntityName(media, name_ + \" output\");\n> -\tret = output_.dev->open();\n> -\tif (ret)\n> -\t\treturn ret;\n> -\n> -\toutput_.pad = PAD_OUTPUT;\n> -\toutput_.name = \"output\";\n> -\n> -\tviewfinder_.dev = V4L2VideoDevice::fromEntityName(media,\n> -\t\t\t\t\t\t\t  name_ + \" viewfinder\");\n> -\tret = viewfinder_.dev->open();\n> -\tif (ret)\n> -\t\treturn ret;\n> -\n> -\tviewfinder_.pad = PAD_VF;\n> -\tviewfinder_.name = \"viewfinder\";\n> -\n> -\tstat_.dev = V4L2VideoDevice::fromEntityName(media, name_ + \" 3a stat\");\n> -\tret = stat_.dev->open();\n> -\tif (ret)\n> -\t\treturn ret;\n> -\n> -\tstat_.pad = PAD_STAT;\n> -\tstat_.name = \"stat\";\n> -\n> -\treturn 0;\n> -}\n> -\n> -/**\n> - * \\brief Configure the ImgU unit input\n> - * \\param[in] size The ImgU input frame size\n> - * \\param[in] inputFormat The format to be applied to ImgU input\n> - * \\return 0 on success or a negative error code otherwise\n> - */\n> -int ImgUDevice::configureInput(const Size &size,\n> -\t\t\t       V4L2DeviceFormat *inputFormat)\n> -{\n> -\t/* Configure the ImgU input video device with the requested sizes. */\n> -\tint ret = input_->setFormat(inputFormat);\n> -\tif (ret)\n> -\t\treturn ret;\n> -\n> -\tLOG(IPU3, Debug) << \"ImgU input format = \" << inputFormat->toString();\n> -\n> -\t/*\n> -\t * \\todo The IPU3 driver implementation shall be changed to use the\n> -\t * input sizes as 'ImgU Input' subdevice sizes, and use the desired\n> -\t * GDC output sizes to configure the crop/compose rectangles.\n> -\t *\n> -\t * The current IPU3 driver implementation uses GDC sizes as the\n> -\t * 'ImgU Input' subdevice sizes, and the input video device sizes\n> -\t * to configure the crop/compose rectangles, contradicting the\n> -\t * V4L2 specification.\n> -\t */\n> -\tRectangle rect = {\n> -\t\t.x = 0,\n> -\t\t.y = 0,\n> -\t\t.width = inputFormat->size.width,\n> -\t\t.height = inputFormat->size.height,\n> -\t};\n> -\tret = imgu_->setSelection(PAD_INPUT, V4L2_SEL_TGT_CROP, &rect);\n> -\tif (ret)\n> -\t\treturn ret;\n> -\n> -\tret = imgu_->setSelection(PAD_INPUT, V4L2_SEL_TGT_COMPOSE, &rect);\n> -\tif (ret)\n> -\t\treturn ret;\n> -\n> -\tLOG(IPU3, Debug) << \"ImgU input feeder and BDS rectangle = \"\n> -\t\t\t << rect.toString();\n> -\n> -\tV4L2SubdeviceFormat imguFormat = {};\n> -\timguFormat.mbus_code = MEDIA_BUS_FMT_FIXED;\n> -\timguFormat.size = size;\n> -\n> -\tret = imgu_->setFormat(PAD_INPUT, &imguFormat);\n> -\tif (ret)\n> -\t\treturn ret;\n> -\n> -\tLOG(IPU3, Debug) << \"ImgU GDC format = \" << imguFormat.toString();\n> -\n> -\treturn 0;\n> -}\n> -\n> -/**\n> - * \\brief Configure the ImgU unit \\a id video output\n> - * \\param[in] output The ImgU output device to configure\n> - * \\param[in] cfg The requested configuration\n> - * \\return 0 on success or a negative error code otherwise\n> - */\n> -int ImgUDevice::configureOutput(ImgUOutput *output,\n> -\t\t\t\tconst StreamConfiguration &cfg,\n> -\t\t\t\tV4L2DeviceFormat *outputFormat)\n> -{\n> -\tV4L2VideoDevice *dev = output->dev;\n> -\tunsigned int pad = output->pad;\n> -\n> -\tV4L2SubdeviceFormat imguFormat = {};\n> -\timguFormat.mbus_code = MEDIA_BUS_FMT_FIXED;\n> -\timguFormat.size = cfg.size;\n> -\n> -\tint ret = imgu_->setFormat(pad, &imguFormat);\n> -\tif (ret)\n> -\t\treturn ret;\n> -\n> -\t/* No need to apply format to the stat node. */\n> -\tif (output == &stat_)\n> -\t\treturn 0;\n> -\n> -\t*outputFormat = {};\n> -\toutputFormat->fourcc = dev->toV4L2PixelFormat(formats::NV12);\n> -\toutputFormat->size = cfg.size;\n> -\toutputFormat->planesCount = 2;\n> -\n> -\tret = dev->setFormat(outputFormat);\n> -\tif (ret)\n> -\t\treturn ret;\n> -\n> -\tLOG(IPU3, Debug) << \"ImgU \" << output->name << \" format = \"\n> -\t\t\t << outputFormat->toString();\n> -\n> -\treturn 0;\n> -}\n> -\n> -/**\n> - * \\brief Allocate buffers for all the ImgU video devices\n> - */\n> -int ImgUDevice::allocateBuffers(unsigned int bufferCount)\n> -{\n> -\t/* Share buffers between CIO2 output and ImgU input. */\n> -\tint ret = input_->importBuffers(bufferCount);\n> -\tif (ret) {\n> -\t\tLOG(IPU3, Error) << \"Failed to import ImgU input buffers\";\n> -\t\treturn ret;\n> -\t}\n> -\n> -\t/*\n> -\t * The kernel fails to start if buffers are not either imported or\n> -\t * allocated for the statisitcs video device. As statistics buffers are\n> -\t * not yet used by the pipeline import buffers to save memory.\n> -\t *\n> -\t * \\todo To be revised when we'll actually use the stat node.\n> -\t */\n> -\tret = stat_.dev->importBuffers(bufferCount);\n> -\tif (ret < 0) {\n> -\t\tLOG(IPU3, Error) << \"Failed to allocate ImgU stat buffers\";\n> -\t\tgoto error;\n> -\t}\n> -\n> -\tret = output_.dev->importBuffers(bufferCount);\n> -\tif (ret < 0) {\n> -\t\tLOG(IPU3, Error) << \"Failed to import ImgU output buffers\";\n> -\t\tgoto error;\n> -\t}\n> -\n> -\tret = viewfinder_.dev->importBuffers(bufferCount);\n> -\tif (ret < 0) {\n> -\t\tLOG(IPU3, Error) << \"Failed to import ImgU viewfinder buffers\";\n> -\t\tgoto error;\n> -\t}\n> -\n> -\treturn 0;\n> -\n> -error:\n> -\tfreeBuffers();\n> -\n> -\treturn ret;\n> -}\n> -\n> -/**\n> - * \\brief Release buffers for all the ImgU video devices\n> - */\n> -void ImgUDevice::freeBuffers()\n> -{\n> -\tint ret;\n> -\n> -\tret = output_.dev->releaseBuffers();\n> -\tif (ret)\n> -\t\tLOG(IPU3, Error) << \"Failed to release ImgU output buffers\";\n> -\n> -\tret = stat_.dev->releaseBuffers();\n> -\tif (ret)\n> -\t\tLOG(IPU3, Error) << \"Failed to release ImgU stat buffers\";\n> -\n> -\tret = viewfinder_.dev->releaseBuffers();\n> -\tif (ret)\n> -\t\tLOG(IPU3, Error) << \"Failed to release ImgU viewfinder buffers\";\n> -\n> -\tret = input_->releaseBuffers();\n> -\tif (ret)\n> -\t\tLOG(IPU3, Error) << \"Failed to release ImgU input buffers\";\n> -}\n> -\n> -int ImgUDevice::start()\n> -{\n> -\tint ret;\n> -\n> -\t/* Start the ImgU video devices. */\n> -\tret = output_.dev->streamOn();\n> -\tif (ret) {\n> -\t\tLOG(IPU3, Error) << \"Failed to start ImgU output\";\n> -\t\treturn ret;\n> -\t}\n> -\n> -\tret = viewfinder_.dev->streamOn();\n> -\tif (ret) {\n> -\t\tLOG(IPU3, Error) << \"Failed to start ImgU viewfinder\";\n> -\t\treturn ret;\n> -\t}\n> -\n> -\tret = stat_.dev->streamOn();\n> -\tif (ret) {\n> -\t\tLOG(IPU3, Error) << \"Failed to start ImgU stat\";\n> -\t\treturn ret;\n> -\t}\n> -\n> -\tret = input_->streamOn();\n> -\tif (ret) {\n> -\t\tLOG(IPU3, Error) << \"Failed to start ImgU input\";\n> -\t\treturn ret;\n> -\t}\n> -\n> -\treturn 0;\n> -}\n> -\n> -int ImgUDevice::stop()\n> -{\n> -\tint ret;\n> -\n> -\tret = output_.dev->streamOff();\n> -\tret |= viewfinder_.dev->streamOff();\n> -\tret |= stat_.dev->streamOff();\n> -\tret |= input_->streamOff();\n> -\n> -\treturn ret;\n> -}\n> -\n> -/**\n> - * \\brief Enable or disable a single link on the ImgU instance\n> - *\n> - * This method assumes the media device associated with the ImgU instance\n> - * is open.\n> - *\n> - * \\return 0 on success or a negative error code otherwise\n> - */\n> -int ImgUDevice::linkSetup(const std::string &source, unsigned int sourcePad,\n> -\t\t\t  const std::string &sink, unsigned int sinkPad,\n> -\t\t\t  bool enable)\n> -{\n> -\tMediaLink *link = media_->link(source, sourcePad, sink, sinkPad);\n> -\tif (!link) {\n> -\t\tLOG(IPU3, Error)\n> -\t\t\t<< \"Failed to get link: '\" << source << \"':\"\n> -\t\t\t<< sourcePad << \" -> '\" << sink << \"':\" << sinkPad;\n> -\t\treturn -ENODEV;\n> -\t}\n> -\n> -\treturn link->setEnabled(enable);\n> -}\n> -\n> -/**\n> - * \\brief Enable or disable all media links in the ImgU instance to prepare\n> - * for capture operations\n> - *\n> - * \\todo This method will probably be removed or changed once links will be\n> - * enabled or disabled selectively.\n> - *\n> - * \\return 0 on success or a negative error code otherwise\n> - */\n> -int ImgUDevice::enableLinks(bool enable)\n> -{\n> -\tstd::string viewfinderName = name_ + \" viewfinder\";\n> -\tstd::string outputName = name_ + \" output\";\n> -\tstd::string statName = name_ + \" 3a stat\";\n> -\tstd::string inputName = name_ + \" input\";\n> -\tint ret;\n> -\n> -\tret = linkSetup(inputName, 0, name_, PAD_INPUT, enable);\n> -\tif (ret)\n> -\t\treturn ret;\n> -\n> -\tret = linkSetup(name_, PAD_OUTPUT, outputName, 0, enable);\n> -\tif (ret)\n> -\t\treturn ret;\n> -\n> -\tret = linkSetup(name_, PAD_VF, viewfinderName, 0, enable);\n> -\tif (ret)\n> -\t\treturn ret;\n> -\n> -\treturn linkSetup(name_, PAD_STAT, statName, 0, enable);\n> -}\n> -\n>  REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3);\n>\n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/ipu3/meson.build b/src/libcamera/pipeline/ipu3/meson.build\n> index b2602d30123f908d..d60e07ae6ccac2bc 100644\n> --- a/src/libcamera/pipeline/ipu3/meson.build\n> +++ b/src/libcamera/pipeline/ipu3/meson.build\n> @@ -2,5 +2,6 @@\n>\n>  libcamera_sources += files([\n>      'cio2.cpp',\n> +    'imgu.cpp',\n>      'ipu3.cpp',\n>  ])\n> --\n> 2.27.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 0C834C2E66\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 27 Jun 2020 09:56:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7C3A9609DD;\n\tSat, 27 Jun 2020 11:56:16 +0200 (CEST)","from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 219F6603B5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 27 Jun 2020 11:56:15 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 4AE2F1BF20B;\n\tSat, 27 Jun 2020 09:56:13 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Sat, 27 Jun 2020 11:59:45 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<20200627095945.2ngwfxixmrlc6kth@uno.localdomain>","References":"<20200627030043.2088585-1-niklas.soderlund@ragnatech.se>\n\t<20200627030043.2088585-6-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200627030043.2088585-6-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH 05/13] libcamera: ipu3: imgu: Move the\n\tImgUDevice class to separate files","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","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>","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\" <libcamera-devel-bounces@lists.libcamera.org>"}}]