| Message ID | 20250611013245.133785-10-bryan.odonoghue@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Bryan, in the subject: s/parans/params/ Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: > Move the initialisation of Bayer params and CCM to a new constructor in the > Debayer class. > > Ensure we call the base class constructor from DebayerCpu's constructor in > the expected constructor order Debayer then DebayerCpu. Oh, is it otherwise by default? How about Object's constructor then (before and after)? > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > src/libcamera/software_isp/debayer.cpp | 11 +++++++++++ > src/libcamera/software_isp/debayer.h | 1 + > src/libcamera/software_isp/debayer_cpu.cpp | 10 +--------- > 3 files changed, 13 insertions(+), 9 deletions(-) > > diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp > index e9e18c48..29fdcbbf 100644 > --- a/src/libcamera/software_isp/debayer.cpp > +++ b/src/libcamera/software_isp/debayer.cpp > @@ -103,6 +103,17 @@ namespace libcamera { > > LOG_DEFINE_CATEGORY(Debayer) > > +Debayer::Debayer() > +{ > + /* Initialize color lookup tables */ > + for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) { > + red_[i] = green_[i] = blue_[i] = i; > + redCcm_[i] = { static_cast<int16_t>(i), 0, 0 }; > + greenCcm_[i] = { 0, static_cast<int16_t>(i), 0 }; > + blueCcm_[i] = { 0, 0, static_cast<int16_t>(i) }; > + } > +} > + > Debayer::~Debayer() > { > } > diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h > index 45da822a..01b6e916 100644 > --- a/src/libcamera/software_isp/debayer.h > +++ b/src/libcamera/software_isp/debayer.h > @@ -32,6 +32,7 @@ LOG_DECLARE_CATEGORY(Debayer) > class Debayer : public Object > { > public: > + Debayer(); > virtual ~Debayer() = 0; > > virtual int configure(const StreamConfiguration &inputCfg, > diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp > index 8d30bf4a..13db8a8c 100644 > --- a/src/libcamera/software_isp/debayer_cpu.cpp > +++ b/src/libcamera/software_isp/debayer_cpu.cpp > @@ -40,7 +40,7 @@ namespace libcamera { > * \param[in] stats Pointer to the stats object to use > */ > DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats) > - : stats_(std::move(stats)) > + : Debayer(), stats_(std::move(stats)) > { > /* > * Reading from uncached buffers may be very slow. > @@ -51,14 +51,6 @@ DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats) > * future. > */ > enableInputMemcpy_ = true; > - > - /* Initialize color lookup tables */ > - for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) { > - red_[i] = green_[i] = blue_[i] = i; > - redCcm_[i] = { static_cast<int16_t>(i), 0, 0 }; > - greenCcm_[i] = { 0, static_cast<int16_t>(i), 0 }; > - blueCcm_[i] = { 0, 0, static_cast<int16_t>(i) }; > - } > } > > DebayerCpu::~DebayerCpu() = default;
Hi 2025. 06. 16. 20:05 keltezéssel, Milan Zamazal írta: > Hi Bryan, > > in the subject: s/parans/params/ > > Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: > >> Move the initialisation of Bayer params and CCM to a new constructor in the >> Debayer class. >> >> Ensure we call the base class constructor from DebayerCpu's constructor in >> the expected constructor order Debayer then DebayerCpu. > > Oh, is it otherwise by default? How about Object's constructor then > (before and after)? The addition of the call to `Debayer()` should not be needed. Is there a compiler warning or something else? Regards, Barnabás Pőcze > >> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> >> --- >> src/libcamera/software_isp/debayer.cpp | 11 +++++++++++ >> src/libcamera/software_isp/debayer.h | 1 + >> src/libcamera/software_isp/debayer_cpu.cpp | 10 +--------- >> 3 files changed, 13 insertions(+), 9 deletions(-) >> >> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp >> index e9e18c48..29fdcbbf 100644 >> --- a/src/libcamera/software_isp/debayer.cpp >> +++ b/src/libcamera/software_isp/debayer.cpp >> @@ -103,6 +103,17 @@ namespace libcamera { >> >> LOG_DEFINE_CATEGORY(Debayer) >> >> +Debayer::Debayer() >> +{ >> + /* Initialize color lookup tables */ >> + for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) { >> + red_[i] = green_[i] = blue_[i] = i; >> + redCcm_[i] = { static_cast<int16_t>(i), 0, 0 }; >> + greenCcm_[i] = { 0, static_cast<int16_t>(i), 0 }; >> + blueCcm_[i] = { 0, 0, static_cast<int16_t>(i) }; >> + } >> +} >> + >> Debayer::~Debayer() >> { >> } >> diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h >> index 45da822a..01b6e916 100644 >> --- a/src/libcamera/software_isp/debayer.h >> +++ b/src/libcamera/software_isp/debayer.h >> @@ -32,6 +32,7 @@ LOG_DECLARE_CATEGORY(Debayer) >> class Debayer : public Object >> { >> public: >> + Debayer(); >> virtual ~Debayer() = 0; >> >> virtual int configure(const StreamConfiguration &inputCfg, >> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp >> index 8d30bf4a..13db8a8c 100644 >> --- a/src/libcamera/software_isp/debayer_cpu.cpp >> +++ b/src/libcamera/software_isp/debayer_cpu.cpp >> @@ -40,7 +40,7 @@ namespace libcamera { >> * \param[in] stats Pointer to the stats object to use >> */ >> DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats) >> - : stats_(std::move(stats)) >> + : Debayer(), stats_(std::move(stats)) >> { >> /* >> * Reading from uncached buffers may be very slow. >> @@ -51,14 +51,6 @@ DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats) >> * future. >> */ >> enableInputMemcpy_ = true; >> - >> - /* Initialize color lookup tables */ >> - for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) { >> - red_[i] = green_[i] = blue_[i] = i; >> - redCcm_[i] = { static_cast<int16_t>(i), 0, 0 }; >> - greenCcm_[i] = { 0, static_cast<int16_t>(i), 0 }; >> - blueCcm_[i] = { 0, 0, static_cast<int16_t>(i) }; >> - } >> } >> >> DebayerCpu::~DebayerCpu() = default; >
On 17/06/2025 12:37, Barnabás Pőcze wrote: >>> >> > Oh, is it otherwise by default? How about Object's constructor then >> (before and after)? > > The addition of the call to `Debayer()` should not be needed. > Is there a compiler warning or something else? > > > Regards, > Barnabás Pőcze I have no memory of why I added this commit. You should only have to invoke a base class constructor if its a non-default constructor and I don't think that stats_(std::move) matters. --- bod
diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp index e9e18c48..29fdcbbf 100644 --- a/src/libcamera/software_isp/debayer.cpp +++ b/src/libcamera/software_isp/debayer.cpp @@ -103,6 +103,17 @@ namespace libcamera { LOG_DEFINE_CATEGORY(Debayer) +Debayer::Debayer() +{ + /* Initialize color lookup tables */ + for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) { + red_[i] = green_[i] = blue_[i] = i; + redCcm_[i] = { static_cast<int16_t>(i), 0, 0 }; + greenCcm_[i] = { 0, static_cast<int16_t>(i), 0 }; + blueCcm_[i] = { 0, 0, static_cast<int16_t>(i) }; + } +} + Debayer::~Debayer() { } diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h index 45da822a..01b6e916 100644 --- a/src/libcamera/software_isp/debayer.h +++ b/src/libcamera/software_isp/debayer.h @@ -32,6 +32,7 @@ LOG_DECLARE_CATEGORY(Debayer) class Debayer : public Object { public: + Debayer(); virtual ~Debayer() = 0; virtual int configure(const StreamConfiguration &inputCfg, diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp index 8d30bf4a..13db8a8c 100644 --- a/src/libcamera/software_isp/debayer_cpu.cpp +++ b/src/libcamera/software_isp/debayer_cpu.cpp @@ -40,7 +40,7 @@ namespace libcamera { * \param[in] stats Pointer to the stats object to use */ DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats) - : stats_(std::move(stats)) + : Debayer(), stats_(std::move(stats)) { /* * Reading from uncached buffers may be very slow. @@ -51,14 +51,6 @@ DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats) * future. */ enableInputMemcpy_ = true; - - /* Initialize color lookup tables */ - for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) { - red_[i] = green_[i] = blue_[i] = i; - redCcm_[i] = { static_cast<int16_t>(i), 0, 0 }; - greenCcm_[i] = { 0, static_cast<int16_t>(i), 0 }; - blueCcm_[i] = { 0, 0, static_cast<int16_t>(i) }; - } } DebayerCpu::~DebayerCpu() = default;
Move the initialisation of Bayer params and CCM to a new constructor in the Debayer class. Ensure we call the base class constructor from DebayerCpu's constructor in the expected constructor order Debayer then DebayerCpu. Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- src/libcamera/software_isp/debayer.cpp | 11 +++++++++++ src/libcamera/software_isp/debayer.h | 1 + src/libcamera/software_isp/debayer_cpu.cpp | 10 +--------- 3 files changed, 13 insertions(+), 9 deletions(-)