[libcamera-devel] [PATCH 2/3] pipeline: raspberrypi: Allow registration of multiple cameras
Naushir Patuck
naush at raspberrypi.com
Fri Nov 19 12:27:43 CET 2021
Hi,
On Fri, 19 Nov 2021 at 11:10, Naushir Patuck <naush at raspberrypi.com> wrote:
> Expand the pipeline handler camera registration to correctly handle
> multiple
> cameras attached to the platform. For example, Raspberry Pi Compute Module
> platforms have two camera connectors, and this change would allow the user
> to
> select either of the two cameras to run.
>
> There are associated kernel driver changes for both Unicam and the ISP
> needed
> to correctly advertise multiple media devices and nodes for multi-camera
> usage:
>
> https://github.com/raspberrypi/linux/pull/4140
> https://github.com/raspberrypi/linux/pull/4709
>
> However, this change is backward compatible with kernel builds that do not
> have
> these changes for standard single camera usage.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> .../pipeline/raspberrypi/raspberrypi.cpp | 112 ++++++++++++------
> 1 file changed, 77 insertions(+), 35 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 9aa7e9eef5e7..8ed2ebcaafe7 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -311,7 +311,8 @@ private:
> return static_cast<RPiCameraData *>(camera->_d());
> }
>
> - bool registerCameras();
> + int registerCameras(MediaDevice *unicam, MediaDevice *isp,
> + const std::string &unicamIdStr, const
> std::string &ispIdStr);
> int queueAllBuffers(Camera *camera);
> int prepareBuffers(Camera *camera);
> void freeBuffers(Camera *camera);
> @@ -993,49 +994,87 @@ int PipelineHandlerRPi::queueRequestDevice(Camera
> *camera, Request *request)
>
> bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
> {
> - DeviceMatch unicam("unicam");
> - DeviceMatch isp("bcm2835-isp");
> + unsigned int numCameras = 0;
>
> - unicam.add("unicam-image");
> + /*
> + * String of indexes to append to the entity names when searching
> for
> + * the Unican media devices. The first string is empty
> (un-indexed) to
> + * to maintain backward compatability with old versions of the
> Unicam
> + * kernel driver that did not advertise instance indexes.
> + */
> + for (const std::string &unicamIdStr : { "", "0", "1" }) {
> + MediaDevice *unicamDevice;
>
> - isp.add("bcm2835-isp0-output0"); /* Input */
> - isp.add("bcm2835-isp0-capture1"); /* Output 0 */
> - isp.add("bcm2835-isp0-capture2"); /* Output 1 */
> - isp.add("bcm2835-isp0-capture3"); /* Stats */
> + DeviceMatch unicam("unicam");
> + unicam.add("unicam" + unicamIdStr + "-image");
> + unicamDevice = acquireMediaDevice(enumerator, unicam);
>
> - unicam_ = acquireMediaDevice(enumerator, unicam);
> - if (!unicam_)
> - return false;
> + if (!unicamDevice)
> + continue;
>
> - isp_ = acquireMediaDevice(enumerator, isp);
> - if (!isp_)
> - return false;
> + for (const std::string &ispIdStr : { "0", "1" }) {
> + MediaDevice *ispDevice;
> + int ret;
> +
> + DeviceMatch isp("bcm2835-isp");
> + isp.add("bcm2835-isp" + ispIdStr + "-output0"); /*
> Input */
> + isp.add("bcm2835-isp" + ispIdStr + "-capture1");
> /* Output 0 */
> + isp.add("bcm2835-isp" + ispIdStr + "-capture2");
> /* Output 0 */
> + isp.add("bcm2835-isp" + ispIdStr + "-capture3");
> /* Stats */
> + ispDevice = acquireMediaDevice(enumerator, isp);
> +
> + if (!ispDevice)
> + continue;
> +
> + ret = registerCameras(unicamDevice, ispDevice,
> unicamIdStr, ispIdStr);
> + if (ret) {
> + LOG(RPI, Error) << "Failed to register
> camera: " << ret;
> + ispDevice->release();
> + unicamDevice->release();
>
I am not too sure if the above two lines are
enough. PipelineHandler::acquireMediaDevice()
does a MediaDevice::acquire() but also adds the media device internally to
a private member:
std::vector<std::shared_ptr<MediaDevice>> mediaDevices_;
Perhaps I ought to implement a new public
function PipelineHandler::acquireMediaDevice(MediaDevice *)
that removes the entry in mediaDevices_ as well as does a
MediaDevice::release(). Thoughts?
Naush
+ } else
> + numCameras++;
>
> - return registerCameras();
> + break;
> + }
> + }
> +
> + return !!numCameras;
> }
>
> -bool PipelineHandlerRPi::registerCameras()
> +int PipelineHandlerRPi::registerCameras(MediaDevice *unicam, MediaDevice
> *isp,
> + const std::string &unicamIdStr,
> + const std::string &ispIdStr)
> {
> std::unique_ptr<RPiCameraData> data =
> std::make_unique<RPiCameraData>(this);
> +
> if (!data->dmaHeap_.isValid())
> - return false;
> + return -ENOMEM;
> +
> + MediaEntity *unicamImage = unicam->getEntityByName("unicam" +
> unicamIdStr + "-image");
> + MediaEntity *ispOutput0 = isp->getEntityByName("bcm2835-isp" +
> ispIdStr + "-output0");
> + MediaEntity *ispCapture1 = isp->getEntityByName("bcm2835-isp" +
> ispIdStr + "-capture1");
> + MediaEntity *ispCapture2 = isp->getEntityByName("bcm2835-isp" +
> ispIdStr + "-capture2");
> + MediaEntity *ispCapture3 = isp->getEntityByName("bcm2835-isp" +
> ispIdStr + "-capture3");
> +
> + if (!unicamImage || !ispOutput0 || !ispCapture1 || !ispCapture2 ||
> !ispCapture3)
> + return -ENOENT;
>
> /* Locate and open the unicam video streams. */
> - data->unicam_[Unicam::Image] = RPi::Stream("Unicam Image",
> unicam_->getEntityByName("unicam-image"));
> + data->unicam_[Unicam::Image] = RPi::Stream("Unicam Image",
> unicamImage);
>
> /* An embedded data node will not be present if the sensor does
> not support it. */
> - MediaEntity *embeddedEntity =
> unicam_->getEntityByName("unicam-embedded");
> - if (embeddedEntity) {
> - data->unicam_[Unicam::Embedded] = RPi::Stream("Unicam
> Embedded", embeddedEntity);
> + MediaEntity *unicamEmbedded = unicam->getEntityByName("unicam" +
> unicamIdStr + "-embedded");
> + if (unicamEmbedded) {
> + data->unicam_[Unicam::Embedded] = RPi::Stream("Unicam
> Embedded", unicamEmbedded);
>
> data->unicam_[Unicam::Embedded].dev()->bufferReady.connect(data.get(),
>
> &RPiCameraData::unicamBufferDequeue);
> }
>
> /* Tag the ISP input stream as an import stream. */
> - data->isp_[Isp::Input] = RPi::Stream("ISP Input",
> isp_->getEntityByName("bcm2835-isp0-output0"), true);
> - data->isp_[Isp::Output0] = RPi::Stream("ISP Output0",
> isp_->getEntityByName("bcm2835-isp0-capture1"));
> - data->isp_[Isp::Output1] = RPi::Stream("ISP Output1",
> isp_->getEntityByName("bcm2835-isp0-capture2"));
> - data->isp_[Isp::Stats] = RPi::Stream("ISP Stats",
> isp_->getEntityByName("bcm2835-isp0-capture3"));
> + data->isp_[Isp::Input] = RPi::Stream("ISP Input", ispOutput0,
> true);
> + data->isp_[Isp::Output0] = RPi::Stream("ISP Output0", ispCapture1);
> + data->isp_[Isp::Output1] = RPi::Stream("ISP Output1", ispCapture2);
> + data->isp_[Isp::Stats] = RPi::Stream("ISP Stats", ispCapture3);
>
> /* Wire up all the buffer connections. */
> data->unicam_[Unicam::Image].dev()->frameStart.connect(data.get(),
> &RPiCameraData::frameStarted);
> @@ -1046,7 +1085,7 @@ bool PipelineHandlerRPi::registerCameras()
> data->isp_[Isp::Stats].dev()->bufferReady.connect(data.get(),
> &RPiCameraData::ispOutputDequeue);
>
> /* Identify the sensor. */
> - for (MediaEntity *entity : unicam_->entities()) {
> + for (MediaEntity *entity : unicam->entities()) {
> if (entity->function() == MEDIA_ENT_F_CAM_SENSOR) {
> data->sensor_ =
> std::make_unique<CameraSensor>(entity);
> break;
> @@ -1054,23 +1093,23 @@ bool PipelineHandlerRPi::registerCameras()
> }
>
> if (!data->sensor_)
> - return false;
> + return -EINVAL;
>
> if (data->sensor_->init())
> - return false;
> + return -EINVAL;
>
> data->sensorFormats_ = populateSensorFormats(data->sensor_);
>
> ipa::RPi::SensorConfig sensorConfig;
> if (data->loadIPA(&sensorConfig)) {
> LOG(RPI, Error) << "Failed to load a suitable IPA library";
> - return false;
> + return -EINVAL;
> }
>
> - if (sensorConfig.sensorMetadata ^ !!embeddedEntity) {
> + if (sensorConfig.sensorMetadata ^ !!unicamEmbedded) {
> LOG(RPI, Warning) << "Mismatch between Unicam and
> CamHelper for embedded data usage!";
> sensorConfig.sensorMetadata = false;
> - if (embeddedEntity)
> + if (unicamEmbedded)
>
> data->unicam_[Unicam::Embedded].dev()->bufferReady.disconnect();
> }
>
> @@ -1091,12 +1130,12 @@ bool PipelineHandlerRPi::registerCameras()
>
> for (auto stream : data->streams_) {
> if (stream->dev()->open())
> - return false;
> + continue;
> }
>
> if
> (!data->unicam_[Unicam::Image].dev()->caps().hasMediaController()) {
> LOG(RPI, Error) << "Unicam driver does not use the
> MediaController, please update your kernel!";
> - return false;
> + return -EINVAL;
> }
>
> /*
> @@ -1158,7 +1197,7 @@ bool PipelineHandlerRPi::registerCameras()
>
> if (!bayerFormat.isValid()) {
> LOG(RPI, Error) << "No Bayer format found";
> - return false;
> + return -EINVAL;
> }
> data->nativeBayerOrder_ = bayerFormat.order;
>
> @@ -1178,7 +1217,10 @@ bool PipelineHandlerRPi::registerCameras()
> Camera::create(std::move(data), id, streams);
> registerCamera(std::move(camera));
>
> - return true;
> + LOG(RPI, Info) << "Registered camera " << id
> + << " to instances \"unicam" << unicamIdStr << "\""
> + << " and \"isp" << ispIdStr << "\"";
> + return 0;
> }
>
> int PipelineHandlerRPi::queueAllBuffers(Camera *camera)
> --
> 2.25.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20211119/abb120f3/attachment-0001.htm>
More information about the libcamera-devel
mailing list