[libcamera-devel] [PATCH v2 1/2] android: post_processor: Introduce a PostProcessor interface

[Hirokazu Honda]

Thanks Umang for this work.
Thanks Laurent for letting me know.
I actually recognized these works last Wednesday but haven't had time
to review them.

The patches are similar to my local WIP patches but nicer than mine.
I haven't been able to upload them because I was interrupted by other
works and had some troubles building and testing libcamera on ChromeOS
environment.
Anyway, I very much appreciate Umang to take over this work.


On Fri, Oct 16, 2020 at 2:38 PM Umang Jain <email at uajain.com> wrote:
>
> Introduce a PostProcessor interface for the streams that require any
> kind of processing (refer to CameraStream::Type) for their consumption
> by the HAL layer. The PostProcessor interface can be configured via
> configure() and the actual processing can be initiated using process().
>
> The post-processing layer can be extended to have multiple post
> processors for various stream configurations. As of now, we only have
> one post processor (JPEG), hence the subsequent commit will port its
> function to this interface.
>
> Signed-off-by: Umang Jain <email at uajain.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/android/post_processor.h | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100644 src/android/post_processor.h
>
> diff --git a/src/android/post_processor.h b/src/android/post_processor.h
> new file mode 100644
> index 0000000..a891c43
> --- /dev/null
> +++ b/src/android/post_processor.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * post_processor.h - CameraStream Post Processing Interface
> + */
> +#ifndef __ANDROID_POST_PROCESSOR_H__
> +#define __ANDROID_POST_PROCESSOR_H__
> +
> +#include <libcamera/buffer.h>
> +#include <libcamera/span.h>
> +#include <libcamera/stream.h>
> +
> +class CameraMetadata;
> +
> +class PostProcessor
> +{
> +public:
> +       virtual ~PostProcessor() {}
> +
> +       virtual int configure(const libcamera::StreamConfiguration &inCfg,
> +                             const libcamera::StreamConfiguration &outCfg) = 0;
> +       virtual int process(const libcamera::FrameBuffer *source,
> +                           const libcamera::Span<uint8_t> &destination,
> +                           CameraMetadata *metadata) = 0;
> +};
> +

nit: How about
process(const libcameraFrameBuffer&, libcamera::Span<uint8_t>, CameraMeatadata)?

I think const & is preferred to const* in this interface.
libcamera::Span looks very cheap to construct/copy/move, so that I
would like to use it as a pass-by-value parameter.
cf.) span in chromium
https://source.chromium.org/chromium/chromium/src/+/master:base/containers/span.h;l=157

Reviewed-by: Hirokazu Honda <hiroh at chromium.org>

Regards,
-Hiro

> +#endif /* __ANDROID_POST_PROCESSOR_H__ */
> --
> 2.26.2
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel