[v2,5/9] android: Add CameraDevice::sendCaptureResult()
diff mbox series

Message ID 20241127092632.3145984-6-chenghaoyang@chromium.org
State New
Headers show
Series
  • Signal metadataAvailable and Android partial result
Related show

Commit Message

Cheng-Hao Yang Nov. 27, 2024, 9:25 a.m. UTC 
The new function is separated from sendCaptureResults(), which allows
other functions to send a result out of order, like aborting. It'll be
updated in the upcoming patch, when the usage of error result is
separated.

This function also allows the upcoming partial results to be sent back
to the application without waiting for the requests.

Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
---
 src/android/camera_device.cpp | 57 ++++++++++++++++++-----------------
 src/android/camera_device.h   |  1 +
 2 files changed, 31 insertions(+), 27 deletions(-)

Comments

Jacopo Mondi Nov. 28, 2024, 2:56 p.m. UTC | #1 
Hi Harvey

On Wed, Nov 27, 2024 at 09:25:55AM +0000, Harvey Yang wrote:
> The new function is separated from sendCaptureResults(), which allows
> other functions to send a result out of order, like aborting. It'll be
> updated in the upcoming patch, when the usage of error result is
> separated.
>
> This function also allows the upcoming partial results to be sent back
> to the application without waiting for the requests.
>
> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> ---
>  src/android/camera_device.cpp | 57 ++++++++++++++++++-----------------
>  src/android/camera_device.h   |  1 +
>  2 files changed, 31 insertions(+), 27 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 62f724041..0377cf215 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1324,42 +1324,45 @@ void CameraDevice::sendCaptureResults()
>  		auto descriptor = std::move(descriptors_.front());
>  		descriptors_.pop();
>
> -		camera3_capture_result_t captureResult = {};
> +		sendCaptureResult(descriptor.get());
> +	}
> +}
>
> -		captureResult.frame_number = descriptor->frameNumber_;
> +void CameraDevice::sendCaptureResult(Camera3RequestDescriptor *request) const

I would have named the paramter 'descriptor' but apart from that the
patch doesn't seems to introduce functional changes.

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

> +{
> +	std::vector<camera3_stream_buffer_t> resultBuffers;
> +	resultBuffers.reserve(request->buffers_.size());
>
> -		if (descriptor->resultMetadata_)
> -			captureResult.result =
> -				descriptor->resultMetadata_->getMetadata();
> +	for (auto &buffer : request->buffers_) {
> +		camera3_buffer_status status = CAMERA3_BUFFER_STATUS_ERROR;
>
> -		std::vector<camera3_stream_buffer_t> resultBuffers;
> -		resultBuffers.reserve(descriptor->buffers_.size());
> +		if (buffer.status == StreamBuffer::Status::Success)
> +			status = CAMERA3_BUFFER_STATUS_OK;
>
> -		for (auto &buffer : descriptor->buffers_) {
> -			camera3_buffer_status status = CAMERA3_BUFFER_STATUS_ERROR;
> +		/*
> +		 * Pass the buffer fence back to the camera framework as
> +		 * a release fence. This instructs the framework to wait
> +		 * on the acquire fence in case we haven't done so
> +		 * ourselves for any reason.
> +		 */
> +		resultBuffers.push_back({ buffer.stream->camera3Stream(),
> +					  buffer.camera3Buffer, status,
> +					  -1, buffer.fence.release() });
> +	}
>
> -			if (buffer.status == StreamBuffer::Status::Success)
> -				status = CAMERA3_BUFFER_STATUS_OK;
> +	camera3_capture_result_t captureResult = {};
>
> -			/*
> -			 * Pass the buffer fence back to the camera framework as
> -			 * a release fence. This instructs the framework to wait
> -			 * on the acquire fence in case we haven't done so
> -			 * ourselves for any reason.
> -			 */
> -			resultBuffers.push_back({ buffer.stream->camera3Stream(),
> -						  buffer.camera3Buffer, status,
> -						  -1, buffer.fence.release() });
> -		}
> +	captureResult.frame_number = request->frameNumber_;
> +	captureResult.num_output_buffers = resultBuffers.size();
> +	captureResult.output_buffers = resultBuffers.data();
>
> -		captureResult.num_output_buffers = resultBuffers.size();
> -		captureResult.output_buffers = resultBuffers.data();
> +	if (request->status_ == Camera3RequestDescriptor::Status::Success)
> +		captureResult.partial_result = 1;
>
> -		if (descriptor->status_ == Camera3RequestDescriptor::Status::Success)
> -			captureResult.partial_result = 1;
> +	if (request->resultMetadata_)
> +		captureResult.result = request->resultMetadata_->getMetadata();
>
> -		callbacks_->process_capture_result(callbacks_, &captureResult);
> -	}
> +	callbacks_->process_capture_result(callbacks_, &captureResult);
>  }
>
>  void CameraDevice::setBufferStatus(StreamBuffer &streamBuffer,
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index c92ee1aa4..699aa8f17 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -97,6 +97,7 @@ private:
>  	void completeDescriptor(Camera3RequestDescriptor *descriptor)
>  		LIBCAMERA_TSA_EXCLUDES(descriptorsMutex_);
>  	void sendCaptureResults() LIBCAMERA_TSA_REQUIRES(descriptorsMutex_);
> +	void sendCaptureResult(Camera3RequestDescriptor *request) const;
>  	void setBufferStatus(StreamBuffer &buffer,
>  			     StreamBuffer::Status status);
>  	std::unique_ptr<CameraMetadata> getResultMetadata(
> --
> 2.47.0.338.g60cca15819-goog
>
Cheng-Hao Yang Nov. 29, 2024, 8:50 a.m. UTC | #2 
Hi Jacopo,

On Thu, Nov 28, 2024 at 10:56 PM Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> Hi Harvey
>
> On Wed, Nov 27, 2024 at 09:25:55AM +0000, Harvey Yang wrote:
> > The new function is separated from sendCaptureResults(), which allows
> > other functions to send a result out of order, like aborting. It'll be
> > updated in the upcoming patch, when the usage of error result is
> > separated.
> >
> > This function also allows the upcoming partial results to be sent back
> > to the application without waiting for the requests.
> >
> > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> > ---
> >  src/android/camera_device.cpp | 57 ++++++++++++++++++-----------------
> >  src/android/camera_device.h   |  1 +
> >  2 files changed, 31 insertions(+), 27 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 62f724041..0377cf215 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -1324,42 +1324,45 @@ void CameraDevice::sendCaptureResults()
> >               auto descriptor = std::move(descriptors_.front());
> >               descriptors_.pop();
> >
> > -             camera3_capture_result_t captureResult = {};
> > +             sendCaptureResult(descriptor.get());
> > +     }
> > +}
> >
> > -             captureResult.frame_number = descriptor->frameNumber_;
> > +void CameraDevice::sendCaptureResult(Camera3RequestDescriptor *request) const
>
> I would have named the paramter 'descriptor' but apart from that the
> patch doesn't seems to introduce functional changes.

