[libcamera-devel] android: CameraDevice: factorize generating a capture result
diff mbox series

Message ID 20210426034502.2270770-1-hiroh@chromium.org
State Superseded
Headers show
Series
  • [libcamera-devel] android: CameraDevice: factorize generating a capture result
Related show

Commit Message

Hirokazu Honda April 26, 2021, 3:45 a.m. UTC 
This factorizes a code of generating camera3_capture_result. It
will be useful to report capture result in other places than
CameraDevice::reqeustComplete().

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>

---
I was going to implement
reportShutter() - createCaptureResult() + notifyShutter(), and
reportError() - createCaptureResult() + notifyError().

But reportError() can be called after reportShutter(). It is a
bit less efficient to create capture result twice in the case.
So I only factorize a code of generating a capture result.

---
 src/android/camera_device.cpp | 29 ++++++++++++++++++++---------
 src/android/camera_device.h   |  3 +++
 2 files changed, 23 insertions(+), 9 deletions(-)

--
2.31.1.498.g6c1eba8ee3d-goog

Comments

Laurent Pinchart April 26, 2021, 5:28 a.m. UTC | #1 
Hi Hiro,

Thank you for the patch.

On Mon, Apr 26, 2021 at 12:45:02PM +0900, Hirokazu Honda wrote:
> This factorizes a code of generating camera3_capture_result. It
> will be useful to report capture result in other places than
> CameraDevice::reqeustComplete().
> 
> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> 
> ---
> I was going to implement
> reportShutter() - createCaptureResult() + notifyShutter(), and
> reportError() - createCaptureResult() + notifyError().
> 
> But reportError() can be called after reportShutter(). It is a
> bit less efficient to create capture result twice in the case.
> So I only factorize a code of generating a capture result.

Can't we have a single function to do all that ? Right now we have

	/* Prepare to call back the Android camera stack. */
	camera3_capture_result_t captureResult = {};
	captureResult.frame_number = descriptor.frameNumber_;
	captureResult.num_output_buffers = descriptor.buffers_.size();
	for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
		buffer.acquire_fence = -1;
		buffer.release_fence = -1;
		buffer.status = status;
	}
	captureResult.output_buffers = descriptor.buffers_.data();

	if (status == CAMERA3_BUFFER_STATUS_OK) {
		notifyShutter(descriptor.frameNumber_, timestamp);

		captureResult.partial_result = 1;
		captureResult.result = resultMetadata->get();
	}

	if (status == CAMERA3_BUFFER_STATUS_ERROR || !captureResult.result) {
		/* \todo Improve error handling. In case we notify an error
		 * because the metadata generation fails, a shutter event has
		 * already been notified for this frame number before the error
		 * is here signalled. Make sure the error path plays well with
		 * the camera stack state machine.
		 */
		notifyError(descriptor.frameNumber_,
			    descriptor.buffers_[0].stream);
	}

	callbacks_->process_capture_result(callbacks_, &captureResult);

Could we move all that to a function that takes descriptor, status,
timestamp and resultMedadata as arguments ? In stop(), to implement
flush(), it would be called with status == CAMERA3_BUFFER_STATUS_ERROR,
so notifyShutter() would be skipped.

If you think that would cause issues, this patch is already an
improvement, so we could proceed with it.

