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

[Barnabás Pőcze]

Regards,
Barnabás Pőcze


2024. december 17., kedd 17:19 keltezéssel, Kieran Bingham <kieran.bingham at ideasonboard.com> írta:

> 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:

I forgot to mention that I used this to test:
https://gitlab.freedesktop.org/pobrn/libcamera/-/commit/cd537fa855bb68c1faeab8fb5b94c4d423899b04

There is also a corresponding bug:
https://bugs.libcamera.org/show_bug.cgi?id=245


Regards,
Barnabás Pőcze

> 
> 
> 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