Let's keep it `request`, as there will be `Camera3ResultDescriptor`
in the following patches :)

BR,
Harvey
>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> Thanks
>   j
>
> > +{
> > +     std::vector<camera3_stream_buffer_t> resultBuffers;
> > +     resultBuffers.reserve(request->buffers_.size());
> >
> > -             if (descriptor->resultMetadata_)
> > -                     captureResult.result =
> > -                             descriptor->resultMetadata_->getMetadata();
> > +     for (auto &buffer : request->buffers_) {
> > +             camera3_buffer_status status = CAMERA3_BUFFER_STATUS_ERROR;
> >
> > -             std::vector<camera3_stream_buffer_t> resultBuffers;
> > -             resultBuffers.reserve(descriptor->buffers_.size());
> > +             if (buffer.status == StreamBuffer::Status::Success)
> > +                     status = CAMERA3_BUFFER_STATUS_OK;
> >
> > -             for (auto &buffer : descriptor->buffers_) {
> > -                     camera3_buffer_status status = CAMERA3_BUFFER_STATUS_ERROR;
> > +             /*
> > +              * Pass the buffer fence back to the camera framework as
> > +              * a release fence. This instructs the framework to wait
> > +              * on the acquire fence in case we haven't done so
> > +              * ourselves for any reason.
> > +              */
> > +             resultBuffers.push_back({ buffer.stream->camera3Stream(),
> > +                                       buffer.camera3Buffer, status,
> > +                                       -1, buffer.fence.release() });
> > +     }
> >
> > -                     if (buffer.status == StreamBuffer::Status::Success)
> > -                             status = CAMERA3_BUFFER_STATUS_OK;
> > +     camera3_capture_result_t captureResult = {};
> >
> > -                     /*
> > -                      * Pass the buffer fence back to the camera framework as
> > -                      * a release fence. This instructs the framework to wait
> > -                      * on the acquire fence in case we haven't done so
> > -                      * ourselves for any reason.
> > -                      */
> > -                     resultBuffers.push_back({ buffer.stream->camera3Stream(),
> > -                                               buffer.camera3Buffer, status,
> > -                                               -1, buffer.fence.release() });
> > -             }
> > +     captureResult.frame_number = request->frameNumber_;
> > +     captureResult.num_output_buffers = resultBuffers.size();
> > +     captureResult.output_buffers = resultBuffers.data();
> >
> > -             captureResult.num_output_buffers = resultBuffers.size();
> > -             captureResult.output_buffers = resultBuffers.data();
> > +     if (request->status_ == Camera3RequestDescriptor::Status::Success)
> > +             captureResult.partial_result = 1;
> >
> > -             if (descriptor->status_ == Camera3RequestDescriptor::Status::Success)
> > -                     captureResult.partial_result = 1;
> > +     if (request->resultMetadata_)
> > +             captureResult.result = request->resultMetadata_->getMetadata();
> >
> > -             callbacks_->process_capture_result(callbacks_, &captureResult);
> > -     }
> > +     callbacks_->process_capture_result(callbacks_, &captureResult);
> >  }
> >
> >  void CameraDevice::setBufferStatus(StreamBuffer &streamBuffer,
> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> > index c92ee1aa4..699aa8f17 100644
> > --- a/src/android/camera_device.h
> > +++ b/src/android/camera_device.h
> > @@ -97,6 +97,7 @@ private:
> >       void completeDescriptor(Camera3RequestDescriptor *descriptor)
> >               LIBCAMERA_TSA_EXCLUDES(descriptorsMutex_);
> >       void sendCaptureResults() LIBCAMERA_TSA_REQUIRES(descriptorsMutex_);
> > +     void sendCaptureResult(Camera3RequestDescriptor *request) const;
> >       void setBufferStatus(StreamBuffer &buffer,
> >                            StreamBuffer::Status status);
> >       std::unique_ptr<CameraMetadata> getResultMetadata(
> > --
> > 2.47.0.338.g60cca15819-goog
> >

Patch
diff mbox series 

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 62f724041..0377cf215 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -1324,42 +1324,45 @@  void CameraDevice::sendCaptureResults()
 		auto descriptor = std::move(descriptors_.front());
 		descriptors_.pop();
 
-		camera3_capture_result_t captureResult = {};
+		sendCaptureResult(descriptor.get());
+	}
+}
 