> ---
>  src/android/camera_device.cpp | 29 ++++++++++++++++++++---------
>  src/android/camera_device.h   |  3 +++
>  2 files changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index a71aee2f..ced8efcc 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -2094,15 +2094,8 @@ void CameraDevice::requestComplete(Request *request)
>  	}
> 
>  	/* Prepare to call back the Android camera stack. */
> -	camera3_capture_result_t captureResult = {};
> -	captureResult.frame_number = descriptor.frameNumber_;
> -	captureResult.num_output_buffers = descriptor.buffers_.size();
> -	for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
> -		buffer.acquire_fence = -1;
> -		buffer.release_fence = -1;
> -		buffer.status = status;
> -	}
> -	captureResult.output_buffers = descriptor.buffers_.data();
> +	camera3_capture_result_t captureResult =
> +		createCaptureResult(descriptor, status);
> 
>  	if (status == CAMERA3_BUFFER_STATUS_OK) {
>  		notifyShutter(descriptor.frameNumber_, timestamp);
> @@ -2130,6 +2123,24 @@ std::string CameraDevice::logPrefix() const
>  	return "'" + camera_->id() + "'";
>  }
> 
> +
> +camera3_capture_result_t CameraDevice::createCaptureResult(
> +	Camera3RequestDescriptor &descriptor,
> +	camera3_buffer_status status) const
> +{
> +	camera3_capture_result_t captureResult = {};
> +	captureResult.frame_number = descriptor.frameNumber_;
> +	captureResult.num_output_buffers = descriptor.buffers_.size();
> +	for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
> +		buffer.acquire_fence = -1;
> +		buffer.release_fence = -1;
> +		buffer.status = status;
> +	}
> +	captureResult.output_buffers = descriptor.buffers_.data();
> +
> +	return captureResult;
> +}
> +
>  void CameraDevice::notifyShutter(uint32_t frameNumber, uint64_t timestamp)
>  {
>  	camera3_notify_msg_t notify = {};
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index c63e8e21..a1abcead 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -100,6 +100,9 @@ private:
> 
>  	std::tuple<uint32_t, uint32_t> calculateStaticMetadataSize();
>  	libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer);
> +	camera3_capture_result_t createCaptureResult(
> +		Camera3RequestDescriptor &descriptor,
> +		camera3_buffer_status status) const;
>  	void notifyShutter(uint32_t frameNumber, uint64_t timestamp);
>  	void notifyError(uint32_t frameNumber, camera3_stream_t *stream);
>  	std::unique_ptr<CameraMetadata> requestTemplatePreview();
Hirokazu Honda April 26, 2021, 9:08 a.m. UTC | #2 
Hi Laurent,

On Mon, Apr 26, 2021 at 2:28 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Hiro,
>
> Thank you for the patch.
>
> On Mon, Apr 26, 2021 at 12:45:02PM +0900, Hirokazu Honda wrote:
> > This factorizes a code of generating camera3_capture_result. It
> > will be useful to report capture result in other places than
> > CameraDevice::reqeustComplete().
> >
> > Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> >
> > ---
> > I was going to implement
> > reportShutter() - createCaptureResult() + notifyShutter(), and
> > reportError() - createCaptureResult() + notifyError().
> >
> > But reportError() can be called after reportShutter(). It is a
> > bit less efficient to create capture result twice in the case.
> > So I only factorize a code of generating a capture result.
>
> Can't we have a single function to do all that ? Right now we have
>
>         /* Prepare to call back the Android camera stack. */
>         camera3_capture_result_t captureResult = {};
>         captureResult.frame_number = descriptor.frameNumber_;
>         captureResult.num_output_buffers = descriptor.buffers_.size();
>         for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
>                 buffer.acquire_fence = -1;
>                 buffer.release_fence = -1;
>                 buffer.status = status;
>         }
>         captureResult.output_buffers = descriptor.buffers_.data();
>
>         if (status == CAMERA3_BUFFER_STATUS_OK) {
>                 notifyShutter(descriptor.frameNumber_, timestamp);
>
>                 captureResult.partial_result = 1;
>                 captureResult.result = resultMetadata->get();
>         }
>
>         if (status == CAMERA3_BUFFER_STATUS_ERROR || !captureResult.result) {
>                 /* \todo Improve error handling. In case we notify an error
>                  * because the metadata generation fails, a shutter event has
>                  * already been notified for this frame number before the error
>                  * is here signalled. Make sure the error path plays well with
>                  * the camera stack state machine.
>                  */
>                 notifyError(descriptor.frameNumber_,
>                             descriptor.buffers_[0].stream);
>         }
>
>         callbacks_->process_capture_result(callbacks_, &captureResult);
>
> Could we move all that to a function that takes descriptor, status,
> timestamp and resultMedadata as arguments ? In stop(), to implement
> flush(), it would be called with status == CAMERA3_BUFFER_STATUS_ERROR,
> so notifyShutter() would be skipped.
>
> If you think that would cause issues, this patch is already an
> improvement, so we could proceed with it.
>

