[libcamera-devel] [PATCH v2 3/5] android: camera_device: Postpone mapped streams handling
Umang Jain
umang.jain at ideasonboard.com
Mon May 30 15:41:20 CEST 2022
Hi Jacopo,
On 5/30/22 11:56, Jacopo Mondi via libcamera-devel wrote:
> Hi Paul
>
> On Fri, May 27, 2022 at 06:34:38PM +0900, Paul Elder wrote:
>> From: Jacopo Mondi <jacopo at jmondi.org>
>>
>> Mapped streams are generated by post-processing and always require a
>> source buffer from where to process image data from.
>>
>> In case a Mapped stream is requested but its source stream is not, it
>> is required to allocate a buffer on the fly and add it to the
>> libcamera::Request.
>>
>> Make sure a source stream is available for all mapped streams, and if
>> that's not the case, add a dedicated buffer to the request for that
>> purpose.
>>
>> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
>> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>>
>> ---
>> Changes in v2:
>> - cosmetic changes in code
>> - fix typo in the commit message
>> ---
>> src/android/camera_device.cpp | 80 +++++++++++++++++++++++++++++++----
>> 1 file changed, 72 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
>> index 20599665..95c14f60 100644
>> --- a/src/android/camera_device.cpp
>> +++ b/src/android/camera_device.cpp
>> @@ -9,6 +9,7 @@
>>
>> #include <algorithm>
>> #include <fstream>
>> +#include <set>
>> #include <sys/mman.h>
>> #include <unistd.h>
>> #include <vector>
>> @@ -923,6 +924,32 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>> LOG(HAL, Debug) << "Queueing request " << descriptor->request_->cookie()
>> << " with " << descriptor->buffers_.size() << " streams";
>>
>> + /*
>> + * Collect the CameraStream associated to each requested capture stream.
>> + * Since requestedStreams is an std:set<>, no duplications can happen.
> std::set<>
>
>> + */
>> + std::set<CameraStream *> requestedStreams;
>> + for (const auto &[i, buffer] : utils::enumerate(descriptor->buffers_)) {
>> + CameraStream *cameraStream = buffer.stream;
>> +
>> + switch (cameraStream->type()) {
>> + case CameraStream::Type::Mapped:
>> + requestedStreams.insert(cameraStream->sourceStream());
Can sourceStream field for Mapped streams be nullptr here? Should we
ensure it via an ASSERT?
>> + break;
>> +
>> + case CameraStream::Type::Direct:
>> + case CameraStream::Type::Internal:
>> + requestedStreams.insert(cameraStream);
>> + break;
>> + }
>> + }
>> +
>> + /*
>> + * Process all the Direct and Internal streams, for which the CameraStream
>> + * they refer to is the one that points to the right libcamera::Stream.
>> + *
>> + * Streams of type Mapped will be handled later.
>> + */
>> for (const auto &[i, buffer] : utils::enumerate(descriptor->buffers_)) {
>> CameraStream *cameraStream = buffer.stream;
>> camera3_stream_t *camera3Stream = cameraStream->camera3Stream();
>> @@ -945,14 +972,6 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>>
>> switch (cameraStream->type()) {
>> case CameraStream::Type::Mapped:
>> - /*
>> - * Mapped streams don't need buffers added to the
>> - * Request.
>> - */
>> - LOG(HAL, Debug) << ss.str() << " (mapped)";
>> -
>> - descriptor->pendingStreamsToProcess_.insert(
>> - { cameraStream, &buffer });
>> continue;
>>
>> case CameraStream::Type::Direct:
>> @@ -996,6 +1015,51 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>> auto fence = std::make_unique<Fence>(std::move(acquireFence));
>> descriptor->request_->addBuffer(cameraStream->stream(),
>> frameBuffer, std::move(fence));
>> +
>> + requestedStreams.erase(cameraStream);
>> + }
>> +
>> + /*
>> + * Now handle the Mapped streams. If no buffer has been added for them
>> + * as their corresponding direct source stream has not been requested,
>> + * add it here.
I am wondering a situation where a direct stream D, and a mapped stream
M, is requested and M is mapped onto D so,
M->sourceStream = D;
Provided the requestedStreams is a std::set<> where no duplications can
happen, and given the above scenario:
I see the requestedStreams will consist of { D } while populating the
set in the switch-case above, which then gets
erased from the requestedStreams (above the comment block) so now, the
requestedStreams become an empty set { } here . . .
>> + */
>> + for (const auto &[i, buffer] : utils::enumerate(descriptor->buffers_)) {
>> + CameraStream *cameraStream = buffer.stream;
>> + camera3_stream_t *camera3Stream = cameraStream->camera3Stream();
>> +
>> + if (cameraStream->type() != CameraStream::Type::Mapped)
>> + continue;
>> +
>> + LOG(HAL, Debug) << i << " - (" << camera3Stream->width << "x"
>> + << camera3Stream->height << ")"
>> + << "[" << utils::hex(camera3Stream->format) << "] -> "
>> + << "(" << cameraStream->configuration().size.toString() << ")["
>> + << cameraStream->configuration().pixelFormat.toString() << "]"
>> + << " (mapped)";
>> +
>> + MutexLocker lock(descriptor->streamsProcessMutex_);
>> + descriptor->pendingStreamsToProcess_.insert({ cameraStream, &buffer });
>> +
>> + /*
>> + * Make sure the CameraStream this stream is mapped on has been
>> + * added to the request.
>> + */
>> + CameraStream *sourceStream = cameraStream->sourceStream();
>> + if (requestedStreams.find(sourceStream) == requestedStreams.end())
and then while handling mapped streams, it will try to find { D } in
requestedStreams (which is now empty)
>> + continue;
>> +
>> + /*
>> + * If that's not the case, we need to add a buffer to the request
>> + * for this stream.
>> + */
>> + FrameBuffer *frameBuffer = cameraStream->getBuffer();
>> + buffer.internalBuffer = frameBuffer;
>> +
>> + descriptor->request_->addBuffer(sourceStream->stream(),
>> + frameBuffer, nullptr);
... and add a internal buffer for D
so we have 2 buffers for D in the end ?
>> +
>> + requestedStreams.erase(sourceStream);
>> }
> We could possibily make sure here that now requestedStreams is empty,
> but that's just an additional safety check.
>
> The patch still looks ok to me.
>
>> /*
>> --
>> 2.30.2
>>
More information about the libcamera-devel
mailing list