[RFC,v1,00/54] libcamera: Split requests and buffers

27079 mbox series

This is the first RFC changeset for splitting requests and buffers. It largely
includes https://patchwork.libcamera.org/cover/26964/, but many of those are
already reviewed.

The "interesting" changes start with "test: fence: Disable temporarily".

---

# introduction

The basic idea is that requests no longer have buffers when they are submitted,
and the camera keeps track of a set of buffers for each stream, and buffers will
be attached to requests from this pool in an unspecified order

A new function, `Camera::addBuffer(stream, buffer, fence)` is added to fill the pool;
this can be called in the "Running" state. `Camera::stop()` empties the pool via the
`bufferCompleted` event. Another new function, `Request::enableStream(stream, bool)`
is used to set which streams a request wants frames from.

Pipeline handlers can use `Camera::Private::acquireBuffer(stream)` to retrieve a
buffer for the given stream. If this buffer is not acceptable, then
`Camera::Private::rejectBuffer(buffer)` can be used to reject it. Otherwise
`completeBuffer(request, buffer)` is used normally to attach it to a request.

This new behaviour is opt-in for pipeline handlers for now, with no opt-in,
the `PipelineHandler` base class fills each request with buffers before
calling `queueRequestDevice()`.


# discussion points

## adding buffers before `Camera::start()`

Currently buffers can only be added in the "Running" state. They could
be added in "Configured" as well, but there is no "unconfigure" function,
and putting it into `stop()` would be a bit asymmetric.

## empty pool notification

Currently there is no mechanism for the application to know how many
buffers it should keep in the pool. I think a signal to notify the application
when it should add more buffers for a given stream could be added. But the
main idea was to expose some kind of minimum buffer count quantity.

## requirements towards application wrt. buffers

With this prototype, an application is essentially required to always process *all*
buffers in *all* completed requests, otherwise it might lose buffers. (This
applies to cancelled requests as well.) Is that reasonable?

## returning unused buffers

Currently this is done from `Camera::stop()` with the `bufferCompleted` signal
where `request == nullptr`. This works, but may not be the best interface.

## returning rejected buffers

Pipeline handlers can reject buffers. But how should that be signalled to the
application. Currently this is also done via `bufferCompleted` with `request == nullptr`.
But this is not ideal because there isn't really a way to distinguish it from
an unused buffer since both are returned with `FrameMetadata::Status::FrameCancelled`
because `FrameError` requires some other fields to be valid.

## extending `completeBuffer()` with stream parameter

Initially I planned to add a stream parameter to `completeBuffer()`, but most pipeline handlers
are not trivially convertible, especially something like `imx8-isi`. So I simply went ahead
with storing the associated stream in the `FrameBuffer` itself for now. I still think having
`completeBuffer(req, stream, buf)` is worthwhile, but it is not a "natural" change at the moment.

## fence timeout

Previously a hard-coded timeout was used for fences. This is now removed.
Should there be a hard-coded fixed timeout for fences?


# TODO

# converting more pipeline handlers

Only the uvcvideo pipeline handler is converted to use the buffer pool
directly as an experiment, but more should be converted to really see
if this public api is a good fit for pipeline handlers. Especially something
like `imx8-isi` since that has a bit different architecture from the others.

# minimum buffer count

The applications need to know how many buffers should be kept in the pool
for a given stream for continuous streaming. This should be a property or
similar.

# `FrameBuffer::Private::request()`

After all pipeline handlers are migrated, the request member of `FrameBuffer`
should probably be removed to fully decouple the two.

---