I did so some times ago when I worked with Jacopo for Flush().
Jacopo suggested to not not contain a code for shutter and error in
the same function, as either of them are not executed.
Jacopo, how do you think?

-Hiro

> > ---
> >  src/android/camera_device.cpp | 29 ++++++++++++++++++++---------
> >  src/android/camera_device.h   |  3 +++
> >  2 files changed, 23 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index a71aee2f..ced8efcc 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -2094,15 +2094,8 @@ void CameraDevice::requestComplete(Request *request)
> >       }
> >
> >       /* Prepare to call back the Android camera stack. */
> > -     camera3_capture_result_t captureResult = {};
> > -     captureResult.frame_number = descriptor.frameNumber_;
> > -     captureResult.num_output_buffers = descriptor.buffers_.size();
> > -     for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
> > -             buffer.acquire_fence = -1;
> > -             buffer.release_fence = -1;
> > -             buffer.status = status;
> > -     }
> > -     captureResult.output_buffers = descriptor.buffers_.data();
> > +     camera3_capture_result_t captureResult =
> > +             createCaptureResult(descriptor, status);
> >
> >       if (status == CAMERA3_BUFFER_STATUS_OK) {
> >               notifyShutter(descriptor.frameNumber_, timestamp);
> > @@ -2130,6 +2123,24 @@ std::string CameraDevice::logPrefix() const
> >       return "'" + camera_->id() + "'";
> >  }
> >
> > +
> > +camera3_capture_result_t CameraDevice::createCaptureResult(
> > +     Camera3RequestDescriptor &descriptor,
> > +     camera3_buffer_status status) const
> > +{
> > +     camera3_capture_result_t captureResult = {};
> > +     captureResult.frame_number = descriptor.frameNumber_;
> > +     captureResult.num_output_buffers = descriptor.buffers_.size();
> > +     for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
> > +             buffer.acquire_fence = -1;
> > +             buffer.release_fence = -1;
> > +             buffer.status = status;
> > +     }
> > +     captureResult.output_buffers = descriptor.buffers_.data();
> > +
> > +     return captureResult;
> > +}
> > +
> >  void CameraDevice::notifyShutter(uint32_t frameNumber, uint64_t timestamp)
> >  {
> >       camera3_notify_msg_t notify = {};
> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> > index c63e8e21..a1abcead 100644
> > --- a/src/android/camera_device.h
> > +++ b/src/android/camera_device.h
> > @@ -100,6 +100,9 @@ private:
> >
> >       std::tuple<uint32_t, uint32_t> calculateStaticMetadataSize();
> >       libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer);
> > +     camera3_capture_result_t createCaptureResult(
> > +             Camera3RequestDescriptor &descriptor,
> > +             camera3_buffer_status status) const;
> >       void notifyShutter(uint32_t frameNumber, uint64_t timestamp);
> >       void notifyError(uint32_t frameNumber, camera3_stream_t *stream);
> >       std::unique_ptr<CameraMetadata> requestTemplatePreview();
>
> --
> Regards,
>
> Laurent Pinchart
Jacopo Mondi April 26, 2021, 10:39 a.m. UTC | #3 
Hi Hiro,

