| Message ID | 20211018132923.476242-6-umang.jain@ideasonboard.com |
|---|---|
| State | Superseded |
| Delegated to: | Umang Jain |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, Thank you for the patch. On Mon, Oct 18, 2021 at 06:59:17PM +0530, Umang Jain wrote: > The Camera3RequestDescriptor structure stores, for each stream, the > camera3_stream_buffer_t and the libcamera FrameBuffer in two separate > vectors. This complicates buffer handling, as the code needs to keep > both vectors in sync. Create a new structure to group all data about > per-stream buffers to simplify this. > > As a side effect, we need to create a local vector of > camera3_stream_buffer_t in CameraDevice::sendCaptureResults() as the > camera3_stream_buffer_t instances stored in the new structure in > Camera3RequestDescriptor are not contiguous anymore. This is a small > price to pay for easier handling of buffers, and will be refactored in > subsequent commits anyway. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/android/camera_device.cpp | 75 ++++++++++++++++++---------------- > src/android/camera_request.cpp | 9 +--- > src/android/camera_request.h | 15 ++++++- > 3 files changed, 55 insertions(+), 44 deletions(-) > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > index 3bddb292..59557358 100644 > --- a/src/android/camera_device.cpp > +++ b/src/android/camera_device.cpp > @@ -831,9 +831,9 @@ void CameraDevice::abortRequest(Camera3RequestDescriptor *descriptor) const > notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_REQUEST); > > for (auto &buffer : descriptor->buffers_) { > - buffer.release_fence = buffer.acquire_fence; > - buffer.acquire_fence = -1; > - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > + buffer.buffer.release_fence = buffer.buffer.acquire_fence; > + buffer.buffer.acquire_fence = -1; > + buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > } > > descriptor->status_ = Camera3RequestDescriptor::Status::Error; > @@ -931,8 +931,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques > LOG(HAL, Debug) << "Queueing request " << descriptor->request_->cookie() > << " with " << descriptor->buffers_.size() << " streams"; > > - for (const auto &[i, camera3Buffer] : utils::enumerate(descriptor->buffers_)) { > - camera3_stream *camera3Stream = camera3Buffer.stream; > + for (const auto &[i, buffer] : utils::enumerate(descriptor->buffers_)) { > + camera3_stream *camera3Stream = buffer.buffer.stream; > CameraStream *cameraStream = static_cast<CameraStream *>(camera3Stream->priv); > > std::stringstream ss; > @@ -949,7 +949,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques > * while fences for streams of type Internal and Mapped are > * handled at post-processing time. > */ > - FrameBuffer *buffer = nullptr; > + FrameBuffer *frameBuffer = nullptr; > int acquireFence = -1; > switch (cameraStream->type()) { > case CameraStream::Type::Mapped: > @@ -967,13 +967,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques > * associate it with the Camera3RequestDescriptor for > * lifetime management only. > */ > - descriptor->frameBuffers_.push_back( > - createFrameBuffer(*camera3Buffer.buffer, > + buffer.frameBuffer = > + createFrameBuffer(*buffer.buffer.buffer, > cameraStream->configuration().pixelFormat, > - cameraStream->configuration().size)); > - > - buffer = descriptor->frameBuffers_.back().get(); > - acquireFence = camera3Buffer.acquire_fence; > + cameraStream->configuration().size); > + frameBuffer = buffer.frameBuffer.get(); > + acquireFence = buffer.buffer.acquire_fence; > LOG(HAL, Debug) << ss.str() << " (direct)"; > break; > > @@ -985,18 +984,18 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques > * The buffer has to be returned to the CameraStream > * once it has been processed. > */ > - buffer = cameraStream->getBuffer(); > + frameBuffer = cameraStream->getBuffer(); > LOG(HAL, Debug) << ss.str() << " (internal)"; > break; > } > > - if (!buffer) { > - LOG(HAL, Error) << "Failed to create buffer"; > + if (!frameBuffer) { > + LOG(HAL, Error) << "Failed to create frame buffer"; > return -ENOMEM; > } > > - descriptor->request_->addBuffer(cameraStream->stream(), buffer, > - acquireFence); > + descriptor->request_->addBuffer(cameraStream->stream(), > + frameBuffer, acquireFence); > } > > /* > @@ -1082,9 +1081,9 @@ void CameraDevice::requestComplete(Request *request) > * The buffer status is set to OK and later changed to ERROR if > * post-processing/compression fails. > */ > - for (camera3_stream_buffer_t &buffer : descriptor->buffers_) { > + for (auto &buffer : descriptor->buffers_) { > CameraStream *cameraStream = > - static_cast<CameraStream *>(buffer.stream->priv); > + static_cast<CameraStream *>(buffer.buffer.stream->priv); > > /* > * Streams of type Direct have been queued to the > @@ -1098,9 +1097,9 @@ void CameraDevice::requestComplete(Request *request) > * fence to -1 once it has handled it and remove this check. > */ > if (cameraStream->type() == CameraStream::Type::Direct) > - buffer.acquire_fence = -1; > - buffer.release_fence = -1; > - buffer.status = CAMERA3_BUFFER_STATUS_OK; > + buffer.buffer.acquire_fence = -1; > + buffer.buffer.release_fence = -1; > + buffer.buffer.status = CAMERA3_BUFFER_STATUS_OK; > } > > /* > @@ -1115,15 +1114,15 @@ void CameraDevice::requestComplete(Request *request) > notifyError(descriptor->frameNumber_, nullptr, > CAMERA3_MSG_ERROR_REQUEST); > > - for (camera3_stream_buffer_t &buffer : descriptor->buffers_) { > + for (auto &buffer : descriptor->buffers_) { > /* > * Signal to the framework it has to handle fences that > * have not been waited on by setting the release fence > * to the acquire fence value. > */ > - buffer.release_fence = buffer.acquire_fence; > - buffer.acquire_fence = -1; > - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > + buffer.buffer.release_fence = buffer.buffer.acquire_fence; > + buffer.buffer.acquire_fence = -1; > + buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > } > > descriptor->status_ = Camera3RequestDescriptor::Status::Error; > @@ -1160,9 +1159,9 @@ void CameraDevice::requestComplete(Request *request) > } > > /* Handle post-processing. */ > - for (camera3_stream_buffer_t &buffer : descriptor->buffers_) { > + for (auto &buffer : descriptor->buffers_) { > CameraStream *cameraStream = > - static_cast<CameraStream *>(buffer.stream->priv); > + static_cast<CameraStream *>(buffer.buffer.stream->priv); > > if (cameraStream->type() == CameraStream::Type::Direct) > continue; > @@ -1170,13 +1169,13 @@ void CameraDevice::requestComplete(Request *request) > FrameBuffer *src = request->findBuffer(cameraStream->stream()); > if (!src) { > LOG(HAL, Error) << "Failed to find a source stream buffer"; > - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > - notifyError(descriptor->frameNumber_, buffer.stream, > + buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > + notifyError(descriptor->frameNumber_, buffer.buffer.stream, > CAMERA3_MSG_ERROR_BUFFER); > continue; > } > > - int ret = cameraStream->process(*src, buffer, descriptor); > + int ret = cameraStream->process(*src, buffer.buffer, descriptor); > > /* > * Return the FrameBuffer to the CameraStream now that we're > @@ -1186,8 +1185,8 @@ void CameraDevice::requestComplete(Request *request) > cameraStream->putBuffer(src); > > if (ret) { > - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > - notifyError(descriptor->frameNumber_, buffer.stream, > + buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > + notifyError(descriptor->frameNumber_, buffer.buffer.stream, > CAMERA3_MSG_ERROR_BUFFER); > } > } > @@ -1213,10 +1212,16 @@ void CameraDevice::sendCaptureResults() > camera3_capture_result_t captureResult = {}; > > captureResult.frame_number = descriptor->frameNumber_; > + > if (descriptor->resultMetadata_) > captureResult.result = descriptor->resultMetadata_->get(); > - captureResult.num_output_buffers = descriptor->buffers_.size(); > - captureResult.output_buffers = descriptor->buffers_.data(); > + > + std::vector<camera3_stream_buffer_t> resultBuffers; > + for (const auto &buffer : descriptor->buffers_) > + resultBuffers.emplace_back(buffer.buffer); > + > + captureResult.num_output_buffers = resultBuffers.size(); > + captureResult.output_buffers = resultBuffers.data(); > > if (descriptor->status_ == Camera3RequestDescriptor::Status::Success) > captureResult.partial_result = 1; > diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp > index 16a632b3..614baed4 100644 > --- a/src/android/camera_request.cpp > +++ b/src/android/camera_request.cpp > @@ -23,15 +23,10 @@ Camera3RequestDescriptor::Camera3RequestDescriptor( > > /* Copy the camera3 request stream information for later access. */ > const uint32_t numBuffers = camera3Request->num_output_buffers; > + > buffers_.resize(numBuffers); > for (uint32_t i = 0; i < numBuffers; i++) > - buffers_[i] = camera3Request->output_buffers[i]; > - > - /* > - * FrameBuffer instances created by wrapping a camera3 provided dmabuf > - * are emplaced in this vector of unique_ptr<> for lifetime management. > - */ > - frameBuffers_.reserve(numBuffers); > + buffers_[i].buffer = camera3Request->output_buffers[i]; > > /* Clone the controls associated with the camera3 request. */ > settings_ = CameraMetadata(camera3Request->settings); > diff --git a/src/android/camera_request.h b/src/android/camera_request.h > index db13f624..a030febf 100644 > --- a/src/android/camera_request.h > +++ b/src/android/camera_request.h > @@ -29,6 +29,16 @@ public: > Error, > }; > > + struct StreamBuffer { > + camera3_stream_buffer_t buffer; > + /* > + * FrameBuffer instances created by wrapping a camera3 provided > + * dmabuf are emplaced in this vector of unique_ptr<> for > + * lifetime management. > + */ > + std::unique_ptr<libcamera::FrameBuffer> frameBuffer; > + }; > + > Camera3RequestDescriptor(libcamera::Camera *camera, > const camera3_capture_request_t *camera3Request); > ~Camera3RequestDescriptor(); > @@ -36,8 +46,9 @@ public: > bool isPending() const { return status_ == Status::Pending; } > > uint32_t frameNumber_ = 0; > - std::vector<camera3_stream_buffer_t> buffers_; > - std::vector<std::unique_ptr<libcamera::FrameBuffer>> frameBuffers_; > + > + std::vector<StreamBuffer> buffers_; > + > CameraMetadata settings_; > std::unique_ptr<CaptureRequest> request_; > std::unique_ptr<CameraMetadata> resultMetadata_;
Hi Umang, On Mon, Oct 18, 2021 at 06:59:17PM +0530, Umang Jain wrote: > The Camera3RequestDescriptor structure stores, for each stream, the > camera3_stream_buffer_t and the libcamera FrameBuffer in two separate > vectors. This complicates buffer handling, as the code needs to keep > both vectors in sync. Create a new structure to group all data about > per-stream buffers to simplify this. > > As a side effect, we need to create a local vector of > camera3_stream_buffer_t in CameraDevice::sendCaptureResults() as the > camera3_stream_buffer_t instances stored in the new structure in > Camera3RequestDescriptor are not contiguous anymore. This is a small > price to pay for easier handling of buffers, and will be refactored in > subsequent commits anyway. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/android/camera_device.cpp | 75 ++++++++++++++++++---------------- > src/android/camera_request.cpp | 9 +--- > src/android/camera_request.h | 15 ++++++- > 3 files changed, 55 insertions(+), 44 deletions(-) > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > index 3bddb292..59557358 100644 > --- a/src/android/camera_device.cpp > +++ b/src/android/camera_device.cpp > @@ -831,9 +831,9 @@ void CameraDevice::abortRequest(Camera3RequestDescriptor *descriptor) const > notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_REQUEST); > > for (auto &buffer : descriptor->buffers_) { > - buffer.release_fence = buffer.acquire_fence; > - buffer.acquire_fence = -1; > - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > + buffer.buffer.release_fence = buffer.buffer.acquire_fence; > + buffer.buffer.acquire_fence = -1; > + buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > } > > descriptor->status_ = Camera3RequestDescriptor::Status::Error; > @@ -931,8 +931,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques > LOG(HAL, Debug) << "Queueing request " << descriptor->request_->cookie() > << " with " << descriptor->buffers_.size() << " streams"; > > - for (const auto &[i, camera3Buffer] : utils::enumerate(descriptor->buffers_)) { > - camera3_stream *camera3Stream = camera3Buffer.stream; > + for (const auto &[i, buffer] : utils::enumerate(descriptor->buffers_)) { > + camera3_stream *camera3Stream = buffer.buffer.stream; > CameraStream *cameraStream = static_cast<CameraStream *>(camera3Stream->priv); > > std::stringstream ss; > @@ -949,7 +949,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques > * while fences for streams of type Internal and Mapped are > * handled at post-processing time. > */ > - FrameBuffer *buffer = nullptr; > + FrameBuffer *frameBuffer = nullptr; > int acquireFence = -1; > switch (cameraStream->type()) { > case CameraStream::Type::Mapped: > @@ -967,13 +967,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques > * associate it with the Camera3RequestDescriptor for > * lifetime management only. > */ > - descriptor->frameBuffers_.push_back( > - createFrameBuffer(*camera3Buffer.buffer, > + buffer.frameBuffer = > + createFrameBuffer(*buffer.buffer.buffer, > cameraStream->configuration().pixelFormat, > - cameraStream->configuration().size)); > - > - buffer = descriptor->frameBuffers_.back().get(); > - acquireFence = camera3Buffer.acquire_fence; > + cameraStream->configuration().size); > + frameBuffer = buffer.frameBuffer.get(); > + acquireFence = buffer.buffer.acquire_fence; > LOG(HAL, Debug) << ss.str() << " (direct)"; > break; > > @@ -985,18 +984,18 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques > * The buffer has to be returned to the CameraStream > * once it has been processed. > */ > - buffer = cameraStream->getBuffer(); > + frameBuffer = cameraStream->getBuffer(); > LOG(HAL, Debug) << ss.str() << " (internal)"; > break; > } > > - if (!buffer) { > - LOG(HAL, Error) << "Failed to create buffer"; > + if (!frameBuffer) { > + LOG(HAL, Error) << "Failed to create frame buffer"; > return -ENOMEM; > } > > - descriptor->request_->addBuffer(cameraStream->stream(), buffer, > - acquireFence); > + descriptor->request_->addBuffer(cameraStream->stream(), > + frameBuffer, acquireFence); > } > > /* > @@ -1082,9 +1081,9 @@ void CameraDevice::requestComplete(Request *request) > * The buffer status is set to OK and later changed to ERROR if > * post-processing/compression fails. > */ > - for (camera3_stream_buffer_t &buffer : descriptor->buffers_) { > + for (auto &buffer : descriptor->buffers_) { > CameraStream *cameraStream = > - static_cast<CameraStream *>(buffer.stream->priv); > + static_cast<CameraStream *>(buffer.buffer.stream->priv); > > /* > * Streams of type Direct have been queued to the > @@ -1098,9 +1097,9 @@ void CameraDevice::requestComplete(Request *request) > * fence to -1 once it has handled it and remove this check. > */ > if (cameraStream->type() == CameraStream::Type::Direct) > - buffer.acquire_fence = -1; > - buffer.release_fence = -1; > - buffer.status = CAMERA3_BUFFER_STATUS_OK; > + buffer.buffer.acquire_fence = -1; > + buffer.buffer.release_fence = -1; > + buffer.buffer.status = CAMERA3_BUFFER_STATUS_OK; > } > > /* > @@ -1115,15 +1114,15 @@ void CameraDevice::requestComplete(Request *request) > notifyError(descriptor->frameNumber_, nullptr, > CAMERA3_MSG_ERROR_REQUEST); > > - for (camera3_stream_buffer_t &buffer : descriptor->buffers_) { > + for (auto &buffer : descriptor->buffers_) { > /* > * Signal to the framework it has to handle fences that > * have not been waited on by setting the release fence > * to the acquire fence value. > */ > - buffer.release_fence = buffer.acquire_fence; > - buffer.acquire_fence = -1; > - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > + buffer.buffer.release_fence = buffer.buffer.acquire_fence; > + buffer.buffer.acquire_fence = -1; > + buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > } > > descriptor->status_ = Camera3RequestDescriptor::Status::Error; > @@ -1160,9 +1159,9 @@ void CameraDevice::requestComplete(Request *request) > } > > /* Handle post-processing. */ > - for (camera3_stream_buffer_t &buffer : descriptor->buffers_) { > + for (auto &buffer : descriptor->buffers_) { > CameraStream *cameraStream = > - static_cast<CameraStream *>(buffer.stream->priv); > + static_cast<CameraStream *>(buffer.buffer.stream->priv); > > if (cameraStream->type() == CameraStream::Type::Direct) > continue; > @@ -1170,13 +1169,13 @@ void CameraDevice::requestComplete(Request *request) > FrameBuffer *src = request->findBuffer(cameraStream->stream()); > if (!src) { > LOG(HAL, Error) << "Failed to find a source stream buffer"; > - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > - notifyError(descriptor->frameNumber_, buffer.stream, > + buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > + notifyError(descriptor->frameNumber_, buffer.buffer.stream, > CAMERA3_MSG_ERROR_BUFFER); > continue; > } > > - int ret = cameraStream->process(*src, buffer, descriptor); > + int ret = cameraStream->process(*src, buffer.buffer, descriptor); > > /* > * Return the FrameBuffer to the CameraStream now that we're > @@ -1186,8 +1185,8 @@ void CameraDevice::requestComplete(Request *request) > cameraStream->putBuffer(src); > > if (ret) { > - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > - notifyError(descriptor->frameNumber_, buffer.stream, > + buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > + notifyError(descriptor->frameNumber_, buffer.buffer.stream, > CAMERA3_MSG_ERROR_BUFFER); > } > } > @@ -1213,10 +1212,16 @@ void CameraDevice::sendCaptureResults() > camera3_capture_result_t captureResult = {}; > > captureResult.frame_number = descriptor->frameNumber_; > + > if (descriptor->resultMetadata_) > captureResult.result = descriptor->resultMetadata_->get(); > - captureResult.num_output_buffers = descriptor->buffers_.size(); > - captureResult.output_buffers = descriptor->buffers_.data(); > + > + std::vector<camera3_stream_buffer_t> resultBuffers; Can't you std::vector::reserve(descriptor->streams.size()) to avoid relocations ? I guess I'll find out more in next patches how this change will be used. Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > + for (const auto &buffer : descriptor->buffers_) > + resultBuffers.emplace_back(buffer.buffer); > + > + captureResult.num_output_buffers = resultBuffers.size(); > + captureResult.output_buffers = resultBuffers.data(); > > if (descriptor->status_ == Camera3RequestDescriptor::Status::Success) > captureResult.partial_result = 1; > diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp > index 16a632b3..614baed4 100644 > --- a/src/android/camera_request.cpp > +++ b/src/android/camera_request.cpp > @@ -23,15 +23,10 @@ Camera3RequestDescriptor::Camera3RequestDescriptor( > > /* Copy the camera3 request stream information for later access. */ > const uint32_t numBuffers = camera3Request->num_output_buffers; > + > buffers_.resize(numBuffers); > for (uint32_t i = 0; i < numBuffers; i++) > - buffers_[i] = camera3Request->output_buffers[i]; > - > - /* > - * FrameBuffer instances created by wrapping a camera3 provided dmabuf > - * are emplaced in this vector of unique_ptr<> for lifetime management. > - */ > - frameBuffers_.reserve(numBuffers); > + buffers_[i].buffer = camera3Request->output_buffers[i]; > > /* Clone the controls associated with the camera3 request. */ > settings_ = CameraMetadata(camera3Request->settings); > diff --git a/src/android/camera_request.h b/src/android/camera_request.h > index db13f624..a030febf 100644 > --- a/src/android/camera_request.h > +++ b/src/android/camera_request.h > @@ -29,6 +29,16 @@ public: > Error, > }; > > + struct StreamBuffer { > + camera3_stream_buffer_t buffer; > + /* > + * FrameBuffer instances created by wrapping a camera3 provided > + * dmabuf are emplaced in this vector of unique_ptr<> for > + * lifetime management. > + */ > + std::unique_ptr<libcamera::FrameBuffer> frameBuffer; > + }; > + > Camera3RequestDescriptor(libcamera::Camera *camera, > const camera3_capture_request_t *camera3Request); > ~Camera3RequestDescriptor(); > @@ -36,8 +46,9 @@ public: > bool isPending() const { return status_ == Status::Pending; } > > uint32_t frameNumber_ = 0; > - std::vector<camera3_stream_buffer_t> buffers_; > - std::vector<std::unique_ptr<libcamera::FrameBuffer>> frameBuffers_; > + > + std::vector<StreamBuffer> buffers_; > + > CameraMetadata settings_; > std::unique_ptr<CaptureRequest> request_; > std::unique_ptr<CameraMetadata> resultMetadata_; > -- > 2.31.0 >
Hi Umang, thank you for the patch. On Tue, Oct 19, 2021 at 1:21 AM Jacopo Mondi <jacopo@jmondi.org> wrote: > > Hi Umang, > > On Mon, Oct 18, 2021 at 06:59:17PM +0530, Umang Jain wrote: > > The Camera3RequestDescriptor structure stores, for each stream, the > > camera3_stream_buffer_t and the libcamera FrameBuffer in two separate > > vectors. This complicates buffer handling, as the code needs to keep > > both vectors in sync. Create a new structure to group all data about > > per-stream buffers to simplify this. > > > > As a side effect, we need to create a local vector of > > camera3_stream_buffer_t in CameraDevice::sendCaptureResults() as the > > camera3_stream_buffer_t instances stored in the new structure in > > Camera3RequestDescriptor are not contiguous anymore. This is a small > > price to pay for easier handling of buffers, and will be refactored in > > subsequent commits anyway. > > > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > > --- > > src/android/camera_device.cpp | 75 ++++++++++++++++++---------------- > > src/android/camera_request.cpp | 9 +--- > > src/android/camera_request.h | 15 ++++++- > > 3 files changed, 55 insertions(+), 44 deletions(-) > > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > > index 3bddb292..59557358 100644 > > --- a/src/android/camera_device.cpp > > +++ b/src/android/camera_device.cpp > > @@ -831,9 +831,9 @@ void CameraDevice::abortRequest(Camera3RequestDescriptor *descriptor) const > > notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_REQUEST); > > > > for (auto &buffer : descriptor->buffers_) { > > - buffer.release_fence = buffer.acquire_fence; > > - buffer.acquire_fence = -1; > > - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > > + buffer.buffer.release_fence = buffer.buffer.acquire_fence; > > + buffer.buffer.acquire_fence = -1; > > + buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > > } > > > > descriptor->status_ = Camera3RequestDescriptor::Status::Error; > > @@ -931,8 +931,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques > > LOG(HAL, Debug) << "Queueing request " << descriptor->request_->cookie() > > << " with " << descriptor->buffers_.size() << " streams"; > > > > - for (const auto &[i, camera3Buffer] : utils::enumerate(descriptor->buffers_)) { > > - camera3_stream *camera3Stream = camera3Buffer.stream; > > + for (const auto &[i, buffer] : utils::enumerate(descriptor->buffers_)) { > > + camera3_stream *camera3Stream = buffer.buffer.stream; > > CameraStream *cameraStream = static_cast<CameraStream *>(camera3Stream->priv); > > > > std::stringstream ss; > > @@ -949,7 +949,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques > > * while fences for streams of type Internal and Mapped are > > * handled at post-processing time. > > */ > > - FrameBuffer *buffer = nullptr; > > + FrameBuffer *frameBuffer = nullptr; > > int acquireFence = -1; > > switch (cameraStream->type()) { > > case CameraStream::Type::Mapped: > > @@ -967,13 +967,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques > > * associate it with the Camera3RequestDescriptor for > > * lifetime management only. > > */ > > - descriptor->frameBuffers_.push_back( > > - createFrameBuffer(*camera3Buffer.buffer, > > + buffer.frameBuffer = > > + createFrameBuffer(*buffer.buffer.buffer, > > cameraStream->configuration().pixelFormat, > > - cameraStream->configuration().size)); > > - > > - buffer = descriptor->frameBuffers_.back().get(); > > - acquireFence = camera3Buffer.acquire_fence; > > + cameraStream->configuration().size); > > + frameBuffer = buffer.frameBuffer.get(); > > + acquireFence = buffer.buffer.acquire_fence; > > LOG(HAL, Debug) << ss.str() << " (direct)"; > > break; > > > > @@ -985,18 +984,18 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques > > * The buffer has to be returned to the CameraStream > > * once it has been processed. > > */ > > - buffer = cameraStream->getBuffer(); > > + frameBuffer = cameraStream->getBuffer(); > > LOG(HAL, Debug) << ss.str() << " (internal)"; > > break; > > } > > > > - if (!buffer) { > > - LOG(HAL, Error) << "Failed to create buffer"; > > + if (!frameBuffer) { > > + LOG(HAL, Error) << "Failed to create frame buffer"; > > return -ENOMEM; > > } > > > > - descriptor->request_->addBuffer(cameraStream->stream(), buffer, > > - acquireFence); > > + descriptor->request_->addBuffer(cameraStream->stream(), > > + frameBuffer, acquireFence); > > } > > > > /* > > @@ -1082,9 +1081,9 @@ void CameraDevice::requestComplete(Request *request) > > * The buffer status is set to OK and later changed to ERROR if > > * post-processing/compression fails. > > */ > > - for (camera3_stream_buffer_t &buffer : descriptor->buffers_) { > > + for (auto &buffer : descriptor->buffers_) { > > CameraStream *cameraStream = > > - static_cast<CameraStream *>(buffer.stream->priv); > > + static_cast<CameraStream *>(buffer.buffer.stream->priv); > > > > /* > > * Streams of type Direct have been queued to the > > @@ -1098,9 +1097,9 @@ void CameraDevice::requestComplete(Request *request) > > * fence to -1 once it has handled it and remove this check. > > */ > > if (cameraStream->type() == CameraStream::Type::Direct) > > - buffer.acquire_fence = -1; > > - buffer.release_fence = -1; > > - buffer.status = CAMERA3_BUFFER_STATUS_OK; > > + buffer.buffer.acquire_fence = -1; > > + buffer.buffer.release_fence = -1; > > + buffer.buffer.status = CAMERA3_BUFFER_STATUS_OK; > > } > > > > /* > > @@ -1115,15 +1114,15 @@ void CameraDevice::requestComplete(Request *request) > > notifyError(descriptor->frameNumber_, nullptr, > > CAMERA3_MSG_ERROR_REQUEST); > > > > - for (camera3_stream_buffer_t &buffer : descriptor->buffers_) { > > + for (auto &buffer : descriptor->buffers_) { > > /* > > * Signal to the framework it has to handle fences that > > * have not been waited on by setting the release fence > > * to the acquire fence value. > > */ > > - buffer.release_fence = buffer.acquire_fence; > > - buffer.acquire_fence = -1; > > - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > > + buffer.buffer.release_fence = buffer.buffer.acquire_fence; > > + buffer.buffer.acquire_fence = -1; > > + buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > > } > > > > descriptor->status_ = Camera3RequestDescriptor::Status::Error; > > @@ -1160,9 +1159,9 @@ void CameraDevice::requestComplete(Request *request) > > } > > > > /* Handle post-processing. */ > > - for (camera3_stream_buffer_t &buffer : descriptor->buffers_) { > > + for (auto &buffer : descriptor->buffers_) { > > CameraStream *cameraStream = > > - static_cast<CameraStream *>(buffer.stream->priv); > > + static_cast<CameraStream *>(buffer.buffer.stream->priv); > > > > if (cameraStream->type() == CameraStream::Type::Direct) > > continue; > > @@ -1170,13 +1169,13 @@ void CameraDevice::requestComplete(Request *request) > > FrameBuffer *src = request->findBuffer(cameraStream->stream()); > > if (!src) { > > LOG(HAL, Error) << "Failed to find a source stream buffer"; > > - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > > - notifyError(descriptor->frameNumber_, buffer.stream, > > + buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > > + notifyError(descriptor->frameNumber_, buffer.buffer.stream, > > CAMERA3_MSG_ERROR_BUFFER); > > continue; > > } > > > > - int ret = cameraStream->process(*src, buffer, descriptor); > > + int ret = cameraStream->process(*src, buffer.buffer, descriptor); > > > > /* > > * Return the FrameBuffer to the CameraStream now that we're > > @@ -1186,8 +1185,8 @@ void CameraDevice::requestComplete(Request *request) > > cameraStream->putBuffer(src); > > > > if (ret) { > > - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > > - notifyError(descriptor->frameNumber_, buffer.stream, > > + buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; > > + notifyError(descriptor->frameNumber_, buffer.buffer.stream, > > CAMERA3_MSG_ERROR_BUFFER); > > } > > } > > @@ -1213,10 +1212,16 @@ void CameraDevice::sendCaptureResults() > > camera3_capture_result_t captureResult = {}; > > > > captureResult.frame_number = descriptor->frameNumber_; > > + > > if (descriptor->resultMetadata_) > > captureResult.result = descriptor->resultMetadata_->get(); > > - captureResult.num_output_buffers = descriptor->buffers_.size(); > > - captureResult.output_buffers = descriptor->buffers_.data(); > > + > > + std::vector<camera3_stream_buffer_t> resultBuffers; > > Can't you std::vector::reserve(descriptor->streams.size()) to avoid > relocations ? > > I guess I'll find out more in next patches how this change will be > used. > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > Thanks > j > > > + for (const auto &buffer : descriptor->buffers_) > > + resultBuffers.emplace_back(buffer.buffer); > > + > > + captureResult.num_output_buffers = resultBuffers.size(); > > + captureResult.output_buffers = resultBuffers.data(); > > > > if (descriptor->status_ == Camera3RequestDescriptor::Status::Success) > > captureResult.partial_result = 1; > > diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp > > index 16a632b3..614baed4 100644 > > --- a/src/android/camera_request.cpp > > +++ b/src/android/camera_request.cpp > > @@ -23,15 +23,10 @@ Camera3RequestDescriptor::Camera3RequestDescriptor( > > > > /* Copy the camera3 request stream information for later access. */ > > const uint32_t numBuffers = camera3Request->num_output_buffers; > > + > > buffers_.resize(numBuffers); > > for (uint32_t i = 0; i < numBuffers; i++) > > - buffers_[i] = camera3Request->output_buffers[i]; > > - > > - /* > > - * FrameBuffer instances created by wrapping a camera3 provided dmabuf > > - * are emplaced in this vector of unique_ptr<> for lifetime management. > > - */ > > - frameBuffers_.reserve(numBuffers); > > + buffers_[i].buffer = camera3Request->output_buffers[i]; > > > > /* Clone the controls associated with the camera3 request. */ > > settings_ = CameraMetadata(camera3Request->settings); > > diff --git a/src/android/camera_request.h b/src/android/camera_request.h > > index db13f624..a030febf 100644 > > --- a/src/android/camera_request.h > > +++ b/src/android/camera_request.h > > @@ -29,6 +29,16 @@ public: > > Error, > > }; > > > > + struct StreamBuffer { > > + camera3_stream_buffer_t buffer; > > + /* > > + * FrameBuffer instances created by wrapping a camera3 provided > > + * dmabuf are emplaced in this vector of unique_ptr<> for > > + * lifetime management. > > + */ > > + std::unique_ptr<libcamera::FrameBuffer> frameBuffer; > > + }; > > + > > Camera3RequestDescriptor(libcamera::Camera *camera, > > const camera3_capture_request_t *camera3Request); > > ~Camera3RequestDescriptor(); > > @@ -36,8 +46,9 @@ public: > > bool isPending() const { return status_ == Status::Pending; } > > > > uint32_t frameNumber_ = 0; > > - std::vector<camera3_stream_buffer_t> buffers_; > > - std::vector<std::unique_ptr<libcamera::FrameBuffer>> frameBuffers_; > > + > > + std::vector<StreamBuffer> buffers_; > > + > > CameraMetadata settings_; > > std::unique_ptr<CaptureRequest> request_; > > std::unique_ptr<CameraMetadata> resultMetadata_; > > -- > > 2.31.0 > >
Hi Jacopo, On 10/18/21 9:52 PM, Jacopo Mondi wrote: > Hi Umang, > > On Mon, Oct 18, 2021 at 06:59:17PM +0530, Umang Jain wrote: >> The Camera3RequestDescriptor structure stores, for each stream, the >> camera3_stream_buffer_t and the libcamera FrameBuffer in two separate >> vectors. This complicates buffer handling, as the code needs to keep >> both vectors in sync. Create a new structure to group all data about >> per-stream buffers to simplify this. >> >> As a side effect, we need to create a local vector of >> camera3_stream_buffer_t in CameraDevice::sendCaptureResults() as the >> camera3_stream_buffer_t instances stored in the new structure in >> Camera3RequestDescriptor are not contiguous anymore. This is a small >> price to pay for easier handling of buffers, and will be refactored in >> subsequent commits anyway. >> >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> --- >> src/android/camera_device.cpp | 75 ++++++++++++++++++---------------- >> src/android/camera_request.cpp | 9 +--- >> src/android/camera_request.h | 15 ++++++- >> 3 files changed, 55 insertions(+), 44 deletions(-) >> >> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp >> index 3bddb292..59557358 100644 >> --- a/src/android/camera_device.cpp >> +++ b/src/android/camera_device.cpp >> @@ -831,9 +831,9 @@ void CameraDevice::abortRequest(Camera3RequestDescriptor *descriptor) const >> notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_REQUEST); >> >> for (auto &buffer : descriptor->buffers_) { >> - buffer.release_fence = buffer.acquire_fence; >> - buffer.acquire_fence = -1; >> - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; >> + buffer.buffer.release_fence = buffer.buffer.acquire_fence; >> + buffer.buffer.acquire_fence = -1; >> + buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; >> } >> >> descriptor->status_ = Camera3RequestDescriptor::Status::Error; >> @@ -931,8 +931,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques >> LOG(HAL, Debug) << "Queueing request " << descriptor->request_->cookie() >> << " with " << descriptor->buffers_.size() << " streams"; >> >> - for (const auto &[i, camera3Buffer] : utils::enumerate(descriptor->buffers_)) { >> - camera3_stream *camera3Stream = camera3Buffer.stream; >> + for (const auto &[i, buffer] : utils::enumerate(descriptor->buffers_)) { >> + camera3_stream *camera3Stream = buffer.buffer.stream; >> CameraStream *cameraStream = static_cast<CameraStream *>(camera3Stream->priv); >> >> std::stringstream ss; >> @@ -949,7 +949,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques >> * while fences for streams of type Internal and Mapped are >> * handled at post-processing time. >> */ >> - FrameBuffer *buffer = nullptr; >> + FrameBuffer *frameBuffer = nullptr; >> int acquireFence = -1; >> switch (cameraStream->type()) { >> case CameraStream::Type::Mapped: >> @@ -967,13 +967,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques >> * associate it with the Camera3RequestDescriptor for >> * lifetime management only. >> */ >> - descriptor->frameBuffers_.push_back( >> - createFrameBuffer(*camera3Buffer.buffer, >> + buffer.frameBuffer = >> + createFrameBuffer(*buffer.buffer.buffer, >> cameraStream->configuration().pixelFormat, >> - cameraStream->configuration().size)); >> - >> - buffer = descriptor->frameBuffers_.back().get(); >> - acquireFence = camera3Buffer.acquire_fence; >> + cameraStream->configuration().size); >> + frameBuffer = buffer.frameBuffer.get(); >> + acquireFence = buffer.buffer.acquire_fence; >> LOG(HAL, Debug) << ss.str() << " (direct)"; >> break; >> >> @@ -985,18 +984,18 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques >> * The buffer has to be returned to the CameraStream >> * once it has been processed. >> */ >> - buffer = cameraStream->getBuffer(); >> + frameBuffer = cameraStream->getBuffer(); >> LOG(HAL, Debug) << ss.str() << " (internal)"; >> break; >> } >> >> - if (!buffer) { >> - LOG(HAL, Error) << "Failed to create buffer"; >> + if (!frameBuffer) { >> + LOG(HAL, Error) << "Failed to create frame buffer"; >> return -ENOMEM; >> } >> >> - descriptor->request_->addBuffer(cameraStream->stream(), buffer, >> - acquireFence); >> + descriptor->request_->addBuffer(cameraStream->stream(), >> + frameBuffer, acquireFence); >> } >> >> /* >> @@ -1082,9 +1081,9 @@ void CameraDevice::requestComplete(Request *request) >> * The buffer status is set to OK and later changed to ERROR if >> * post-processing/compression fails. >> */ >> - for (camera3_stream_buffer_t &buffer : descriptor->buffers_) { >> + for (auto &buffer : descriptor->buffers_) { >> CameraStream *cameraStream = >> - static_cast<CameraStream *>(buffer.stream->priv); >> + static_cast<CameraStream *>(buffer.buffer.stream->priv); >> >> /* >> * Streams of type Direct have been queued to the >> @@ -1098,9 +1097,9 @@ void CameraDevice::requestComplete(Request *request) >> * fence to -1 once it has handled it and remove this check. >> */ >> if (cameraStream->type() == CameraStream::Type::Direct) >> - buffer.acquire_fence = -1; >> - buffer.release_fence = -1; >> - buffer.status = CAMERA3_BUFFER_STATUS_OK; >> + buffer.buffer.acquire_fence = -1; >> + buffer.buffer.release_fence = -1; >> + buffer.buffer.status = CAMERA3_BUFFER_STATUS_OK; >> } >> >> /* >> @@ -1115,15 +1114,15 @@ void CameraDevice::requestComplete(Request *request) >> notifyError(descriptor->frameNumber_, nullptr, >> CAMERA3_MSG_ERROR_REQUEST); >> >> - for (camera3_stream_buffer_t &buffer : descriptor->buffers_) { >> + for (auto &buffer : descriptor->buffers_) { >> /* >> * Signal to the framework it has to handle fences that >> * have not been waited on by setting the release fence >> * to the acquire fence value. >> */ >> - buffer.release_fence = buffer.acquire_fence; >> - buffer.acquire_fence = -1; >> - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; >> + buffer.buffer.release_fence = buffer.buffer.acquire_fence; >> + buffer.buffer.acquire_fence = -1; >> + buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; >> } >> >> descriptor->status_ = Camera3RequestDescriptor::Status::Error; >> @@ -1160,9 +1159,9 @@ void CameraDevice::requestComplete(Request *request) >> } >> >> /* Handle post-processing. */ >> - for (camera3_stream_buffer_t &buffer : descriptor->buffers_) { >> + for (auto &buffer : descriptor->buffers_) { >> CameraStream *cameraStream = >> - static_cast<CameraStream *>(buffer.stream->priv); >> + static_cast<CameraStream *>(buffer.buffer.stream->priv); >> >> if (cameraStream->type() == CameraStream::Type::Direct) >> continue; >> @@ -1170,13 +1169,13 @@ void CameraDevice::requestComplete(Request *request) >> FrameBuffer *src = request->findBuffer(cameraStream->stream()); >> if (!src) { >> LOG(HAL, Error) << "Failed to find a source stream buffer"; >> - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; >> - notifyError(descriptor->frameNumber_, buffer.stream, >> + buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; >> + notifyError(descriptor->frameNumber_, buffer.buffer.stream, >> CAMERA3_MSG_ERROR_BUFFER); >> continue; >> } >> >> - int ret = cameraStream->process(*src, buffer, descriptor); >> + int ret = cameraStream->process(*src, buffer.buffer, descriptor); >> >> /* >> * Return the FrameBuffer to the CameraStream now that we're >> @@ -1186,8 +1185,8 @@ void CameraDevice::requestComplete(Request *request) >> cameraStream->putBuffer(src); >> >> if (ret) { >> - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; >> - notifyError(descriptor->frameNumber_, buffer.stream, >> + buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; >> + notifyError(descriptor->frameNumber_, buffer.buffer.stream, >> CAMERA3_MSG_ERROR_BUFFER); >> } >> } >> @@ -1213,10 +1212,16 @@ void CameraDevice::sendCaptureResults() >> camera3_capture_result_t captureResult = {}; >> >> captureResult.frame_number = descriptor->frameNumber_; >> + >> if (descriptor->resultMetadata_) >> captureResult.result = descriptor->resultMetadata_->get(); >> - captureResult.num_output_buffers = descriptor->buffers_.size(); >> - captureResult.output_buffers = descriptor->buffers_.data(); >> + >> + std::vector<camera3_stream_buffer_t> resultBuffers; > Can't you std::vector::reserve(descriptor->streams.size()) to avoid > relocations ? > > I guess I'll find out more in next patches how this change will be > used. If you haven't found already, .reserve() is used in 9/11 to avoid relocations. :-) > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > Thanks > j > >> + for (const auto &buffer : descriptor->buffers_) >> + resultBuffers.emplace_back(buffer.buffer); >> + >> + captureResult.num_output_buffers = resultBuffers.size(); >> + captureResult.output_buffers = resultBuffers.data(); >> >> if (descriptor->status_ == Camera3RequestDescriptor::Status::Success) >> captureResult.partial_result = 1; >> diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp >> index 16a632b3..614baed4 100644 >> --- a/src/android/camera_request.cpp >> +++ b/src/android/camera_request.cpp >> @@ -23,15 +23,10 @@ Camera3RequestDescriptor::Camera3RequestDescriptor( >> >> /* Copy the camera3 request stream information for later access. */ >> const uint32_t numBuffers = camera3Request->num_output_buffers; >> + >> buffers_.resize(numBuffers); >> for (uint32_t i = 0; i < numBuffers; i++) >> - buffers_[i] = camera3Request->output_buffers[i]; >> - >> - /* >> - * FrameBuffer instances created by wrapping a camera3 provided dmabuf >> - * are emplaced in this vector of unique_ptr<> for lifetime management. >> - */ >> - frameBuffers_.reserve(numBuffers); >> + buffers_[i].buffer = camera3Request->output_buffers[i]; >> >> /* Clone the controls associated with the camera3 request. */ >> settings_ = CameraMetadata(camera3Request->settings); >> diff --git a/src/android/camera_request.h b/src/android/camera_request.h >> index db13f624..a030febf 100644 >> --- a/src/android/camera_request.h >> +++ b/src/android/camera_request.h >> @@ -29,6 +29,16 @@ public: >> Error, >> }; >> >> + struct StreamBuffer { >> + camera3_stream_buffer_t buffer; >> + /* >> + * FrameBuffer instances created by wrapping a camera3 provided >> + * dmabuf are emplaced in this vector of unique_ptr<> for >> + * lifetime management. >> + */ >> + std::unique_ptr<libcamera::FrameBuffer> frameBuffer; >> + }; >> + >> Camera3RequestDescriptor(libcamera::Camera *camera, >> const camera3_capture_request_t *camera3Request); >> ~Camera3RequestDescriptor(); >> @@ -36,8 +46,9 @@ public: >> bool isPending() const { return status_ == Status::Pending; } >> >> uint32_t frameNumber_ = 0; >> - std::vector<camera3_stream_buffer_t> buffers_; >> - std::vector<std::unique_ptr<libcamera::FrameBuffer>> frameBuffers_; >> + >> + std::vector<StreamBuffer> buffers_; >> + >> CameraMetadata settings_; >> std::unique_ptr<CaptureRequest> request_; >> std::unique_ptr<CameraMetadata> resultMetadata_; >> -- >> 2.31.0 >>
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 3bddb292..59557358 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -831,9 +831,9 @@ void CameraDevice::abortRequest(Camera3RequestDescriptor *descriptor) const notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_REQUEST); for (auto &buffer : descriptor->buffers_) { - buffer.release_fence = buffer.acquire_fence; - buffer.acquire_fence = -1; - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; + buffer.buffer.release_fence = buffer.buffer.acquire_fence; + buffer.buffer.acquire_fence = -1; + buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; } descriptor->status_ = Camera3RequestDescriptor::Status::Error; @@ -931,8 +931,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques LOG(HAL, Debug) << "Queueing request " << descriptor->request_->cookie() << " with " << descriptor->buffers_.size() << " streams"; - for (const auto &[i, camera3Buffer] : utils::enumerate(descriptor->buffers_)) { - camera3_stream *camera3Stream = camera3Buffer.stream; + for (const auto &[i, buffer] : utils::enumerate(descriptor->buffers_)) { + camera3_stream *camera3Stream = buffer.buffer.stream; CameraStream *cameraStream = static_cast<CameraStream *>(camera3Stream->priv); std::stringstream ss; @@ -949,7 +949,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques * while fences for streams of type Internal and Mapped are * handled at post-processing time. */ - FrameBuffer *buffer = nullptr; + FrameBuffer *frameBuffer = nullptr; int acquireFence = -1; switch (cameraStream->type()) { case CameraStream::Type::Mapped: @@ -967,13 +967,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques * associate it with the Camera3RequestDescriptor for * lifetime management only. */ - descriptor->frameBuffers_.push_back( - createFrameBuffer(*camera3Buffer.buffer, + buffer.frameBuffer = + createFrameBuffer(*buffer.buffer.buffer, cameraStream->configuration().pixelFormat, - cameraStream->configuration().size)); - - buffer = descriptor->frameBuffers_.back().get(); - acquireFence = camera3Buffer.acquire_fence; + cameraStream->configuration().size); + frameBuffer = buffer.frameBuffer.get(); + acquireFence = buffer.buffer.acquire_fence; LOG(HAL, Debug) << ss.str() << " (direct)"; break; @@ -985,18 +984,18 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques * The buffer has to be returned to the CameraStream * once it has been processed. */ - buffer = cameraStream->getBuffer(); + frameBuffer = cameraStream->getBuffer(); LOG(HAL, Debug) << ss.str() << " (internal)"; break; } - if (!buffer) { - LOG(HAL, Error) << "Failed to create buffer"; + if (!frameBuffer) { + LOG(HAL, Error) << "Failed to create frame buffer"; return -ENOMEM; } - descriptor->request_->addBuffer(cameraStream->stream(), buffer, - acquireFence); + descriptor->request_->addBuffer(cameraStream->stream(), + frameBuffer, acquireFence); } /* @@ -1082,9 +1081,9 @@ void CameraDevice::requestComplete(Request *request) * The buffer status is set to OK and later changed to ERROR if * post-processing/compression fails. */ - for (camera3_stream_buffer_t &buffer : descriptor->buffers_) { + for (auto &buffer : descriptor->buffers_) { CameraStream *cameraStream = - static_cast<CameraStream *>(buffer.stream->priv); + static_cast<CameraStream *>(buffer.buffer.stream->priv); /* * Streams of type Direct have been queued to the @@ -1098,9 +1097,9 @@ void CameraDevice::requestComplete(Request *request) * fence to -1 once it has handled it and remove this check. */ if (cameraStream->type() == CameraStream::Type::Direct) - buffer.acquire_fence = -1; - buffer.release_fence = -1; - buffer.status = CAMERA3_BUFFER_STATUS_OK; + buffer.buffer.acquire_fence = -1; + buffer.buffer.release_fence = -1; + buffer.buffer.status = CAMERA3_BUFFER_STATUS_OK; } /* @@ -1115,15 +1114,15 @@ void CameraDevice::requestComplete(Request *request) notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_REQUEST); - for (camera3_stream_buffer_t &buffer : descriptor->buffers_) { + for (auto &buffer : descriptor->buffers_) { /* * Signal to the framework it has to handle fences that * have not been waited on by setting the release fence * to the acquire fence value. */ - buffer.release_fence = buffer.acquire_fence; - buffer.acquire_fence = -1; - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; + buffer.buffer.release_fence = buffer.buffer.acquire_fence; + buffer.buffer.acquire_fence = -1; + buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; } descriptor->status_ = Camera3RequestDescriptor::Status::Error; @@ -1160,9 +1159,9 @@ void CameraDevice::requestComplete(Request *request) } /* Handle post-processing. */ - for (camera3_stream_buffer_t &buffer : descriptor->buffers_) { + for (auto &buffer : descriptor->buffers_) { CameraStream *cameraStream = - static_cast<CameraStream *>(buffer.stream->priv); + static_cast<CameraStream *>(buffer.buffer.stream->priv); if (cameraStream->type() == CameraStream::Type::Direct) continue; @@ -1170,13 +1169,13 @@ void CameraDevice::requestComplete(Request *request) FrameBuffer *src = request->findBuffer(cameraStream->stream()); if (!src) { LOG(HAL, Error) << "Failed to find a source stream buffer"; - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; - notifyError(descriptor->frameNumber_, buffer.stream, + buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; + notifyError(descriptor->frameNumber_, buffer.buffer.stream, CAMERA3_MSG_ERROR_BUFFER); continue; } - int ret = cameraStream->process(*src, buffer, descriptor); + int ret = cameraStream->process(*src, buffer.buffer, descriptor); /* * Return the FrameBuffer to the CameraStream now that we're @@ -1186,8 +1185,8 @@ void CameraDevice::requestComplete(Request *request) cameraStream->putBuffer(src); if (ret) { - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; - notifyError(descriptor->frameNumber_, buffer.stream, + buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; + notifyError(descriptor->frameNumber_, buffer.buffer.stream, CAMERA3_MSG_ERROR_BUFFER); } } @@ -1213,10 +1212,16 @@ void CameraDevice::sendCaptureResults() camera3_capture_result_t captureResult = {}; captureResult.frame_number = descriptor->frameNumber_; + if (descriptor->resultMetadata_) captureResult.result = descriptor->resultMetadata_->get(); - captureResult.num_output_buffers = descriptor->buffers_.size(); - captureResult.output_buffers = descriptor->buffers_.data(); + + std::vector<camera3_stream_buffer_t> resultBuffers; + for (const auto &buffer : descriptor->buffers_) + resultBuffers.emplace_back(buffer.buffer); + + captureResult.num_output_buffers = resultBuffers.size(); + captureResult.output_buffers = resultBuffers.data(); if (descriptor->status_ == Camera3RequestDescriptor::Status::Success) captureResult.partial_result = 1; diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp index 16a632b3..614baed4 100644 --- a/src/android/camera_request.cpp +++ b/src/android/camera_request.cpp @@ -23,15 +23,10 @@ Camera3RequestDescriptor::Camera3RequestDescriptor( /* Copy the camera3 request stream information for later access. */ const uint32_t numBuffers = camera3Request->num_output_buffers; + buffers_.resize(numBuffers); for (uint32_t i = 0; i < numBuffers; i++) - buffers_[i] = camera3Request->output_buffers[i]; - - /* - * FrameBuffer instances created by wrapping a camera3 provided dmabuf - * are emplaced in this vector of unique_ptr<> for lifetime management. - */ - frameBuffers_.reserve(numBuffers); + buffers_[i].buffer = camera3Request->output_buffers[i]; /* Clone the controls associated with the camera3 request. */ settings_ = CameraMetadata(camera3Request->settings); diff --git a/src/android/camera_request.h b/src/android/camera_request.h index db13f624..a030febf 100644 --- a/src/android/camera_request.h +++ b/src/android/camera_request.h @@ -29,6 +29,16 @@ public: Error, }; + struct StreamBuffer { + camera3_stream_buffer_t buffer; + /* + * FrameBuffer instances created by wrapping a camera3 provided + * dmabuf are emplaced in this vector of unique_ptr<> for + * lifetime management. + */ + std::unique_ptr<libcamera::FrameBuffer> frameBuffer; + }; + Camera3RequestDescriptor(libcamera::Camera *camera, const camera3_capture_request_t *camera3Request); ~Camera3RequestDescriptor(); @@ -36,8 +46,9 @@ public: bool isPending() const { return status_ == Status::Pending; } uint32_t frameNumber_ = 0; - std::vector<camera3_stream_buffer_t> buffers_; - std::vector<std::unique_ptr<libcamera::FrameBuffer>> frameBuffers_; + + std::vector<StreamBuffer> buffers_; + CameraMetadata settings_; std::unique_ptr<CaptureRequest> request_; std::unique_ptr<CameraMetadata> resultMetadata_;