[libcamera-devel,v2,2/3] libcamera: use const reference for range loops
diff mbox series

Message ID 20220816183826.102989-2-Rauch.Christian@gmx.de
State Superseded
Headers show
Series
  • [libcamera-devel,v2,1/3] libcamera: remove unused headers
Related show

Commit Message

Christian Rauch Aug. 16, 2022, 6:38 p.m. UTC 
A const reference prevents unnecessary copies of the loop elements.

Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
---
 src/libcamera/camera_manager.cpp             | 2 +-
 src/libcamera/device_enumerator.cpp          | 2 +-
 src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 6 +-----
 src/libcamera/pipeline/vimc/vimc.cpp         | 2 +-
 src/libcamera/pipeline_handler.cpp           | 2 +-
 5 files changed, 5 insertions(+), 9 deletions(-)

--
2.34.1

Comments

Laurent Pinchart Aug. 16, 2022, 7:26 p.m. UTC | #1 
Hi Christian,

Thank you for the patch.

On Tue, Aug 16, 2022 at 08:38:25PM +0200, Christian Rauch via libcamera-devel wrote:
> A const reference prevents unnecessary copies of the loop elements.

This should also mention that PipelineHandlerUVC::processControls()
switches to structured bindings. With that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

You can propose an updated commit message in a reply to this e-mail,
there's no need to resubmit the series.

> Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
> ---
>  src/libcamera/camera_manager.cpp             | 2 +-
>  src/libcamera/device_enumerator.cpp          | 2 +-
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 6 +-----
>  src/libcamera/pipeline/vimc/vimc.cpp         | 2 +-
>  src/libcamera/pipeline_handler.cpp           | 2 +-
>  5 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index 70d73822..7ff4bc6f 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -189,7 +189,7 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,
>  {
>  	MutexLocker locker(mutex_);
> 
> -	for (std::shared_ptr<Camera> c : cameras_) {
> +	for (const std::shared_ptr<Camera> &c : cameras_) {
>  		if (c->id() == camera->id()) {
>  			LOG(Camera, Fatal)
>  				<< "Trying to register a camera with a duplicated ID '"
> diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
> index d1258050..f2e055de 100644
> --- a/src/libcamera/device_enumerator.cpp
> +++ b/src/libcamera/device_enumerator.cpp
> @@ -161,7 +161,7 @@ std::unique_ptr<DeviceEnumerator> DeviceEnumerator::create()
> 
>  DeviceEnumerator::~DeviceEnumerator()
>  {
> -	for (std::shared_ptr<MediaDevice> media : devices_) {
> +	for (const std::shared_ptr<MediaDevice> &media : devices_) {
>  		if (media->busy())
>  			LOG(DeviceEnumerator, Error)
>  				<< "Removing media device " << media->deviceNode()
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index fbe02cdc..9cbf126a 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -340,12 +340,8 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)
>  {
>  	ControlList controls(data->video_->controls());
> 
> -	for (auto it : request->controls()) {
> -		unsigned int id = it.first;
> -		ControlValue &value = it.second;
> -
> +	for (const auto &[id, value] : request->controls())
>  		processControl(&controls, id, value);
> -	}
> 
>  	for (const auto &ctrl : controls)
>  		LOG(UVC, Debug)
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index 153cf849..d2f2e460 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -378,7 +378,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
>  {
>  	ControlList controls(data->sensor_->controls());
> 
> -	for (auto it : request->controls()) {
> +	for (const auto &it : request->controls()) {
>  		unsigned int id = it.first;
>  		unsigned int offset;
>  		uint32_t cid;
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 7d2d00ef..e5cb751c 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -616,7 +616,7 @@ void PipelineHandler::disconnect()
>  	 */
>  	std::vector<std::weak_ptr<Camera>> cameras{ std::move(cameras_) };
> 
> -	for (std::weak_ptr<Camera> ptr : cameras) {
> +	for (const std::weak_ptr<Camera> &ptr : cameras) {
>  		std::shared_ptr<Camera> camera = ptr.lock();
>  		if (!camera)
>  			continue;
Christian Rauch Aug. 17, 2022, 6:54 p.m. UTC | #2 
Hi Laurent,

I am not sure how to update the commit message via email :-) So I will
just paste the new commit message here:

    libcamera: use const reference for range loops



    A const reference prevents unnecessary copies of the loop elements.
Also,

    this changes looping over controls in
PipelineHandlerUVC::processControls

    to structured bindings.



    Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>

    Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


Best,
Christian


Am 16.08.22 um 21:26 schrieb Laurent Pinchart:
> Hi Christian,
>
> Thank you for the patch.
>
> On Tue, Aug 16, 2022 at 08:38:25PM +0200, Christian Rauch via libcamera-devel wrote:
>> A const reference prevents unnecessary copies of the loop elements.
>
> This should also mention that PipelineHandlerUVC::processControls()
> switches to structured bindings. With that,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> You can propose an updated commit message in a reply to this e-mail,
> there's no need to resubmit the series.
>
>> Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
>> ---
>>  src/libcamera/camera_manager.cpp             | 2 +-
>>  src/libcamera/device_enumerator.cpp          | 2 +-
>>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 6 +-----
>>  src/libcamera/pipeline/vimc/vimc.cpp         | 2 +-
>>  src/libcamera/pipeline_handler.cpp           | 2 +-
>>  5 files changed, 5 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
>> index 70d73822..7ff4bc6f 100644
>> --- a/src/libcamera/camera_manager.cpp
>> +++ b/src/libcamera/camera_manager.cpp
>> @@ -189,7 +189,7 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,
>>  {
>>  	MutexLocker locker(mutex_);
>>
>> -	for (std::shared_ptr<Camera> c : cameras_) {
>> +	for (const std::shared_ptr<Camera> &c : cameras_) {
>>  		if (c->id() == camera->id()) {
>>  			LOG(Camera, Fatal)
>>  				<< "Trying to register a camera with a duplicated ID '"
>> diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
>> index d1258050..f2e055de 100644
>> --- a/src/libcamera/device_enumerator.cpp
>> +++ b/src/libcamera/device_enumerator.cpp
>> @@ -161,7 +161,7 @@ std::unique_ptr<DeviceEnumerator> DeviceEnumerator::create()
>>
>>  DeviceEnumerator::~DeviceEnumerator()
>>  {
>> -	for (std::shared_ptr<MediaDevice> media : devices_) {
>> +	for (const std::shared_ptr<MediaDevice> &media : devices_) {
>>  		if (media->busy())
>>  			LOG(DeviceEnumerator, Error)
>>  				<< "Removing media device " << media->deviceNode()
>> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
>> index fbe02cdc..9cbf126a 100644
>> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
>> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
>> @@ -340,12 +340,8 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)
>>  {
>>  	ControlList controls(data->video_->controls());
>>
>> -	for (auto it : request->controls()) {
>> -		unsigned int id = it.first;
>> -		ControlValue &value = it.second;
>> -
>> +	for (const auto &[id, value] : request->controls())
>>  		processControl(&controls, id, value);
>> -	}
>>
>>  	for (const auto &ctrl : controls)
>>  		LOG(UVC, Debug)
>> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
>> index 153cf849..d2f2e460 100644
>> --- a/src/libcamera/pipeline/vimc/vimc.cpp
>> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
>> @@ -378,7 +378,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
>>  {
>>  	ControlList controls(data->sensor_->controls());
>>
>> -	for (auto it : request->controls()) {
>> +	for (const auto &it : request->controls()) {
>>  		unsigned int id = it.first;
>>  		unsigned int offset;
>>  		uint32_t cid;
>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
>> index 7d2d00ef..e5cb751c 100644
>> --- a/src/libcamera/pipeline_handler.cpp
>> +++ b/src/libcamera/pipeline_handler.cpp
>> @@ -616,7 +616,7 @@ void PipelineHandler::disconnect()
>>  	 */
>>  	std::vector<std::weak_ptr<Camera>> cameras{ std::move(cameras_) };
>>
>> -	for (std::weak_ptr<Camera> ptr : cameras) {
>> +	for (const std::weak_ptr<Camera> &ptr : cameras) {
>>  		std::shared_ptr<Camera> camera = ptr.lock();
>>  		if (!camera)
>>  			continue;
>

Patch
diff mbox series 

diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
index 70d73822..7ff4bc6f 100644
--- a/src/libcamera/camera_manager.cpp
+++ b/src/libcamera/camera_manager.cpp
@@ -189,7 +189,7 @@  void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,
 {
 	MutexLocker locker(mutex_);

-	for (std::shared_ptr<Camera> c : cameras_) {
+	for (const std::shared_ptr<Camera> &c : cameras_) {
 		if (c->id() == camera->id()) {
 			LOG(Camera, Fatal)
 				<< "Trying to register a camera with a duplicated ID '"
diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
index d1258050..f2e055de 100644
--- a/src/libcamera/device_enumerator.cpp
+++ b/src/libcamera/device_enumerator.cpp
@@ -161,7 +161,7 @@  std::unique_ptr<DeviceEnumerator> DeviceEnumerator::create()

 DeviceEnumerator::~DeviceEnumerator()
 {
-	for (std::shared_ptr<MediaDevice> media : devices_) {
+	for (const std::shared_ptr<MediaDevice> &media : devices_) {
 		if (media->busy())
 			LOG(DeviceEnumerator, Error)
 				<< "Removing media device " << media->deviceNode()
diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
index fbe02cdc..9cbf126a 100644
--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
@@ -340,12 +340,8 @@  int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)
 {
 	ControlList controls(data->video_->controls());

-	for (auto it : request->controls()) {
-		unsigned int id = it.first;
-		ControlValue &value = it.second;
-
+	for (const auto &[id, value] : request->controls())
 		processControl(&controls, id, value);
-	}

 	for (const auto &ctrl : controls)
 		LOG(UVC, Debug)
diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
index 153cf849..d2f2e460 100644
--- a/src/libcamera/pipeline/vimc/vimc.cpp
+++ b/src/libcamera/pipeline/vimc/vimc.cpp
@@ -378,7 +378,7 @@  int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
 {
 	ControlList controls(data->sensor_->controls());

-	for (auto it : request->controls()) {
+	for (const auto &it : request->controls()) {
 		unsigned int id = it.first;
 		unsigned int offset;
 		uint32_t cid;
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index 7d2d00ef..e5cb751c 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -616,7 +616,7 @@  void PipelineHandler::disconnect()
 	 */
 	std::vector<std::weak_ptr<Camera>> cameras{ std::move(cameras_) };

-	for (std::weak_ptr<Camera> ptr : cameras) {
+	for (const std::weak_ptr<Camera> &ptr : cameras) {
 		std::shared_ptr<Camera> camera = ptr.lock();
 		if (!camera)
 			continue;