[libcamera-devel] [PATCH 2/3] src: rkisp1: return error from ipa's configure method if it fails
paul.elder at ideasonboard.com
paul.elder at ideasonboard.com
Wed Feb 17 09:42:33 CET 2021
Hi Dafna,
On Tue, Feb 16, 2021 at 01:26:23PM +0100, Dafna Hirschfeld wrote:
> The IPA of rkisp1 relay some of the camera's controls.
> Therefore it can't work if those controls don't exist.
> Return a negative int in that case.
> Also return error from the 'configure' method of the pipeline
> handler if the IPA configure fails.
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld at collabora.com>
Looks good to me.
Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
> include/libcamera/ipa/rkisp1.mojom | 2 +-
> src/ipa/rkisp1/rkisp1.cpp | 11 ++++++-----
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 +++++--
> 3 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
> index 9270f9c7..95fa0d93 100644
> --- a/include/libcamera/ipa/rkisp1.mojom
> +++ b/include/libcamera/ipa/rkisp1.mojom
> @@ -31,7 +31,7 @@ interface IPARkISP1Interface {
>
> configure(CameraSensorInfo sensorInfo,
> map<uint32, IPAStream> streamConfig,
> - map<uint32, ControlInfoMap> entityControls) => ();
> + map<uint32, ControlInfoMap> entityControls) => (int32 ret);
>
> mapBuffers(array<IPABuffer> buffers);
> unmapBuffers(array<uint32> ids);
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 67bac986..b0641faf 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -38,7 +38,7 @@ public:
> int start() override { return 0; }
> void stop() override {}
>
> - void configure(const CameraSensorInfo &info,
> + int configure(const CameraSensorInfo &info,
> const std::map<uint32_t, IPAStream> &streamConfig,
> const std::map<uint32_t, ControlInfoMap> &entityControls) override;
> void mapBuffers(const std::vector<IPABuffer> &buffers) override;
> @@ -75,25 +75,25 @@ private:
> * assemble one. Make sure the reported sensor information are relevant
> * before accessing them.
> */
> -void IPARkISP1::configure([[maybe_unused]] const CameraSensorInfo &info,
> +int IPARkISP1::configure([[maybe_unused]] const CameraSensorInfo &info,
> [[maybe_unused]] const std::map<uint32_t, IPAStream> &streamConfig,
> const std::map<uint32_t, ControlInfoMap> &entityControls)
> {
> if (entityControls.empty())
> - return;
> + return -1;
>
> ctrls_ = entityControls.at(0);
>
> const auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);
> if (itExp == ctrls_.end()) {
> LOG(IPARkISP1, Error) << "Can't find exposure control";
> - return;
> + return -1;
> }
>
> const auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN);
> if (itGain == ctrls_.end()) {
> LOG(IPARkISP1, Error) << "Can't find gain control";
> - return;
> + return -1;
> }
>
> autoExposure_ = true;
> @@ -111,6 +111,7 @@ void IPARkISP1::configure([[maybe_unused]] const CameraSensorInfo &info,
> << " Gain: " << minGain_ << "-" << maxGain_;
>
> setControls(0);
> + return 0;
> }
>
> void IPARkISP1::mapBuffers(const std::vector<IPABuffer> &buffers)
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 1dd878fc..dad8e5d4 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -653,8 +653,11 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> std::map<uint32_t, ControlInfoMap> entityControls;
> entityControls.emplace(0, data->sensor_->controls());
>
> - data->ipa_->configure(sensorInfo, streamConfig, entityControls);
> -
> + ret = data->ipa_->configure(sensorInfo, streamConfig, entityControls);
> + if (ret) {
> + LOG(RkISP1, Error) << "failed configuring IPA (" << ret << ")";
> + return ret;
> + }
> return 0;
> }
>
> --
> 2.17.1
>
More information about the libcamera-devel
mailing list