| Message ID | 20250521-djrscally-c55-cleanup-v1-2-aa171b2aed38@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Dan, Thank you for the patch. The commit message should start with "libcamera: mali-c55:". Same for patch 1/2. I would actually have squashed the two patches together, but that's up to you. On Wed, May 21, 2025 at 02:21:02PM +0100, Daniel Scally wrote: > The tpgSizes_ vector is only used within the initTPGData() function. > Drop it and use a local variable instead. > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > --- > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > index 35372ee1ed6f694166c395088c65509d32b9d1f2..e8c03ee9aafb44d47e3853e9987f8749dc385bb2 100644 > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > @@ -131,7 +131,6 @@ private: > void setSensorControls(const ControlList &sensorControls); > > std::string id_; > - std::vector<Size> tpgSizes_; > Size tpgResolution_; > }; > > @@ -174,6 +173,8 @@ int MaliC55CameraData::init() > > void MaliC55CameraData::initTPGData() > { > + std::vector<Size> tpgSizes; > + Move this... > /* Replicate the CameraSensor implementation for TPG. */ > V4L2Subdevice::Formats formats = sd_->formats(0); > if (formats.empty()) > @@ -181,11 +182,11 @@ void MaliC55CameraData::initTPGData() > ... here. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > for (const auto &format : formats) { > const std::vector<SizeRange> &ranges = format.second; > - std::transform(ranges.begin(), ranges.end(), std::back_inserter(tpgSizes_), > + std::transform(ranges.begin(), ranges.end(), std::back_inserter(tpgSizes), > [](const SizeRange &range) { return range.max; }); > } > > - tpgResolution_ = tpgSizes_.back(); > + tpgResolution_ = tpgSizes.back(); > } > > void MaliC55CameraData::setSensorControls(const ControlList &sensorControls)
Quoting Laurent Pinchart (2025-05-21 17:18:45) > Hi Dan, > > Thank you for the patch. > > The commit message should start with "libcamera: mali-c55:". Same for > patch 1/2. I would actually have squashed the two patches together, but > that's up to you. > > On Wed, May 21, 2025 at 02:21:02PM +0100, Daniel Scally wrote: > > The tpgSizes_ vector is only used within the initTPGData() function. > > Drop it and use a local variable instead. > > > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > > --- > > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > index 35372ee1ed6f694166c395088c65509d32b9d1f2..e8c03ee9aafb44d47e3853e9987f8749dc385bb2 100644 > > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > @@ -131,7 +131,6 @@ private: > > void setSensorControls(const ControlList &sensorControls); > > > > std::string id_; > > - std::vector<Size> tpgSizes_; > > Size tpgResolution_; > > }; > > > > @@ -174,6 +173,8 @@ int MaliC55CameraData::init() > > > > void MaliC55CameraData::initTPGData() > > { > > + std::vector<Size> tpgSizes; > > + > > Move this... > > > /* Replicate the CameraSensor implementation for TPG. */ > > V4L2Subdevice::Formats formats = sd_->formats(0); > > if (formats.empty()) > > @@ -181,11 +182,11 @@ void MaliC55CameraData::initTPGData() > > > > ... here. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Or squashed - either way. > > > for (const auto &format : formats) { > > const std::vector<SizeRange> &ranges = format.second; > > - std::transform(ranges.begin(), ranges.end(), std::back_inserter(tpgSizes_), > > + std::transform(ranges.begin(), ranges.end(), std::back_inserter(tpgSizes), > > [](const SizeRange &range) { return range.max; }); > > } > > > > - tpgResolution_ = tpgSizes_.back(); > > + tpgResolution_ = tpgSizes.back(); > > } > > > > void MaliC55CameraData::setSensorControls(const ControlList &sensorControls) > > -- > Regards, > > Laurent Pinchart
diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp index 35372ee1ed6f694166c395088c65509d32b9d1f2..e8c03ee9aafb44d47e3853e9987f8749dc385bb2 100644 --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp @@ -131,7 +131,6 @@ private: void setSensorControls(const ControlList &sensorControls); std::string id_; - std::vector<Size> tpgSizes_; Size tpgResolution_; }; @@ -174,6 +173,8 @@ int MaliC55CameraData::init() void MaliC55CameraData::initTPGData() { + std::vector<Size> tpgSizes; + /* Replicate the CameraSensor implementation for TPG. */ V4L2Subdevice::Formats formats = sd_->formats(0); if (formats.empty()) @@ -181,11 +182,11 @@ void MaliC55CameraData::initTPGData() for (const auto &format : formats) { const std::vector<SizeRange> &ranges = format.second; - std::transform(ranges.begin(), ranges.end(), std::back_inserter(tpgSizes_), + std::transform(ranges.begin(), ranges.end(), std::back_inserter(tpgSizes), [](const SizeRange &range) { return range.max; }); } - tpgResolution_ = tpgSizes_.back(); + tpgResolution_ = tpgSizes.back(); } void MaliC55CameraData::setSensorControls(const ControlList &sensorControls)
The tpgSizes_ vector is only used within the initTPGData() function. Drop it and use a local variable instead. Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> --- src/libcamera/pipeline/mali-c55/mali-c55.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)