| Message ID | 20230203094424.25243-4-naush@raspberrypi.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
On Fri, Feb 03, 2023 at 09:44:19AM +0000, Naushir Patuck via libcamera-devel wrote: > Look for MandatoryStream flag in the hints field of the ISP Output0 > StreamConfiguration structure. If this flag is set, it guarantees that > the application will provide buffers for the ISP, do not allocate any > internal buffers for the device. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > .../pipeline/raspberrypi/raspberrypi.cpp | 29 ++++++++++++++----- > 1 file changed, 22 insertions(+), 7 deletions(-) > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index 7d786fe839f7..b6ed66a1cf46 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -1522,7 +1522,7 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera) > RPiCameraData *data = cameraData(camera); > unsigned int minUnicamBuffers = data->config_.minUnicamBuffers; > unsigned int minTotalUnicamBuffers = data->config_.minTotalUnicamBuffers; > - unsigned int numRawBuffers = 0; > + unsigned int numRawBuffers = 0, minIspBuffers = 1; nit: I guess we prefer one variable per line (not sure how much we can enforce it though) > int ret; > > for (Stream *s : camera->streams()) { > @@ -1546,8 +1546,21 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera) > minTotalUnicamBuffers = 0; > } > } > - > - break; > + } else if (s == &data->isp_[Isp::Output0]) { > + /* > + * Since the ISP runs synchronous with the IPA and requests, > + * we only ever need a maximum of one internal buffer. Any > + * buffers the application wants to hold onto will already > + * be exported through PipelineHandlerRPi::exportFrameBuffers(). > + * > + * However, as above, if the application provides a guarantee > + * that the buffer will always be provided for the ISP Output0 > + * stream in a Request, we don't need any internal buffers > + * allocated. > + */ > + if (!data->dropFrameCount_ && > + s->configuration().hints & StreamConfiguration::Hint::MandatoryStream) > + minIspBuffers = 0; > } > } > > @@ -1583,12 +1596,14 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera) > * so allocate the minimum required to avoid frame drops. > */ > numBuffers = data->config_.minTotalUnicamBuffers; > + } else if (stream == &data->isp_[Isp::Output0]) { > + /* Buffer count for this is handled in the earlier loop above. */ > + numBuffers = minIspBuffers; > } else { > /* > - * Since the ISP runs synchronous with the IPA and requests, > - * we only ever need one set of internal buffers. Any buffers > - * the application wants to hold onto will already be exported > - * through PipelineHandlerRPi::exportFrameBuffers(). > + * Same reasoning as for ISP Output 0, we only ever need Isn't it different ? I got that Ouput1 is not a concern and we can unconditionally allocate a buffer due to its smaller resolution ? > + * a maximum of one internal buffer for Output1 (required > + * for colour denoise) and ISP statistics. > */ > numBuffers = 1; > } > -- > 2.25.1 >
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 7d786fe839f7..b6ed66a1cf46 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1522,7 +1522,7 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera) RPiCameraData *data = cameraData(camera); unsigned int minUnicamBuffers = data->config_.minUnicamBuffers; unsigned int minTotalUnicamBuffers = data->config_.minTotalUnicamBuffers; - unsigned int numRawBuffers = 0; + unsigned int numRawBuffers = 0, minIspBuffers = 1; int ret; for (Stream *s : camera->streams()) { @@ -1546,8 +1546,21 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera) minTotalUnicamBuffers = 0; } } - - break; + } else if (s == &data->isp_[Isp::Output0]) { + /* + * Since the ISP runs synchronous with the IPA and requests, + * we only ever need a maximum of one internal buffer. Any + * buffers the application wants to hold onto will already + * be exported through PipelineHandlerRPi::exportFrameBuffers(). + * + * However, as above, if the application provides a guarantee + * that the buffer will always be provided for the ISP Output0 + * stream in a Request, we don't need any internal buffers + * allocated. + */ + if (!data->dropFrameCount_ && + s->configuration().hints & StreamConfiguration::Hint::MandatoryStream) + minIspBuffers = 0; } } @@ -1583,12 +1596,14 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera) * so allocate the minimum required to avoid frame drops. */ numBuffers = data->config_.minTotalUnicamBuffers; + } else if (stream == &data->isp_[Isp::Output0]) { + /* Buffer count for this is handled in the earlier loop above. */ + numBuffers = minIspBuffers; } else { /* - * Since the ISP runs synchronous with the IPA and requests, - * we only ever need one set of internal buffers. Any buffers - * the application wants to hold onto will already be exported - * through PipelineHandlerRPi::exportFrameBuffers(). + * Same reasoning as for ISP Output 0, we only ever need + * a maximum of one internal buffer for Output1 (required + * for colour denoise) and ISP statistics. */ numBuffers = 1; }