[v14,5/7] libcamera: virtual: Add ImageFrameGenerator

21419 diff mbox series

Besides TestPatternGenerator, this patch adds ImageFrameGenerator that
loads real images (jpg / jpeg for now) as the source and generates
scaled frames.

Signed-off-by: Konami Shu <konamiz@google.com>
Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
Co-developed-by: Yunke Cao <yunkec@chromium.org>
Co-developed-by: Tomasz Figa <tfiga@chromium.org>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 .../virtual/image_frame_generator.cpp         | 175 ++++++++++++++++++
 .../pipeline/virtual/image_frame_generator.h  |  51 +++++
 src/libcamera/pipeline/virtual/meson.build    |   4 +
 3 files changed, 230 insertions(+)
 create mode 100644 src/libcamera/pipeline/virtual/image_frame_generator.cpp
 create mode 100644 src/libcamera/pipeline/virtual/image_frame_generator.h

Quoting Harvey Yang (2024-09-30 07:29:46)
> Besides TestPatternGenerator, this patch adds ImageFrameGenerator that
> loads real images (jpg / jpeg for now) as the source and generates
> scaled frames.
> 
> Signed-off-by: Konami Shu <konamiz@google.com>
> Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> Co-developed-by: Yunke Cao <yunkec@chromium.org>
> Co-developed-by: Tomasz Figa <tfiga@chromium.org>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  .../virtual/image_frame_generator.cpp         | 175 ++++++++++++++++++
>  .../pipeline/virtual/image_frame_generator.h  |  51 +++++
>  src/libcamera/pipeline/virtual/meson.build    |   4 +
>  3 files changed, 230 insertions(+)
>  create mode 100644 src/libcamera/pipeline/virtual/image_frame_generator.cpp
>  create mode 100644 src/libcamera/pipeline/virtual/image_frame_generator.h
> 
> diff --git a/src/libcamera/pipeline/virtual/image_frame_generator.cpp b/src/libcamera/pipeline/virtual/image_frame_generator.cpp
> new file mode 100644
> index 00000000..a1915b24
> --- /dev/null
> +++ b/src/libcamera/pipeline/virtual/image_frame_generator.cpp
> @@ -0,0 +1,175 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Google Inc.
> + *
> + * image_frame_generator.cpp - Derived class of FrameGenerator for
> + * generating frames from images
> + */
> +
> +#include "image_frame_generator.h"
> +
> +#include <string>
> +
> +#include <libcamera/base/file.h>
> +#include <libcamera/base/log.h>
> +
> +#include <libcamera/framebuffer.h>
> +
> +#include "libcamera/internal/mapped_framebuffer.h"
> +
> +#include "libyuv/convert.h"
> +#include "libyuv/scale.h"
> +
> +namespace libcamera {
> +
> +LOG_DECLARE_CATEGORY(Virtual)
> +
> +/*
> + * Factory function to create an ImageFrameGenerator object.
> + * Read the images and convert them to buffers in NV12 format.
> + * Store the pointers to the buffers to a list (imageFrameDatas)
> + */
> +std::unique_ptr<ImageFrameGenerator>
> +ImageFrameGenerator::create(ImageFrames &imageFrames)
> +{
> +       std::unique_ptr<ImageFrameGenerator> imageFrameGenerator =
> +               std::make_unique<ImageFrameGenerator>();
> +       imageFrameGenerator->imageFrames_ = &imageFrames;
> +
> +       /*
> +        * For each file in the directory, load the image,
> +        * convert it to NV12, and store the pointer.

What happens if the directory has 200 images? Will this load all images
into memory?

Should we impose any limits? Or would you see issues there?


> +        */
> +       for (unsigned int i = 0; i < imageFrames.number.value_or(1); i++) {
> +               std::filesystem::path path;
> +               if (!imageFrames.number)
> +                       /* If the path is to an image */
> +                       path = imageFrames.path;
> +               else
> +                       /* If the path is to a directory */
> +                       path = imageFrames.path / (std::to_string(i) + ".jpg");
> +
> +               File file(path);
> +               if (!file.open(File::OpenModeFlag::ReadOnly)) {
> +                       LOG(Virtual, Error) << "Failed to open image file " << file.fileName()
> +                                           << ": " << strerror(file.error());
> +                       return nullptr;
> +               }
> +
> +               /* Read the image file to data */
> +               auto fileSize = file.size();
> +               auto buffer = std::make_unique<uint8_t[]>(file.size());

not fileSize?

> +               if (file.read({ buffer.get(), static_cast<size_t>(fileSize) }) != fileSize) {
> +                       LOG(Virtual, Error) << "Failed to read file " << file.fileName()
> +                                           << ": " << strerror(file.error());
> +                       return nullptr;
> +               }
> +
> +               /* Get the width and height of the image */
> +               int width, height;

unsigned int?

> +               if (libyuv::MJPGSize(buffer.get(), fileSize, &width, &height)) {
> +                       LOG(Virtual, Error) << "Failed to get the size of the image file: "
> +                                           << file.fileName();
> +                       return nullptr;
> +               }
> +
> +               std::unique_ptr<uint8_t[]> dstY =
> +                       std::make_unique<uint8_t[]>(width * height);
> +               std::unique_ptr<uint8_t[]> dstUV =
> +                       std::make_unique<uint8_t[]>(width * height / 2);
> +               int ret = libyuv::MJPGToNV12(buffer.get(), fileSize,
> +                                            dstY.get(), width, dstUV.get(),
> +                                            width, width, height, width, height);
> +               if (ret != 0)
> +                       LOG(Virtual, Error) << "MJPGToNV12() failed with " << ret;
> +
> +               imageFrameGenerator->imageFrameDatas_.emplace_back(
> +                       ImageFrameData{ std::move(dstY), std::move(dstUV),
> +                                       Size(width, height) });
> +       }
> +
> +       return imageFrameGenerator;
> +}
> +
> +/* Scale the buffers for image frames. */
> +void ImageFrameGenerator::configure(const Size &size)
> +{
> +       /* Reset the source images to prevent multiple configuration calls */
> +       scaledFrameDatas_.clear();
> +       frameCount_ = 0;
> +       parameter_ = 0;
> +
> +       for (unsigned int i = 0; i < imageFrames_->number.value_or(1); i++) {
> +               /* Scale the imageFrameDatas_ to scaledY and scaledUV */
> +               unsigned int halfSizeWidth = (size.width + 1) / 2;
> +               unsigned int halfSizeHeight = (size.height + 1) / 2;
> +               std::unique_ptr<uint8_t[]> scaledY =
> +                       std::make_unique<uint8_t[]>(size.width * size.height);
> +               std::unique_ptr<uint8_t[]> scaledUV =
> +                       std::make_unique<uint8_t[]>(halfSizeWidth * halfSizeHeight * 2);
> +               auto &src = imageFrameDatas_[i];
> +
> +               /*
> +                * \todo Some platforms might enforce stride due to GPU, like
> +                * ChromeOS ciri (64). The weight needs to be a multiple of

weight ?

And I think this isn't anything to do with Ciri. We don't know (or care)
what a Ciri is inside libcamera, but we do care about externally
allocated buffers or buffers with defined stride requirements.

The stride should always be correctly considered for any output buffer
managed by a pipeline handler.

I wonder if we can make some specific unit tests in lc-compliance to
validate that too.


> +                * the stride to work properly for now.
> +                */
> +               libyuv::NV12Scale(src.Y.get(), src.size.width,
> +                                 src.UV.get(), src.size.width,
> +                                 src.size.width, src.size.height,
> +                                 scaledY.get(), size.width, scaledUV.get(), size.width,
> +                                 size.width, size.height, libyuv::FilterMode::kFilterBilinear);
> +
> +               scaledFrameDatas_.emplace_back(
> +                       ImageFrameData{ std::move(scaledY), std::move(scaledUV), size });
> +       }
> +}
> +
> +int ImageFrameGenerator::generateFrame(const Size &size, const FrameBuffer *buffer)
> +{
> +       /* Don't do anything when the list of buffers is empty*/

missing space between empty*/

But ... well an ASSERT is definitely a way to not do anything at all
ever again ;-)

> +       ASSERT(!scaledFrameDatas_.empty());
> +
> +       MappedFrameBuffer mappedFrameBuffer(buffer, MappedFrameBuffer::MapFlag::Write);
> +
> +       auto planes = mappedFrameBuffer.planes();
> +
> +       /* Make sure the frameCount does not over the number of images */

	/* Loop only around the number of images available */

> +       frameCount_ %= imageFrames_->number.value_or(1);

Then, I don't think this is a frameCount. It's a frameIndex_.

A FrameCount would be something you would set as the sequence numbers in
the FrameBuffers.

Do you set those?

> +
> +       /* Write the scaledY and scaledUV to the mapped frame buffer */
> +       libyuv::NV12Copy(scaledFrameDatas_[frameCount_].Y.get(), size.width,
> +                        scaledFrameDatas_[frameCount_].UV.get(), size.width, planes[0].begin(),
> +                        size.width, planes[1].begin(), size.width,
> +                        size.width, size.height);
> +
> +       /* Proceed an image every 4 frames */

	/* Proceed to the next image every 4 frames */

> +       /* \todo Consider setting the proceed interval in the config file  */

s/proceed interval/frameInterval/

But now I wonder if it should be frameRepeat ... this isn't an interval
is it ?

> +       parameter_++;
> +       if (parameter_ % frameInterval == 0)
> +               frameCount_++;
> +
> +       return 0;
> +}
> +
> +/*
> + * \var ImageFrameGenerator::imageFrameDatas_
> + * \brief List of pointers to the not scaled image buffers
> + */
> +
> +/*
> + * \var ImageFrameGenerator::scaledFrameDatas_
> + * \brief List of pointers to the scaled image buffers
> + */
> +
> +/*
> + * \var ImageFrameGenerator::imageFrames_
> + * \brief Pointer to the imageFrames_ in VirtualCameraData
> + */
> +
> +/*
> + * \var ImageFrameGenerator::parameter_
> + * \brief Speed parameter. Change to the next image every parameter_ frames
> + */
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/virtual/image_frame_generator.h b/src/libcamera/pipeline/virtual/image_frame_generator.h
> new file mode 100644
> index 00000000..cd243816
> --- /dev/null
> +++ b/src/libcamera/pipeline/virtual/image_frame_generator.h
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Google Inc.
> + *
> + * image_frame_generator.h - Derived class of FrameGenerator for
> + * generating frames from images
> + */
> +
> +#pragma once
> +
> +#include <filesystem>
> +#include <memory>
> +#include <optional>
> +#include <stdint.h>
> +#include <sys/types.h>
> +
> +#include "frame_generator.h"
> +
> +namespace libcamera {
> +
> +/* Frame configuration provided by the config file */
> +struct ImageFrames {
> +       std::filesystem::path path;
> +       std::optional<unsigned int> number;
> +};
> +
> +class ImageFrameGenerator : public FrameGenerator
> +{
> +public:
> +       static std::unique_ptr<ImageFrameGenerator> create(ImageFrames &imageFrames);
> +
> +private:

A brief comment here would help to say what a frameInterval is.

But I don't know if it's named correctly. It's a frameRepeat from my
understanding... I.e. it causes a loaded frame to be repeated for N
frames.

> +       static constexpr unsigned int frameInterval = 4;
> +
> +       struct ImageFrameData {
> +               std::unique_ptr<uint8_t[]> Y;
> +               std::unique_ptr<uint8_t[]> UV;
> +               Size size;
> +       };
> +
> +       void configure(const Size &size) override;
> +       int generateFrame(const Size &size, const FrameBuffer *buffer) override;
> +
> +       std::vector<ImageFrameData> imageFrameDatas_;
> +       std::vector<ImageFrameData> scaledFrameDatas_;
> +       ImageFrames *imageFrames_;
> +       unsigned int frameCount_;
> +       unsigned int parameter_;
> +};
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/virtual/meson.build b/src/libcamera/pipeline/virtual/meson.build
> index 0c537777..bb38c193 100644
> --- a/src/libcamera/pipeline/virtual/meson.build
> +++ b/src/libcamera/pipeline/virtual/meson.build
> @@ -1,8 +1,12 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
>  libcamera_internal_sources += files([
> +    'image_frame_generator.cpp',
>      'test_pattern_generator.cpp',
>      'virtual.cpp',
>  ])
>  
> +libjpeg = dependency('libjpeg', required : true)
> +
>  libcamera_deps += [libyuv_dep]
> +libcamera_deps += [libjpeg]
> -- 
> 2.46.1.824.gd892dcdcdd-goog
>

Hi Kieran,

On Thu, Oct 3, 2024 at 6:32 PM Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Quoting Harvey Yang (2024-09-30 07:29:46)
> > Besides TestPatternGenerator, this patch adds ImageFrameGenerator that
> > loads real images (jpg / jpeg for now) as the source and generates
> > scaled frames.
> >
> > Signed-off-by: Konami Shu <konamiz@google.com>
> > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> > Co-developed-by: Yunke Cao <yunkec@chromium.org>
> > Co-developed-by: Tomasz Figa <tfiga@chromium.org>
> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
> >  .../virtual/image_frame_generator.cpp         | 175 ++++++++++++++++++
> >  .../pipeline/virtual/image_frame_generator.h  |  51 +++++
> >  src/libcamera/pipeline/virtual/meson.build    |   4 +
> >  3 files changed, 230 insertions(+)
> >  create mode 100644 src/libcamera/pipeline/virtual/image_frame_generator.cpp
> >  create mode 100644 src/libcamera/pipeline/virtual/image_frame_generator.h
> >
> > diff --git a/src/libcamera/pipeline/virtual/image_frame_generator.cpp b/src/libcamera/pipeline/virtual/image_frame_generator.cpp
> > new file mode 100644
> > index 00000000..a1915b24
> > --- /dev/null
> > +++ b/src/libcamera/pipeline/virtual/image_frame_generator.cpp
> > @@ -0,0 +1,175 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2024, Google Inc.
> > + *
> > + * image_frame_generator.cpp - Derived class of FrameGenerator for
> > + * generating frames from images
> > + */
> > +
> > +#include "image_frame_generator.h"
> > +
> > +#include <string>
> > +
> > +#include <libcamera/base/file.h>
> > +#include <libcamera/base/log.h>
> > +
> > +#include <libcamera/framebuffer.h>
> > +
> > +#include "libcamera/internal/mapped_framebuffer.h"
> > +
> > +#include "libyuv/convert.h"
> > +#include "libyuv/scale.h"
> > +
> > +namespace libcamera {
> > +
> > +LOG_DECLARE_CATEGORY(Virtual)
> > +
> > +/*
> > + * Factory function to create an ImageFrameGenerator object.
> > + * Read the images and convert them to buffers in NV12 format.
> > + * Store the pointers to the buffers to a list (imageFrameDatas)
> > + */
> > +std::unique_ptr<ImageFrameGenerator>
> > +ImageFrameGenerator::create(ImageFrames &imageFrames)
> > +{
> > +       std::unique_ptr<ImageFrameGenerator> imageFrameGenerator =
> > +               std::make_unique<ImageFrameGenerator>();
> > +       imageFrameGenerator->imageFrames_ = &imageFrames;
> > +
> > +       /*
> > +        * For each file in the directory, load the image,
> > +        * convert it to NV12, and store the pointer.
>
> What happens if the directory has 200 images? Will this load all images
> into memory?

Currently, yes.

>
> Should we impose any limits? Or would you see issues there?

We can either have a limit of file size or number of files, while I think
we can keep it as is for now, when we have more tests depending on
virtual pipeline handler. Over-designing might take much more time
than needed here IMO.

Another idea: We can also pre-load a certain amount of files, and
drop some images to save memory usage.

>
>
> > +        */
> > +       for (unsigned int i = 0; i < imageFrames.number.value_or(1); i++) {
> > +               std::filesystem::path path;
> > +               if (!imageFrames.number)
> > +                       /* If the path is to an image */
> > +                       path = imageFrames.path;
> > +               else
> > +                       /* If the path is to a directory */
> > +                       path = imageFrames.path / (std::to_string(i) + ".jpg");
> > +
> > +               File file(path);
> > +               if (!file.open(File::OpenModeFlag::ReadOnly)) {
> > +                       LOG(Virtual, Error) << "Failed to open image file " << file.fileName()
> > +                                           << ": " << strerror(file.error());
> > +                       return nullptr;
> > +               }
> > +
> > +               /* Read the image file to data */
> > +               auto fileSize = file.size();
> > +               auto buffer = std::make_unique<uint8_t[]>(file.size());
>
> not fileSize?

Ah right. Updated.

>
> > +               if (file.read({ buffer.get(), static_cast<size_t>(fileSize) }) != fileSize) {
> > +                       LOG(Virtual, Error) << "Failed to read file " << file.fileName()
> > +                                           << ": " << strerror(file.error());
> > +                       return nullptr;
> > +               }
> > +
> > +               /* Get the width and height of the image */
> > +               int width, height;
>
> unsigned int?

No, as `libyuv::MJPGToNV12` takes `int` instead of `unsigned int`.

>
> > +               if (libyuv::MJPGSize(buffer.get(), fileSize, &width, &height)) {
> > +                       LOG(Virtual, Error) << "Failed to get the size of the image file: "
> > +                                           << file.fileName();
> > +                       return nullptr;
> > +               }
> > +
> > +               std::unique_ptr<uint8_t[]> dstY =
> > +                       std::make_unique<uint8_t[]>(width * height);
> > +               std::unique_ptr<uint8_t[]> dstUV =
> > +                       std::make_unique<uint8_t[]>(width * height / 2);
> > +               int ret = libyuv::MJPGToNV12(buffer.get(), fileSize,
> > +                                            dstY.get(), width, dstUV.get(),
> > +                                            width, width, height, width, height);
> > +               if (ret != 0)
> > +                       LOG(Virtual, Error) << "MJPGToNV12() failed with " << ret;
> > +
> > +               imageFrameGenerator->imageFrameDatas_.emplace_back(
> > +                       ImageFrameData{ std::move(dstY), std::move(dstUV),
> > +                                       Size(width, height) });
> > +       }
> > +
> > +       return imageFrameGenerator;
> > +}
> > +
> > +/* Scale the buffers for image frames. */
> > +void ImageFrameGenerator::configure(const Size &size)
> > +{
> > +       /* Reset the source images to prevent multiple configuration calls */
> > +       scaledFrameDatas_.clear();
> > +       frameCount_ = 0;
> > +       parameter_ = 0;
> > +
> > +       for (unsigned int i = 0; i < imageFrames_->number.value_or(1); i++) {
> > +               /* Scale the imageFrameDatas_ to scaledY and scaledUV */
> > +               unsigned int halfSizeWidth = (size.width + 1) / 2;
> > +               unsigned int halfSizeHeight = (size.height + 1) / 2;
> > +               std::unique_ptr<uint8_t[]> scaledY =
> > +                       std::make_unique<uint8_t[]>(size.width * size.height);
> > +               std::unique_ptr<uint8_t[]> scaledUV =
> > +                       std::make_unique<uint8_t[]>(halfSizeWidth * halfSizeHeight * 2);
> > +               auto &src = imageFrameDatas_[i];
> > +
> > +               /*
> > +                * \todo Some platforms might enforce stride due to GPU, like
> > +                * ChromeOS ciri (64). The weight needs to be a multiple of
>
> weight ?

Should be width. Thanks!

>
> And I think this isn't anything to do with Ciri. We don't know (or care)
> what a Ciri is inside libcamera, but we do care about externally
> allocated buffers or buffers with defined stride requirements.
>
> The stride should always be correctly considered for any output buffer
> managed by a pipeline handler.

True. Let me remove Ciri as an example for now.

>
> I wonder if we can make some specific unit tests in lc-compliance to
> validate that too.
>
>
> > +                * the stride to work properly for now.
> > +                */
> > +               libyuv::NV12Scale(src.Y.get(), src.size.width,
> > +                                 src.UV.get(), src.size.width,
> > +                                 src.size.width, src.size.height,
> > +                                 scaledY.get(), size.width, scaledUV.get(), size.width,
> > +                                 size.width, size.height, libyuv::FilterMode::kFilterBilinear);
> > +
> > +               scaledFrameDatas_.emplace_back(
> > +                       ImageFrameData{ std::move(scaledY), std::move(scaledUV), size });
> > +       }
> > +}
> > +
> > +int ImageFrameGenerator::generateFrame(const Size &size, const FrameBuffer *buffer)
> > +{
> > +       /* Don't do anything when the list of buffers is empty*/
>
> missing space between empty*/
>
> But ... well an ASSERT is definitely a way to not do anything at all
> ever again ;-)

Yeah the comment doesn't make much sense...
Let me remove it.

>
> > +       ASSERT(!scaledFrameDatas_.empty());
> > +
> > +       MappedFrameBuffer mappedFrameBuffer(buffer, MappedFrameBuffer::MapFlag::Write);
> > +
> > +       auto planes = mappedFrameBuffer.planes();
> > +
> > +       /* Make sure the frameCount does not over the number of images */
>
>         /* Loop only around the number of images available */

Done

>
> > +       frameCount_ %= imageFrames_->number.value_or(1);
>
> Then, I don't think this is a frameCount. It's a frameIndex_.

Right, updated.

>
> A FrameCount would be something you would set as the sequence numbers in
> the FrameBuffers.
>
> Do you set those?

No. In which member variable of FrameBuffer?

>
> > +
> > +       /* Write the scaledY and scaledUV to the mapped frame buffer */
> > +       libyuv::NV12Copy(scaledFrameDatas_[frameCount_].Y.get(), size.width,
> > +                        scaledFrameDatas_[frameCount_].UV.get(), size.width, planes[0].begin(),
> > +                        size.width, planes[1].begin(), size.width,
> > +                        size.width, size.height);
> > +
> > +       /* Proceed an image every 4 frames */
>
>         /* Proceed to the next image every 4 frames */

Done

>
> > +       /* \todo Consider setting the proceed interval in the config file  */
>
> s/proceed interval/frameInterval/
>
> But now I wonder if it should be frameRepeat ... this isn't an interval
> is it ?
>
> > +       parameter_++;
> > +       if (parameter_ % frameInterval == 0)
> > +               frameCount_++;
> > +
> > +       return 0;
> > +}
> > +
> > +/*
> > + * \var ImageFrameGenerator::imageFrameDatas_
> > + * \brief List of pointers to the not scaled image buffers
> > + */
> > +
> > +/*
> > + * \var ImageFrameGenerator::scaledFrameDatas_
> > + * \brief List of pointers to the scaled image buffers
> > + */
> > +
> > +/*
> > + * \var ImageFrameGenerator::imageFrames_
> > + * \brief Pointer to the imageFrames_ in VirtualCameraData
> > + */
> > +
> > +/*
> > + * \var ImageFrameGenerator::parameter_
> > + * \brief Speed parameter. Change to the next image every parameter_ frames
> > + */
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/libcamera/pipeline/virtual/image_frame_generator.h b/src/libcamera/pipeline/virtual/image_frame_generator.h
> > new file mode 100644
> > index 00000000..cd243816
> > --- /dev/null
> > +++ b/src/libcamera/pipeline/virtual/image_frame_generator.h
> > @@ -0,0 +1,51 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2024, Google Inc.
> > + *
> > + * image_frame_generator.h - Derived class of FrameGenerator for
> > + * generating frames from images
> > + */
> > +
> > +#pragma once
> > +
> > +#include <filesystem>
> > +#include <memory>
> > +#include <optional>
> > +#include <stdint.h>
> > +#include <sys/types.h>
> > +
> > +#include "frame_generator.h"
> > +
> > +namespace libcamera {
> > +
> > +/* Frame configuration provided by the config file */
> > +struct ImageFrames {
> > +       std::filesystem::path path;
> > +       std::optional<unsigned int> number;
> > +};
> > +
> > +class ImageFrameGenerator : public FrameGenerator
> > +{
> > +public:
> > +       static std::unique_ptr<ImageFrameGenerator> create(ImageFrames &imageFrames);
> > +
> > +private:
>
> A brief comment here would help to say what a frameInterval is.
>
> But I don't know if it's named correctly. It's a frameRepeat from my
> understanding... I.e. it causes a loaded frame to be repeated for N
> frames.

Yeah right, frameRepeat makes more sense. Added a brief comment.


>
> > +       static constexpr unsigned int frameInterval = 4;
> > +
> > +       struct ImageFrameData {
> > +               std::unique_ptr<uint8_t[]> Y;
> > +               std::unique_ptr<uint8_t[]> UV;
> > +               Size size;
> > +       };
> > +
> > +       void configure(const Size &size) override;
> > +       int generateFrame(const Size &size, const FrameBuffer *buffer) override;
> > +
> > +       std::vector<ImageFrameData> imageFrameDatas_;
> > +       std::vector<ImageFrameData> scaledFrameDatas_;
> > +       ImageFrames *imageFrames_;
> > +       unsigned int frameCount_;
> > +       unsigned int parameter_;
> > +};
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/libcamera/pipeline/virtual/meson.build b/src/libcamera/pipeline/virtual/meson.build
> > index 0c537777..bb38c193 100644
> > --- a/src/libcamera/pipeline/virtual/meson.build
> > +++ b/src/libcamera/pipeline/virtual/meson.build
> > @@ -1,8 +1,12 @@
> >  # SPDX-License-Identifier: CC0-1.0
> >
> >  libcamera_internal_sources += files([
> > +    'image_frame_generator.cpp',
> >      'test_pattern_generator.cpp',
> >      'virtual.cpp',
> >  ])
> >
> > +libjpeg = dependency('libjpeg', required : true)
> > +
> >  libcamera_deps += [libyuv_dep]
> > +libcamera_deps += [libjpeg]
> > --
> > 2.46.1.824.gd892dcdcdd-goog
> >