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

[Kieran Bingham]

Quoting Barnabás Pőcze (2024-12-11 15:25:56)
> 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>

Hrm, rabbit holes a plenty trying to test this.

First qcam isn't working on my main development PC as Qt6 does not
supply a pkg-config file in the version available on Ubuntu 22.04 LTS.

Next up, I'm running cam with the SDL output which works:

./build/gcc/src/apps/cam/cam -c2 -S -C

136838.194958 (0.00 fps) cam0-stream0 seq: 031860 bytesused: 0/0
136838.194951 (0.00 fps) cam0-stream0 seq: 031860 bytesused: 0/0
0.000000 (0.00 fps) cam0-stream0 seq: 000000 bytesused: 0/0
0.000000 (0.00 fps) cam0-stream0 seq: 000000 bytesused: 0/0
136838.194958 (0.00 fps) cam0-stream0 seq: 031860 bytesused: 0/0
136838.194951 (0.00 fps) cam0-stream0 seq: 031860 bytesused: 0/0
0.000000 (0.00 fps) cam0-stream0 seq: 000000 bytesused: 0/0
0.000000 (0.00 fps) cam0-stream0 seq: 000000 bytesused: 0/0
136838.194958 (0.00 fps) cam0-stream0 seq: 031860 bytesused: 0/0
136838.194951 (0.00 fps) cam0-stream0 seq: 031860 bytesused: 0/0
0.000000 (0.00 fps) cam0-stream0 seq: 000000 bytesused: 0/0
0.000000 (0.00 fps) cam0-stream0 seq: 000000 bytesused: 0/0
136838.194958 (0.00 fps) cam0-stream0 seq: 031860 bytesused: 0/0
136838.194951 (0.00 fps) cam0-stream0 seq: 031860 bytesused: 0/0


but the virtual pipeline handler is clearly not handling sequence
numbers, timetamps, bytesused - or presumably any of the required
metadata correctly.

But none of that is a fault of your patch which is working so:


Tested-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.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)
> +               return;
> +
> +       const size_t stride = width * SampleSize;
> +       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);
> +       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
> -- 
> 2.47.1
> 
>