[libcamera-devel] [PATCH v5 00/10] Zero-copy RAW stream work

[Niklas Söderlund]

Hi Naushir,

On 2020-08-01 11:32:19 +0100, Naushir Patuck wrote:
> Hi Niklas,
> 
> 
> On Sat, 1 Aug 2020, 10:22 am Niklas Söderlund, <
> niklas.soderlund at ragnatech.se> wrote:
> 
> > Hi Naushir,
> >
> > On 2020-07-30 09:39:38 +0100, Naushir Patuck wrote:
> > > Hi all,
> > >
> > > Is there any further changes anyone would like me to make for this
> > > series?  I know Laurent had comments on what I was trying to achieve
> > > in patches 09/10 and 10/10 - I have made some minor changes with
> > > variable names and comments to hopefully explain the code better.  If
> > > there are other clarifications to be made, please let me know and I
> > > will be happy to do so.
> >
> > On one hand I think this series improves the pipeline but at the same
> > time it's hard to know if the direction will solve the zero-copy problem
> > as that patch have been dropped from the series. If you and others are
> > pushing for this series to be integrated I would be fine doing so, but
> > if possible it would be nice to see a version with the zero-copy raw
> > feature added back.
> >
> 
> All the patches in the series will enable zero-copy RAW (or any other
> stream really) captures in the raspberry pi platform. I've been testing
> this with cam, qcam, and our in-house libcamera application.

You are right, what version of the series did I look at? I thought the 
copyFrom() was still present in the pipeline. My bad, sorry about that.

> 
> The only feature missing is that applications cannot use buffers that have
> not been allocated or exported by the v4l2 videodevice (e.g. a random
> dmabuf), but none of the above applications do that right now as far as I
> can tell. I have a patch series waiting to add support for this and will
> submit for review once this is gone through. Hope that is ok?
> 

This is unfortunately a problem as it introduces a regression. What's in 
master work's with external buffers, but just in a zero-copy fashion.  
The Android HAL component uses external buffers which libcamera can't 
control so I think removing the ability to work with external buffers 
does more harm then good.

Would it be possible to merge the two series? Or in some other way break 
out the cleanup in one and the entablement of zero-copy in another?

> Regards,
> Naush
> 
> 
> 
> > >
> > > Regards,
> > > Naush
> > >
> > > On Fri, 24 Jul 2020 at 08:22, Naushir Patuck <naush at raspberrypi.com>
> > wrote:
> > > >
> > > > Hi,
> > > >
> > > > This is patchset v5 of the zero copy RAW stream work for the Raspberry
> > Pi platform.
> > > > All minor changes in the review feedback have been addressed.  The
> > only other difference
> > > > is in patch 9/10 where I have renamed requeueBuffers_ to
> > requestBuffers_ and made minor
> > > > changes to the comments to hopefully make the usage slightly more
> > understandable.  I
> > > > have left the review tags in place, as there is no functional change,
> > hope that is ok.
> > > >
> > > > Regards,
> > > > Naush
> > > >
> > > > Naushir Patuck (10):
> > > >   libcamera: pipeline: raspberrypi: Move RPiStream into a separate file
> > > >   libcamera: pipeline: ipa: raspberrypi: Rework drop frame signalling
> > > >   libcamera: request: Add log point on a completed request
> > > >   libcamera: pipeline: raspberrypi: Add some debug logging
> > > >   libcamera: pipeline: raspberrypi: Increase the number of RAW buffers
> > > >   libcamera: pipeline: raspberrypi: Remove const qualifier from
> > > >     RPiStream
> > > >   libcamera: pipeline: raspberrypi: Rework stream buffer logic for
> > > >     zero-copy
> > > >   libcamera: pipeline: raspberrypi: Fix bug in passing configuration to
> > > >     IPA
> > > >   libcamera: pipeline: raspberrypi: Add more robust stream buffer logic
> > > >   libcamera: pipeline: ipa: raspberrypi: Remove use of FrameBuffer
> > > >     cookie
> > > >
> > > >  include/libcamera/ipa/raspberrypi.h           |   2 +-
> > > >  src/ipa/raspberrypi/raspberrypi.cpp           |  20 +-
> > > >  .../pipeline/raspberrypi/meson.build          |   1 +
> > > >  .../pipeline/raspberrypi/raspberrypi.cpp      | 522 +++++++-----------
> > > >  .../pipeline/raspberrypi/rpi_stream.cpp       | 216 ++++++++
> > > >  .../pipeline/raspberrypi/rpi_stream.h         | 131 +++++
> > > >  src/libcamera/request.cpp                     |   3 +
> > > >  7 files changed, 563 insertions(+), 332 deletions(-)
> > > >  create mode 100644 src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> > > >  create mode 100644 src/libcamera/pipeline/raspberrypi/rpi_stream.h
> > > >
> > > > --
> > > > 2.25.1
> > > >
> > > _______________________________________________
> > > libcamera-devel mailing list
> > > libcamera-devel at lists.libcamera.org
> > > https://lists.libcamera.org/listinfo/libcamera-devel
> >
> > --
> > Regards,
> > Niklas Söderlund
> >

-- 
Regards,
Niklas Söderlund