[libcamera-devel,v2] pipeline: rpi: vc4: Allocate more embedded data buffers
diff mbox series

Message ID 20231018125241.17838-1-william.vinnicombe@raspberrypi.com
State Accepted
Headers show
Series
  • [libcamera-devel,v2] pipeline: rpi: vc4: Allocate more embedded data buffers
Related show

Commit Message

William Vinnicombe Oct. 18, 2023, 12:52 p.m. UTC 
If the pipeline runs out of embedded data buffers, then it will pass
the frame to the IPA without the metadata. The IPA then has to use the
delayed controls as inputs to the algorithms. This can cause problems
with the subsequent algorithms if the sensor did not action the
controls, especially with the autofocus as that doesn't have controls
which can be passed in lieu of the metadata.

Reduce the likelihood of this by increasing the number of embedded data
buffers, as they are small so a generous number can be allocated.

Signed-off-by: William Vinnicombe <william.vinnicombe@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
---
 src/libcamera/pipeline/rpi/vc4/vc4.cpp | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

Comments

Kieran Bingham Oct. 18, 2023, 1:16 p.m. UTC | #1 
Quoting William Vinnicombe via libcamera-devel (2023-10-18 13:52:41)
> If the pipeline runs out of embedded data buffers, then it will pass
> the frame to the IPA without the metadata. The IPA then has to use the
> delayed controls as inputs to the algorithms. This can cause problems
> with the subsequent algorithms if the sensor did not action the
> controls, especially with the autofocus as that doesn't have controls
> which can be passed in lieu of the metadata.
> 
> Reduce the likelihood of this by increasing the number of embedded data
> buffers, as they are small so a generous number can be allocated.
> 
> Signed-off-by: William Vinnicombe <william.vinnicombe@raspberrypi.com>
> Reviewed-by: Naushir Patuck <naush@raspberrypi.com>


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Applying...


> ---
>  src/libcamera/pipeline/rpi/vc4/vc4.cpp | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> index 425ab9ae..d7279d02 100644
> --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> @@ -258,10 +258,21 @@ int PipelineHandlerVc4::prepareBuffers(Camera *camera)
>  
>                 } else if (stream == &data->unicam_[Unicam::Embedded]) {
>                         /*
> -                        * Embedded data buffers are (currently) for internal use,
> -                        * so allocate the minimum required to avoid frame drops.
> +                        * Embedded data buffers are (currently) for internal use, and
> +                        * are small enough (typically 1-2KB) that we can
> +                        * allocate them generously to avoid causing problems in the
> +                        * IPA when we cannot supply the metadata.
> +                        * 
> +                        * 12 are allocated as a typical application will have 8-10
> +                        * input buffers, so allocating more embedded buffers than that
> +                        * is a sensible choice.
> +                        * 
> +                        * The lifetimes of these buffers are smaller than those of the
> +                        * raw buffers, so allocating a fixed number will still suffice
> +                        * if the application requests a greater number of raw
> +                        * buffers, as these will be recycled quicker.
>                          */
> -                       numBuffers = minBuffers;
> +                       numBuffers = 12;
>                 } else {
>                         /*
>                          * Since the ISP runs synchronous with the IPA and requests,
> -- 
> 2.39.2
>

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
index 425ab9ae..d7279d02 100644
--- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp
+++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
@@ -258,10 +258,21 @@  int PipelineHandlerVc4::prepareBuffers(Camera *camera)
 
 		} else if (stream == &data->unicam_[Unicam::Embedded]) {
 			/*
-			 * Embedded data buffers are (currently) for internal use,
-			 * so allocate the minimum required to avoid frame drops.
+			 * Embedded data buffers are (currently) for internal use, and
+			 * are small enough (typically 1-2KB) that we can
+			 * allocate them generously to avoid causing problems in the
+			 * IPA when we cannot supply the metadata.
+			 * 
+			 * 12 are allocated as a typical application will have 8-10
+			 * input buffers, so allocating more embedded buffers than that
+			 * is a sensible choice.
+			 * 
+			 * The lifetimes of these buffers are smaller than those of the
+			 * raw buffers, so allocating a fixed number will still suffice
+			 * if the application requests a greater number of raw
+			 * buffers, as these will be recycled quicker.
 			 */
-			numBuffers = minBuffers;
+			numBuffers = 12;
 		} else {
 			/*
 			 * Since the ISP runs synchronous with the IPA and requests,