[libcamera-devel] [PATCH v4 2/2] pipeline: raspberrypi: Tidy the camera enumeration and registration logic
Dave Stevenson
dave.stevenson at raspberrypi.com
Mon Nov 29 15:51:08 CET 2021
On Mon, 29 Nov 2021 at 14:41, Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> Quoting Naushir Patuck (2021-11-26 15:35:38)
> > When acquiring the media device, it is not necessary to match all entity names,
> > so remove it. Aditionally, we do not need to keep the MediaEntity pointers for
> > the Unicam and ISP devices stored within the PipelineHandlerRPi class. Instead
> > these can be stored locally in PipelineHandlerRPi::match().
> >
> > PipelineHandlerRPi::registerCamera() now returns an int error code instead of a
> > boolean for pass/fail.
>
> This looks good. I think it's clearer even for a single camera too, and
> it will be nice to see when we get multiple camera's running through.
>
> I know of the StereoPi use case already, and I guess there will be
> others with the CM4 using this.
This is two simultaneous but independent cameras.
Stereoscopic requires a bit more work to be able to combine the output
of two ISP passes into one buffer for side-by-side, or top-bottom.
I suspect there will be a load of discussion required for how the
libcamera frameworks wish to support stereoscopic, and how IPAs are
expected to handle stereo.
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> > .../pipeline/raspberrypi/raspberrypi.cpp | 89 +++++++++++--------
> > 1 file changed, 52 insertions(+), 37 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index 5a6dfa4e8b2a..e31fa3b23859 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -314,14 +314,11 @@ private:
> > return static_cast<RPiCameraData *>(camera->_d());
> > }
> >
> > - bool registerCamera();
> > + int registerCamera(MediaDevice *unicam, MediaDevice *isp);
> > int queueAllBuffers(Camera *camera);
> > int prepareBuffers(Camera *camera);
> > void freeBuffers(Camera *camera);
> > void mapBuffers(Camera *camera, const RPi::BufferMap &buffers, unsigned int mask);
> > -
> > - MediaDevice *unicam_;
> > - MediaDevice *isp_;
> > };
> >
> > RPiCameraConfiguration::RPiCameraConfiguration(const RPiCameraData *data)
> > @@ -512,7 +509,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> > }
> >
> > PipelineHandlerRPi::PipelineHandlerRPi(CameraManager *manager)
> > - : PipelineHandler(manager), unicam_(nullptr), isp_(nullptr)
> > + : PipelineHandler(manager)
> > {
> > }
> >
> > @@ -997,48 +994,62 @@ int PipelineHandlerRPi::queueRequestDevice(Camera *camera, Request *request)
> > bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
> > {
> > DeviceMatch unicam("unicam");
> > - DeviceMatch isp("bcm2835-isp");
> > + MediaDevice *unicamDevice = acquireMediaDevice(enumerator, unicam);
> >
> > - unicam.add("unicam-image");
> > + if (!unicamDevice) {
> > + LOG(RPI, Debug) << "Unable to acquire a Unicam instance";
> > + return false;
> > + }
> >
> > - 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 isp("bcm2835-isp");
> > + MediaDevice *ispDevice = acquireMediaDevice(enumerator, isp);
> >
> > - unicam_ = acquireMediaDevice(enumerator, unicam);
> > - if (!unicam_)
> > + if (!ispDevice) {
> > + LOG(RPI, Debug) << "Unable to acquire ISP instance";
> > return false;
> > + }
> >
> > - isp_ = acquireMediaDevice(enumerator, isp);
> > - if (!isp_)
> > + int ret = registerCamera(unicamDevice, ispDevice);
> > + if (ret) {
> > + LOG(RPI, Error) << "Failed to register camera: " << ret;
> > return false;
> > + }
> >
> > - return registerCamera();
> > + return true;
> > }
> >
> > -bool PipelineHandlerRPi::registerCamera()
> > +int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp)
> > {
> > std::unique_ptr<RPiCameraData> data = std::make_unique<RPiCameraData>(this);
> > +
> > if (!data->dmaHeap_.isValid())
> > - return false;
> > + return -ENOMEM;
> > +
> > + MediaEntity *unicamImage = unicam->getEntityByName("unicam-image");
> > + MediaEntity *ispOutput0 = isp->getEntityByName("bcm2835-isp0-output0");
> > + MediaEntity *ispCapture1 = isp->getEntityByName("bcm2835-isp0-capture1");
> > + MediaEntity *ispCapture2 = isp->getEntityByName("bcm2835-isp0-capture2");
> > + MediaEntity *ispCapture3 = isp->getEntityByName("bcm2835-isp0-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-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);
> > @@ -1049,7 +1060,7 @@ bool PipelineHandlerRPi::registerCamera()
> > 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;
> > @@ -1057,23 +1068,23 @@ bool PipelineHandlerRPi::registerCamera()
> > }
> >
> > 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();
> > }
> >
> > @@ -1093,13 +1104,14 @@ bool PipelineHandlerRPi::registerCamera()
> > data->streams_.push_back(&stream);
> >
> > for (auto stream : data->streams_) {
> > - if (stream->dev()->open())
> > - return false;
> > + int ret = stream->dev()->open();
> > + if (ret)
> > + return ret;
> > }
> >
> > 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;
> > }
> >
> > /*
> > @@ -1161,7 +1173,7 @@ bool PipelineHandlerRPi::registerCamera()
> >
> > if (!bayerFormat.isValid()) {
> > LOG(RPI, Error) << "No Bayer format found";
> > - return false;
> > + return -EINVAL;
> > }
> > data->nativeBayerOrder_ = bayerFormat.order;
> >
> > @@ -1181,7 +1193,10 @@ bool PipelineHandlerRPi::registerCamera()
> > Camera::create(std::move(data), id, streams);
> > PipelineHandler::registerCamera(std::move(camera));
> >
> > - return true;
> > + LOG(RPI, Info) << "Registered camera " << id
> > + << " to Unicam device " << unicam->deviceNode()
> > + << " and ISP device " << isp->deviceNode();
> > + return 0;
> > }
> >
> > int PipelineHandlerRPi::queueAllBuffers(Camera *camera)
> > --
> > 2.25.1
> >
More information about the libcamera-devel
mailing list