[libcamera-devel,v4,3/4] android: camera_device: Mark abortRequest() and notifyError() as const
diff mbox series

Message ID 20210929133030.401961-4-umang.jain@ideasonboard.com
State Accepted
Delegated to: Umang Jain
Headers show
Series
  • Camera3RequestDescriptors std::map => std::queue
Related show

Commit Message

Umang Jain Sept. 29, 2021, 1:30 p.m. UTC 
abortRequest() and notifyError() do not access any members of
CameraDevice hence, these functions can be const.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 src/android/camera_device.cpp | 4 ++--
 src/android/camera_device.h   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Laurent Pinchart Sept. 29, 2021, 2:27 p.m. UTC | #1 
Hi Umang,

Thank you for the patch.

On Wed, Sep 29, 2021 at 07:00:29PM +0530, Umang Jain wrote:
> abortRequest() and notifyError() do not access any members of
> CameraDevice hence, these functions can be const.

They both access callbacks_.

> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  src/android/camera_device.cpp | 4 ++--
>  src/android/camera_device.h   | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index e815f7a0..45350563 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -860,7 +860,7 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
>  	return 0;
>  }
>  
> -void CameraDevice::abortRequest(camera3_capture_request_t *request)
> +void CameraDevice::abortRequest(camera3_capture_request_t *request) const
>  {
>  	notifyError(request->frame_number, nullptr, CAMERA3_MSG_ERROR_REQUEST);
>  
> @@ -1258,7 +1258,7 @@ void CameraDevice::notifyShutter(uint32_t frameNumber, uint64_t timestamp)
>  }
>  
>  void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream,
> -			       camera3_error_msg_code code)
> +			       camera3_error_msg_code code) const
>  {
>  	camera3_notify_msg_t notify = {};
>  
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 9ec510d5..85497921 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -99,11 +99,11 @@ private:
>  	createFrameBuffer(const buffer_handle_t camera3buffer,
>  			  libcamera::PixelFormat pixelFormat,
>  			  const libcamera::Size &size);
> -	void abortRequest(camera3_capture_request_t *request);
> +	void abortRequest(camera3_capture_request_t *request) const;
>  	bool isValidRequest(camera3_capture_request_t *request) const;
>  	void notifyShutter(uint32_t frameNumber, uint64_t timestamp);
>  	void notifyError(uint32_t frameNumber, camera3_stream_t *stream,
> -			 camera3_error_msg_code code);
> +			 camera3_error_msg_code code) const;
>  	int processControls(Camera3RequestDescriptor *descriptor);
>  	std::unique_ptr<CameraMetadata> getResultMetadata(
>  		const Camera3RequestDescriptor &descriptor) const;
Umang Jain Sept. 29, 2021, 5:41 p.m. UTC | #2 
Hi Laurent,

On 9/29/21 7:57 PM, Laurent Pinchart wrote:
> Hi Umang,
>
> Thank you for the patch.
>
> On Wed, Sep 29, 2021 at 07:00:29PM +0530, Umang Jain wrote:
>> abortRequest() and notifyError() do not access any members of
>> CameraDevice hence, these functions can be const.
> They both access callbacks_.


Oops,

The commit message should be updated to:

     abortRequest() and notifyError do not modify any members of 
CameraDevice,
     hence, these functions can be const.


