[libcamera-devel] [PATCH 2/6] pipeline: raspberrypi: Convert the pipeline handler to use media controller
David Plowman
david.plowman at raspberrypi.com
Fri Oct 22 15:09:39 CEST 2021
Hi Naush
Thanks for this patch!
On Fri, 22 Oct 2021 at 12:55, Naushir Patuck <naush at raspberrypi.com> wrote:
>
> Switch the pipeline handler to use the new Unicam media controller based driver.
> With this change, we directly talk to the sensor device driver to set controls
> and set/get formats in the pipeline handler.
>
> This change requires the accompanying Raspberry Pi linux kernel change at
> https://github.com/raspberrypi/linux/pull/4645. If this kernel change is not
> present, the pipeline handler will fail to run with an error message informing
> the user to update the kernel build.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> .../pipeline/raspberrypi/raspberrypi.cpp | 113 +++++++++++-------
> 1 file changed, 67 insertions(+), 46 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 1634ca98f481..730f1575095c 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -48,6 +48,19 @@ LOG_DEFINE_CATEGORY(RPI)
>
> namespace {
>
> +/* Map of mbus codes to supported sizes reported by the sensor. */
> +using SensorFormats = std::map<unsigned int, std::vector<Size>>;
> +
> +SensorFormats populateSensorFormats(std::unique_ptr<CameraSensor> &sensor)
> +{
> + SensorFormats formats;
> +
> + for (auto const mbusCode : sensor->mbusCodes())
> + formats.emplace(mbusCode, sensor->sizes(mbusCode));
> +
> + return formats;
> +}
> +
> bool isRaw(PixelFormat &pixFmt)
> {
> /*
> @@ -74,8 +87,7 @@ double scoreFormat(double desired, double actual)
> return score;
> }
>
> -V4L2DeviceFormat findBestMode(V4L2VideoDevice::Formats &formatsMap,
> - const Size &req)
> +V4L2DeviceFormat findBestMode(const SensorFormats &formatsMap, const Size &req)
I wonder whether this function should return something more like an
mbus code and a size. The packing is not really relevant to what comes
out of the sensor and should, I suspect, actually be determined by any
raw stream that has been requested by the application. This all starts
to impinge a bit on the sensor-mode-hint discussions I've been having.
One thing I wonder at this point is how an application could express a
wish for packed or unpacked raw streams. I think generateConfiguration
should probably fill in whatever it wants, probably "packed" as that's
more efficient. Then the application would have to be able to change
this to "unpacked" - which we could make less annoying by making the
BayerFormat publicly available.
> {
> double bestScore = std::numeric_limits<double>::max(), score;
> V4L2DeviceFormat bestMode;
> @@ -88,18 +100,17 @@ V4L2DeviceFormat findBestMode(V4L2VideoDevice::Formats &formatsMap,
>
> /* Calculate the closest/best mode from the user requested size. */
> for (const auto &iter : formatsMap) {
> - V4L2PixelFormat v4l2Format = iter.first;
> + const unsigned int mbus_code = iter.first;
> + const V4L2PixelFormat v4l2Format = BayerFormat::fromMbusCode(mbus_code).toV4L2PixelFormat();
> const PixelFormatInfo &info = PixelFormatInfo::info(v4l2Format);
>
> - for (const SizeRange &sz : iter.second) {
> - double modeWidth = sz.contains(req) ? req.width : sz.max.width;
> - double modeHeight = sz.contains(req) ? req.height : sz.max.height;
> + for (const Size &sz : iter.second) {
> double reqAr = static_cast<double>(req.width) / req.height;
> - double modeAr = modeWidth / modeHeight;
> + double modeAr = sz.width / sz.height;
>
> /* Score the dimensions for closeness. */
> - score = scoreFormat(req.width, modeWidth);
> - score += scoreFormat(req.height, modeHeight);
> + score = scoreFormat(req.width, sz.width);
> + score += scoreFormat(req.height, sz.height);
> score += PENALTY_AR * scoreFormat(reqAr, modeAr);
>
> /* Add any penalties... this is not an exact science! */
> @@ -116,10 +127,10 @@ V4L2DeviceFormat findBestMode(V4L2VideoDevice::Formats &formatsMap,
> if (score <= bestScore) {
> bestScore = score;
> bestMode.fourcc = v4l2Format;
> - bestMode.size = Size(modeWidth, modeHeight);
> + bestMode.size = sz;
> }
>
> - LOG(RPI, Info) << "Mode: " << modeWidth << "x" << modeHeight
> + LOG(RPI, Info) << "Mode: " << sz.width << "x" << sz.height
> << " fmt " << v4l2Format.toString()
> << " Score: " << score
> << " (best " << bestScore << ")";
> @@ -170,6 +181,7 @@ public:
> std::unique_ptr<ipa::RPi::IPAProxyRPi> ipa_;
>
> std::unique_ptr<CameraSensor> sensor_;
> + SensorFormats sensorFormats_;
> /* Array of Unicam and ISP device streams and associated buffers/streams. */
> RPi::Device<Unicam, 2> unicam_;
> RPi::Device<Isp, 4> isp_;
> @@ -352,8 +364,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> * Calculate the best sensor mode we can use based on
> * the user request.
> */
> - V4L2VideoDevice::Formats fmts = data_->unicam_[Unicam::Image].dev()->formats();
> - V4L2DeviceFormat sensorFormat = findBestMode(fmts, cfg.size);
> + V4L2DeviceFormat sensorFormat = findBestMode(data_->sensorFormats_, cfg.size);
> int ret = data_->unicam_[Unicam::Image].dev()->tryFormat(&sensorFormat);
> if (ret)
> return Invalid;
> @@ -487,8 +498,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
> switch (role) {
> case StreamRole::Raw:
> size = data->sensor_->resolution();
> - fmts = data->unicam_[Unicam::Image].dev()->formats();
> - sensorFormat = findBestMode(fmts, size);
> + sensorFormat = findBestMode(data->sensorFormats_, size);
> pixelFormat = sensorFormat.fourcc.toPixelFormat();
> ASSERT(pixelFormat.isValid());
> bufferCount = 2;
> @@ -599,32 +609,32 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> }
>
> /* First calculate the best sensor mode we can use based on the user request. */
> - V4L2VideoDevice::Formats fmts = data->unicam_[Unicam::Image].dev()->formats();
> - V4L2DeviceFormat sensorFormat = findBestMode(fmts, rawStream ? sensorSize : maxSize);
> + V4L2DeviceFormat unicamFormat = findBestMode(data->sensorFormats_, rawStream ? sensorSize : maxSize);
> +
> + unsigned int mbus_code = BayerFormat::fromV4L2PixelFormat(unicamFormat.fourcc).toMbusCode();
> + V4L2SubdeviceFormat sensorFormat { .mbus_code = mbus_code, .size = unicamFormat.size };
> +
> + ret = data->sensor_->setFormat(&sensorFormat);
> + if (ret)
> + return ret;
As discussed above, the unicamFormat probably wants to take account of
the packing (or not) in any raw stream (if present).
>
> /*
> * Unicam image output format. The ISP input format gets set at start,
> * just in case we have swapped bayer orders due to flips.
> */
> - ret = data->unicam_[Unicam::Image].dev()->setFormat(&sensorFormat);
> + ret = data->unicam_[Unicam::Image].dev()->setFormat(&unicamFormat);
> if (ret)
> return ret;
>
> - /*
> - * The control ranges associated with the sensor may need updating
> - * after a format change.
> - * \todo Use the CameraSensor::setFormat API instead.
> - */
> - data->sensor_->updateControlInfo();
Hmm, nice, I wonder if we could delete updateControlInfo entirely?
> -
> LOG(RPI, Info) << "Sensor: " << camera->id()
> - << " - Selected mode: " << sensorFormat.toString();
> + << " - Selected sensor mode: " << sensorFormat.toString()
> + << " - Selected unicam mode: " << unicamFormat.toString();
>
> /*
> * This format may be reset on start() if the bayer order has changed
> * because of flips in the sensor.
> */
> - ret = data->isp_[Isp::Input].dev()->setFormat(&sensorFormat);
> + ret = data->isp_[Isp::Input].dev()->setFormat(&unicamFormat);
> if (ret)
> return ret;
>
> @@ -746,8 +756,8 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> data->ispMinCropSize_ = testCrop.size();
>
> /* Adjust aspect ratio by providing crops on the input image. */
> - Size size = sensorFormat.size.boundedToAspectRatio(maxSize);
> - Rectangle crop = size.centeredTo(Rectangle(sensorFormat.size).center());
> + Size size = unicamFormat.size.boundedToAspectRatio(maxSize);
> + Rectangle crop = size.centeredTo(Rectangle(unicamFormat.size).center());
> data->ispCrop_ = crop;
>
> data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &crop);
> @@ -761,10 +771,11 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> * supports it.
> */
> if (data->sensorMetadata_) {
> - format = {};
> - format.fourcc = V4L2PixelFormat(V4L2_META_FMT_SENSOR_DATA);
> + V4L2SubdeviceFormat embeddedFormat;
>
> - LOG(RPI, Debug) << "Setting embedded data format.";
> + data->sensor_->device()->getFormat(1, &embeddedFormat);
> + format.fourcc = V4L2PixelFormat(V4L2_META_FMT_SENSOR_DATA);
> + format.planes[0].size = embeddedFormat.size.width * embeddedFormat.size.height;
> ret = data->unicam_[Unicam::Embedded].dev()->setFormat(&format);
> if (ret) {
> LOG(RPI, Error) << "Failed to set format on Unicam embedded: "
> @@ -847,9 +858,14 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)
> * IPA configure may have changed the sensor flips - hence the bayer
> * order. Get the sensor format and set the ISP input now.
> */
> - V4L2DeviceFormat sensorFormat;
> - data->unicam_[Unicam::Image].dev()->getFormat(&sensorFormat);
> - ret = data->isp_[Isp::Input].dev()->setFormat(&sensorFormat);
> + V4L2SubdeviceFormat sensorFormat;
> + data->sensor_->device()->getFormat(0, &sensorFormat);
> +
> + V4L2DeviceFormat ispFormat;
> + ispFormat.fourcc = BayerFormat::fromMbusCode(sensorFormat.mbus_code).toV4L2PixelFormat();
> + ispFormat.size = sensorFormat.size;
> +
> + ret = data->isp_[Isp::Input].dev()->setFormat(&ispFormat);
> if (ret) {
> stop(camera);
> return ret;
> @@ -1004,6 +1020,8 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
> if (data->sensor_->init())
> return false;
>
> + data->sensorFormats_ = populateSensorFormats(data->sensor_);
> +
> ipa::RPi::SensorConfig sensorConfig;
> if (data->loadIPA(&sensorConfig)) {
> LOG(RPI, Error) << "Failed to load a suitable IPA library";
> @@ -1030,6 +1048,11 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
> return false;
> }
>
> + if (!(data->unicam_[Unicam::Image].dev()->caps().device_caps() & V4L2_CAP_IO_MC)) {
> + LOG(RPI, Error) << "Unicam driver did not advertise V4L2_CAP_IO_MC, please update your kernel!";
> + return false;
> + }
> +
> /*
> * Setup our delayed control writer with the sensor default
> * gain and exposure delays. Mark VBLANK for priority write.
> @@ -1039,7 +1062,7 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
> { V4L2_CID_EXPOSURE, { sensorConfig.exposureDelay, false } },
> { V4L2_CID_VBLANK, { sensorConfig.vblankDelay, true } }
> };
> - data->delayedCtrls_ = std::make_unique<DelayedControls>(data->unicam_[Unicam::Image].dev(), params);
> + data->delayedCtrls_ = std::make_unique<DelayedControls>(data->sensor_->device(), params);
> data->sensorMetadata_ = sensorConfig.sensorMetadata;
>
> /* Register the controls that the Raspberry Pi IPA can handle. */
> @@ -1066,15 +1089,14 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
> * As part of answering the final question, we reset the camera to
> * no transform at all.
> */
> -
> - V4L2VideoDevice *dev = data->unicam_[Unicam::Image].dev();
> - const struct v4l2_query_ext_ctrl *hflipCtrl = dev->controlInfo(V4L2_CID_HFLIP);
> + const V4L2Subdevice *sensor = data->sensor_->device();
> + const struct v4l2_query_ext_ctrl *hflipCtrl = sensor->controlInfo(V4L2_CID_HFLIP);
> if (hflipCtrl) {
> /* We assume it will support vflips too... */
> data->supportsFlips_ = true;
> data->flipsAlterBayerOrder_ = hflipCtrl->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT;
>
> - ControlList ctrls(dev->controls());
> + ControlList ctrls(data->sensor_->controls());
> ctrls.set(V4L2_CID_HFLIP, 0);
> ctrls.set(V4L2_CID_VFLIP, 0);
> data->setSensorControls(ctrls);
> @@ -1082,9 +1104,8 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
>
> /* Look for a valid Bayer format. */
> BayerFormat bayerFormat;
> - for (const auto &iter : dev->formats()) {
> - V4L2PixelFormat v4l2Format = iter.first;
> - bayerFormat = BayerFormat::fromV4L2PixelFormat(v4l2Format);
> + for (const auto &iter : data->sensorFormats_) {
> + bayerFormat = BayerFormat::fromMbusCode(iter.first);
> if (bayerFormat.isValid())
> break;
> }
> @@ -1271,7 +1292,7 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)
> }
> }
>
> - entityControls.emplace(0, unicam_[Unicam::Image].dev()->controls());
> + entityControls.emplace(0, sensor_->controls());
> entityControls.emplace(1, isp_[Isp::Input].dev()->controls());
>
> /* Always send the user transform to the IPA. */
> @@ -1406,10 +1427,10 @@ void RPiCameraData::setSensorControls(ControlList &controls)
> ControlList vblank_ctrl;
>
> vblank_ctrl.set(V4L2_CID_VBLANK, controls.get(V4L2_CID_VBLANK));
> - unicam_[Unicam::Image].dev()->setControls(&vblank_ctrl);
> + sensor_->setControls(&vblank_ctrl);
> }
>
> - unicam_[Unicam::Image].dev()->setControls(&controls);
> + sensor_->setControls(&controls);
> }
>
> void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)
> --
> 2.25.1
>
Thanks!
David
More information about the libcamera-devel
mailing list