[libcamera-devel] [PATCH v2 2/2] android: libyuv: Introduce PostProcessorLibyuv
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Jan 27 12:38:53 CET 2021
Hi Hiro,
Thank you for the patch.
On Wed, Jan 27, 2021 at 04:44:25AM +0000, Hirokazu Honda wrote:
> This adds PostProcessorLibyuv. It supports NV12 buffer scaling.
>
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
>
> ---
> src/android/libyuv/post_processor_libyuv.cpp | 126 +++++++++++++++++++
> src/android/libyuv/post_processor_libyuv.h | 38 ++++++
> src/android/meson.build | 1 +
> 3 files changed, 165 insertions(+)
> create mode 100644 src/android/libyuv/post_processor_libyuv.cpp
> create mode 100644 src/android/libyuv/post_processor_libyuv.h
>
> diff --git a/src/android/libyuv/post_processor_libyuv.cpp b/src/android/libyuv/post_processor_libyuv.cpp
> new file mode 100644
> index 00000000..5f40fd25
> --- /dev/null
> +++ b/src/android/libyuv/post_processor_libyuv.cpp
> @@ -0,0 +1,126 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * post_processor_libyuv.cpp - Post Processor using libyuv
> + */
> +
> +#include "post_processor_libyuv.h"
> +
> +#include <libyuv/scale.h>
> +
> +#include <libcamera/formats.h>
> +#include <libcamera/geometry.h>
> +#include <libcamera/internal/formats.h>
> +#include <libcamera/internal/log.h>
> +#include <libcamera/pixel_format.h>
> +
> +using namespace libcamera;
> +
> +LOG_DEFINE_CATEGORY(LIBYUV)
> +
> +namespace {
Missing blank line.
> +void getNV12LengthAndStride(Size size, unsigned int length[2],
> + unsigned int stride[2])
> +{
> + const auto nv12Info = PixelFormatInfo::info(PixelFormat(formats::NV12));
formats::NV12 is already a PixelFormat, so you can write
const auto nv12Info = PixelFormatInfo::info(formats::NV12);
I'd write out the type explicitly though:
const PixelFormatInfo &nv12Info = PixelFormatInfo::info(formats::NV12);
> + for (unsigned int i = 0; i < 2; i++) {
> + const unsigned int vertSubSample =
> + nv12Info.planes[i].verticalSubSampling;
> + length[i] = nv12Info.stride(size.width, i, /*align=*/1) *
> + ((size.height + vertSubSample - 1) / vertSubSample);
> + stride[i] = nv12Info.stride(size.width, i, 1);
> + }
> +}
Missing blank line here too.
> +} // namespace
We could also make this a private static member function. For classes
exposed through the public API it could polute the public headers
unnecessarily, but for internal classes, it's less of an issue.
> +
> +PostProcessorLibyuv::PostProcessorLibyuv() = default;
Is this needed ? The compiler should generate a default constructor. You
could drop this line and the declaration of the constructor in the
class definition.
> +
> +int PostProcessorLibyuv::configure(const StreamConfiguration &inCfg,
> + const StreamConfiguration &outCfg)
> +{
> + if (inCfg.pixelFormat != outCfg.pixelFormat) {
> + LOG(LIBYUV, Error)
> + << "Pixel format conversion is not supported"
> + << "-In: " << inCfg.toString()
> + << "-Out: " << outCfg.toString();
This will print something like
"Pixel format conversion is not supported-In: NV12-Out: NV24"
How about the following ?
LOG(LIBYUV, Error)
<< "Pixel format conversion is not supported"
<< " (from " << inCfg.toString()
<< " to " << outCfg.toString() << ")";
> + return -EINVAL;
> + }
Blank line.
> + if (inCfg.size < outCfg.size) {
> + LOG(LIBYUV, Error) << "Up-scaling is not supported"
> + << ", -In: " << inCfg.toString()
> + << ", -Out: " << outCfg.toString();
Same here for the message.
> + return -EINVAL;
> + }
Here too.
> + if (inCfg.pixelFormat == formats::NV12) {
> + LOG(LIBYUV, Error) << "Only NV12 is supported"
> + << ", -In: " << inCfg.toString()
> + << ", -Out: " << outCfg.toString();
LOG(LIBYUV, Error)
<< "Unsupported format " << inCfg.pixelFormat
<< " (only NV12 is supported)";
> + return -EINVAL;
> + }
> +
> + getNV12LengthAndStride(inCfg.size, sourceLength_, sourceStride_);
> + getNV12LengthAndStride(outCfg.size, destinationLength_,
> + destinationStride_);
> + return 0;
> +}
> +
> +int PostProcessorLibyuv::process(const FrameBuffer &source,
> + libcamera::MappedBuffer *destination,
> + const CameraMetadata & /*requestMetadata*/,
> + CameraMetadata * /*metadata*/)
> +{
> + if (!IsValidNV12Buffers(source, *destination)) {
> + LOG(LIBYUV, Error) << "Invalid source and destination";
I think you could drop this message as IsValidNV12Buffers() already logs
errors.
> + return -EINVAL;
> + }
> +
> + const MappedFrameBuffer sourceMapped(&source, PROT_READ);
> + if (!sourceMapped.isValid()) {
> + LOG(LIBYUV, Error) << "Failed to mmap camera frame buffer";
> + return -EINVAL;
> + }
> +
> + return 0 == libyuv::NV12Scale(sourceMapped.maps()[0].data(),
> + sourceStride_[0],
> + sourceMapped.maps()[1].data(),
> + sourceStride_[1],
> + sourceSize_.width, sourceSize_.height,
> + destination->maps()[0].data(),
> + destinationStride_[0],
> + destination->maps()[1].data(),
> + destinationStride_[1],
> + destinationSize_.width,
> + destinationSize_.height,
> + libyuv::FilterMode::kFilterBilinear);
> +}
> +
> +bool PostProcessorLibyuv::IsValidNV12Buffers(
s/IsValidNV12Buffers/isValidNV12Buffers/
> + const FrameBuffer &source,
> + const libcamera::MappedBuffer &destination) const
> +{
> + if (source.planes().size() != 2u) {
> + LOG(LIBYUV, Error) << "The number of source planes is not 2";
> + return false;
> + }
> + if (destination.maps().size() != 2u) {
> + LOG(LIBYUV, Error)
> + << "The number of destination planes is not 2";
> + return false;
> + }
> +
> + if (source.planes()[0].length < sourceLength_[0] ||
> + source.planes()[1].length < sourceLength_[1]) {
> + LOG(LIBYUV, Error)
> + << "The source planes lengths are too small";
> + return false;
> + }
> + if (destination.maps()[0].size() < destinationLength_[0] ||
> + destination.maps()[1].size() < destinationLength_[1]) {
> + LOG(LIBYUV, Error)
> + << "The destination planes lengths are too small";
> + return false;
> + }
> +
> + return true;
> +}
> diff --git a/src/android/libyuv/post_processor_libyuv.h b/src/android/libyuv/post_processor_libyuv.h
> new file mode 100644
> index 00000000..40c635a1
> --- /dev/null
> +++ b/src/android/libyuv/post_processor_libyuv.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * post_processor_libyuv.h - Post Processor using libyuv
> + */
> +#ifndef __ANDROID_POST_PROCESSOR_LIBYUV_H__
> +#define __ANDROID_POST_PROCESSOR_LIBYUV_H__
> +
> +#include "../post_processor.h"
> +
> +class CameraDevice;
> +
> +class PostProcessorLibyuv : public PostProcessor
> +{
> +public:
> + PostProcessorLibyuv();
> +
> + int configure(const libcamera::StreamConfiguration &incfg,
> + const libcamera::StreamConfiguration &outcfg) override;
> + int process(const libcamera::FrameBuffer &source,
> + libcamera::MappedBuffer *destination,
> + const CameraMetadata & /*requestMetadata*/,
You can use [[maybe_unused]].
> + CameraMetadata *metadata) override;
> +
> +private:
> + bool IsValidNV12Buffers(const libcamera::FrameBuffer &source,
> + const libcamera::MappedBuffer &destination) const;
> +
> + libcamera::Size sourceSize_;
> + libcamera::Size destinationSize_;
> + unsigned int sourceLength_[2] = {};
> + unsigned int destinationLength_[2] = {};
> + unsigned int sourceStride_[2] = {};
> + unsigned int destinationStride_[2] = {};
> +};
> +
> +#endif /* __ANDROID_POST_PROCESSOR_LIBYUV_H__ */
> diff --git a/src/android/meson.build b/src/android/meson.build
> index 3d4d3be4..ff067d12 100644
> --- a/src/android/meson.build
> +++ b/src/android/meson.build
> @@ -26,6 +26,7 @@ android_hal_sources = files([
> 'jpeg/exif.cpp',
> 'jpeg/post_processor_jpeg.cpp',
> 'jpeg/thumbnailer.cpp',
> + 'libyuv/post_processor_libyuv.cpp'
> ])
>
> android_camera_metadata_sources = files([
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list