[libcamera-devel,v8,01/17] libcamera: property: Add MinimumRequests property

13464 diff mbox series

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@collabora.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@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(-)

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@collabora.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@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
>

Hi Nicolas,

Thank you for your patch.

On Tue, 24 Aug 2021 at 20:57, Nícolas F. R. A. Prado <
nfraprado@collabora.com> 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@collabora.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@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);
> +
>

I'm not sure about the number 3 in all use cases... see below...


>         /*
>          * 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).
>

I have slight concerns over the definition wording here.  There is no
practical number
that can guarantee no frame drops.  Should we perhaps reword this to say
minimise
frame drops instead?

Having the bufferCount as part of the StreamConfiguration object did
provide one big
advantage over the MinimumRequests property - you can specify different
numbers for
different StreamRoles.  So for example viewfinder could use a different
number to a
high resolution stills capture use case.  With a static property this is
not the case any more.
This might be sub-optimal for many scenarios.  Not sure what the solution
would be for this.

Regards,
Naush



> +
>    #
> ----------------------------------------------------------------------------
>    # Draft properties section
>
> --
> 2.33.0
>
>