[libcamera-devel] [PATCH v5 01/10] libcamera: property: Add MinNumRequests property
Naushir Patuck
naush at raspberrypi.com
Thu Jul 8 15:19:07 CEST 2021
Hi Nícolas,
On Thu, 8 Jul 2021 at 13:44, Nícolas F. R. A. Prado <nfraprado at collabora.com>
wrote:
> Hi Naushir,
>
> On Thu, Jul 08, 2021 at 10:26:54AM +0100, Naushir Patuck wrote:
> > Hi Nicolas,
> >
> > Thank you for your patch.
> >
> > On Wed, 7 Jul 2021 at 15:42, Nícolas F. R. A. Prado <
> nfraprado at collabora.com>
> > wrote:
> >
> > > The MinNumRequests property reports the bare minimum number of requests
> > > needed in the camera pipeline for capture to be possible. It is equal
> to
> > > the number of buffers required by the driver. At this quantity, there's
> > > no guarantee that frames won't be dropped or that manual per-frame
> > > controls will apply correctly. The quantity needed for those may be
> > > added as separate properties in the future.
> > >
> > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado at collabora.com>
> > > ---
> > > src/libcamera/pipeline/ipu3/ipu3.cpp | 6 ++++++
> > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 6 ++++++
> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 ++
> > > src/libcamera/pipeline/simple/simple.cpp | 2 ++
> > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 ++
> > > src/libcamera/pipeline/vimc/vimc.cpp | 3 +++
> > > src/libcamera/property_ids.yaml | 9 +++++++++
> > > 7 files changed, 30 insertions(+)
> > >
> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > index 76c3bb3d8aa9..017018c845fa 100644
> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > @@ -1111,6 +1111,12 @@ int PipelineHandlerIPU3::registerCameras()
> > > /* Initialize the camera properties. */
> > > data->properties_ = cio2->sensor()->properties();
> > >
> > > + /*
> > > + * \todo Make sure this is correct even after the
> stream is
> > > + * configured to CIO2
> > > + */
> > > + data->properties_.set(properties::MinNumRequests, 1);
> > > +
> > > ret = initControls(data.get());
> > > if (ret)
> > > continue;
> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > index 082eb1ee1c23..f99a21de6918 100644
> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > @@ -1045,6 +1045,12 @@ bool PipelineHandlerRPi::match(DeviceEnumerator
> > > *enumerator)
> > > /* Initialize the camera properties. */
> > > data->properties_ = data->sensor_->properties();
> > >
> > > + /*
> > > + * \todo Make sure this is correct after the stream is
> configured
> > > for
> > > + * any of the roles
> > > + */
> > > + data->properties_.set(properties::MinNumRequests, 1);
> > > +
> > >
> >
> > Could you elaborate a bit on the above \todo, I'm not entirely sure it is
> > relevant for the
> > Raspberry Pi pipeline handler.
>
> Sure. I added it because I am worried about the usage of different values
> for
> bufferCount depending on the StreamRole used. I saw that you use 2 buffers
> for
> Raw, 1 for StillCapture, 4 for VideoRecording and Viewfinder, but I don't
> quite understand the reason for them.
>
I don't clearly know what to set bufferCount to. I assumed it is a
"reasonable guess"
as to what a user should request for a given use case.
>
> Given that I'm removing those bufferCount assignments in patch 10, and
> that the
> MinNumRequests is one global property for the pipeline handler, not
> specific to
> a single StreamRole, I just added that TODO so we could have this
> discussion
> eventually :).
>
> So, do you think 1 works fine independent of the StreamRole configuration?
>
Yes, 1 should work fine, regardless of the use case. So perhaps the todo
is not
needed here.
Regards,
Naush
>
> Thanks,
> Nícolas
>
> >
> > Thanks,
> > Naush
> >
> > /*
> > > * 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 00df4f0b3e6b..b5cbf2394b1c 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>
> > >
> > > @@ -944,6 +945,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity
> > > *sensor)
> > >
> > > /* Initialize the camera properties. */
> > > data->properties_ = data->sensor_->properties();
> > > + data->properties_.set(properties::MinNumRequests, 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 b29fff9820e5..c4adea61519f 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>
> > >
> > > @@ -436,6 +437,7 @@ int SimpleCameraData::init()
> > > }
> > >
> > > properties_ = sensor_->properties();
> > > + properties_.set(properties::MinNumRequests, 3);
> > >
> > > return 0;
> > > }
> > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > index 0f634b8da609..0258111ad6cf 100644
> > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > @@ -525,6 +525,8 @@ int UVCCameraData::init(MediaDevice *media)
> > > properties_.set(properties::PixelArraySize, resolution);
> > > properties_.set(properties::PixelArrayActiveAreas, {
> > > Rectangle(resolution) });
> > >
> > > + properties_.set(properties::MinNumRequests, 1);
> > > +
> > > /* Initialise the supported controls. */
> > > ControlInfoMap::Map ctrls;
> > >
> > > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp
> > > b/src/libcamera/pipeline/vimc/vimc.cpp
> > > index 12f7517fd0ae..8c3f7ccb46bd 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>
> > >
> > > @@ -517,6 +518,8 @@ int VimcCameraData::init()
> > > /* Initialize the camera properties. */
> > > properties_ = sensor_->properties();
> > >
> > > + properties_.set(properties::MinNumRequests, 2);
> > > +
> > > return 0;
> > > }
> > >
> > > diff --git a/src/libcamera/property_ids.yaml
> > > b/src/libcamera/property_ids.yaml
> > > index 12ecbce5eed4..4efa952ee393 100644
> > > --- a/src/libcamera/property_ids.yaml
> > > +++ b/src/libcamera/property_ids.yaml
> > > @@ -678,6 +678,15 @@ controls:
> > > \todo Turn this property into a "maximum control value" for
> the
> > > ScalerCrop control once "dynamic" controls have been
> implemented.
> > >
> > > + - MinNumRequests:
> > > + type: int32_t
> > > + description: |
> > > + The bare minimum number of requests needed in the camera
> pipeline
> > > for
> > > + capture to be possible, as required by the driver. Note that
> this
> > > + quantity does not guarantee that frames won't be dropped or
> that
> > > manual
> > > + per-frame controls will be applied properly.
> > > +
> > > +
> > > #
> > >
> ----------------------------------------------------------------------------
> > > # Draft properties section
> > >
> > > --
> > > 2.32.0
> > >
> > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210708/e650ee57/attachment-0001.htm>
More information about the libcamera-devel
mailing list