From b80c01843cf0228fc51f2429f47eee186ebdcb3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20S=C3=B6derlund?= Date: Sat, 27 Jun 2020 03:32:56 +0200 Subject: libcamera: ipu3: imgu: Use specific functions to configure each sink MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the IPU3 pipeline only provided streams to applications that came from the ImgU it made sense to have a generic function to configure all the different outputs. With the addition of the RAW stream this begins to be cumbersome to read and make sense of in the PipelineHandlerIPU3 code. Replace the generic function that takes a specific argument for which sink to configure with a specific function for each sink. This makes the code easier to follow as it's always clear which of the ImgU sinks are being configured without knowing the content of a generically named variable. It also paves the way for future improvements. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- src/libcamera/pipeline/ipu3/imgu.cpp | 20 ++++++++++---------- src/libcamera/pipeline/ipu3/imgu.h | 28 ++++++++++++++++++++++++++-- src/libcamera/pipeline/ipu3/ipu3.cpp | 31 +++++++++++++++++-------------- 3 files changed, 53 insertions(+), 26 deletions(-) (limited to 'src') diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp index 8a0b77a4..cbe31023 100644 --- a/src/libcamera/pipeline/ipu3/imgu.cpp +++ b/src/libcamera/pipeline/ipu3/imgu.cpp @@ -139,18 +139,17 @@ int ImgUDevice::configureInput(const Size &size, } /** - * \brief Configure the ImgU unit \a id video output - * \param[in] output The ImgU output device to configure + * \brief Configure a video device on the ImgU + * \param[in] dev The video device to configure + * \param[in] pad The pad of the ImgU subdevice * \param[in] cfg The requested configuration + * \param[out] outputFormat The format set on the video device * \return 0 on success or a negative error code otherwise */ -int ImgUDevice::configureOutput(ImgUOutput *output, - const StreamConfiguration &cfg, - V4L2DeviceFormat *outputFormat) +int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, + const StreamConfiguration &cfg, + V4L2DeviceFormat *outputFormat) { - V4L2VideoDevice *dev = output->dev; - unsigned int pad = output->pad; - V4L2SubdeviceFormat imguFormat = {}; imguFormat.mbus_code = MEDIA_BUS_FMT_FIXED; imguFormat.size = cfg.size; @@ -160,7 +159,7 @@ int ImgUDevice::configureOutput(ImgUOutput *output, return ret; /* No need to apply format to the stat node. */ - if (output == &stat_) + if (dev == stat_.dev) return 0; *outputFormat = {}; @@ -172,7 +171,8 @@ int ImgUDevice::configureOutput(ImgUOutput *output, if (ret) return ret; - LOG(IPU3, Debug) << "ImgU " << output->name << " format = " + const char *name = dev == output_.dev ? "output" : "viewfinder"; + LOG(IPU3, Debug) << "ImgU " << name << " format = " << outputFormat->toString(); return 0; diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h index 31f4d75c..2ad2a65d 100644 --- a/src/libcamera/pipeline/ipu3/imgu.h +++ b/src/libcamera/pipeline/ipu3/imgu.h @@ -47,9 +47,29 @@ public: } int init(MediaDevice *media, unsigned int index); + int configureInput(const Size &size, V4L2DeviceFormat *inputFormat); - int configureOutput(ImgUOutput *output, const StreamConfiguration &cfg, - V4L2DeviceFormat *outputFormat); + + int configureOutput(const StreamConfiguration &cfg, + V4L2DeviceFormat *outputFormat) + { + return configureVideoDevice(output_.dev, PAD_OUTPUT, cfg, + outputFormat); + } + + int configureViewfinder(const StreamConfiguration &cfg, + V4L2DeviceFormat *outputFormat) + { + return configureVideoDevice(viewfinder_.dev, PAD_VF, cfg, + outputFormat); + } + + int configureStat(const StreamConfiguration &cfg, + V4L2DeviceFormat *outputFormat) + { + return configureVideoDevice(stat_.dev, PAD_STAT, cfg, + outputFormat); + } int allocateBuffers(unsigned int bufferCount); void freeBuffers(); @@ -76,6 +96,10 @@ private: const std::string &sink, unsigned int sinkPad, bool enable); + int configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, + const StreamConfiguration &cfg, + V4L2DeviceFormat *outputFormat); + std::string name_; MediaDevice *media_; }; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index b41a789e..093d415e 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -504,19 +504,24 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) stream->active_ = true; cfg.setStream(stream); - /* - * The RAW still capture stream just copies buffers from the - * internal queue and doesn't need any specific configuration. - */ - if (stream->raw_) { - cfg.stride = cio2Format.planes[0].bpl; - } else { - ret = imgu->configureOutput(stream->device_, cfg, - &outputFormat); + if (stream == outStream) { + ret = imgu->configureOutput(cfg, &outputFormat); if (ret) return ret; cfg.stride = outputFormat.planes[0].bpl; + } else if (stream == vfStream) { + ret = imgu->configureViewfinder(cfg, &outputFormat); + if (ret) + return ret; + + cfg.stride = outputFormat.planes[0].bpl; + } else { + /* + * The RAW stream is configured as part of the CIO2 and + * no configuration is needed for the ImgU. + */ + cfg.stride = cio2Format.planes[0].bpl; } } @@ -526,15 +531,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) * be at least one active stream in the configuration request). */ if (!outStream->active_) { - ret = imgu->configureOutput(outStream->device_, config->at(0), - &outputFormat); + ret = imgu->configureOutput(config->at(0), &outputFormat); if (ret) return ret; } if (!vfStream->active_) { - ret = imgu->configureOutput(vfStream->device_, config->at(0), - &outputFormat); + ret = imgu->configureViewfinder(config->at(0), &outputFormat); if (ret) return ret; } @@ -546,7 +549,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) StreamConfiguration statCfg = {}; statCfg.size = cio2Format.size; - ret = imgu->configureOutput(&imgu->stat_, statCfg, &outputFormat); + ret = imgu->configureStat(statCfg, &outputFormat); if (ret) return ret; -- cgit v1.2.1