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

[Kieran Bingham]

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.


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
>