[PATCH v14 5/7] libcamera: virtual: Add ImageFrameGenerator
Kieran Bingham
kieran.bingham at ideasonboard.com
Thu Oct 3 12:32:14 CEST 2024
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 at google.com>
> Co-developed-by: Harvey Yang <chenghaoyang at chromium.org>
> Co-developed-by: Yunke Cao <yunkec at chromium.org>
> Co-developed-by: Tomasz Figa <tfiga at chromium.org>
> Reviewed-by: Jacopo Mondi <jacopo.mondi at 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
>
More information about the libcamera-devel
mailing list