>
>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
>> ---
>>   src/android/camera_device.cpp | 4 ++--
>>   src/android/camera_device.h   | 4 ++--
>>   2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
>> index e815f7a0..45350563 100644
>> --- a/src/android/camera_device.cpp
>> +++ b/src/android/camera_device.cpp
>> @@ -860,7 +860,7 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
>>   	return 0;
>>   }
>>   
>> -void CameraDevice::abortRequest(camera3_capture_request_t *request)
>> +void CameraDevice::abortRequest(camera3_capture_request_t *request) const
>>   {
>>   	notifyError(request->frame_number, nullptr, CAMERA3_MSG_ERROR_REQUEST);
>>   
>> @@ -1258,7 +1258,7 @@ void CameraDevice::notifyShutter(uint32_t frameNumber, uint64_t timestamp)
>>   }
>>   
>>   void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream,
>> -			       camera3_error_msg_code code)
>> +			       camera3_error_msg_code code) const
>>   {
>>   	camera3_notify_msg_t notify = {};
>>   
>> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
>> index 9ec510d5..85497921 100644
>> --- a/src/android/camera_device.h
>> +++ b/src/android/camera_device.h
>> @@ -99,11 +99,11 @@ private:
>>   	createFrameBuffer(const buffer_handle_t camera3buffer,
>>   			  libcamera::PixelFormat pixelFormat,
>>   			  const libcamera::Size &size);
>> -	void abortRequest(camera3_capture_request_t *request);
>> +	void abortRequest(camera3_capture_request_t *request) const;
>>   	bool isValidRequest(camera3_capture_request_t *request) const;
>>   	void notifyShutter(uint32_t frameNumber, uint64_t timestamp);
>>   	void notifyError(uint32_t frameNumber, camera3_stream_t *stream,
>> -			 camera3_error_msg_code code);
>> +			 camera3_error_msg_code code) const;
>>   	int processControls(Camera3RequestDescriptor *descriptor);
>>   	std::unique_ptr<CameraMetadata> getResultMetadata(
>>   		const Camera3RequestDescriptor &descriptor) const;
Laurent Pinchart Sept. 29, 2021, 5:46 p.m. UTC | #3 
On Wed, Sep 29, 2021 at 11:11:38PM +0530, Umang Jain wrote:
> Hi Laurent,
> 
> On 9/29/21 7:57 PM, Laurent Pinchart wrote:
> > Hi Umang,
> >
> > Thank you for the patch.
> >
> > On Wed, Sep 29, 2021 at 07:00:29PM +0530, Umang Jain wrote:
> >> abortRequest() and notifyError() do not access any members of
> >> CameraDevice hence, these functions can be const.
> > They both access callbacks_.
> 
> 
> Oops,
> 
> The commit message should be updated to:
> 
>      abortRequest() and notifyError do not modify any members of 
> CameraDevice,
>      hence, these functions can be const.

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

> >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> >> ---
> >>   src/android/camera_device.cpp | 4 ++--
> >>   src/android/camera_device.h   | 4 ++--
> >>   2 files changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> >> index e815f7a0..45350563 100644
> >> --- a/src/android/camera_device.cpp
> >> +++ b/src/android/camera_device.cpp
> >> @@ -860,7 +860,7 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
> >>   	return 0;
> >>   }
> >>   
> >> -void CameraDevice::abortRequest(camera3_capture_request_t *request)
> >> +void CameraDevice::abortRequest(camera3_capture_request_t *request) const
> >>   {
> >>   	notifyError(request->frame_number, nullptr, CAMERA3_MSG_ERROR_REQUEST);
> >>   
> >> @@ -1258,7 +1258,7 @@ void CameraDevice::notifyShutter(uint32_t frameNumber, uint64_t timestamp)
> >>   }
> >>   
> >>   void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream,
> >> -			       camera3_error_msg_code code)
> >> +			       camera3_error_msg_code code) const
> >>   {
> >>   	camera3_notify_msg_t notify = {};
> >>   
> >> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> >> index 9ec510d5..85497921 100644
> >> --- a/src/android/camera_device.h
> >> +++ b/src/android/camera_device.h
> >> @@ -99,11 +99,11 @@ private:
> >>   	createFrameBuffer(const buffer_handle_t camera3buffer,
> >>   			  libcamera::PixelFormat pixelFormat,
> >>   			  const libcamera::Size &size);
> >> -	void abortRequest(camera3_capture_request_t *request);
> >> +	void abortRequest(camera3_capture_request_t *request) const;
> >>   	bool isValidRequest(camera3_capture_request_t *request) const;
> >>   	void notifyShutter(uint32_t frameNumber, uint64_t timestamp);
> >>   	void notifyError(uint32_t frameNumber, camera3_stream_t *stream,
> >> -			 camera3_error_msg_code code);
> >> +			 camera3_error_msg_code code) const;
> >>   	int processControls(Camera3RequestDescriptor *descriptor);
> >>   	std::unique_ptr<CameraMetadata> getResultMetadata(
> >>   		const Camera3RequestDescriptor &descriptor) const;
Jacopo Mondi Sept. 29, 2021, 6:41 p.m. UTC | #4 
Hi Umang,

