[libcamera-devel] [PATCH v3 8/9] pipeline: raspberrypi: Account for a missing Unicam embedded data node

[Naushir Patuck]

Hi Kieran,

Thank you for your feedback,

On Wed, 27 Oct 2021 at 13:23, Kieran Bingham <
kieran.bingham at ideasonboard.com> wrote:

> Quoting Naushir Patuck (2021-10-27 10:28:02)
> > The unicam driver no longer registers an embedded data node if the
> sensor does
> > not provide this stream. Account for this in the pipeline handler match
> routine
> > by not assuming it is always present.
> >
> > Add a warning if Unicam and the CamHelper do not agree on the presense
> of sensor
> > embedded data, and disable its usage in these cases.
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  .../pipeline/raspberrypi/raspberrypi.cpp         | 16 +++++++++++++---
> >  1 file changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index e01359b20fd9..52521857b61c 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -1015,7 +1015,6 @@ bool PipelineHandlerRPi::match(DeviceEnumerator
> *enumerator)
> >         DeviceMatch unicam("unicam");
> >         DeviceMatch isp("bcm2835-isp");
> >
> > -       unicam.add("unicam-embedded");
> >         unicam.add("unicam-image");
> >
> >         isp.add("bcm2835-isp0-output0"); /* Input */
> > @@ -1036,9 +1035,16 @@ bool PipelineHandlerRPi::match(DeviceEnumerator
> *enumerator)
> >                 return false;
> >
> >         /* Locate and open the unicam video streams. */
> > -       data->unicam_[Unicam::Embedded] = RPi::Stream("Unicam Embedded",
> unicam_->getEntityByName("unicam-embedded"));
> >         data->unicam_[Unicam::Image] = RPi::Stream("Unicam Image",
> unicam_->getEntityByName("unicam-image"));
> >
> > +       /* 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);
> > +
>  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"));
> > @@ -1048,7 +1054,6 @@ bool PipelineHandlerRPi::match(DeviceEnumerator
> *enumerator)
> >         /* Wire up all the buffer connections. */
> >
>  data->unicam_[Unicam::Image].dev()->frameStart.connect(data.get(),
> &RPiCameraData::frameStarted);
> >
>  data->unicam_[Unicam::Image].dev()->bufferReady.connect(data.get(),
> &RPiCameraData::unicamBufferDequeue);
> > -
>  data->unicam_[Unicam::Embedded].dev()->bufferReady.connect(data.get(),
> &RPiCameraData::unicamBufferDequeue);
> >         data->isp_[Isp::Input].dev()->bufferReady.connect(data.get(),
> &RPiCameraData::ispInputDequeue);
> >         data->isp_[Isp::Output0].dev()->bufferReady.connect(data.get(),
> &RPiCameraData::ispOutputDequeue);
> >         data->isp_[Isp::Output1].dev()->bufferReady.connect(data.get(),
> &RPiCameraData::ispOutputDequeue);
> > @@ -1076,6 +1081,11 @@ bool PipelineHandlerRPi::match(DeviceEnumerator
> *enumerator)
> >                 return false;
> >         }
> >
> > +       if (sensorConfig.sensorMetadata ^ !!embeddedEntity) {
> > +               LOG(RPI, Warning) << "Mismatch between Unicam and
> CamHelper for embedded data usage!";
> > +               sensorConfig.sensorMetadata = false;
>
> Should the data->unicam_[Unicam::Embedded] stream/ signal be
> disconnected here? (if they were created) or will they just be fine
> sitting idle?
>
> I presume they'll be fine and keep otherwise quiet on the pipeline, and
> get cleaned up at the end anyway.
>

It shouldn't matter as it will never be triggered, but I will add a
disconnect() call
in these cases.

Regards,
Naush


>
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> > +       }
> > +
> >         /*
> >          * Open all Unicam and ISP streams. The exception is the
> embedded data
> >          * stream, which only gets opened below if the IPA reports that
> the sensor
> > --
> > 2.25.1
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20211027/2611778a/attachment.htm>