<div dir="ltr"><div dir="ltr">Hi Nícolas,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 8 Jul 2021 at 13:44, Nícolas F. R. A. Prado <<a href="mailto:nfraprado@collabora.com">nfraprado@collabora.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naushir,<br>
<br>
On Thu, Jul 08, 2021 at 10:26:54AM +0100, Naushir Patuck wrote:<br>
> Hi Nicolas,<br>
> <br>
> Thank you for your patch.<br>
> <br>
> On Wed, 7 Jul 2021 at 15:42, Nícolas F. R. A. Prado <<a href="mailto:nfraprado@collabora.com" target="_blank">nfraprado@collabora.com</a>><br>
> wrote:<br>
> <br>
> > The MinNumRequests property reports the bare minimum number of requests<br>
> > needed in the camera pipeline for capture to be possible. It is equal to<br>
> > the number of buffers required by the driver. At this quantity, there's<br>
> > no guarantee that frames won't be dropped or that manual per-frame<br>
> > controls will apply correctly. The quantity needed for those may be<br>
> > added as separate properties in the future.<br>
> ><br>
> > Signed-off-by: Nícolas F. R. A. Prado <<a href="mailto:nfraprado@collabora.com" target="_blank">nfraprado@collabora.com</a>><br>
> > ---<br>
> > src/libcamera/pipeline/ipu3/ipu3.cpp | 6 ++++++<br>
> > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 6 ++++++<br>
> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 ++<br>
> > src/libcamera/pipeline/simple/simple.cpp | 2 ++<br>
> > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 ++<br>
> > src/libcamera/pipeline/vimc/vimc.cpp | 3 +++<br>
> > src/libcamera/property_ids.yaml | 9 +++++++++<br>
> > 7 files changed, 30 insertions(+)<br>
> ><br>
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp<br>
> > b/src/libcamera/pipeline/ipu3/ipu3.cpp<br>
> > index 76c3bb3d8aa9..017018c845fa 100644<br>
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp<br>
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp<br>
> > @@ -1111,6 +1111,12 @@ int PipelineHandlerIPU3::registerCameras()<br>
> > /* Initialize the camera properties. */<br>
> > data->properties_ = cio2->sensor()->properties();<br>
> ><br>
> > + /*<br>
> > + * \todo Make sure this is correct even after the stream is<br>
> > + * configured to CIO2<br>
> > + */<br>
> > + data->properties_.set(properties::MinNumRequests, 1);<br>
> > +<br>
> > ret = initControls(data.get());<br>
> > if (ret)<br>
> > continue;<br>
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > index 082eb1ee1c23..f99a21de6918 100644<br>
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > @@ -1045,6 +1045,12 @@ bool PipelineHandlerRPi::match(DeviceEnumerator<br>
> > *enumerator)<br>
> > /* Initialize the camera properties. */<br>
> > data->properties_ = data->sensor_->properties();<br>
> ><br>
> > + /*<br>
> > + * \todo Make sure this is correct after the stream is configured<br>
> > for<br>
> > + * any of the roles<br>
> > + */<br>
> > + data->properties_.set(properties::MinNumRequests, 1);<br>
> > +<br>
> ><br>
> <br>
> Could you elaborate a bit on the above \todo, I'm not entirely sure it is<br>
> relevant for the<br>
> Raspberry Pi pipeline handler.<br>
<br>
Sure. I added it because I am worried about the usage of different values for<br>
bufferCount depending on the StreamRole used. I saw that you use 2 buffers for<br>
Raw, 1 for StillCapture, 4 for VideoRecording and Viewfinder, but I don't<br>
quite understand the reason for them.<br></blockquote><div><br></div><div>I don't clearly know what to set bufferCount to. I assumed it is a "reasonable guess"</div><div>as to what a user should request for a given use case.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Given that I'm removing those bufferCount assignments in patch 10, and that the<br>
MinNumRequests is one global property for the pipeline handler, not specific to<br>
a single StreamRole, I just added that TODO so we could have this discussion<br>
eventually :).<br>
<br>
So, do you think 1 works fine independent of the StreamRole configuration?<br></blockquote><div><br></div><div>Yes, 1 should work fine, regardless of the use case. So perhaps the todo is not</div><div>needed here.</div><div><br></div><div>Regards,</div><div>Naush</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Thanks,<br>
Nícolas<br>
<br>
> <br>
> Thanks,<br>
> Naush<br>
> <br>
> /*<br>
> > * Set a default value for the ScalerCropMaximum property to show<br>
> > * that we support its use, however, initialise it to zero because<br>
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp<br>
> > b/src/libcamera/pipeline/rkisp1/rkisp1.cpp<br>
> > index 00df4f0b3e6b..b5cbf2394b1c 100644<br>
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp<br>
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp<br>
> > @@ -24,6 +24,7 @@<br>
> > #include <libcamera/ipa/core_ipa_interface.h><br>
> > #include <libcamera/ipa/rkisp1_ipa_interface.h><br>
> > #include <libcamera/ipa/rkisp1_ipa_proxy.h><br>
> > +#include <libcamera/property_ids.h><br>
> > #include <libcamera/request.h><br>
> > #include <libcamera/stream.h><br>
> ><br>
> > @@ -944,6 +945,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity<br>
> > *sensor)<br>
> ><br>
> > /* Initialize the camera properties. */<br>
> > data->properties_ = data->sensor_->properties();<br>
> > + data->properties_.set(properties::MinNumRequests, 3);<br>
> ><br>
> > /*<br>
> > * \todo Read dealy values from the sensor itself or from a<br>
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp<br>
> > b/src/libcamera/pipeline/simple/simple.cpp<br>
> > index b29fff9820e5..c4adea61519f 100644<br>
> > --- a/src/libcamera/pipeline/simple/simple.cpp<br>
> > +++ b/src/libcamera/pipeline/simple/simple.cpp<br>
> > @@ -25,6 +25,7 @@<br>
> ><br>
> > #include <libcamera/camera.h><br>
> > #include <libcamera/control_ids.h><br>
> > +#include <libcamera/property_ids.h><br>
> > #include <libcamera/request.h><br>
> > #include <libcamera/stream.h><br>
> ><br>
> > @@ -436,6 +437,7 @@ int SimpleCameraData::init()<br>
> > }<br>
> ><br>
> > properties_ = sensor_->properties();<br>
> > + properties_.set(properties::MinNumRequests, 3);<br>
> ><br>
> > return 0;<br>
> > }<br>
> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp<br>
> > b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp<br>
> > index 0f634b8da609..0258111ad6cf 100644<br>
> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp<br>
> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp<br>
> > @@ -525,6 +525,8 @@ int UVCCameraData::init(MediaDevice *media)<br>
> > properties_.set(properties::PixelArraySize, resolution);<br>
> > properties_.set(properties::PixelArrayActiveAreas, {<br>
> > Rectangle(resolution) });<br>
> ><br>
> > + properties_.set(properties::MinNumRequests, 1);<br>
> > +<br>
> > /* Initialise the supported controls. */<br>
> > ControlInfoMap::Map ctrls;<br>
> ><br>
> > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp<br>
> > b/src/libcamera/pipeline/vimc/vimc.cpp<br>
> > index 12f7517fd0ae..8c3f7ccb46bd 100644<br>
> > --- a/src/libcamera/pipeline/vimc/vimc.cpp<br>
> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp<br>
> > @@ -21,6 +21,7 @@<br>
> > #include <libcamera/control_ids.h><br>
> > #include <libcamera/controls.h><br>
> > #include <libcamera/formats.h><br>
> > +#include <libcamera/property_ids.h><br>
> > #include <libcamera/request.h><br>
> > #include <libcamera/stream.h><br>
> ><br>
> > @@ -517,6 +518,8 @@ int VimcCameraData::init()<br>
> > /* Initialize the camera properties. */<br>
> > properties_ = sensor_->properties();<br>
> ><br>
> > + properties_.set(properties::MinNumRequests, 2);<br>
> > +<br>
> > return 0;<br>
> > }<br>
> ><br>
> > diff --git a/src/libcamera/property_ids.yaml<br>
> > b/src/libcamera/property_ids.yaml<br>
> > index 12ecbce5eed4..4efa952ee393 100644<br>
> > --- a/src/libcamera/property_ids.yaml<br>
> > +++ b/src/libcamera/property_ids.yaml<br>
> > @@ -678,6 +678,15 @@ controls:<br>
> > \todo Turn this property into a "maximum control value" for the<br>
> > ScalerCrop control once "dynamic" controls have been implemented.<br>
> ><br>
> > + - MinNumRequests:<br>
> > + type: int32_t<br>
> > + description: |<br>
> > + The bare minimum number of requests needed in the camera pipeline<br>
> > for<br>
> > + capture to be possible, as required by the driver. Note that this<br>
> > + quantity does not guarantee that frames won't be dropped or that<br>
> > manual<br>
> > + per-frame controls will be applied properly.<br>
> > +<br>
> > +<br>
> > #<br>
> > ----------------------------------------------------------------------------<br>
> > # Draft properties section<br>
> ><br>
> > --<br>
> > 2.32.0<br>
> ><br>
> ><br>
</blockquote></div></div>