[libcamera-devel,v3,00/10] libcamera: ipu3: Allow zero-copy RAW stream
mbox series

Message ID 20200623020930.1781469-1-niklas.soderlund@ragnatech.se
Headers show
Series
  • libcamera: ipu3: Allow zero-copy RAW stream
Related show

Message

Niklas Söderlund June 23, 2020, 2:09 a.m. UTC
Hi,

This series removes the need to copy the buffer when capturing RAW
buffers from the IPU3 pipeline. This is made possible by allocating an
internal queue of buffers in the pipeline handler which is used when the
application does not provide an buffer for the RAW stream.

There is on issue with this series. If the camera is configured to
supply the application with more then one stream and one of them is a
RAW stream. Then the sequence number of the RAW buffers in all requests
are set to 0 before the request completes. This is due to that if two or
more streams are used the RAW buffer is always queued to the ImgU input
to serve as input to the vf and/or output buffers. When the RAW buffers
is dequeued from the ImgU input device the kernel sets the sequence
number to zero. This should either be solved in the kernel or by 
reworking how sequence numbers handled going out of libcamera, does it 
make sens to have seq numbers on the buffer level ?

As the RPi pipeline handler already started using the copy approach we
can not yet rename to role nor remove the copyFrom() helper. I aim to
work on that once the approach taken in this series is agreed upon.

Niklas Söderlund (10):
  libcamera: camera_sensor: Make mbusCodes() return a set
  libcamera: ipu3: Fold mediaBusToFormat() into only caller
  libcamera: ipu3: Breakout stream assignment to new function
  libcamera: ipu3: Calculate number of buffers for ImgU
  libcamera: ipu3: cio2: Move the CIO2Device class to separate files
  libcamera: ipu3: cio2: Consolidate information about formats
  libcamera: ipu3: cio2: Add function to generate configuration
  libcamera: ipu3: cio2: Make the V4L2 devices private
  libcamera: ipu3: cio2: Hide buffer allocation and freeing from users
  libcamera: ipu3: Allow zero-copy RAW stream capture

 include/libcamera/internal/camera_sensor.h |   4 +-
 include/libcamera/internal/formats.h       |   2 +-
 src/libcamera/camera_sensor.cpp            |   1 -
 src/libcamera/formats.cpp                  |   7 +-
 src/libcamera/pipeline/ipu3/cio2.cpp       | 298 ++++++++++++
 src/libcamera/pipeline/ipu3/cio2.h         |  66 +++
 src/libcamera/pipeline/ipu3/ipu3.cpp       | 497 +++++----------------
 src/libcamera/pipeline/ipu3/meson.build    |   1 +
 test/camera-sensor.cpp                     |   2 +-
 9 files changed, 475 insertions(+), 403 deletions(-)
 create mode 100644 src/libcamera/pipeline/ipu3/cio2.cpp
 create mode 100644 src/libcamera/pipeline/ipu3/cio2.h

Comments

Laurent Pinchart June 25, 2020, 1:40 a.m. UTC | #1
Hi Niklas,

On Tue, Jun 23, 2020 at 04:09:20AM +0200, Niklas Söderlund wrote:
> Hi,
> 
> This series removes the need to copy the buffer when capturing RAW
> buffers from the IPU3 pipeline. This is made possible by allocating an
> internal queue of buffers in the pipeline handler which is used when the
> application does not provide an buffer for the RAW stream.
> 
> There is on issue with this series. If the camera is configured to
> supply the application with more then one stream and one of them is a
> RAW stream. Then the sequence number of the RAW buffers in all requests
> are set to 0 before the request completes. This is due to that if two or
> more streams are used the RAW buffer is always queued to the ImgU input
> to serve as input to the vf and/or output buffers. When the RAW buffers
> is dequeued from the ImgU input device the kernel sets the sequence
> number to zero. This should either be solved in the kernel or by 
> reworking how sequence numbers handled going out of libcamera, does it 
> make sens to have seq numbers on the buffer level ?
> 
> As the RPi pipeline handler already started using the copy approach we
> can not yet rename to role nor remove the copyFrom() helper. I aim to
> work on that once the approach taken in this series is agreed upon.

I think the approach is good. It will be a good occasion to add support
in the core for what CIO2Device::tryReturnBuffer() does in private :-)

> Niklas Söderlund (10):
>   libcamera: camera_sensor: Make mbusCodes() return a set
>   libcamera: ipu3: Fold mediaBusToFormat() into only caller
>   libcamera: ipu3: Breakout stream assignment to new function
>   libcamera: ipu3: Calculate number of buffers for ImgU
>   libcamera: ipu3: cio2: Move the CIO2Device class to separate files
>   libcamera: ipu3: cio2: Consolidate information about formats
>   libcamera: ipu3: cio2: Add function to generate configuration
>   libcamera: ipu3: cio2: Make the V4L2 devices private
>   libcamera: ipu3: cio2: Hide buffer allocation and freeing from users
>   libcamera: ipu3: Allow zero-copy RAW stream capture
> 
>  include/libcamera/internal/camera_sensor.h |   4 +-
>  include/libcamera/internal/formats.h       |   2 +-
>  src/libcamera/camera_sensor.cpp            |   1 -
>  src/libcamera/formats.cpp                  |   7 +-
>  src/libcamera/pipeline/ipu3/cio2.cpp       | 298 ++++++++++++
>  src/libcamera/pipeline/ipu3/cio2.h         |  66 +++
>  src/libcamera/pipeline/ipu3/ipu3.cpp       | 497 +++++----------------
>  src/libcamera/pipeline/ipu3/meson.build    |   1 +
>  test/camera-sensor.cpp                     |   2 +-
>  9 files changed, 475 insertions(+), 403 deletions(-)
>  create mode 100644 src/libcamera/pipeline/ipu3/cio2.cpp
>  create mode 100644 src/libcamera/pipeline/ipu3/cio2.h