[libcamera-devel] [PATCH] libcamera: ipa_interface: Make configure() return an int

Naushir Patuck naush at raspberrypi.com
Fri Dec 4 11:17:44 CET 2020


Hi Laurent,

On Tue, 1 Dec 2020 at 18:47, Naushir Patuck <naush at raspberrypi.com> wrote:

> Hi Laurent,
>
> Thank you for the patch.  This does exactly what I need!
>
> On Tue, 1 Dec 2020 at 09:59, Laurent Pinchart <
> laurent.pinchart at ideasonboard.com> wrote:
>
>> 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>
>>
>
> Reviewed-by: Naushir Patuck <naush at raspberrypi.com>
> Tested-by: Naushir Patuck <naush at raspberrypi.com>
>

Did you reach any conclusion on whether this is the preferred approach to
use for this?

Naush



> ---
>>  .../libcamera/internal/ipa_context_wrapper.h  | 10 +++---
>>  include/libcamera/ipa/ipa_interface.h         | 22 ++++++------
>>  src/ipa/libipa/ipa_interface_wrapper.cpp      | 16 ++++-----
>>  src/ipa/libipa/ipa_interface_wrapper.h        | 12 +++----
>>  src/ipa/raspberrypi/raspberrypi.cpp           | 23 +++++++------
>>  src/ipa/rkisp1/rkisp1.cpp                     | 27 ++++++++-------
>>  src/ipa/vimc/vimc.cpp                         | 10 +++---
>>  src/libcamera/ipa_context_wrapper.cpp         | 17 +++++-----
>>  src/libcamera/ipa_interface.cpp               |  4 +++
>>  src/libcamera/proxy/ipa_proxy_linux.cpp       | 10 +++---
>>  src/libcamera/proxy/ipa_proxy_thread.cpp      | 24 ++++++-------
>>  test/ipa/ipa_wrappers_test.cpp                | 34 ++++++++++++-------
>>  12 files changed, 113 insertions(+), 96 deletions(-)
>>
>> 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
>> --
>> Regards,
>>
>> Laurent Pinchart
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20201204/c47de24c/attachment-0001.htm>


More information about the libcamera-devel mailing list