On Mon, Apr 26, 2021 at 06:08:10PM +0900, Hirokazu Honda wrote:
> Hi Laurent,
>
> On Mon, Apr 26, 2021 at 2:28 PM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> >
> > Hi Hiro,
> >
> > Thank you for the patch.
> >
> > On Mon, Apr 26, 2021 at 12:45:02PM +0900, Hirokazu Honda wrote:
> > > This factorizes a code of generating camera3_capture_result. It
> > > will be useful to report capture result in other places than
> > > CameraDevice::reqeustComplete().
> > >
> > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> > >
> > > ---
> > > I was going to implement
> > > reportShutter() - createCaptureResult() + notifyShutter(), and
> > > reportError() - createCaptureResult() + notifyError().
> > >
> > > But reportError() can be called after reportShutter(). It is a
> > > bit less efficient to create capture result twice in the case.
> > > So I only factorize a code of generating a capture result.
> >
> > Can't we have a single function to do all that ? Right now we have
> >
> >         /* Prepare to call back the Android camera stack. */
> >         camera3_capture_result_t captureResult = {};
> >         captureResult.frame_number = descriptor.frameNumber_;
> >         captureResult.num_output_buffers = descriptor.buffers_.size();
> >         for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
> >                 buffer.acquire_fence = -1;
> >                 buffer.release_fence = -1;
> >                 buffer.status = status;
> >         }
> >         captureResult.output_buffers = descriptor.buffers_.data();
> >
> >         if (status == CAMERA3_BUFFER_STATUS_OK) {
> >                 notifyShutter(descriptor.frameNumber_, timestamp);
> >
> >                 captureResult.partial_result = 1;
> >                 captureResult.result = resultMetadata->get();
> >         }
> >
> >         if (status == CAMERA3_BUFFER_STATUS_ERROR || !captureResult.result) {
> >                 /* \todo Improve error handling. In case we notify an error
> >                  * because the metadata generation fails, a shutter event has
> >                  * already been notified for this frame number before the error
> >                  * is here signalled. Make sure the error path plays well with
> >                  * the camera stack state machine.
> >                  */
> >                 notifyError(descriptor.frameNumber_,
> >                             descriptor.buffers_[0].stream);
> >         }
> >
> >         callbacks_->process_capture_result(callbacks_, &captureResult);
> >
> > Could we move all that to a function that takes descriptor, status,
> > timestamp and resultMedadata as arguments ? In stop(), to implement
> > flush(), it would be called with status == CAMERA3_BUFFER_STATUS_ERROR,
> > so notifyShutter() would be skipped.
> >
> > If you think that would cause issues, this patch is already an
> > improvement, so we could proceed with it.
> >
>
> I did so some times ago when I worked with Jacopo for Flush().
> Jacopo suggested to not not contain a code for shutter and error in
> the same function, as either of them are not executed.
> Jacopo, how do you think?
>

It's terribily hard to have a sense of how a patch is used without
context with all the pending work we have in flux.

If I recall correctly, in the context of the flush() implementation we
discussed, was that having all in one function was not that beneficial
as notifications will have to be sent to the framework asynchronously
in the long term, and part of the function was dead code.

Again, memory can fail me, there's really a lot of things moving and
keep the full picture in mind is hard.

Thanks
   j