On Wed, Sep 29, 2021 at 07:00:29PM +0530, Umang Jain wrote:
> abortRequest() and notifyError() do not access any members of
> CameraDevice hence, these functions can be const.

I know it's -techincally- correct, but I find it confusing to a
function that sets the state of the descriptor at hand as const :)

Anyway, I'll go with the majority

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j

>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  src/android/camera_device.cpp | 4 ++--
>  src/android/camera_device.h   | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index e815f7a0..45350563 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -860,7 +860,7 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
>  	return 0;
>  }
>
> -void CameraDevice::abortRequest(camera3_capture_request_t *request)
> +void CameraDevice::abortRequest(camera3_capture_request_t *request) const
>  {
>  	notifyError(request->frame_number, nullptr, CAMERA3_MSG_ERROR_REQUEST);
>
> @@ -1258,7 +1258,7 @@ void CameraDevice::notifyShutter(uint32_t frameNumber, uint64_t timestamp)
>  }
>
>  void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream,
> -			       camera3_error_msg_code code)
> +			       camera3_error_msg_code code) const
>  {
>  	camera3_notify_msg_t notify = {};
>
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 9ec510d5..85497921 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -99,11 +99,11 @@ private:
>  	createFrameBuffer(const buffer_handle_t camera3buffer,
>  			  libcamera::PixelFormat pixelFormat,
>  			  const libcamera::Size &size);
> -	void abortRequest(camera3_capture_request_t *request);
> +	void abortRequest(camera3_capture_request_t *request) const;
>  	bool isValidRequest(camera3_capture_request_t *request) const;
>  	void notifyShutter(uint32_t frameNumber, uint64_t timestamp);
>  	void notifyError(uint32_t frameNumber, camera3_stream_t *stream,
> -			 camera3_error_msg_code code);
> +			 camera3_error_msg_code code) const;
>  	int processControls(Camera3RequestDescriptor *descriptor);
>  	std::unique_ptr<CameraMetadata> getResultMetadata(
>  		const Camera3RequestDescriptor &descriptor) const;
> --
> 2.31.0
>

Patch
diff mbox series 

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index e815f7a0..45350563 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -860,7 +860,7 @@  int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
 	return 0;
 }
 
-void CameraDevice::abortRequest(camera3_capture_request_t *request)
+void CameraDevice::abortRequest(camera3_capture_request_t *request) const
 {
 	notifyError(request->frame_number, nullptr, CAMERA3_MSG_ERROR_REQUEST);
 
@@ -1258,7 +1258,7 @@  void CameraDevice::notifyShutter(uint32_t frameNumber, uint64_t timestamp)
 }
 
 void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream,
-			       camera3_error_msg_code code)
+			       camera3_error_msg_code code) const
 {
 	camera3_notify_msg_t notify = {};
 
diff --git a/src/android/camera_device.h b/src/android/camera_device.h
index 9ec510d5..85497921 100644
--- a/src/android/camera_device.h
+++ b/src/android/camera_device.h
@@ -99,11 +99,11 @@  private:
 	createFrameBuffer(const buffer_handle_t camera3buffer,
 			  libcamera::PixelFormat pixelFormat,
 			  const libcamera::Size &size);
-	void abortRequest(camera3_capture_request_t *request);
+	void abortRequest(camera3_capture_request_t *request) const;
 	bool isValidRequest(camera3_capture_request_t *request) const;
 	void notifyShutter(uint32_t frameNumber, uint64_t timestamp);
 	void notifyError(uint32_t frameNumber, camera3_stream_t *stream,
-			 camera3_error_msg_code code);
+			 camera3_error_msg_code code) const;
 	int processControls(Camera3RequestDescriptor *descriptor);
 	std::unique_ptr<CameraMetadata> getResultMetadata(
 		const Camera3RequestDescriptor &descriptor) const;