[libcamera-devel] [PATCH v8 01/17] libcamera: property: Add MinimumRequests property
Paul Elder
paul.elder at ideasonboard.com
Wed Nov 30 12:18:46 CET 2022
On Tue, Aug 24, 2021 at 04:56:20PM -0300, Nícolas F. R. A. Prado wrote:
> The MinimumRequests property reports the minimum number of capture
> requests that the camera pipeline requires to have queued in order to
> sustain capture operations without frame drops. At this quantity,
> there's no guarantee that manual per-frame controls will apply
> correctly. The quantity needed for that may be added as a separate
> property in the future.
>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado at collabora.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
>
> ---
>
> Changes in v8:
> - Changed the MinimumRequests property meaning to require that frames aren't
> dropped
> - Set MinimumRequests on SimplePipeline depending on device and usage of
> converter
> - Undid definition of default MinimumRequests on CameraSensor constructor
> - Updated pipeline-handler guides with new MinimumRequests property
>
> Changes in v7:
> - Renamed property from MinNumRequests to MinimumRequests
> - Changed MinimumRequests property's description
>
> Changes in v6:
> - Removed comment from Raspberrypi MinNumRequests setting
> - Removed an extra blank line
>
> Documentation/guides/pipeline-handler.rst | 22 ++++++++---
> src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +
> .../pipeline/raspberrypi/raspberrypi.cpp | 2 +
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +
> src/libcamera/pipeline/simple/simple.cpp | 38 +++++++++++++++++--
> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 +
> src/libcamera/pipeline/vimc/vimc.cpp | 2 +
> src/libcamera/property_ids.yaml | 21 ++++++++++
> 8 files changed, 83 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst
> index 54c8e7f1f553..32e3fc518d91 100644
> --- a/Documentation/guides/pipeline-handler.rst
> +++ b/Documentation/guides/pipeline-handler.rst
> @@ -658,19 +658,31 @@ associated with immutable values, which represent static characteristics that ca
> be used by applications to identify camera devices in the system. Properties can be
> registered by inspecting the values of V4L2 controls from the video devices and
> camera sensor (for example to retrieve the position and orientation of a camera)
> -or to express other immutable characteristics. The example pipeline handler does
> -not register any property, but examples are available in the libcamera code
> -base.
> +or to express other immutable characteristics.
>
> -.. TODO: Add a property example to the pipeline handler. At least the model.
> +A required property is ``MinimumRequests``, which indicates how many requests
> +need to be queued in the pipeline for capture without frame drops to be
> +possible.
> +
> +In our case, the vivid driver requires two buffers before it'll start streaming
> +(can be seen in the ``min_buffers_needed`` property for the ``vid_cap`` queue in
> +vivid's driver code). In order to not drop frames we should have one extra
> +buffer queued to the driver so that it is used as soon as the previous buffer
> +completes. Therefore we will set our ``MinimumRequests`` to three. Append the
> +following line to ``init()``:
> +
> +.. code-block:: cpp
> +
> + properties_.set(properties::MinimumRequests, 3);
>
> At this point you need to add the following includes to the top of the file for
> -handling controls:
> +handling controls and properties:
>
> .. code-block:: cpp
>
> #include <libcamera/controls.h>
> #include <libcamera/control_ids.h>
> + #include <libcamera/property_ids.h>
>
> Generating a default configuration
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index a98d7effc1bb..2309609093cc 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -1090,6 +1090,8 @@ int PipelineHandlerIPU3::registerCameras()
> /* Initialize the camera properties. */
> data->properties_ = cio2->sensor()->properties();
>
> + data->properties_.set(properties::MinimumRequests, 3);
> +
> ret = initControls(data.get());
> if (ret)
> continue;
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index b2674ac02109..d35b9714f0c3 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -1046,6 +1046,8 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
> /* Initialize the camera properties. */
> data->properties_ = data->sensor_->properties();
>
> + data->properties_.set(properties::MinimumRequests, 3);
> +
> /*
> * Set a default value for the ScalerCropMaximum property to show
> * that we support its use, however, initialise it to zero because
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 980088628523..cc663c983b3b 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -24,6 +24,7 @@
> #include <libcamera/ipa/core_ipa_interface.h>
> #include <libcamera/ipa/rkisp1_ipa_interface.h>
> #include <libcamera/ipa/rkisp1_ipa_proxy.h>
> +#include <libcamera/property_ids.h>
> #include <libcamera/request.h>
> #include <libcamera/stream.h>
>
> @@ -948,6 +949,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>
> /* Initialize the camera properties. */
> data->properties_ = data->sensor_->properties();
> + data->properties_.set(properties::MinimumRequests, 3);
>
> /*
> * \todo Read dealy values from the sensor itself or from a
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 8ff954722fed..26c59e601a76 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -25,6 +25,7 @@
>
> #include <libcamera/camera.h>
> #include <libcamera/control_ids.h>
> +#include <libcamera/property_ids.h>
> #include <libcamera/request.h>
> #include <libcamera/stream.h>
>
> @@ -130,6 +131,10 @@ class SimplePipelineHandler;
>
> struct SimplePipelineInfo {
> const char *driver;
> + /*
> + * Minimum number of buffers required by the driver to start streaming.
> + */
> + unsigned int minimumBuffers;
> /*
> * Each converter in the list contains the name
> * and the number of streams it supports.
> @@ -140,9 +145,9 @@ struct SimplePipelineInfo {
> namespace {
>
> static const SimplePipelineInfo supportedDevices[] = {
> - { "imx7-csi", { { "pxp", 1 } } },
> - { "qcom-camss", {} },
> - { "sun6i-csi", {} },
> + { "imx7-csi", 2, { { "pxp", 1 } } },
> + { "qcom-camss", 1, {} },
> + { "sun6i-csi", 3, {} },
> };
>
> } /* namespace */
> @@ -191,6 +196,8 @@ public:
> std::vector<std::unique_ptr<FrameBuffer>> converterBuffers_;
> bool useConverter_;
> std::queue<std::map<unsigned int, FrameBuffer *>> converterQueue_;
> +
> + const SimplePipelineInfo *deviceInfo;
> };
>
> class SimpleCameraConfiguration : public CameraConfiguration
> @@ -774,6 +781,29 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
> inputCfg.stride = captureFormat.planes[0].bpl;
> inputCfg.bufferCount = kNumInternalBuffers;
>
> + /* Set the MinimumRequests property. */
> + unsigned int minimumRequests;
> +
> + if (data->useConverter_) {
> + /*
> + * The application will interact only with the capture node of
> + * the converter. Require two buffers for a frame drop free
> + * conversion, plus one extra to account for requeue delays.
> + */
> + minimumRequests = 3;
> + } else {
> + /*
> + * The application will interact directly with the video capture
> + * device. Require the minimum required by the driver, plus one
> + * extra to account for requeue delays. Force at least three
> + * buffers in order to not drop frames.
> + */
> + minimumRequests = std::max(data->deviceInfo->minimumBuffers + 1,
> + 3U);
> + }
> +
> + data->properties_.set(properties::MinimumRequests, minimumRequests);
> +
> return converter_->configure(inputCfg, outputCfgs);
> }
>
> @@ -1040,6 +1070,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
> bool registered = false;
>
> for (std::unique_ptr<SimpleCameraData> &data : pipelines) {
> + data->deviceInfo = info;
> +
> int ret = data->init();
> if (ret < 0)
> continue;
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 973ecd5b835e..75d57300555c 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -526,6 +526,8 @@ int UVCCameraData::init(MediaDevice *media)
> properties_.set(properties::PixelArraySize, resolution);
> properties_.set(properties::PixelArrayActiveAreas, { Rectangle(resolution) });
>
> + properties_.set(properties::MinimumRequests, 3);
> +
> /* Initialise the supported controls. */
> ControlInfoMap::Map ctrls;
>
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index baeb6a7e6fa6..a0d8fd6c48c3 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -21,6 +21,7 @@
> #include <libcamera/control_ids.h>
> #include <libcamera/controls.h>
> #include <libcamera/formats.h>
> +#include <libcamera/property_ids.h>
> #include <libcamera/request.h>
> #include <libcamera/stream.h>
>
> @@ -560,6 +561,7 @@ int VimcCameraData::init()
>
> /* Initialize the camera properties. */
> properties_ = sensor_->properties();
> + properties_.set(properties::MinimumRequests, 3);
>
> return 0;
> }
> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> index 12ecbce5eed4..9df131265b9f 100644
> --- a/src/libcamera/property_ids.yaml
> +++ b/src/libcamera/property_ids.yaml
> @@ -678,6 +678,27 @@ controls:
> \todo Turn this property into a "maximum control value" for the
> ScalerCrop control once "dynamic" controls have been implemented.
>
> + - MinimumRequests:
> + type: int32_t
> + description: |
> + The minimum number of capture requests that the camera pipeline requires
> + to have queued in order to sustain capture operations without frame
> + drops. At this quantity, there's no guarantee that manual per-frame
> + controls will apply correctly.
> +
> + This property is based on the minimum number of memory buffers
> + needed to fill the capture pipeline composed of hardware processing
> + blocks plus as many buffers as needed to take into account propagation
> + delays and avoid dropping frames.
> +
> + \todo Should this be a per-stream property?
> +
> + \todo Extend this property to expose the different minimum buffer and
> + request counts (the minimum number of buffers to be able to capture at
> + all, the minimum number of buffers to sustain capture without frame
> + drop, and the minimum number of requests to ensure proper operation of
> + per-frame controls).
> +
> # ----------------------------------------------------------------------------
> # Draft properties section
>
> --
> 2.33.0
>
More information about the libcamera-devel
mailing list