> -Hiro
>
> > > ---
> > >  src/android/camera_device.cpp | 29 ++++++++++++++++++++---------
> > >  src/android/camera_device.h   |  3 +++
> > >  2 files changed, 23 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > > index a71aee2f..ced8efcc 100644
> > > --- a/src/android/camera_device.cpp
> > > +++ b/src/android/camera_device.cpp
> > > @@ -2094,15 +2094,8 @@ void CameraDevice::requestComplete(Request *request)
> > >       }
> > >
> > >       /* Prepare to call back the Android camera stack. */
> > > -     camera3_capture_result_t captureResult = {};
> > > -     captureResult.frame_number = descriptor.frameNumber_;
> > > -     captureResult.num_output_buffers = descriptor.buffers_.size();
> > > -     for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
> > > -             buffer.acquire_fence = -1;
> > > -             buffer.release_fence = -1;
> > > -             buffer.status = status;
> > > -     }
> > > -     captureResult.output_buffers = descriptor.buffers_.data();
> > > +     camera3_capture_result_t captureResult =
> > > +             createCaptureResult(descriptor, status);
> > >
> > >       if (status == CAMERA3_BUFFER_STATUS_OK) {
> > >               notifyShutter(descriptor.frameNumber_, timestamp);
> > > @@ -2130,6 +2123,24 @@ std::string CameraDevice::logPrefix() const
> > >       return "'" + camera_->id() + "'";
> > >  }
> > >
> > > +
> > > +camera3_capture_result_t CameraDevice::createCaptureResult(
> > > +     Camera3RequestDescriptor &descriptor,
> > > +     camera3_buffer_status status) const
> > > +{
> > > +     camera3_capture_result_t captureResult = {};
> > > +     captureResult.frame_number = descriptor.frameNumber_;
> > > +     captureResult.num_output_buffers = descriptor.buffers_.size();
> > > +     for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
> > > +             buffer.acquire_fence = -1;
> > > +             buffer.release_fence = -1;
> > > +             buffer.status = status;
> > > +     }
> > > +     captureResult.output_buffers = descriptor.buffers_.data();
> > > +
> > > +     return captureResult;
> > > +}
> > > +
> > >  void CameraDevice::notifyShutter(uint32_t frameNumber, uint64_t timestamp)
> > >  {
> > >       camera3_notify_msg_t notify = {};
> > > diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> > > index c63e8e21..a1abcead 100644
> > > --- a/src/android/camera_device.h
> > > +++ b/src/android/camera_device.h
> > > @@ -100,6 +100,9 @@ private:
> > >
> > >       std::tuple<uint32_t, uint32_t> calculateStaticMetadataSize();
> > >       libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer);
> > > +     camera3_capture_result_t createCaptureResult(
> > > +             Camera3RequestDescriptor &descriptor,
> > > +             camera3_buffer_status status) const;
> > >       void notifyShutter(uint32_t frameNumber, uint64_t timestamp);
> > >       void notifyError(uint32_t frameNumber, camera3_stream_t *stream);
> > >       std::unique_ptr<CameraMetadata> requestTemplatePreview();
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
Hirokazu Honda April 26, 2021, 11:01 a.m. UTC | #4 
Hi Jacopo,

