[libcamera-devel] [PATCH v7 06/13] libcamera: ipu3: Apply image format to the pipeline
Niklas Söderlund
niklas.soderlund at ragnatech.se
Tue Apr 2 20:22:28 CEST 2019
Hi Jacopo,
Thanks for your work.
On 2019-04-02 19:13:02 +0200, Jacopo Mondi wrote:
> Apply the requested stream configuration to the CIO2 device, and
> propagate formats through the the ImgU subdevice and its input and
> output video devices.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
I like the end result of this patch, nice work.
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> src/libcamera/pipeline/ipu3/ipu3.cpp | 304 ++++++++++++++++++++++-----
> 1 file changed, 252 insertions(+), 52 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 53e6b3b461a1..57e4bb89eaa7 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -5,6 +5,7 @@
> * ipu3.cpp - Pipeline handler for Intel IPU3
> */
>
> +#include <iomanip>
> #include <memory>
> #include <vector>
>
> @@ -50,22 +51,35 @@ public:
> static constexpr unsigned int PAD_VF = 3;
> static constexpr unsigned int PAD_STAT = 4;
>
> + /* ImgU output descriptor: group data specific to an ImgU output. */
> + struct ImgUOutput {
> + V4L2Device *dev;
> + unsigned int pad;
> + std::string name;
> + };
> +
> ImgUDevice()
> - : imgu_(nullptr), input_(nullptr), output_(nullptr),
> - viewfinder_(nullptr), stat_(nullptr)
> + : imgu_(nullptr), input_(nullptr)
> {
> + output_.dev = nullptr;
> + viewfinder_.dev = nullptr;
> + stat_.dev = nullptr;
> }
>
> ~ImgUDevice()
> {
> delete imgu_;
> delete input_;
> - delete output_;
> - delete viewfinder_;
> - delete stat_;
> + delete output_.dev;
> + delete viewfinder_.dev;
> + delete stat_.dev;
> }
>
> int init(MediaDevice *media, unsigned int index);
> + int configureInput(const StreamConfiguration &config,
> + V4L2DeviceFormat *inputFormat);
> + int configureOutput(const ImgUOutput *output,
> + const StreamConfiguration &config);
>
> unsigned int index_;
> std::string name_;
> @@ -73,9 +87,9 @@ public:
>
> V4L2Subdevice *imgu_;
> V4L2Device *input_;
> - V4L2Device *output_;
> - V4L2Device *viewfinder_;
> - V4L2Device *stat_;
> + ImgUOutput output_;
> + ImgUOutput viewfinder_;
> + ImgUOutput stat_;
> /* \todo Add param video device for 3A tuning */
> };
>
> @@ -95,6 +109,8 @@ public:
> }
>
> int init(const MediaDevice *media, unsigned int index);
> + int configure(const StreamConfiguration &config,
> + V4L2DeviceFormat *format);
>
> V4L2Device *output_;
> V4L2Subdevice *csi2_;
> @@ -204,60 +220,62 @@ int PipelineHandlerIPU3::configureStreams(Camera *camera,
> std::map<Stream *, StreamConfiguration> &config)
> {
> IPU3CameraData *data = cameraData(camera);
> - StreamConfiguration *cfg = &config[&data->stream_];
> - V4L2Subdevice *sensor = data->cio2_.sensor_;
> - V4L2Subdevice *csi2 = data->cio2_.csi2_;
> - V4L2Device *cio2 = data->cio2_.output_;
> - V4L2SubdeviceFormat subdevFormat = {};
> - V4L2DeviceFormat devFormat = {};
> + const StreamConfiguration &cfg = config[&data->stream_];
> + CIO2Device *cio2 = &data->cio2_;
> + ImgUDevice *imgu = data->imgu_;
> int ret;
>
> + LOG(IPU3, Info)
> + << "Requested image format " << cfg.width << "x"
> + << cfg.height << "-0x" << std::hex << std::setfill('0')
> + << std::setw(8) << cfg.pixelFormat << " on camera '"
> + << camera->name() << "'";
> +
> /*
> - * FIXME: as of now, the format gets applied to the sensor and is
> - * propagated along the pipeline. It should instead be applied on the
> - * capture device and the sensor format calculated accordingly.
> + * Verify that the requested size respects the IPU3 alignement
> + * requirements (the image width shall be a multiple of 8 pixels and
> + * its height a multiple of 4 pixels) and the camera maximum sizes.
> + *
> + * \todo: consider the BDS scaling factor requirements:
> + * "the downscaling factor must be an integer value multiple of 1/32"
> */
> + if (cfg.width % 8 || cfg.height % 4) {
> + LOG(IPU3, Error) << "Invalid stream size: bad alignment";
> + return -EINVAL;
> + }
>
> - ret = sensor->getFormat(0, &subdevFormat);
> - if (ret)
> - return ret;
> -
> - subdevFormat.width = cfg->width;
> - subdevFormat.height = cfg->height;
> - ret = sensor->setFormat(0, &subdevFormat);
> - if (ret)
> - return ret;
> -
> - /* Return error if the requested format cannot be applied to sensor. */
> - if (subdevFormat.width != cfg->width ||
> - subdevFormat.height != cfg->height) {
> + if (cfg.width > cio2->maxSize_.width ||
> + cfg.height > cio2->maxSize_.height) {
> LOG(IPU3, Error)
> - << "Failed to apply image format "
> - << subdevFormat.width << "x" << subdevFormat.height
> - << " - got: " << cfg->width << "x" << cfg->height;
> + << "Invalid stream size: larger than sensor resolution";
> return -EINVAL;
> }
>
> - ret = csi2->setFormat(0, &subdevFormat);
> + /*
> + * Pass the requested stream size to the CIO2 unit and get back the
> + * adjusted format to be propagated to the ImgU output devices.
> + */
> + V4L2DeviceFormat cio2Format = {};
> + ret = cio2->configure(cfg, &cio2Format);
> if (ret)
> return ret;
>
> - ret = cio2->getFormat(&devFormat);
> + ret = imgu->configureInput(cfg, &cio2Format);
> if (ret)
> return ret;
>
> - devFormat.width = subdevFormat.width;
> - devFormat.height = subdevFormat.height;
> - devFormat.fourcc = cfg->pixelFormat;
> + /* Apply the format to the ImgU output, viewfinder and stat. */
> + ret = imgu->configureOutput(&imgu->output_, cfg);
> + if (ret)
> + return ret;
>
> - ret = cio2->setFormat(&devFormat);
> + ret = imgu->configureOutput(&imgu->viewfinder_, cfg);
> if (ret)
> return ret;
>
> - LOG(IPU3, Info) << cio2->driverName() << ": "
> - << devFormat.width << "x" << devFormat.height
> - << "- 0x" << std::hex << devFormat.fourcc << " planes: "
> - << devFormat.planes;
> + ret = imgu->configureOutput(&imgu->stat_, cfg);
> + if (ret)
> + return ret;
>
> return 0;
> }
> @@ -519,9 +537,9 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index)
> media_ = media;
>
> /*
> - * The media entities presence has been verified by the match()
> - * function, no need to check for newly created video devices
> - * and subdevice validity here.
> + * The media entities presence in the media device has been verified
> + * by the match() function: no need to check for newly created
> + * video devices and subdevice validity here.
> */
> imgu_ = V4L2Subdevice::fromEntityName(media, name_);
> ret = imgu_->open();
> @@ -533,21 +551,131 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index)
> if (ret)
> return ret;
>
> - output_ = V4L2Device::fromEntityName(media, name_ + " output");
> - ret = output_->open();
> + output_.dev = V4L2Device::fromEntityName(media, name_ + " output");
> + ret = output_.dev->open();
> + if (ret)
> + return ret;
> +
> + output_.pad = PAD_OUTPUT;
> + output_.name = "output";
> +
> + viewfinder_.dev = V4L2Device::fromEntityName(media,
> + name_ + " viewfinder");
> + ret = viewfinder_.dev->open();
> + if (ret)
> + return ret;
> +
> + viewfinder_.pad = PAD_VF;
> + viewfinder_.name = "viewfinder";
> +
> + stat_.dev = V4L2Device::fromEntityName(media, name_ + " 3a stat");
> + ret = stat_.dev->open();
> + if (ret)
> + return ret;
> +
> + stat_.pad = PAD_STAT;
> + stat_.name = "stat";
> +
> + return 0;
> +}
> +
> +/**
> + * \brief Configure the ImgU unit input
> + * \param[in] config The requested stream configuration
> + * \param[in] inputFormat The format to be applied to ImgU input
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int ImgUDevice::configureInput(const StreamConfiguration &config,
> + V4L2DeviceFormat *inputFormat)
> +{
> + /* Configure the ImgU input video device with the requested sizes. */
> + int ret = input_->setFormat(inputFormat);
> + if (ret)
> + return ret;
> +
> + LOG(IPU3, Debug) << "ImgU input format = " << inputFormat->toString();
> +
> + /*
> + * \todo The IPU3 driver implementation shall be changed to use the
> + * input sizes as 'ImgU Input' subdevice sizes, and use the desired
> + * GDC output sizes to configure the crop/compose rectangles.
> + *
> + * The current IPU3 driver implementation uses GDC sizes as the
> + * 'ImgU Input' subdevice sizes, and the input video device sizes
> + * to configure the crop/compose rectangles, contradicting the
> + * V4L2 specification.
> + */
> + Rectangle rect = {
> + .x = 0,
> + .y = 0,
> + .w = inputFormat->width,
> + .h = inputFormat->height,
> + };
> + ret = imgu_->setCrop(PAD_INPUT, &rect);
> + if (ret)
> + return ret;
> +
> + ret = imgu_->setCompose(PAD_INPUT, &rect);
> + if (ret)
> + return ret;
> +
> + LOG(IPU3, Debug) << "ImgU input feeder and BDS rectangle = "
> + << rect.toString();
> +
> + V4L2SubdeviceFormat imguFormat = {};
> + imguFormat.width = config.width;
> + imguFormat.height = config.height;
> + imguFormat.mbus_code = MEDIA_BUS_FMT_FIXED;
> +
> + ret = imgu_->setFormat(PAD_INPUT, &imguFormat);
> if (ret)
> return ret;
>
> - viewfinder_ = V4L2Device::fromEntityName(media, name_ + " viewfinder");
> - ret = viewfinder_->open();
> + LOG(IPU3, Debug) << "ImgU GDC format = " << imguFormat.toString();
> +
> + return 0;
> +}
> +
> +/**
> + * \brief Configure the ImgU unit \a id video output
> + * \param[in] output The ImgU output device to configure
> + * \param[in] config The requested configuration
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int ImgUDevice::configureOutput(const ImgUOutput *output,
> + const StreamConfiguration &config)
> +{
> + V4L2Device *dev = output->dev;
> + unsigned int pad = output->pad;
> +
> + V4L2SubdeviceFormat imguFormat = {};
> + imguFormat.width = config.width;
> + imguFormat.height = config.height;
> + imguFormat.mbus_code = MEDIA_BUS_FMT_FIXED;
> +
> + int ret = imgu_->setFormat(pad, &imguFormat);
> if (ret)
> return ret;
>
> - stat_ = V4L2Device::fromEntityName(media, name_ + " 3a stat");
> - ret = stat_->open();
> + /* No need to apply format to the stat node. */
> + if (output == &stat_)
> + return 0;
> +
> + V4L2DeviceFormat outputFormat = {};
> + outputFormat.width = config.width;
> + outputFormat.height = config.height;
> + outputFormat.fourcc = V4L2_PIX_FMT_NV12;
> + outputFormat.planesCount = 2;
> +
> + ret = dev->setFormat(&outputFormat);
> if (ret)
> return ret;
>
> + LOG(IPU3, Debug) << "ImgU " << output->name << " format = "
> + << outputFormat.toString();
> +
> return 0;
> }
>
> @@ -645,6 +773,78 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)
> return 0;
> }
>
> +/**
> + * \brief Configure the CIO2 unit
> + * \param[in] config The requested configuration
> + * \param[out] cio2Format The CIO2 unit output image format
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int CIO2Device::configure(const StreamConfiguration &config,
> + V4L2DeviceFormat *cio2Format)
> +{
> + unsigned int imageSize = config.width * config.height;
> + V4L2SubdeviceFormat sensorFormat = {};
> + unsigned int best = ~0;
> + int ret;
> +
> + for (auto it : sensor_->formats(0)) {
> + /* Only consider formats consumable by the CIO2 unit. */
> + if (mediaBusToCIO2Format(it.first) < 0)
> + continue;
> +
> + for (const SizeRange &size : it.second) {
> + /*
> + * Only select formats bigger than the requested sizes
> + * as the IPU3 cannot up-scale.
> + *
> + * \todo: Unconditionally scale on the sensor as much
> + * as possible. This will need to be revisited when
> + * implementing the scaling policy.
> + */
> + if (size.maxWidth < config.width ||
> + size.maxHeight < config.height)
> + continue;
> +
> + unsigned int diff = size.maxWidth * size.maxHeight
> + - imageSize;
> + if (diff >= best)
> + continue;
> +
> + best = diff;
> +
> + sensorFormat.width = size.maxWidth;
> + sensorFormat.height = size.maxHeight;
> + sensorFormat.mbus_code = it.first;
> + }
> + }
> +
> + /*
> + * Apply the selected format to the sensor, the CSI-2 receiver and
> + * the CIO2 output device.
> + */
> + ret = sensor_->setFormat(0, &sensorFormat);
> + if (ret)
> + return ret;
> +
> + ret = csi2_->setFormat(0, &sensorFormat);
> + if (ret)
> + return ret;
> +
> + cio2Format->width = sensorFormat.width;
> + cio2Format->height = sensorFormat.height;
> + cio2Format->fourcc = mediaBusToCIO2Format(sensorFormat.mbus_code);
> + cio2Format->planesCount = 1;
> +
> + ret = output_->setFormat(cio2Format);
> + if (ret)
> + return ret;
> +
> + LOG(IPU3, Debug) << "CIO2 output format " << cio2Format->toString();
> +
> + return 0;
> +}
> +
> REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3);
>
> } /* namespace libcamera */
> --
> 2.21.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list