[libcamera-devel] [PATCH v2] pipeline: ipa: raspberrypi: Handle failures during IPA configuration
Naushir Patuck
naush at raspberrypi.com
Tue Dec 1 10:59:57 CET 2020
Hi Laurent,
On Tue, 1 Dec 2020 at 09:54, Naushir Patuck <naush at raspberrypi.com> wrote:
> Hi Laurent,
>
> On Tue, 1 Dec 2020 at 01:55, Laurent Pinchart <
> laurent.pinchart at ideasonboard.com> wrote:
>
>> Hi Naush,
>>
>> Thank you for the patch.
>>
>> On Fri, Nov 27, 2020 at 02:56:12PM +0000, Naushir Patuck wrote:
>> > If the IPA fails during configuration, return an error flag to the
>> > pipeline handler and fail the use case gracefully.
>> >
>> > At present, the IPA configuration can fail for the following reasons:
>> > - The sensor is not recognised, and fails to open a CamHelper object.
>> > - The pipeline handler did not pass in controls for the ISP and sensor.
>> >
>> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
>> > Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>>
>> Wouldn't it be simpler to modify the configure() function to return an
>> int ? How about the following ? If it works for you I'll submit a proper
>> patch.
>>
>
> Yes, that works equally well for me. Happy for you to submit the patch,
> or would you prefer if I add your commit below to a new patch set that will
> also include the Raspberry Pi IPA/PH changes?
>
Sorry, should have been a bit clearer. I have two more patches (not yet
submitted for review) that are based on this work. I could include all of
this in a new series including your patch below. Otherwise, I am happy to
submit them separately once this has gone through.
>
> Regards,
> Naush
>
>
>>
>> Author: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>> Date: Tue Dec 1 03:54:18 2020 +0200
>>
>> libcamera: ipa_interface: Make configure() return an int
>>
>> It's useful for the IPAInterface::configure() function to be able to
>> return a status. Make it return an int, to avoid forcing IPAs to
>> return
>> a status encoded in the IPAOperationData in a custom way.
>>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>
>> diff --git a/include/libcamera/internal/ipa_context_wrapper.h
>> b/include/libcamera/internal/ipa_context_wrapper.h
>> index 8f767e844221..a00b5e7b92eb 100644
>> --- a/include/libcamera/internal/ipa_context_wrapper.h
>> +++ b/include/libcamera/internal/ipa_context_wrapper.h
>> @@ -22,11 +22,11 @@ public:
>> int init(const IPASettings &settings) override;
>> int start() override;
>> void stop() override;
>> - void configure(const CameraSensorInfo &sensorInfo,
>> - const std::map<unsigned int, IPAStream>
>> &streamConfig,
>> - const std::map<unsigned int, const ControlInfoMap
>> &> &entityControls,
>> - const IPAOperationData &ipaConfig,
>> - IPAOperationData *result) override;
>> + int configure(const CameraSensorInfo &sensorInfo,
>> + const std::map<unsigned int, IPAStream>
>> &streamConfig,
>> + const std::map<unsigned int, const ControlInfoMap
>> &> &entityControls,
>> + const IPAOperationData &ipaConfig,
>> + IPAOperationData *result) override;
>>
>> void mapBuffers(const std::vector<IPABuffer> &buffers) override;
>> void unmapBuffers(const std::vector<unsigned int> &ids) override;
>> diff --git a/include/libcamera/ipa/ipa_interface.h
>> b/include/libcamera/ipa/ipa_interface.h
>> index 322b7079070e..1d8cf8dc1c56 100644
>> --- a/include/libcamera/ipa/ipa_interface.h
>> +++ b/include/libcamera/ipa/ipa_interface.h
>> @@ -95,12 +95,12 @@ struct ipa_context_ops {
>> void (*register_callbacks)(struct ipa_context *ctx,
>> const struct ipa_callback_ops
>> *callbacks,
>> void *cb_ctx);
>> - void (*configure)(struct ipa_context *ctx,
>> - const struct ipa_sensor_info *sensor_info,
>> - const struct ipa_stream *streams,
>> - unsigned int num_streams,
>> - const struct ipa_control_info_map *maps,
>> - unsigned int num_maps);
>> + int (*configure)(struct ipa_context *ctx,
>> + const struct ipa_sensor_info *sensor_info,
>> + const struct ipa_stream *streams,
>> + unsigned int num_streams,
>> + const struct ipa_control_info_map *maps,
>> + unsigned int num_maps);
>> void (*map_buffers)(struct ipa_context *ctx,
>> const struct ipa_buffer *buffers,
>> size_t num_buffers);
>> @@ -156,11 +156,11 @@ public:
>> virtual int start() = 0;
>> virtual void stop() = 0;
>>
>> - virtual void configure(const CameraSensorInfo &sensorInfo,
>> - const std::map<unsigned int, IPAStream>
>> &streamConfig,
>> - const std::map<unsigned int, const
>> ControlInfoMap &> &entityControls,
>> - const IPAOperationData &ipaConfig,
>> - IPAOperationData *result) = 0;
>> + virtual int configure(const CameraSensorInfo &sensorInfo,
>> + const std::map<unsigned int, IPAStream>
>> &streamConfig,
>> + const std::map<unsigned int, const
>> ControlInfoMap &> &entityControls,
>> + const IPAOperationData &ipaConfig,
>> + IPAOperationData *result) = 0;
>>
>> virtual void mapBuffers(const std::vector<IPABuffer> &buffers) =
>> 0;
>> virtual void unmapBuffers(const std::vector<unsigned int> &ids) =
>> 0;
>> diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp
>> b/src/ipa/libipa/ipa_interface_wrapper.cpp
>> index cee532e3a747..74d7bc6e1c9b 100644
>> --- a/src/ipa/libipa/ipa_interface_wrapper.cpp
>> +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp
>> @@ -115,12 +115,12 @@ void IPAInterfaceWrapper::register_callbacks(struct
>> ipa_context *_ctx,
>> ctx->cb_ctx_ = cb_ctx;
>> }
>>
>> -void IPAInterfaceWrapper::configure(struct ipa_context *_ctx,
>> - const struct ipa_sensor_info
>> *sensor_info,
>> - const struct ipa_stream *streams,
>> - unsigned int num_streams,
>> - const struct ipa_control_info_map
>> *maps,
>> - unsigned int num_maps)
>> +int IPAInterfaceWrapper::configure(struct ipa_context *_ctx,
>> + const struct ipa_sensor_info
>> *sensor_info,
>> + const struct ipa_stream *streams,
>> + unsigned int num_streams,
>> + const struct ipa_control_info_map
>> *maps,
>> + unsigned int num_maps)
>> {
>> IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper
>> *>(_ctx);
>>
>> @@ -168,8 +168,8 @@ void IPAInterfaceWrapper::configure(struct
>> ipa_context *_ctx,
>>
>> /* \todo Translate the ipaConfig and result. */
>> IPAOperationData ipaConfig;
>> - ctx->ipa_->configure(sensorInfo, ipaStreams, entityControls,
>> ipaConfig,
>> - nullptr);
>> + return ctx->ipa_->configure(sensorInfo, ipaStreams,
>> entityControls,
>> + ipaConfig, nullptr);
>> }
>>
>> void IPAInterfaceWrapper::map_buffers(struct ipa_context *_ctx,
>> diff --git a/src/ipa/libipa/ipa_interface_wrapper.h
>> b/src/ipa/libipa/ipa_interface_wrapper.h
>> index a1c701599b56..acd3160039b1 100644
>> --- a/src/ipa/libipa/ipa_interface_wrapper.h
>> +++ b/src/ipa/libipa/ipa_interface_wrapper.h
>> @@ -30,12 +30,12 @@ private:
>> static void register_callbacks(struct ipa_context *ctx,
>> const struct ipa_callback_ops
>> *callbacks,
>> void *cb_ctx);
>> - static void configure(struct ipa_context *ctx,
>> - const struct ipa_sensor_info *sensor_info,
>> - const struct ipa_stream *streams,
>> - unsigned int num_streams,
>> - const struct ipa_control_info_map *maps,
>> - unsigned int num_maps);
>> + static int configure(struct ipa_context *ctx,
>> + const struct ipa_sensor_info *sensor_info,
>> + const struct ipa_stream *streams,
>> + unsigned int num_streams,
>> + const struct ipa_control_info_map *maps,
>> + unsigned int num_maps);
>> static void map_buffers(struct ipa_context *ctx,
>> const struct ipa_buffer *c_buffers,
>> size_t num_buffers);
>> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
>> b/src/ipa/raspberrypi/raspberrypi.cpp
>> index 9853a343c892..75f7af3430ef 100644
>> --- a/src/ipa/raspberrypi/raspberrypi.cpp
>> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
>> @@ -81,11 +81,11 @@ public:
>> int start() override { return 0; }
>> void stop() override {}
>>
>> - void configure(const CameraSensorInfo &sensorInfo,
>> - const std::map<unsigned int, IPAStream>
>> &streamConfig,
>> - const std::map<unsigned int, const ControlInfoMap
>> &> &entityControls,
>> - const IPAOperationData &data,
>> - IPAOperationData *response) override;
>> + int configure(const CameraSensorInfo &sensorInfo,
>> + const std::map<unsigned int, IPAStream>
>> &streamConfig,
>> + const std::map<unsigned int, const ControlInfoMap
>> &> &entityControls,
>> + const IPAOperationData &data,
>> + IPAOperationData *response) override;
>> void mapBuffers(const std::vector<IPABuffer> &buffers) override;
>> void unmapBuffers(const std::vector<unsigned int> &ids) override;
>> void processEvent(const IPAOperationData &event) override;
>> @@ -191,14 +191,14 @@ void IPARPi::setMode(const CameraSensorInfo
>> &sensorInfo)
>> mode_.line_length = 1e9 * sensorInfo.lineLength /
>> sensorInfo.pixelRate;
>> }
>>
>> -void IPARPi::configure(const CameraSensorInfo &sensorInfo,
>> - [[maybe_unused]] const std::map<unsigned int,
>> IPAStream> &streamConfig,
>> - const std::map<unsigned int, const ControlInfoMap
>> &> &entityControls,
>> - const IPAOperationData &ipaConfig,
>> - IPAOperationData *result)
>> +int IPARPi::configure(const CameraSensorInfo &sensorInfo,
>> + [[maybe_unused]] const std::map<unsigned int,
>> IPAStream> &streamConfig,
>> + const std::map<unsigned int, const ControlInfoMap
>> &> &entityControls,
>> + const IPAOperationData &ipaConfig,
>> + IPAOperationData *result)
>> {
>> if (entityControls.empty())
>> - return;
>> + return -EINVAL;
>>
>> result->operation = 0;
>>
>> @@ -314,6 +314,7 @@ void IPARPi::configure(const CameraSensorInfo
>> &sensorInfo,
>> }
>>
>> lastMode_ = mode_;
>> + return 0;
>> }
>>
>> void IPARPi::mapBuffers(const std::vector<IPABuffer> &buffers)
>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
>> index 07d7f1b2ddd8..78d78c5ac521 100644
>> --- a/src/ipa/rkisp1/rkisp1.cpp
>> +++ b/src/ipa/rkisp1/rkisp1.cpp
>> @@ -40,11 +40,11 @@ public:
>> int start() override { return 0; }
>> void stop() override {}
>>
>> - void configure(const CameraSensorInfo &info,
>> - const std::map<unsigned int, IPAStream>
>> &streamConfig,
>> - const std::map<unsigned int, const ControlInfoMap
>> &> &entityControls,
>> - const IPAOperationData &ipaConfig,
>> - IPAOperationData *response) override;
>> + int configure(const CameraSensorInfo &info,
>> + const std::map<unsigned int, IPAStream>
>> &streamConfig,
>> + const std::map<unsigned int, const ControlInfoMap
>> &> &entityControls,
>> + const IPAOperationData &ipaConfig,
>> + IPAOperationData *response) override;
>> void mapBuffers(const std::vector<IPABuffer> &buffers) override;
>> void unmapBuffers(const std::vector<unsigned int> &ids) override;
>> void processEvent(const IPAOperationData &event) override;
>> @@ -79,27 +79,27 @@ private:
>> * assemble one. Make sure the reported sensor information are relevant
>> * before accessing them.
>> */
>> -void IPARkISP1::configure([[maybe_unused]] const CameraSensorInfo &info,
>> - [[maybe_unused]] const std::map<unsigned int,
>> IPAStream> &streamConfig,
>> - const std::map<unsigned int, const
>> ControlInfoMap &> &entityControls,
>> - [[maybe_unused]] const IPAOperationData
>> &ipaConfig,
>> - [[maybe_unused]] IPAOperationData *result)
>> +int IPARkISP1::configure([[maybe_unused]] const CameraSensorInfo &info,
>> + [[maybe_unused]] const std::map<unsigned int,
>> IPAStream> &streamConfig,
>> + const std::map<unsigned int, const
>> ControlInfoMap &> &entityControls,
>> + [[maybe_unused]] const IPAOperationData
>> &ipaConfig,
>> + [[maybe_unused]] IPAOperationData *result)
>> {
>> if (entityControls.empty())
>> - return;
>> + return -EINVAL;
>>
>> 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 -EINVAL;
>> }
>>
>> const auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN);
>> if (itGain == ctrls_.end()) {
>> LOG(IPARkISP1, Error) << "Can't find gain control";
>> - return;
>> + return -EINVAL;
>> }
>>
>> autoExposure_ = true;
>> @@ -117,6 +117,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/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
>> index cf8411359e40..79c8c2fb8927 100644
>> --- a/src/ipa/vimc/vimc.cpp
>> +++ b/src/ipa/vimc/vimc.cpp
>> @@ -37,11 +37,11 @@ public:
>> int start() override;
>> void stop() override;
>>
>> - void configure([[maybe_unused]] const CameraSensorInfo
>> &sensorInfo,
>> - [[maybe_unused]] const std::map<unsigned int,
>> IPAStream> &streamConfig,
>> - [[maybe_unused]] const std::map<unsigned int,
>> const ControlInfoMap &> &entityControls,
>> - [[maybe_unused]] const IPAOperationData &ipaConfig,
>> - [[maybe_unused]] IPAOperationData *result)
>> override {}
>> + int configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,
>> + [[maybe_unused]] const std::map<unsigned int,
>> IPAStream> &streamConfig,
>> + [[maybe_unused]] const std::map<unsigned int, const
>> ControlInfoMap &> &entityControls,
>> + [[maybe_unused]] const IPAOperationData &ipaConfig,
>> + [[maybe_unused]] IPAOperationData *result) override
>> { return 0; }
>> void mapBuffers([[maybe_unused]] const std::vector<IPABuffer>
>> &buffers) override {}
>> void unmapBuffers([[maybe_unused]] const std::vector<unsigned
>> int> &ids) override {}
>> void processEvent([[maybe_unused]] const IPAOperationData &event)
>> override {}
>> diff --git a/src/libcamera/ipa_context_wrapper.cpp
>> b/src/libcamera/ipa_context_wrapper.cpp
>> index 231300ce0bec..f63ad830c003 100644
>> --- a/src/libcamera/ipa_context_wrapper.cpp
>> +++ b/src/libcamera/ipa_context_wrapper.cpp
>> @@ -108,18 +108,18 @@ void IPAContextWrapper::stop()
>> ctx_->ops->stop(ctx_);
>> }
>>
>> -void IPAContextWrapper::configure(const CameraSensorInfo &sensorInfo,
>> - const std::map<unsigned int, IPAStream>
>> &streamConfig,
>> - const std::map<unsigned int, const
>> ControlInfoMap &> &entityControls,
>> - const IPAOperationData &ipaConfig,
>> - IPAOperationData *result)
>> +int IPAContextWrapper::configure(const CameraSensorInfo &sensorInfo,
>> + const std::map<unsigned int, IPAStream>
>> &streamConfig,
>> + const std::map<unsigned int, const
>> ControlInfoMap &> &entityControls,
>> + const IPAOperationData &ipaConfig,
>> + IPAOperationData *result)
>> {
>> if (intf_)
>> return intf_->configure(sensorInfo, streamConfig,
>> entityControls, ipaConfig,
>> result);
>>
>> if (!ctx_)
>> - return;
>> + return 0;
>>
>> serializer_.reset();
>>
>> @@ -178,8 +178,9 @@ void IPAContextWrapper::configure(const
>> CameraSensorInfo &sensorInfo,
>> }
>>
>> /* \todo Translate the ipaConfig and reponse */
>> - ctx_->ops->configure(ctx_, &sensor_info, c_streams,
>> streamConfig.size(),
>> - c_info_maps, entityControls.size());
>> + return ctx_->ops->configure(ctx_, &sensor_info, c_streams,
>> + streamConfig.size(), c_info_maps,
>> + entityControls.size());
>> }
>>
>> void IPAContextWrapper::mapBuffers(const std::vector<IPABuffer> &buffers)
>> diff --git a/src/libcamera/ipa_interface.cpp
>> b/src/libcamera/ipa_interface.cpp
>> index 23fc56d7d48e..516a8ecd4b53 100644
>> --- a/src/libcamera/ipa_interface.cpp
>> +++ b/src/libcamera/ipa_interface.cpp
>> @@ -347,6 +347,8 @@
>> * \param[in] num_maps The number of entries in the \a maps array
>> *
>> * \sa libcamera::IPAInterface::configure()
>> + *
>> + * \return 0 on success or a negative error code on failure
>> */
>>
>> /**
>> @@ -573,6 +575,8 @@ namespace libcamera {
>> * pipeline handler to the IPA and back. The pipeline handler may set
>> the \a
>> * result parameter to null if the IPA protocol doesn't need to pass a
>> result
>> * back through the configure() function.
>> + *
>> + * \return 0 on success or a negative error code on failure
>> */
>>
>> /**
>> diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp
>> b/src/libcamera/proxy/ipa_proxy_linux.cpp
>> index b78a0e4535f5..ed250ce79c17 100644
>> --- a/src/libcamera/proxy/ipa_proxy_linux.cpp
>> +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp
>> @@ -32,11 +32,11 @@ public:
>> }
>> int start() override { return 0; }
>> void stop() override {}
>> - void configure([[maybe_unused]] const CameraSensorInfo
>> &sensorInfo,
>> - [[maybe_unused]] const std::map<unsigned int,
>> IPAStream> &streamConfig,
>> - [[maybe_unused]] const std::map<unsigned int,
>> const ControlInfoMap &> &entityControls,
>> - [[maybe_unused]] const IPAOperationData &ipaConfig,
>> - [[maybe_unused]] IPAOperationData *result)
>> override {}
>> + int configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,
>> + [[maybe_unused]] const std::map<unsigned int,
>> IPAStream> &streamConfig,
>> + [[maybe_unused]] const std::map<unsigned int, const
>> ControlInfoMap &> &entityControls,
>> + [[maybe_unused]] const IPAOperationData &ipaConfig,
>> + [[maybe_unused]] IPAOperationData *result) override
>> { return 0; }
>> void mapBuffers([[maybe_unused]] const std::vector<IPABuffer>
>> &buffers) override {}
>> void unmapBuffers([[maybe_unused]] const std::vector<unsigned
>> int> &ids) override {}
>> void processEvent([[maybe_unused]] const IPAOperationData &event)
>> override {}
>> diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp
>> b/src/libcamera/proxy/ipa_proxy_thread.cpp
>> index eead2883708d..fd91726c4840 100644
>> --- a/src/libcamera/proxy/ipa_proxy_thread.cpp
>> +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp
>> @@ -29,11 +29,11 @@ public:
>> int start() override;
>> void stop() override;
>>
>> - void configure(const CameraSensorInfo &sensorInfo,
>> - const std::map<unsigned int, IPAStream>
>> &streamConfig,
>> - const std::map<unsigned int, const ControlInfoMap
>> &> &entityControls,
>> - const IPAOperationData &ipaConfig,
>> - IPAOperationData *result) override;
>> + int configure(const CameraSensorInfo &sensorInfo,
>> + const std::map<unsigned int, IPAStream>
>> &streamConfig,
>> + const std::map<unsigned int, const ControlInfoMap
>> &> &entityControls,
>> + const IPAOperationData &ipaConfig,
>> + IPAOperationData *result) override;
>> void mapBuffers(const std::vector<IPABuffer> &buffers) override;
>> void unmapBuffers(const std::vector<unsigned int> &ids) override;
>> void processEvent(const IPAOperationData &event) override;
>> @@ -132,14 +132,14 @@ void IPAProxyThread::stop()
>> thread_.wait();
>> }
>>
>> -void IPAProxyThread::configure(const CameraSensorInfo &sensorInfo,
>> - const std::map<unsigned int, IPAStream>
>> &streamConfig,
>> - const std::map<unsigned int, const
>> ControlInfoMap &> &entityControls,
>> - const IPAOperationData &ipaConfig,
>> - IPAOperationData *result)
>> +int IPAProxyThread::configure(const CameraSensorInfo &sensorInfo,
>> + const std::map<unsigned int, IPAStream>
>> &streamConfig,
>> + const std::map<unsigned int, const
>> ControlInfoMap &> &entityControls,
>> + const IPAOperationData &ipaConfig,
>> + IPAOperationData *result)
>> {
>> - ipa_->configure(sensorInfo, streamConfig, entityControls,
>> ipaConfig,
>> - result);
>> + return ipa_->configure(sensorInfo, streamConfig, entityControls,
>> + ipaConfig, result);
>> }
>>
>> void IPAProxyThread::mapBuffers(const std::vector<IPABuffer> &buffers)
>> diff --git a/test/ipa/ipa_wrappers_test.cpp
>> b/test/ipa/ipa_wrappers_test.cpp
>> index 59d991cbbf6a..ad0fb0386865 100644
>> --- a/test/ipa/ipa_wrappers_test.cpp
>> +++ b/test/ipa/ipa_wrappers_test.cpp
>> @@ -67,55 +67,63 @@ public:
>> report(Op_stop, TestPass);
>> }
>>
>> - void configure(const CameraSensorInfo &sensorInfo,
>> - const std::map<unsigned int, IPAStream>
>> &streamConfig,
>> - const std::map<unsigned int, const ControlInfoMap
>> &> &entityControls,
>> - [[maybe_unused]] const IPAOperationData &ipaConfig,
>> - [[maybe_unused]] IPAOperationData *result) override
>> + int configure(const CameraSensorInfo &sensorInfo,
>> + const std::map<unsigned int, IPAStream>
>> &streamConfig,
>> + const std::map<unsigned int, const ControlInfoMap
>> &> &entityControls,
>> + [[maybe_unused]] const IPAOperationData &ipaConfig,
>> + [[maybe_unused]] IPAOperationData *result) override
>> {
>> /* Verify sensorInfo. */
>> if (sensorInfo.outputSize.width != 2560 ||
>> sensorInfo.outputSize.height != 1940) {
>> cerr << "configure(): Invalid sensor info size "
>> << sensorInfo.outputSize.toString();
>> + report(Op_configure, TestFail);
>> + return -EINVAL;
>> }
>>
>> /* Verify streamConfig. */
>> if (streamConfig.size() != 2) {
>> cerr << "configure(): Invalid number of streams "
>> << streamConfig.size() << endl;
>> - return report(Op_configure, TestFail);
>> + report(Op_configure, TestFail);
>> + return -EINVAL;
>> }
>>
>> auto iter = streamConfig.find(1);
>> if (iter == streamConfig.end()) {
>> cerr << "configure(): No configuration for stream
>> 1" << endl;
>> - return report(Op_configure, TestFail);
>> + report(Op_configure, TestFail);
>> + return -EINVAL;
>> }
>> const IPAStream *stream = &iter->second;
>> if (stream->pixelFormat != V4L2_PIX_FMT_YUYV ||
>> stream->size != Size{ 1024, 768 }) {
>> cerr << "configure(): Invalid configuration for
>> stream 1" << endl;
>> - return report(Op_configure, TestFail);
>> + report(Op_configure, TestFail);
>> + return -EINVAL;
>> }
>>
>> iter = streamConfig.find(2);
>> if (iter == streamConfig.end()) {
>> cerr << "configure(): No configuration for stream
>> 2" << endl;
>> - return report(Op_configure, TestFail);
>> + report(Op_configure, TestFail);
>> + return -EINVAL;
>> }
>> stream = &iter->second;
>> if (stream->pixelFormat != V4L2_PIX_FMT_NV12 ||
>> stream->size != Size{ 800, 600 }) {
>> cerr << "configure(): Invalid configuration for
>> stream 2" << endl;
>> - return report(Op_configure, TestFail);
>> + report(Op_configure, TestFail);
>> + return -EINVAL;
>> }
>>
>> /* Verify entityControls. */
>> auto ctrlIter = entityControls.find(42);
>> if (ctrlIter == entityControls.end()) {
>> cerr << "configure(): Controls not found" << endl;
>> - return report(Op_configure, TestFail);
>> + report(Op_configure, TestFail);
>> + return -EINVAL;
>> }
>>
>> const ControlInfoMap &infoMap = ctrlIter->second;
>> @@ -124,10 +132,12 @@ public:
>> infoMap.count(V4L2_CID_CONTRAST) != 1 ||
>> infoMap.count(V4L2_CID_SATURATION) != 1) {
>> cerr << "configure(): Invalid control IDs" <<
>> endl;
>> - return report(Op_configure, TestFail);
>> + report(Op_configure, TestFail);
>> + return -EINVAL;
>> }
>>
>> report(Op_configure, TestPass);
>> + return 0;
>> }
>>
>> void mapBuffers(const std::vector<IPABuffer> &buffers) override
>>
>> > ---
>> > include/libcamera/ipa/raspberrypi.h | 1 +
>> > src/ipa/raspberrypi/raspberrypi.cpp | 12 +++++++++++-
>> > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 5 +++++
>> > 3 files changed, 17 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/include/libcamera/ipa/raspberrypi.h
>> b/include/libcamera/ipa/raspberrypi.h
>> > index 2b55dbfc..86c97ffa 100644
>> > --- a/include/libcamera/ipa/raspberrypi.h
>> > +++ b/include/libcamera/ipa/raspberrypi.h
>> > @@ -21,6 +21,7 @@ enum ConfigParameters {
>> > IPA_CONFIG_STAGGERED_WRITE = (1 << 1),
>> > IPA_CONFIG_SENSOR = (1 << 2),
>> > IPA_CONFIG_DROP_FRAMES = (1 << 3),
>> > + IPA_CONFIG_FAILED = (1 << 4),
>> > };
>> >
>> > enum Operations {
>> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
>> b/src/ipa/raspberrypi/raspberrypi.cpp
>> > index 9853a343..57dd9c61 100644
>> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
>> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
>> > @@ -197,8 +197,11 @@ void IPARPi::configure(const CameraSensorInfo
>> &sensorInfo,
>> > const IPAOperationData &ipaConfig,
>> > IPAOperationData *result)
>> > {
>> > - if (entityControls.empty())
>> > + if (entityControls.size() != 2) {
>> > + LOG(IPARPI, Error) << "No ISP or sensor controls found.";
>> > + result->operation = RPi::IPA_CONFIG_FAILED;
>> > return;
>> > + }
>> >
>> > result->operation = 0;
>> >
>> > @@ -217,6 +220,13 @@ void IPARPi::configure(const CameraSensorInfo
>> &sensorInfo,
>> > if (!helper_) {
>> > helper_ =
>> std::unique_ptr<RPiController::CamHelper>(RPiController::CamHelper::Create(cameraName));
>> >
>> > + if (!helper_) {
>> > + LOG(IPARPI, Error) << "Could not create camera
>> helper for "
>> > + << cameraName;
>> > + result->operation = RPi::IPA_CONFIG_FAILED;
>> > + return;
>> > + }
>> > +
>> > /*
>> > * Pass out the sensor config to the pipeline handler in
>> order
>> > * to setup the staggered writer class.
>> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
>> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
>> > index 6fcdf557..76252806 100644
>> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
>> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
>> > @@ -1194,6 +1194,11 @@ int RPiCameraData::configureIPA(const
>> CameraConfiguration *config)
>> > ipa_->configure(sensorInfo_, streamConfig, entityControls,
>> ipaConfig,
>> > &result);
>> >
>> > + if (result.operation & RPi::IPA_CONFIG_FAILED) {
>> > + LOG(RPI, Error) << "IPA configuration failed!";
>> > + return -EPIPE;
>> > + }
>> > +
>> > unsigned int resultIdx = 0;
>> > if (result.operation & RPi::IPA_CONFIG_STAGGERED_WRITE) {
>> > /*
>>
>> --
>> Regards,
>>
>> Laurent Pinchart
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20201201/16192ba8/attachment-0001.htm>
More information about the libcamera-devel
mailing list