On Mon, Apr 26, 2021 at 7:38 PM Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> Hi Hiro,
>
> On Mon, Apr 26, 2021 at 06:08:10PM +0900, Hirokazu Honda wrote:
> > Hi Laurent,
> >
> > On Mon, Apr 26, 2021 at 2:28 PM Laurent Pinchart
> > <laurent.pinchart@ideasonboard.com> wrote:
> > >
> > > Hi Hiro,
> > >
> > > Thank you for the patch.
> > >
> > > On Mon, Apr 26, 2021 at 12:45:02PM +0900, Hirokazu Honda wrote:
> > > > This factorizes a code of generating camera3_capture_result. It
> > > > will be useful to report capture result in other places than
> > > > CameraDevice::reqeustComplete().
> > > >
> > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> > > >
> > > > ---
> > > > I was going to implement
> > > > reportShutter() - createCaptureResult() + notifyShutter(), and
> > > > reportError() - createCaptureResult() + notifyError().
> > > >
> > > > But reportError() can be called after reportShutter(). It is a
> > > > bit less efficient to create capture result twice in the case.
> > > > So I only factorize a code of generating a capture result.
> > >
> > > Can't we have a single function to do all that ? Right now we have
> > >
> > >         /* Prepare to call back the Android camera stack. */
> > >         camera3_capture_result_t captureResult = {};
> > >         captureResult.frame_number = descriptor.frameNumber_;
> > >         captureResult.num_output_buffers = descriptor.buffers_.size();
> > >         for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
> > >                 buffer.acquire_fence = -1;
> > >                 buffer.release_fence = -1;
> > >                 buffer.status = status;
> > >         }
> > >         captureResult.output_buffers = descriptor.buffers_.data();
> > >
> > >         if (status == CAMERA3_BUFFER_STATUS_OK) {
> > >                 notifyShutter(descriptor.frameNumber_, timestamp);
> > >
> > >                 captureResult.partial_result = 1;
> > >                 captureResult.result = resultMetadata->get();
> > >         }
> > >
> > >         if (status == CAMERA3_BUFFER_STATUS_ERROR || !captureResult.result) {
> > >                 /* \todo Improve error handling. In case we notify an error
> > >                  * because the metadata generation fails, a shutter event has
> > >                  * already been notified for this frame number before the error
> > >                  * is here signalled. Make sure the error path plays well with
> > >                  * the camera stack state machine.
> > >                  */
> > >                 notifyError(descriptor.frameNumber_,
> > >                             descriptor.buffers_[0].stream);
> > >         }
> > >
> > >         callbacks_->process_capture_result(callbacks_, &captureResult);
> > >
> > > Could we move all that to a function that takes descriptor, status,
> > > timestamp and resultMedadata as arguments ? In stop(), to implement
> > > flush(), it would be called with status == CAMERA3_BUFFER_STATUS_ERROR,
> > > so notifyShutter() would be skipped.
> > >
> > > If you think that would cause issues, this patch is already an
> > > improvement, so we could proceed with it.
> > >
> >
> > I did so some times ago when I worked with Jacopo for Flush().
> > Jacopo suggested to not not contain a code for shutter and error in
> > the same function, as either of them are not executed.
> > Jacopo, how do you think?
> >
>
> It's terribily hard to have a sense of how a patch is used without
> context with all the pending work we have in flux.
>
> If I recall correctly, in the context of the flush() implementation we
> discussed, was that having all in one function was not that beneficial
> as notifications will have to be sent to the framework asynchronously
> in the long term, and part of the function was dead code.
>
> Again, memory can fail me, there's really a lot of things moving and
> keep the full picture in mind is hard.
>

I uploaded the next patch series. Let's discuss there.

> Thanks
>    j
>
> > -Hiro
> >
> > > > ---
> > > >  src/android/camera_device.cpp | 29 ++++++++++++++++++++---------
> > > >  src/android/camera_device.h   |  3 +++
> > > >  2 files changed, 23 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > > > index a71aee2f..ced8efcc 100644
> > > > --- a/src/android/camera_device.cpp
> > > > +++ b/src/android/camera_device.cpp
> > > > @@ -2094,15 +2094,8 @@ void CameraDevice::requestComplete(Request *request)
> > > >       }
> > > >
> > > >       /* Prepare to call back the Android camera stack. */
> > > > -     camera3_capture_result_t captureResult = {};
> > > > -     captureResult.frame_number = descriptor.frameNumber_;
> > > > -     captureResult.num_output_buffers = descriptor.buffers_.size();
> > > > -     for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
> > > > -             buffer.acquire_fence = -1;
> > > > -             buffer.release_fence = -1;
> > > > -             buffer.status = status;
> > > > -     }
> > > > -     captureResult.output_buffers = descriptor.buffers_.data();
> > > > +     camera3_capture_result_t captureResult =
> > > > +             createCaptureResult(descriptor, status);
> > > >
> > > >       if (status == CAMERA3_BUFFER_STATUS_OK) {
> > > >               notifyShutter(descriptor.frameNumber_, timestamp);
> > > > @@ -2130,6 +2123,24 @@ std::string CameraDevice::logPrefix() const
> > > >       return "'" + camera_->id() + "'";
> > > >  }
> > > >
> > > > +
> > > > +camera3_capture_result_t CameraDevice::createCaptureResult(
> > > > +     Camera3RequestDescriptor &descriptor,
> > > > +     camera3_buffer_status status) const
> > > > +{
> > > > +     camera3_capture_result_t captureResult = {};
> > > > +     captureResult.frame_number = descriptor.frameNumber_;
> > > > +     captureResult.num_output_buffers = descriptor.buffers_.size();
> > > > +     for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
> > > > +             buffer.acquire_fence = -1;
> > > > +             buffer.release_fence = -1;
> > > > +             buffer.status = status;
> > > > +     }
> > > > +     captureResult.output_buffers = descriptor.buffers_.data();
> > > > +
> > > > +     return captureResult;
> > > > +}
> > > > +
> > > >  void CameraDevice::notifyShutter(uint32_t frameNumber, uint64_t timestamp)
> > > >  {
> > > >       camera3_notify_msg_t notify = {};
> > > > diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> > > > index c63e8e21..a1abcead 100644
> > > > --- a/src/android/camera_device.h
> > > > +++ b/src/android/camera_device.h
> > > > @@ -100,6 +100,9 @@ private:
> > > >
> > > >       std::tuple<uint32_t, uint32_t> calculateStaticMetadataSize();
> > > >       libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer);
> > > > +     camera3_capture_result_t createCaptureResult(
> > > > +             Camera3RequestDescriptor &descriptor,
> > > > +             camera3_buffer_status status) const;
> > > >       void notifyShutter(uint32_t frameNumber, uint64_t timestamp);
> > > >       void notifyError(uint32_t frameNumber, camera3_stream_t *stream);
> > > >       std::unique_ptr<CameraMetadata> requestTemplatePreview();
> > >
> > > --
> > > Regards,
> > >
> > > Laurent Pinchart

