[libcamera-devel] [PATCH v3 12/13] libcamera: pipeline: ipu3: Use the new CameraSensor class
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Apr 18 17:08:28 CEST 2019
Hi Jacopo,
On Thu, Apr 18, 2019 at 04:40:52PM +0200, Jacopo Mondi wrote:
> On Thu, Apr 18, 2019 at 05:14:36PM +0300, Laurent Pinchart wrote:
> > Replace the manual handling of sensor formats with usage of the new
> > CameraSensor class.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> > Changes since v2:
> >
> > - Properly sort the CIO2 supported media bus codes numerically
> >
> > Changes since v1:
> >
> > - Move sensor entity function check to CameraSensor class
> > - Verify sensor media bus codes at init time
> > ---
> > src/libcamera/pipeline/ipu3/ipu3.cpp | 107 +++++++++------------------
> > 1 file changed, 36 insertions(+), 71 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index f43969099b48..c677ee9b51da 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 <algorithm>
>
> What do you need this for?
We don't, I've removed it. It was a leftover.
> > #include <iomanip>
> > #include <memory>
> > #include <vector>
> > @@ -15,6 +16,7 @@
> > #include <libcamera/request.h>
> > #include <libcamera/stream.h>
> >
> > +#include "camera_sensor.h"
> > #include "device_enumerator.h"
> > #include "log.h"
> > #include "media_device.h"
> > @@ -124,11 +126,7 @@ public:
> >
> > V4L2Device *output_;
> > V4L2Subdevice *csi2_;
> > - V4L2Subdevice *sensor_;
> > -
> > - /* Maximum sizes and the mbus code used to produce them. */
> > - unsigned int mbusCode_;
> > - Size maxSize_;
> > + CameraSensor *sensor_;
> >
> > BufferPool pool_;
> > };
> > @@ -257,8 +255,9 @@ int PipelineHandlerIPU3::configureStreams(Camera *camera,
> > return -EINVAL;
> > }
> >
> > - if (cfg.width > cio2->maxSize_.width ||
> > - cfg.height > cio2->maxSize_.height) {
> > + const Size &resolution = cio2->sensor_->resolution();
> > + if (cfg.width > resolution.width ||
> > + cfg.height > resolution.height) {
> > LOG(IPU3, Error)
> > << "Invalid stream size: larger than sensor resolution";
> > return -EINVAL;
> > @@ -1032,45 +1031,39 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)
> >
> > MediaLink *link = links[0];
> > MediaEntity *sensorEntity = link->source()->entity();
> > - if (sensorEntity->function() != MEDIA_ENT_F_CAM_SENSOR)
> > - return -ENODEV;
> > + sensor_ = new CameraSensor(sensorEntity);
> > + ret = sensor_->init();
> > + if (ret)
> > + return ret;
> >
> > ret = link->setEnabled(true);
> > if (ret)
> > return ret;
> >
> > /*
> > - * Now that we're sure a sensor subdevice is connected, make sure it
> > - * produces at least one image format compatible with CIO2 requirements
> > - * and cache the camera maximum size.
> > + * Make sure the sensor produces at least one format compatible with
> > + * the CIO2 requirements.
> > *
> > + * utils::set_overlap requires the ranges to be sorted, keep the
> > + * cio2Codes vector sorted in ascending order.
> > + */
> > + const std::vector<unsigned int> cio2Codes{ MEDIA_BUS_FMT_SBGGR10_1X10,
> > + MEDIA_BUS_FMT_SGRBG10_1X10,
> > + MEDIA_BUS_FMT_SGBRG10_1X10,
>
> As you mentioned you ordered them, shouldn't SGBR come before SGRB ?
In alphabetical order, yes, in numerical order, no.
>
> Minors apart:
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>
> I'll rebase my multiple stream support series on top of this patches!
Looking forward to that :-)
> > + MEDIA_BUS_FMT_SRGGB10_1X10 };
> > + const std::vector<unsigned int> &sensorCodes = sensor_->mbusCodes();
> > + if (!utils::set_overlap(sensorCodes.begin(), sensorCodes.end(),
> > + cio2Codes.begin(), cio2Codes.end())) {
> > + LOG(IPU3, Error)
> > + << "Sensor " << sensor_->entity()->name()
> > + << " has not format compatible with the IPU3";
> > + return -EINVAL;
> > + }
> > +
> > + /*
> > * \todo Define when to open and close video device nodes, as they
> > * might impact on power consumption.
> > */
> > - sensor_ = new V4L2Subdevice(sensorEntity);
> > - ret = sensor_->open();
> > - if (ret)
> > - return ret;
> > -
> > - for (auto it : sensor_->formats(0)) {
> > - int mbusCode = mediaBusToFormat(it.first);
> > - if (mbusCode < 0)
> > - continue;
> > -
> > - for (const SizeRange &size : it.second) {
> > - if (maxSize_.width < size.max.width &&
> > - maxSize_.height < size.max.height) {
> > - maxSize_ = size.max;
> > - mbusCode_ = mbusCode;
> > - }
> > - }
> > - }
> > - if (maxSize_.width == 0) {
> > - LOG(IPU3, Info) << "Sensor '" << sensor_->entity()->name()
> > - << "' detected, but no supported image format "
> > - << " found: skip camera creation";
> > - return -ENODEV;
> > - }
> >
> > csi2_ = new V4L2Subdevice(csi2Entity);
> > ret = csi2_->open();
> > @@ -1096,47 +1089,19 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)
> > int CIO2Device::configure(const StreamConfiguration &config,
> > V4L2DeviceFormat *outputFormat)
> > {
> > - unsigned int imageSize = config.width * config.height;
> > - V4L2SubdeviceFormat sensorFormat = {};
> > - unsigned int best = ~0;
> > + V4L2SubdeviceFormat sensorFormat;
> > int ret;
> >
> > - for (auto it : sensor_->formats(0)) {
> > - /* Only consider formats consumable by the CIO2 unit. */
> > - if (mediaBusToFormat(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.max.width < config.width ||
> > - size.max.height < config.height)
> > - continue;
> > -
> > - unsigned int diff = size.max.width * size.max.height
> > - - imageSize;
> > - if (diff >= best)
> > - continue;
> > -
> > - best = diff;
> > -
> > - sensorFormat.width = size.max.width;
> > - sensorFormat.height = size.max.height;
> > - 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);
> > + sensorFormat = sensor_->getFormat({ MEDIA_BUS_FMT_SBGGR10_1X10,
> > + MEDIA_BUS_FMT_SGBRG10_1X10,
> > + MEDIA_BUS_FMT_SGRBG10_1X10,
> > + MEDIA_BUS_FMT_SRGGB10_1X10 },
> > + Size(config.width, config.height));
> > + ret = sensor_->setFormat(&sensorFormat);
> > if (ret)
> > return ret;
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list