<div dir="ltr"><div dir="ltr">Hi Kieran,<div><br></div><div>Thank you for your feedback,</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 27 Oct 2021 at 13:23, Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Quoting Naushir Patuck (2021-10-27 10:28:02)<br>
> The unicam driver no longer registers an embedded data node if the sensor does<br>
> not provide this stream. Account for this in the pipeline handler match routine<br>
> by not assuming it is always present.<br>
> <br>
> Add a warning if Unicam and the CamHelper do not agree on the presense of sensor<br>
> embedded data, and disable its usage in these cases.<br>
> <br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> Reviewed-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
> Reviewed-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br>
> ---<br>
>  .../pipeline/raspberrypi/raspberrypi.cpp         | 16 +++++++++++++---<br>
>  1 file changed, 13 insertions(+), 3 deletions(-)<br>
> <br>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> index e01359b20fd9..52521857b61c 100644<br>
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> @@ -1015,7 +1015,6 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)<br>
>         DeviceMatch unicam("unicam");<br>
>         DeviceMatch isp("bcm2835-isp");<br>
>  <br>
> -       unicam.add("unicam-embedded");<br>
>         unicam.add("unicam-image");<br>
>  <br>
>         isp.add("bcm2835-isp0-output0"); /* Input */<br>
> @@ -1036,9 +1035,16 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)<br>
>                 return false;<br>
>  <br>
>         /* Locate and open the unicam video streams. */<br>
> -       data->unicam_[Unicam::Embedded] = RPi::Stream("Unicam Embedded", unicam_->getEntityByName("unicam-embedded"));<br>
>         data->unicam_[Unicam::Image] = RPi::Stream("Unicam Image", unicam_->getEntityByName("unicam-image"));<br>
>  <br>
> +       /* An embedded data node will not be present if the sensor does not support it. */<br>
> +       MediaEntity *embeddedEntity = unicam_->getEntityByName("unicam-embedded");<br>
> +       if (embeddedEntity) {<br>
> +               data->unicam_[Unicam::Embedded] = RPi::Stream("Unicam Embedded", embeddedEntity);<br>
> +               data->unicam_[Unicam::Embedded].dev()->bufferReady.connect(data.get(),<br>
> +                                                                          &RPiCameraData::unicamBufferDequeue);<br>
> +       }<br>
> +<br>
>         /* Tag the ISP input stream as an import stream. */<br>
>         data->isp_[Isp::Input] = RPi::Stream("ISP Input", isp_->getEntityByName("bcm2835-isp0-output0"), true);<br>
>         data->isp_[Isp::Output0] = RPi::Stream("ISP Output0", isp_->getEntityByName("bcm2835-isp0-capture1"));<br>
> @@ -1048,7 +1054,6 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)<br>
>         /* Wire up all the buffer connections. */<br>
>         data->unicam_[Unicam::Image].dev()->frameStart.connect(data.get(), &RPiCameraData::frameStarted);<br>
>         data->unicam_[Unicam::Image].dev()->bufferReady.connect(data.get(), &RPiCameraData::unicamBufferDequeue);<br>
> -       data->unicam_[Unicam::Embedded].dev()->bufferReady.connect(data.get(), &RPiCameraData::unicamBufferDequeue);<br>
>         data->isp_[Isp::Input].dev()->bufferReady.connect(data.get(), &RPiCameraData::ispInputDequeue);<br>
>         data->isp_[Isp::Output0].dev()->bufferReady.connect(data.get(), &RPiCameraData::ispOutputDequeue);<br>
>         data->isp_[Isp::Output1].dev()->bufferReady.connect(data.get(), &RPiCameraData::ispOutputDequeue);<br>
> @@ -1076,6 +1081,11 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)<br>
>                 return false;<br>
>         }<br>
>  <br>
> +       if (sensorConfig.sensorMetadata ^ !!embeddedEntity) {<br>
> +               LOG(RPI, Warning) << "Mismatch between Unicam and CamHelper for embedded data usage!";<br>
> +               sensorConfig.sensorMetadata = false;<br>
<br>
Should the data->unicam_[Unicam::Embedded] stream/ signal be<br>
disconnected here? (if they were created) or will they just be fine<br>
sitting idle?<br>
<br>
I presume they'll be fine and keep otherwise quiet on the pipeline, and<br>
get cleaned up at the end anyway.<br></blockquote><div><br></div><div>It shouldn't matter as it will never be triggered, but I will add a disconnect() call</div><div>in these cases.</div><div><br></div><div>Regards,</div><div>Naush</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
Reviewed-by: Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com" target="_blank">kieran.bingham@ideasonboard.com</a>><br>
<br>
> +       }<br>
> +<br>
>         /*<br>
>          * Open all Unicam and ISP streams. The exception is the embedded data<br>
>          * stream, which only gets opened below if the IPA reports that the sensor<br>
> -- <br>
> 2.25.1<br>
><br>
</blockquote></div></div>