Patch
diff mbox series 

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index a71aee2f..ced8efcc 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -2094,15 +2094,8 @@  void CameraDevice::requestComplete(Request *request)
 	}

 	/* Prepare to call back the Android camera stack. */
-	camera3_capture_result_t captureResult = {};
-	captureResult.frame_number = descriptor.frameNumber_;
-	captureResult.num_output_buffers = descriptor.buffers_.size();
-	for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
-		buffer.acquire_fence = -1;
-		buffer.release_fence = -1;
-		buffer.status = status;
-	}
-	captureResult.output_buffers = descriptor.buffers_.data();
+	camera3_capture_result_t captureResult =
+		createCaptureResult(descriptor, status);

 	if (status == CAMERA3_BUFFER_STATUS_OK) {
 		notifyShutter(descriptor.frameNumber_, timestamp);
@@ -2130,6 +2123,24 @@  std::string CameraDevice::logPrefix() const
 	return "'" + camera_->id() + "'";
 }

+
+camera3_capture_result_t CameraDevice::createCaptureResult(
+	Camera3RequestDescriptor &descriptor,
+	camera3_buffer_status status) const
+{
+	camera3_capture_result_t captureResult = {};
+	captureResult.frame_number = descriptor.frameNumber_;
+	captureResult.num_output_buffers = descriptor.buffers_.size();
+	for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
+		buffer.acquire_fence = -1;
+		buffer.release_fence = -1;
+		buffer.status = status;
+	}
+	captureResult.output_buffers = descriptor.buffers_.data();
+
+	return captureResult;
+}
+
 void CameraDevice::notifyShutter(uint32_t frameNumber, uint64_t timestamp)
 {
 	camera3_notify_msg_t notify = {};
diff --git a/src/android/camera_device.h b/src/android/camera_device.h
index c63e8e21..a1abcead 100644
--- a/src/android/camera_device.h
+++ b/src/android/camera_device.h
@@ -100,6 +100,9 @@  private:

 	std::tuple<uint32_t, uint32_t> calculateStaticMetadataSize();
 	libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer);
+	camera3_capture_result_t createCaptureResult(
+		Camera3RequestDescriptor &descriptor,
+		camera3_buffer_status status) const;
 	void notifyShutter(uint32_t frameNumber, uint64_t timestamp);
 	void notifyError(uint32_t frameNumber, camera3_stream_t *stream);
 	std::unique_ptr<CameraMetadata> requestTemplatePreview();