[libcamera-devel] [PATCH 2/3] pipeline: raspberrypi: Allow registration of multiple cameras
Naushir Patuck
naush at raspberrypi.com
Fri Nov 19 12:28:39 CET 2021
On Fri, 19 Nov 2021 at 11:27, Naushir Patuck <naush at raspberrypi.com> wrote:
> 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 *)
>
Sorry, that should read PipelineHandler::releaseMediaDevice(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/5b11cbee/attachment-0001.htm>
More information about the libcamera-devel
mailing list