<div dir="ltr"><div dir="ltr">Hi Laurent,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 23 Mar 2021 at 16:36, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@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">Hi Naush,<br>
<br>
On Tue, Mar 23, 2021 at 02:36:04PM +0000, Naushir Patuck wrote:<br>
> Pass the sensor model string to the IPA init() method through the<br>
> IPASettings structure.<br>
> <br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> Tested-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>
> include/libcamera/ipa/core.mojom | 8 ++++++++<br>
> src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ++-<br>
> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 3 ++-<br>
> src/libcamera/pipeline/vimc/vimc.cpp | 2 +-<br>
> test/ipa/ipa_interface_test.cpp | 2 +-<br>
> 5 files changed, 14 insertions(+), 4 deletions(-)<br>
> <br>
> diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom<br>
> index 5236a672461c..5363f1c5b48b 100644<br>
> --- a/include/libcamera/ipa/core.mojom<br>
> +++ b/include/libcamera/ipa/core.mojom<br>
> @@ -145,8 +145,16 @@ struct IPABuffer {<br>
> * This field may be an empty string if the IPA doesn't require a configuration<br>
> * file.<br>
> */<br>
> +<br>
> + /**<br>
> + * \var IPASettings::sensorModel<br>
> + * \brief The sensor model name<br>
> + *<br>
> + * Provides the sensor model name to the IPA.<br>
> + */<br>
> struct IPASettings {<br>
> string configurationFile;<br>
> + string sensorModel;<br>
> };<br>
> <br>
> /**<br>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp<br>
> index 2ea13ec9e1b9..c27120710323 100644<br>
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp<br>
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp<br>
> @@ -1144,7 +1144,8 @@ int IPU3CameraData::loadIPA()<br>
> <br>
> ipa_->queueFrameAction.connect(this, &IPU3CameraData::queueFrameAction);<br>
> <br>
> - ipa_->init(IPASettings{});<br>
> + CameraSensor *sensor = cio2_.sensor();<br>
> + ipa_->init(IPASettings{ "", sensor->model() });<br>
> <br>
> return 0;<br>
> }<br>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> index 00fa62c972ed..2c8ae31a28ad 100644<br>
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> @@ -1227,7 +1227,8 @@ int RPiCameraData::loadIPA()<br>
> ipa_->setIspControls.connect(this, &RPiCameraData::setIspControls);<br>
> ipa_->setDelayedControls.connect(this, &RPiCameraData::setDelayedControls);<br>
> <br>
> - IPASettings settings(ipa_->configurationFile(sensor_->model() + ".json"));<br>
> + IPASettings settings(ipa_->configurationFile(sensor_->model() + ".json"),<br>
> + sensor_->model());<br>
> <br>
> return ipa_->init(settings);<br>
> }<br>
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp<br>
> index 57c65b021c46..2dfb63ecf2ef 100644<br>
> --- a/src/libcamera/pipeline/vimc/vimc.cpp<br>
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp<br>
> @@ -425,7 +425,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)<br>
> data->ipa_ = IPAManager::createIPA<ipa::vimc::IPAProxyVimc>(this, 0, 0);<br>
> if (data->ipa_ != nullptr) {<br>
> std::string conf = data->ipa_->configurationFile("vimc.conf");<br>
> - data->ipa_->init(IPASettings{ conf });<br>
> + data->ipa_->init(IPASettings{ conf, data->sensor_->model() });<br>
> } else {<br>
> LOG(VIMC, Warning) << "no matching IPA found";<br>
> }<br>
<br>
This crashes, as data->sensor_ is null here :-S This fixes it.<br></blockquote><div><br></div><div>Sorry for breaking that :(</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>
diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp<br>
index 2dfb63ecf2ef..8f5f4ba30953 100644<br>
--- a/src/libcamera/pipeline/vimc/vimc.cpp<br>
+++ b/src/libcamera/pipeline/vimc/vimc.cpp<br>
@@ -422,6 +422,10 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)<br>
<br>
std::unique_ptr<VimcCameraData> data = std::make_unique<VimcCameraData>(this, media);<br>
<br>
+ /* Locate and open the capture video node. */<br>
+ if (data->init())<br>
+ return false;<br>
+<br>
data->ipa_ = IPAManager::createIPA<ipa::vimc::IPAProxyVimc>(this, 0, 0);<br>
if (data->ipa_ != nullptr) {<br>
std::string conf = data->ipa_->configurationFile("vimc.conf");<br>
@@ -430,10 +434,6 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)<br>
LOG(VIMC, Warning) << "no matching IPA found";<br>
}<br>
<br>
- /* Locate and open the capture video node. */<br>
- if (data->init())<br>
- return false;<br>
-<br>
/* Create and register the camera. */<br>
std::set<Stream *> streams{ &data->stream_ };<br>
std::shared_ptr<Camera> camera =<br>
<br>
Could you run "ninja test" on future patch series ? It requires the<br>
vimc, vivid and vim2m modules to be loaded, and you can do so on an x86<br>
machine.<br></blockquote><div><br></div><div>I'll be sure to run through these tests before submitting next time.</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>
I'll fold this fix in when merging.<br>
<br>
> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp<br>
> index 4b88806f8f67..d6ca6f5137b0 100644<br>
> --- a/test/ipa/ipa_interface_test.cpp<br>
> +++ b/test/ipa/ipa_interface_test.cpp<br>
> @@ -104,7 +104,7 @@ protected:<br>
> <br>
> /* Test initialization of IPA module. */<br>
> std::string conf = ipa_->configurationFile("vimc.conf");<br>
> - int ret = ipa_->init(IPASettings{ conf });<br>
> + int ret = ipa_->init(IPASettings{ conf, "vimc" });<br>
> if (ret < 0) {<br>
> cerr << "IPA interface init() failed" << endl;<br>
> return TestFail;<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>