Barnabás Pőcze (54):
  apps: cam: Simplify buffer reuse
  libcamera: request: Disassociate buffer when cancelling
  libcamera: pipeline: Replace open-coded request cancellation
  libcamera: pipeline: mali-c55: Remove `setRequest()` calls
  libcamera: pipeline: virtual: Make copy of request's buffer map
  libcamera: request: completeBuffer(): Emit `bufferCompleted` here
  libcamera: pipeline_handler: completeBuffer(): Inline and `static`
  v4l2: v4l2_camera: Avoid a level of indirection
  v4l2: v4l2_camera: Remove repated index checks
  v4l2: v4l2_camera_proxy: Remove `bufferCount_`
  v4l2: v4l2_camera: Use actually allocated buffer count
  v4l2: v4l2_camera: Use buffer cookie for indexing
  v4l2: v4l2_camera: Rename `Buffer` to `CompletedBuffer`
  v4l2: v4l2_camera: Always clear pending requests
  v4l2: v4l2_camera: Clear completed requests when stopping
  v4l2: v4l2_camera: Provide buffers one by one
  test: fence: Disable temporarily
  libcamera: request: Remove `ReuseBuffers`
  libcamera: framebuffer: request(): Move to private type
  libcamera: framebuffer: Store associated Stream
  libcamera: camera: Add `StreamData`
  libcamera: camera: Add buffer pool
  libcamera: request: addBuffer(): Remove impl
  libcamera: request: Remove fence support
  libcamera: request: Store count of pending buffers
  libcamera: request: doCancelRequest(): Remove
  libcamera: pipeline_handler: Move constructor options to a separate
    type
  libcamera: pipeline_handler: Acquire buffers if not using pool
  libcamera: request: enableStream(): Add
  libcamera: camera: acquireBuffer(): Add
  libcamera: camera: rejectBuffer(): Add
  libcamera: pipeline_handler: buffersAddedDevice(): New virtual
    function
  libcamera: pipeline_handler: Use `std::deque`
  libcamera: pipeline_handler: completeRequest(): Return request count
  libcamera: pipeline: uvcvideo: Use buffer pool prototype
  libcamera: request: completeBuffer(): Emit `bufferCompleted` last
  libcamera: camera: bufferCompleted: Pass `Stream` as well
  libcamera: camera: queueRequest(): Adjust buffer map empty error
    message
  libcamera: camera: queueBuffer(): Reject if it has buffers
  apps: cam: Use camera buffer pool
  app: lc-compliance: Use camera buffer pool
  apps: lc-compliance: Add buffer pool tests
  test: Use camera buffer pool
  apps: qcam: Use camera buffer pool
  v4l2: Use camera buffer pool
  py: Use camera buffer pool
  gstreamer: Use camera buffer pool
  libcamera: request: Remove `ReuseFlag`
  android: Update imported files
  android: camera_request: Add helper for buffer conversion
  android: camera_device: Move fence restoration into
    `prepareToReturn()`
  android: Use camera buffer pool
  libcamera: request: Remove `addBuffer()`
  test: fence: Enable

 .../guides/application-developer.rst          |    9 -
 Documentation/guides/pipeline-handler.rst     |   10 +-
 .../libhardware/include/hardware/camera3.h    |  360 +-
 .../include/hardware/camera_common.h          |  304 +-
 .../libhardware/include/hardware/gralloc.h    |   99 +-
 .../android/metadata/camera_metadata_hidden.h |   11 +-
 .../android/metadata/system/camera_metadata.h |   41 +-
 .../metadata/system/camera_metadata_tags.h    |  633 +-
 .../metadata/system/camera_vendor_tags.h      |   10 +-
 .../android/system/core/include/android/log.h |  450 +-
 .../core/include/cutils/native_handle.h       |   62 +-
 .../system/core/include/system/camera.h       |    4 +-
 .../core/include/system/graphics-base-v1.2.h  |   36 +
 .../core/include/system/graphics-base.h       |    1 +
 .../system/core/include/system/graphics.h     |   10 +-
 include/libcamera/camera.h                    |    5 +-
 include/libcamera/framebuffer.h               |    1 -
 include/libcamera/internal/camera.h           |   46 +-
 include/libcamera/internal/framebuffer.h      |    6 +
 include/libcamera/internal/pipeline_handler.h |   31 +-
 include/libcamera/internal/request.h          |   24 +-
 include/libcamera/request.h                   |   10 +-
 src/android/camera3_hal.cpp                   |    3 +
 src/android/camera_capabilities.cpp           |    3 +
 src/android/camera_device.cpp                 |  287 +-
 src/android/camera_device.h                   |    4 +
 src/android/camera_hal_manager.cpp            |    2 +-
 src/android/camera_ops.cpp                    |   16 +-
 src/android/camera_request.cpp                |   53 +-
 src/android/camera_request.h                  |   11 +-
 src/android/metadata/camera_metadata.c        |  183 +-
 .../metadata/camera_metadata_tag_info.c       | 5478 ++++++++++++++++-
 src/apps/cam/camera_session.cpp               |   36 +-
 src/apps/cam/camera_session.h                 |    2 +-
 src/apps/lc-compliance/helpers/capture.cpp    |   23 +-
 src/apps/lc-compliance/tests/capture_test.cpp |  261 +
 src/apps/qcam/main_window.cpp                 |   82 +-
 src/apps/qcam/main_window.h                   |    1 -
 src/gstreamer/gstlibcamerasrc.cpp             |  136 +-
 src/libcamera/camera.cpp                      |  164 +-
 src/libcamera/fence.cpp                       |   15 +-
 src/libcamera/framebuffer.cpp                 |   57 +-
 src/libcamera/pipeline/imx8-isi/imx8-isi.cpp  |    5 +-
 src/libcamera/pipeline/ipu3/ipu3.cpp          |   16 +-
 src/libcamera/pipeline/mali-c55/mali-c55.cpp  |    7 +-
 src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp |    3 +-
 src/libcamera/pipeline/mali-c55/rzg2l-cru.h   |    3 +-
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      |    7 +-
 .../pipeline/rpi/common/pipeline_base.cpp     |   16 +-
 .../pipeline/rpi/common/pipeline_base.h       |    2 +-
 src/libcamera/pipeline/simple/simple.cpp      |   17 +-
 src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |   72 +-
 src/libcamera/pipeline/vimc/vimc.cpp          |   11 +-
 src/libcamera/pipeline/virtual/virtual.cpp    |   44 +-
 src/libcamera/pipeline_handler.cpp            |  260 +-
 src/libcamera/request.cpp                     |  290 +-
 src/py/cam/cam.py                             |   20 +-
 src/py/examples/simple-cam.py                 |   13 +-
 src/py/examples/simple-capture.py             |   20 +-
 src/py/examples/simple-continuous-capture.py  |   32 +-
 src/py/libcamera/py_main.cpp                  |   65 +-
 src/v4l2/v4l2_camera.cpp                      |  150 +-
 src/v4l2/v4l2_camera.h                        |   25 +-
 src/v4l2/v4l2_camera_proxy.cpp                |  118 +-
 src/v4l2/v4l2_camera_proxy.h                  |    3 -
 test/camera/buffer_import.cpp                 |   22 +-
 test/camera/camera_reconfigure.cpp            |   22 +-
 test/camera/capture.cpp                       |   24 +-
 test/camera/statemachine.cpp                  |    4 +-
 test/fence.cpp                                |  214 +-
 70 files changed, 9182 insertions(+), 1283 deletions(-)
 create mode 100644 include/android/system/core/include/system/graphics-base-v1.2.h

--
2.54.0