[libcamera-devel] [PATCH v2 0/2] Raspberry Pi: Metadata parsing improvements (II)

[Naushir Patuck]

On Tue, 22 Jun 2021 at 14:20, Naushir Patuck <naush at raspberrypi.com> wrote:

> Hi,
>
> Here is version 2 of this series.  The following changes have been
> introduced
> over v1:
>
> - Rework patch 1/2 to use a unique_ptr to store the parser object in the
> CamHelper class.
> - Switch to using std::initialiser_list in the constructor of MdParserSmia.
> - All suggestions from Laurent's feedback have been addressed in patch 2/2.
>
> The constructor for the unique_ptr<MdParserSmia> in patch 2/2 looks a bit
> awkward now since I have to explicitly write std::initialiser_list within
> std::make_unique, but I cannot see nice way around this.
>

Something like the following could arguably be neater, but functionally
equivalent in patch 2/2:

diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp
b/src/ipa/raspberrypi/cam_helper_imx219.cpp
index 18f5c3e7e520..bd42536085d9 100644
--- a/src/ipa/raspberrypi/cam_helper_imx219.cpp
+++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp
@@ -30,6 +30,7 @@ using namespace RPiController;
 constexpr uint32_t gainReg = 0x157;
 constexpr uint32_t expHiReg = 0x15a;
 constexpr uint32_t expLoReg = 0x15b;
+constexpr std::initializer_list<uint32_t> registerList = { expHiReg,
expLoReg, gainReg };

 class CamHelperImx219 : public CamHelper
 {
@@ -53,9 +54,7 @@ private:

 CamHelperImx219::CamHelperImx219()
 #if ENABLE_EMBEDDED_DATA
-       : CamHelper(std::make_unique<MdParserSmia>
-                       (std::initializer_list<uint32_t>({ expHiReg,
expLoReg, gainReg })),
-                   frameIntegrationDiff)
+       : CamHelper(std::make_unique<MdParserSmia>(registerList),
frameIntegrationDiff)
 #else
        : CamHelper({}, frameIntegrationDiff)
 #endif
diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp
b/src/ipa/raspberrypi/cam_helper_imx477.cpp
index 8869af6620cf..4c7f7fd9561b 100644
--- a/src/ipa/raspberrypi/cam_helper_imx477.cpp
+++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp
@@ -23,6 +23,7 @@ constexpr uint32_t expHiReg = 0x0202;
 constexpr uint32_t expLoReg = 0x0203;
 constexpr uint32_t gainHiReg = 0x0204;
 constexpr uint32_t gainLoReg = 0x0205;
+constexpr std::initializer_list<uint32_t> registerList = { expHiReg,
expLoReg, gainHiReg, gainLoReg };

 class CamHelperImx477 : public CamHelper
 {
@@ -46,9 +47,7 @@ private:
 };

 CamHelperImx477::CamHelperImx477()
-       : CamHelper(std::make_unique<MdParserSmia>
-                       (std::initializer_list<uint32_t>({ expHiReg,
expLoReg, gainHiReg, gainLoReg })),
-                   frameIntegrationDiff)
+       : CamHelper(std::make_unique<MdParserSmia>(registerList),
frameIntegrationDiff)
 {
 }

If folks think this is better, I am happy to change it.

Naush



>
> I have removed all previous tags from 1/2, as this is a completely
> different
> approach to the previous revision.
>
> Thanks,
> Naush
>
> Naushir Patuck (2):
>   ipa: raspberrypi: Use a unique_ptr for the metadata parser
>   ipa: raspberrypi: Generalise the SMIA metadata parser
>
>  src/ipa/raspberrypi/cam_helper.cpp        |  38 ++++---
>  src/ipa/raspberrypi/cam_helper.hpp        |   7 +-
>  src/ipa/raspberrypi/cam_helper_imx219.cpp | 117 ++++----------------
>  src/ipa/raspberrypi/cam_helper_imx290.cpp |   2 +-
>  src/ipa/raspberrypi/cam_helper_imx477.cpp | 123 ++++------------------
>  src/ipa/raspberrypi/cam_helper_ov5647.cpp |   2 +-
>  src/ipa/raspberrypi/md_parser.hpp         |  42 +++++---
>  src/ipa/raspberrypi/md_parser_smia.cpp    |  66 ++++++++++--
>  8 files changed, 155 insertions(+), 242 deletions(-)
>
> --
> 2.25.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210622/5bf085c9/attachment-0001.htm>