[libcamera-devel] [PATCH v1 5/5] cam: Move request processing to main thread

[Laurent Pinchart]

Hi Marvin,

On Fri, Nov 13, 2020 at 02:31:09PM +0100, Marvin Schmidt wrote:
> Am Fr., 13. Nov. 2020 um 13:45 Uhr schrieb Laurent Pinchart:
> > On Fri, Nov 13, 2020 at 01:35:40PM +0100, Marvin Schmidt wrote:
> > > Hey Laurent,
> > >
> > > Thanks for your work
> > >
> > > Am Fr., 13. Nov. 2020 um 07:38 Uhr schrieb Laurent Pinchart:
> > > >
> > > > The request completion handler is invoked in the camera manager thread,
> > > > which shouldn't be blocked for large amounts of time. As writing the
> > > > frames to disk can be a time-consuming process, move request processing
> > > > to the main thread by queueing an event to the event loop.
> > > >
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > > ---
> > > >  src/cam/capture.cpp | 9 +++++++++
> > > >  src/cam/capture.h   | 1 +
> > > >  2 files changed, 10 insertions(+)
> > > >
> > > > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> > > > index 7580f798288c..43b109d099f6 100644
> > > > --- a/src/cam/capture.cpp
> > > > +++ b/src/cam/capture.cpp
> > > > @@ -157,6 +157,15 @@ void Capture::requestComplete(Request *request)
> > > >         if (request->status() == Request::RequestCancelled)
> > > >                 return;
> > > >
> > > > +       /*
> > > > +        * Defer processing of the completed request to the event loop, to avoid
> > > > +        * blocking the camera manager thread.
> > > > +        */
> > > > +       loop_->callLater(std::bind(&Capture::processRequest, this, request));
> > >
> > > You could use a lambda here instead of std::bind:
> > >
> > >     [=] { processRequest(request); }
> > >
> > > std::bind would otherwise require including `<functional>`
> >
> > <functional> is included by event_loop.h, as the callLater() function
> > takes an std::function parameter. But a lambda would certainly work. I
> > wonder, between std::bind and the proposed lambda function, what would
> > be more efficient ?
> 
> Since lambdas are a language feature I would imagine that they can be
> better optimized by compilers. Other people seem to agree with that and
> found that lambdas are faster (at least in their use-case / benchmark):
> https://stackoverflow.com/questions/24852764/stdbind-vs-lambda-performance
> 
> Also the existence of clang-tidy's `modernize-avoid-bind` check[1] makes me
> think that lambdas should be used where possible :-)
> 
> [1] https://clang.llvm.org/extra/clang-tidy/checks/modernize-avoid-bind.html

Thanks for the interesting information. I'll switch to using a lambda.

> > > > +}
> > > > +
> > > > +void Capture::processRequest(Request *request)
> > > > +{
> > > >         const Request::BufferMap &buffers = request->buffers();
> > > >
> > > >         /*
> > > > diff --git a/src/cam/capture.h b/src/cam/capture.h
> > > > index 45e5e8a9ba27..d21c95a26ce7 100644
> > > > --- a/src/cam/capture.h
> > > > +++ b/src/cam/capture.h
> > > > @@ -33,6 +33,7 @@ private:
> > > >         int capture(libcamera::FrameBufferAllocator *allocator);
> > > >
> > > >         void requestComplete(libcamera::Request *request);
> > > > +       void processRequest(libcamera::Request *request);
> > > >
> > > >         std::shared_ptr<libcamera::Camera> camera_;
> > > >         libcamera::CameraConfiguration *config_;

-- 
Regards,

Laurent Pinchart