| Message ID | 20210526114814.650016-1-umang.jain@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, Thank you for the patch. On Wed, May 26, 2021 at 05:18:14PM +0530, Umang Jain wrote: > The ControlInfoMap of entityControls member in IPAConfigInfo struct, > was not able to correctly match to the ControlInfoMap defined in > core.mojom. This resulted in a FATAL breakage when IPU3 IPA is meant to > run: > > FATAL IPADataSerializer ipa_data_serializer.cpp:437 ControlSerializer > not provided for serialization of ControlInfoMap Fixes: c76ca01323d8 ("ipa: ipu3: Introduce IPAConfigInfo in IPC") > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> As this fixes a breakage in master, I'll merge it right away. Paul, is there a way this issue could have been detected at compile time ? If that's not doable, could you send at least a documentation patch ? > --- > include/libcamera/ipa/ipu3.mojom | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom > index 6b6b431f..32c046ad 100644 > --- a/include/libcamera/ipa/ipu3.mojom > +++ b/include/libcamera/ipa/ipu3.mojom > @@ -32,7 +32,7 @@ struct IPU3Action { > > struct IPAConfigInfo { > libcamera.IPACameraSensorInfo sensorInfo; > - map<uint32, ControlInfoMap> entityControls; > + map<uint32, libcamera.ControlInfoMap> entityControls; > libcamera.Size bdsOutputSize; > libcamera.Size iif; > };
Hi Paul, On Wed, May 26, 2021 at 03:07:37PM +0300, Laurent Pinchart wrote: > On Wed, May 26, 2021 at 05:18:14PM +0530, Umang Jain wrote: > > The ControlInfoMap of entityControls member in IPAConfigInfo struct, > > was not able to correctly match to the ControlInfoMap defined in > > core.mojom. This resulted in a FATAL breakage when IPU3 IPA is meant to > > run: > > > > FATAL IPADataSerializer ipa_data_serializer.cpp:437 ControlSerializer > > not provided for serialization of ControlInfoMap > > Fixes: c76ca01323d8 ("ipa: ipu3: Introduce IPAConfigInfo in IPC") > > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > As this fixes a breakage in master, I'll merge it right away. > > Paul, is there a way this issue could have been detected at compile time > ? If that's not doable, could you send at least a documentation patch ? Beside the ControlInfoMap vs. libcamera::ControlInfoMap, here's the diff in the generated code. diff -Naur x86-gcc-10.2.0-source/include/libcamera/ipa/ipu3_ipa_serializer.h x86-gcc-11.1.0-source/include/libcamera/ipa/ipu3_ipa_serializer.h --- x86-gcc-10.2.0-source/include/libcamera/ipa/ipu3_ipa_serializer.h 2021-05-26 15:33:16.154762388 +0300 +++ x86-gcc-11.1.0-source/include/libcamera/ipa/ipu3_ipa_serializer.h 2021-05-25 12:19:18.146693517 +0300 @@ -265,7 +265,7 @@ public: static std::tuple<std::vector<uint8_t>, std::vector<int32_t>> serialize(const ipa::ipu3::IPAConfigInfo &data, + [[maybe_unused]] ControlSerializer *cs = nullptr) - ControlSerializer *cs) { std::vector<uint8_t> retData; @@ -298,7 +298,7 @@ static ipa::ipu3::IPAConfigInfo deserialize(std::vector<uint8_t> &data, + ControlSerializer *cs = nullptr) - ControlSerializer *cs) { return IPADataSerializer<ipa::ipu3::IPAConfigInfo>::deserialize(data.cbegin(), data.cend(), cs); } @@ -307,7 +307,7 @@ static ipa::ipu3::IPAConfigInfo deserialize(std::vector<uint8_t>::const_iterator dataBegin, std::vector<uint8_t>::const_iterator dataEnd, + [[maybe_unused]] ControlSerializer *cs = nullptr) - ControlSerializer *cs) { ipa::ipu3::IPAConfigInfo ret; std::vector<uint8_t>::const_iterator m = dataBegin; diff -Naur x86-gcc-10.2.0-source/src/libcamera/proxy/ipu3_ipa_proxy.cpp x86-gcc-11.1.0-source/src/libcamera/proxy/ipu3_ipa_proxy.cpp --- x86-gcc-10.2.0-source/src/libcamera/proxy/ipu3_ipa_proxy.cpp 2021-05-26 15:33:16.226762395 +0300 +++ x86-gcc-11.1.0-source/src/libcamera/proxy/ipu3_ipa_proxy.cpp 2021-05-25 12:19:18.262693528 +0300 @@ -272,7 +272,7 @@ std::vector<uint8_t> configInfoBuf; std::tie(configInfoBuf, std::ignore) = + IPADataSerializer<IPAConfigInfo>::serialize(configInfo); - IPADataSerializer<IPAConfigInfo>::serialize(configInfo, &controlSerializer_); _ipcInputBuf.data().insert(_ipcInputBuf.data().end(), configInfoBuf.begin(), configInfoBuf.end()); diff -Naur x86-gcc-10.2.0-source/src/libcamera/proxy/worker/ipu3_ipa_proxy_worker.cpp x86-gcc-11.1.0-source/src/libcamera/proxy/worker/ipu3_ipa_proxy_worker.cpp --- x86-gcc-10.2.0-source/src/libcamera/proxy/worker/ipu3_ipa_proxy_worker.cpp 2021-05-26 15:33:16.224762394 +0300 +++ x86-gcc-11.1.0-source/src/libcamera/proxy/worker/ipu3_ipa_proxy_worker.cpp 2021-05-25 12:19:18.389693540 +0300 @@ -146,7 +146,8 @@ IPAConfigInfo configInfo = IPADataSerializer<IPAConfigInfo>::deserialize( _ipcMessage.data().cbegin() + configInfoStart, - _ipcMessage.data().cend()); + _ipcMessage.data().cend(), + &controlSerializer_); ipa_->configure(configInfo); > > --- > > include/libcamera/ipa/ipu3.mojom | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom > > index 6b6b431f..32c046ad 100644 > > --- a/include/libcamera/ipa/ipu3.mojom > > +++ b/include/libcamera/ipa/ipu3.mojom > > @@ -32,7 +32,7 @@ struct IPU3Action { > > > > struct IPAConfigInfo { > > libcamera.IPACameraSensorInfo sensorInfo; > > - map<uint32, ControlInfoMap> entityControls; > > + map<uint32, libcamera.ControlInfoMap> entityControls; > > libcamera.Size bdsOutputSize; > > libcamera.Size iif; > > };
diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom index 6b6b431f..32c046ad 100644 --- a/include/libcamera/ipa/ipu3.mojom +++ b/include/libcamera/ipa/ipu3.mojom @@ -32,7 +32,7 @@ struct IPU3Action { struct IPAConfigInfo { libcamera.IPACameraSensorInfo sensorInfo; - map<uint32, ControlInfoMap> entityControls; + map<uint32, libcamera.ControlInfoMap> entityControls; libcamera.Size bdsOutputSize; libcamera.Size iif; };
The ControlInfoMap of entityControls member in IPAConfigInfo struct, was not able to correctly match to the ControlInfoMap defined in core.mojom. This resulted in a FATAL breakage when IPU3 IPA is meant to run: FATAL IPADataSerializer ipa_data_serializer.cpp:437 ControlSerializer not provided for serialization of ControlInfoMap Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- include/libcamera/ipa/ipu3.mojom | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)