[PATCH v4 2/7] android: Set StreamBuffer::srcBuffer in CameraDevice::processCaptureRequest

[Cheng-Hao Yang]

Hi Jacopo,

On Wed, Dec 11, 2024 at 5:30 PM Jacopo Mondi
<jacopo.mondi at ideasonboard.com> wrote:
>
> Hi Harvey
>
> On Tue, Dec 10, 2024 at 02:23:55PM +0000, Harvey Yang wrote:
> > StreamBuffer::srcBuffer was set right before being processed, while it
> > was already determined when being constructed. This patch sets the value
> > in CameraDevice::processCaptureRequest. `requestedStreams` is refactored
> > from std::set to std::map to find the correct srcBuffer accordingly.
> >
> > The change also helps cleaning up the post process pipeline's dependency
> > on `Camera3RequestDescriptor::pendingStreamsToProcess_`, which is going
> > to be cleaned up in the following patches.
>
> Does this still apply ?

Ah I should say `series`. Updated.

>
> >
> > Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> > Co-developed-by: Harvey Yang <chenghaoyang at chromium.org>
> > Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> > ---
> >  src/android/camera_device.cpp | 28 +++++++++++++++++++---------
> >  1 file changed, 19 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 4e3bdc9cc..f6dadaf22 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -9,7 +9,6 @@
> >
> >  #include <algorithm>
> >  #include <fstream>
> > -#include <set>
> >  #include <sys/mman.h>
> >  #include <unistd.h>
> >  #include <vector>
> > @@ -967,9 +966,9 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> >        * to a libcamera stream. Streams of type Mapped will be handled later.
> >        *
> >        * Collect the CameraStream associated to each requested capture stream.
> > -      * Since requestedStreams is an std:set<>, no duplications can happen.
> > +      * Since requestedStreams is an std:map<>, no duplications can happen.
> >        */
> > -     std::set<CameraStream *> requestedStreams;
> > +     std::map<CameraStream *, libcamera::FrameBuffer *> requestedBuffers;
> >       for (const auto &[i, buffer] : utils::enumerate(descriptor->buffers_)) {
> >               CameraStream *cameraStream = buffer.stream;
> >               camera3_stream_t *camera3Stream = cameraStream->camera3Stream();
> > @@ -1021,6 +1020,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> >                        */
> >                       frameBuffer = cameraStream->getBuffer();
> >                       buffer.internalBuffer = frameBuffer;
> > +                     buffer.srcBuffer = frameBuffer;
> > +
> >                       LOG(HAL, Debug) << ss.str() << " (internal)";
> >
> >                       descriptor->pendingStreamsToProcess_.insert(
> > @@ -1037,7 +1038,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> >               descriptor->request_->addBuffer(cameraStream->stream(),
> >                                               frameBuffer, std::move(fence));
> >
> > -             requestedStreams.insert(cameraStream);
> > +             requestedBuffers[cameraStream] = frameBuffer;
> >       }
> >
> >       /*
> > @@ -1067,9 +1068,19 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> >                * added to the request.
> >                */
> >               CameraStream *sourceStream = cameraStream->sourceStream();
> > -             ASSERT(sourceStream);
> > -             if (requestedStreams.find(sourceStream) != requestedStreams.end())
> > +             ASSERT(sourceStream && sourceStream->type() == CameraStream::Type::Direct);
> > +
> > +             /*
> > +              * If the buffer for the source stream has been requested as
> > +              * Direct, use its framebuffer as the source buffer for
> > +              * post-processing. No need to recycle the buffer since it's
> > +              * owned by Android.
> > +              */
> > +             auto iterDirectBuffer = requestedBuffers.find(sourceStream);
> > +             if (iterDirectBuffer != requestedBuffers.end()) {
> > +                     buffer.srcBuffer = iterDirectBuffer->second;
> >                       continue;
> > +             }
> >
> >               /*
> >                * If that's not the case, we need to add a buffer to the request
> > @@ -1077,11 +1088,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> >                */
> >               FrameBuffer *frameBuffer = cameraStream->getBuffer();
> >               buffer.internalBuffer = frameBuffer;
> > +             buffer.srcBuffer = frameBuffer;
> >
> >               descriptor->request_->addBuffer(sourceStream->stream(),
> >                                               frameBuffer, nullptr);
> >
> > -             requestedStreams.insert(sourceStream);
> > +             requestedBuffers[sourceStream] = frameBuffer;
> >       }
> >
> >       /*
> > @@ -1236,8 +1248,6 @@ void CameraDevice::requestComplete(Request *request)
> >                       continue;
> >               }
> >
> > -             buffer->srcBuffer = src;
> > -
>
> In this new version of the series I see that setting srcBuffer earlire
> allows to validate the buffer error status in 5/7, so

Yes, this also helps with that :)

BR,
Harvey

>
> Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
>
> Thanks
>   j
>
> >               ++iter;
> >               int ret = stream->process(buffer);
> >               if (ret) {
> > --
> > 2.47.0.338.g60cca15819-goog
> >