[PATCH v1 3/3] libcamera: virtual: Speed up test pattern animation

[Laurent Pinchart]

Hi Barnabás,

Thank you for the patch.

On Wed, Dec 11, 2024 at 03:25:56PM +0000, Barnabás Pőcze wrote:
> After the initial generation, when a frame is requested,
> the test pattern generator rotates the image left by 1 column.
> 
> The current approach has two shortcomings:
>   (1) it allocates a temporary buffer to hold one column;
>   (2) it swaps two columns at a time.
> 
> The test patterns are simple ARGB images, in row-major order,
> so doing (2) works against memory prefetching. This can be
> addressed by doing the rotation one row at a time as that way
> the image is addressed in a purely linear fashion. Doing so also
> eliminates the need for a dynamically allocated temporary buffer,
> as the required buffer now only needs to hold one sample,
> which is 4 bytes in this case.
> 
> In an optimized build, this results in about a 2x increase in the
> number of frames per second as reported by `cam`. In an unoptimized,
> ASAN and UBSAN intrumented build, the difference is even bigger,
> which is useful for running lc-compliance in CI in a reasonable time.
> 
> Signed-off-by: Barnabás Pőcze <pobrn at protonmail.com>
> ---
>  .../virtual/test_pattern_generator.cpp        | 68 +++++++++----------
>  .../pipeline/virtual/test_pattern_generator.h |  4 --
>  2 files changed, 34 insertions(+), 38 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/virtual/test_pattern_generator.cpp b/src/libcamera/pipeline/virtual/test_pattern_generator.cpp
> index 47d341919..eeadc1646 100644
> --- a/src/libcamera/pipeline/virtual/test_pattern_generator.cpp
> +++ b/src/libcamera/pipeline/virtual/test_pattern_generator.cpp
> @@ -5,6 +5,8 @@
>   * Derived class of FrameGenerator for generating test patterns
>   */
>  
> +#include <string.h>
> +
>  #include "test_pattern_generator.h"
>  
>  #include <libcamera/base/log.h>
> @@ -13,6 +15,37 @@
>  
>  #include <libyuv/convert_from_argb.h>
>  
> +namespace {
> +
> +template<size_t SampleSize>
> +void rotateLeft1Sample(uint8_t *samples, size_t width)
> +{
> +	if (width <= 0)

width is unsigned, so it will never be smaller than 0. As the caller
already ensures that the width is >= 2, I'd drop this check.

> +		return;
> +
> +	const size_t stride = width * SampleSize;

You could pass the stride value to this function, instead of recomputing
it for each line.

> +	uint8_t first[SampleSize];
> +
> +	memcpy(first, &samples[0], SampleSize);
> +	for (size_t i = 0; i < stride - SampleSize; i += SampleSize)
> +		memcpy(&samples[i], &samples[i + SampleSize], SampleSize);

Have you considered replacing this with

	memmove(&samples[0], &samples[SampleSize], stride - SampleSize);

?

This patch is a good improvement, so

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

with the above comments taken into account (regardless of whether the
suggestions are accepted or rejected).

I think the test pattern generator could do with a whole rewrite. I
would compute the test pattern image at configure time, and store it in
NV12 and in the output resolution, and handle the rotation when copying
it to the frame buffer. That should significantly speed up operation.

> +	memcpy(&samples[stride - SampleSize], first, SampleSize);
> +}
> +
> +template<size_t SampleSize>
> +void rotateLeft1Column(const libcamera::Size &size, uint8_t *image)
> +{
> +	if (size.width < 2)
> +		return;
> +
> +	const size_t stride = size.width * SampleSize;
> +
> +	for (size_t i = 0; i < size.height; i++, image += stride)
> +		rotateLeft1Sample<SampleSize>(image, size.width);
> +}
> +
> +}
> +
>  namespace libcamera {
>  
>  LOG_DECLARE_CATEGORY(Virtual)
> @@ -27,7 +60,7 @@ int TestPatternGenerator::generateFrame(const Size &size,
>  
>  	const auto &planes = mappedFrameBuffer.planes();
>  
> -	shiftLeft(size);
> +	rotateLeft1Column<kARGBSize>(size, template_.get());
>  
>  	/* Convert the template_ to the frame buffer */
>  	int ret = libyuv::ARGBToNV12(template_.get(), size.width * kARGBSize,
> @@ -40,39 +73,6 @@ int TestPatternGenerator::generateFrame(const Size &size,
>  	return ret;
>  }
>  
> -void TestPatternGenerator::shiftLeft(const Size &size)
> -{
> -	/* Store the first column temporarily */
> -	auto firstColumn = std::make_unique<uint8_t[]>(size.height * kARGBSize);
> -	for (size_t h = 0; h < size.height; h++) {
> -		unsigned int index = h * size.width * kARGBSize;
> -		unsigned int index1 = h * kARGBSize;
> -		firstColumn[index1] = template_[index];
> -		firstColumn[index1 + 1] = template_[index + 1];
> -		firstColumn[index1 + 2] = template_[index + 2];
> -		firstColumn[index1 + 3] = 0x00;
> -	}
> -
> -	/* Overwrite template_ */
> -	uint8_t *buf = template_.get();
> -	for (size_t h = 0; h < size.height; h++) {
> -		for (size_t w = 0; w < size.width - 1; w++) {
> -			/* Overwrite with the pixel on the right */
> -			unsigned int index = (h * size.width + w + 1) * kARGBSize;
> -			*buf++ = template_[index]; /* B */
> -			*buf++ = template_[index + 1]; /* G */
> -			*buf++ = template_[index + 2]; /* R */
> -			*buf++ = 0x00; /* A */
> -		}
> -		/* Overwrite the new last column with the original first column */
> -		unsigned int index1 = h * kARGBSize;
> -		*buf++ = firstColumn[index1]; /* B */
> -		*buf++ = firstColumn[index1 + 1]; /* G */
> -		*buf++ = firstColumn[index1 + 2]; /* R */
> -		*buf++ = 0x00; /* A */
> -	}
> -}
> -
>  void ColorBarsGenerator::configure(const Size &size)
>  {
>  	constexpr uint8_t kColorBar[8][3] = {
> diff --git a/src/libcamera/pipeline/virtual/test_pattern_generator.h b/src/libcamera/pipeline/virtual/test_pattern_generator.h
> index 05f4ab7a7..2a51bd31a 100644
> --- a/src/libcamera/pipeline/virtual/test_pattern_generator.h
> +++ b/src/libcamera/pipeline/virtual/test_pattern_generator.h
> @@ -29,10 +29,6 @@ public:
>  protected:
>  	/* Buffer of test pattern template */
>  	std::unique_ptr<uint8_t[]> template_;
> -
> -private:
> -	/* Shift the buffer by 1 pixel left each frame */
> -	void shiftLeft(const Size &size);
>  };
>  
>  class ColorBarsGenerator : public TestPatternGenerator

-- 
Regards,

Laurent Pinchart