-		captureResult.frame_number = descriptor->frameNumber_;
+void CameraDevice::sendCaptureResult(Camera3RequestDescriptor *request) const
+{
+	std::vector<camera3_stream_buffer_t> resultBuffers;
+	resultBuffers.reserve(request->buffers_.size());
 
-		if (descriptor->resultMetadata_)
-			captureResult.result =
-				descriptor->resultMetadata_->getMetadata();
+	for (auto &buffer : request->buffers_) {
+		camera3_buffer_status status = CAMERA3_BUFFER_STATUS_ERROR;
 
-		std::vector<camera3_stream_buffer_t> resultBuffers;
-		resultBuffers.reserve(descriptor->buffers_.size());
+		if (buffer.status == StreamBuffer::Status::Success)
+			status = CAMERA3_BUFFER_STATUS_OK;
 
-		for (auto &buffer : descriptor->buffers_) {
-			camera3_buffer_status status = CAMERA3_BUFFER_STATUS_ERROR;
+		/*
+		 * Pass the buffer fence back to the camera framework as
+		 * a release fence. This instructs the framework to wait
+		 * on the acquire fence in case we haven't done so
+		 * ourselves for any reason.
+		 */
+		resultBuffers.push_back({ buffer.stream->camera3Stream(),
+					  buffer.camera3Buffer, status,
+					  -1, buffer.fence.release() });
+	}
 
-			if (buffer.status == StreamBuffer::Status::Success)
-				status = CAMERA3_BUFFER_STATUS_OK;
+	camera3_capture_result_t captureResult = {};
 
-			/*
-			 * Pass the buffer fence back to the camera framework as
-			 * a release fence. This instructs the framework to wait
-			 * on the acquire fence in case we haven't done so
-			 * ourselves for any reason.
-			 */
-			resultBuffers.push_back({ buffer.stream->camera3Stream(),
-						  buffer.camera3Buffer, status,
-						  -1, buffer.fence.release() });
-		}
+	captureResult.frame_number = request->frameNumber_;
+	captureResult.num_output_buffers = resultBuffers.size();
+	captureResult.output_buffers = resultBuffers.data();
 
-		captureResult.num_output_buffers = resultBuffers.size();
-		captureResult.output_buffers = resultBuffers.data();
+	if (request->status_ == Camera3RequestDescriptor::Status::Success)
+		captureResult.partial_result = 1;
 
-		if (descriptor->status_ == Camera3RequestDescriptor::Status::Success)
-			captureResult.partial_result = 1;
+	if (request->resultMetadata_)
+		captureResult.result = request->resultMetadata_->getMetadata();
 
-		callbacks_->process_capture_result(callbacks_, &captureResult);
-	}
+	callbacks_->process_capture_result(callbacks_, &captureResult);
 }
 
 void CameraDevice::setBufferStatus(StreamBuffer &streamBuffer,
diff --git a/src/android/camera_device.h b/src/android/camera_device.h
index c92ee1aa4..699aa8f17 100644
--- a/src/android/camera_device.h
+++ b/src/android/camera_device.h
@@ -97,6 +97,7 @@  private:
 	void completeDescriptor(Camera3RequestDescriptor *descriptor)
 		LIBCAMERA_TSA_EXCLUDES(descriptorsMutex_);
 	void sendCaptureResults() LIBCAMERA_TSA_REQUIRES(descriptorsMutex_);
+	void sendCaptureResult(Camera3RequestDescriptor *request) const;
 	void setBufferStatus(StreamBuffer &buffer,
 			     StreamBuffer::Status status);
 	std::unique_ptr<CameraMetadata> getResultMetadata(