[libcamera-devel] [PATCH v3 7/8] android: camera_device: Add buffers for each stream to Requests
Jacopo Mondi
jacopo at jmondi.org
Fri Jul 3 15:24:40 CEST 2020
Hi Kieran,
On Fri, Jul 03, 2020 at 01:39:18PM +0100, Kieran Bingham wrote:
> Construct a FrameBuffer for every buffer given in the camera3Request
> and add it to the libcamera Request on the appropriate stream.
>
> The correct stream is obtained from the private data of the camera3_stream
> associated with the camera3_buffer.
>
> Comments regarding supporting only one buffer are now removed.
>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> src/android/camera_device.cpp | 42 ++++++++++++++++-------------------
> 1 file changed, 19 insertions(+), 23 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 28334751a26e..3d21e59af258 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -991,6 +991,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>
> /* Maintain internal state of all stream mappings. */
> streams_[i].index = streamIndex++;
> + stream->priv = static_cast<void *>(&streams_[i]);
> }
>
> switch (config_->validate()) {
> @@ -1049,9 +1050,6 @@ FrameBuffer *CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer
>
> int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Request)
> {
> - StreamConfiguration *streamConfiguration = &config_->at(0);
> - Stream *stream = streamConfiguration->stream();
> -
> if (camera3Request->num_output_buffers != 1) {
> LOG(HAL, Error) << "Invalid number of output buffers: "
> << camera3Request->num_output_buffers;
> @@ -1089,30 +1087,32 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> camera_->createRequest(reinterpret_cast<uint64_t>(descriptor));
>
> for (unsigned int i = 0; i < descriptor->numBuffers; ++i) {
> + CameraStream *cameraStream = static_cast<CameraStream *>(camera3Buffers[i].stream->priv);
> +
> /*
> * Keep track of which stream the request belongs to and store
> * the native buffer handles.
> - *
> - * \todo Currently we only support one capture buffer. Copy
> - * all of them to be ready once we'll support more.
> */
> descriptor->buffers[i].stream = camera3Buffers[i].stream;
> descriptor->buffers[i].buffer = camera3Buffers[i].buffer;
> - }
>
> - /*
> - * Create a libcamera buffer using the dmabuf descriptors of the first
> - * and (currently) only supported request buffer.
> - */
> - FrameBuffer *buffer = createFrameBuffer(*camera3Buffers[0].buffer);
> - if (!buffer) {
> - LOG(HAL, Error) << "Failed to create buffer";
> - delete request;
> - delete descriptor;
> - return -ENOMEM;
> - }
> + /*
> + * Create a libcamera buffer using the dmabuf descriptors of the
> + * first and (currently) only supported request buffer.
> + */
> + FrameBuffer *buffer = createFrameBuffer(*camera3Buffers[0].buffer);
> + if (!buffer) {
> + LOG(HAL, Error) << "Failed to create buffer";
> + delete request;
> + delete descriptor;
> + return -ENOMEM;
> + }
>
> - request->addBuffer(stream, buffer);
> + StreamConfiguration *streamConfiguration = &config_->at(cameraStream->index);
> + Stream *stream = streamConfiguration->stream();
> +
> + request->addBuffer(stream, buffer);
As we support multiple streams/buffers now, they should be handled in
CameraDevice::requestComplete(), as of now only the first frame buffer
is correctly deleted.
> + }
>
> int ret = camera_->queueRequest(request);
> if (ret) {
Looking at the error path here, should we free the just allocated
FrameBuffers if queueing a request fails ?
Would it make sense to track all the allocated FrameBuffers as part of
the Camera3RequestDescriptor, and free all of them here and at the end
of requestComplete() ?
> @@ -1146,10 +1146,6 @@ void CameraDevice::requestComplete(Request *request)
> captureResult.frame_number = descriptor->frameNumber;
> captureResult.num_output_buffers = descriptor->numBuffers;
> for (unsigned int i = 0; i < descriptor->numBuffers; ++i) {
> - /*
> - * \todo Currently we only support one capture buffer. Prepare
> - * all of them to be ready once we'll support more.
> - */
> descriptor->buffers[i].acquire_fence = -1;
> descriptor->buffers[i].release_fence = -1;
> descriptor->buffers[i].status = status;
> --
> 2.25.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
More information about the libcamera-devel
mailing list