[libcamera-devel] [PATCH 05/15] android: camera_device: Move processing to CameraStream
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Oct 5 14:08:37 CEST 2020
Hi Jacopo,
Thank you for the patch.
On Mon, Oct 05, 2020 at 01:46:39PM +0300, Laurent Pinchart wrote:
> From: Jacopo Mondi <jacopo at jmondi.org>
>
> Move the JPEG processing procedure to the individual CameraStream
> by augmenting the class with a CameraStream::process() method.
>
> This allows removing the CameraStream::encoder() method.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> src/android/camera_device.cpp | 68 ++---------------------------------
> src/android/camera_device.h | 8 +++++
> src/android/camera_stream.cpp | 63 ++++++++++++++++++++++++++++++++
> src/android/camera_stream.h | 7 +++-
> 4 files changed, 80 insertions(+), 66 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 2c4dd4dee28c..c44904300e0a 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -23,9 +23,6 @@
> #include "camera_metadata.h"
> #include "system/graphics.h"
>
> -#include "jpeg/encoder_libjpeg.h"
> -#include "jpeg/exif.h"
> -
> using namespace libcamera;
>
> namespace {
> @@ -133,12 +130,6 @@ const std::map<int, const Camera3Format> camera3FormatsMap = {
>
> LOG_DECLARE_CATEGORY(HAL);
>
> -class MappedCamera3Buffer : public MappedBuffer
> -{
> -public:
> - MappedCamera3Buffer(const buffer_handle_t camera3buffer, int flags);
> -};
> -
> MappedCamera3Buffer::MappedCamera3Buffer(const buffer_handle_t camera3buffer,
> int flags)
> {
> @@ -1471,12 +1462,6 @@ void CameraDevice::requestComplete(Request *request)
> if (cameraStream->outputFormat() != HAL_PIXEL_FORMAT_BLOB)
> continue;
>
> - Encoder *encoder = cameraStream->encoder();
> - if (!encoder) {
> - LOG(HAL, Error) << "Failed to identify encoder";
> - continue;
> - }
> -
> StreamConfiguration *streamConfiguration = &config_->at(cameraStream->index());
> Stream *stream = streamConfiguration->stream();
> FrameBuffer *buffer = request->findBuffer(stream);
> @@ -1497,59 +1482,12 @@ void CameraDevice::requestComplete(Request *request)
> continue;
> }
>
> - /* Set EXIF metadata for various tags. */
> - Exif exif;
> - /* \todo Set Make and Model from external vendor tags. */
> - exif.setMake("libcamera");
> - exif.setModel("cameraModel");
> - exif.setOrientation(orientation_);
> - exif.setSize(cameraStream->size());
> - /*
> - * We set the frame's EXIF timestamp as the time of encode.
> - * Since the precision we need for EXIF timestamp is only one
> - * second, it is good enough.
> - */
> - exif.setTimestamp(std::time(nullptr));
> - if (exif.generate() != 0)
> - LOG(HAL, Error) << "Failed to generate valid EXIF data";
> -
> - int jpeg_size = encoder->encode(buffer, mapped.maps()[0], exif.data());
> - if (jpeg_size < 0) {
> - LOG(HAL, Error) << "Failed to encode stream image";
> + int ret = cameraStream->process(buffer, &mapped,
> + resultMetadata.get());
> + if (ret) {
> status = CAMERA3_BUFFER_STATUS_ERROR;
> continue;
> }
> -
> - /*
> - * Fill in the JPEG blob header.
> - *
> - * The mapped size of the buffer is being returned as
> - * substantially larger than the requested JPEG_MAX_SIZE
> - * (which is referenced from maxJpegBufferSize_). Utilise
> - * this static size to ensure the correct offset of the blob is
> - * determined.
> - *
> - * \todo Investigate if the buffer size mismatch is an issue or
> - * expected behaviour.
> - */
> - uint8_t *resultPtr = mapped.maps()[0].data() +
> - maxJpegBufferSize_ -
> - sizeof(struct camera3_jpeg_blob);
> - auto *blob = reinterpret_cast<struct camera3_jpeg_blob *>(resultPtr);
> - blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;
> - blob->jpeg_size = jpeg_size;
> -
> - /* Update the JPEG result Metadata. */
> - resultMetadata->addEntry(ANDROID_JPEG_SIZE,
> - &jpeg_size, 1);
> -
> - const uint32_t jpeg_quality = 95;
> - resultMetadata->addEntry(ANDROID_JPEG_QUALITY,
> - &jpeg_quality, 1);
> -
> - const uint32_t jpeg_orientation = 0;
> - resultMetadata->addEntry(ANDROID_JPEG_ORIENTATION,
> - &jpeg_orientation, 1);
> }
>
> /* Prepare to call back the Android camera stack. */
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 4e326fa3e1fb..826023b453f7 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -20,6 +20,7 @@
> #include <libcamera/request.h>
> #include <libcamera/stream.h>
>
> +#include "libcamera/internal/buffer.h"
> #include "libcamera/internal/log.h"
> #include "libcamera/internal/message.h"
>
> @@ -28,6 +29,12 @@
>
> class CameraMetadata;
>
> +class MappedCamera3Buffer : public libcamera::MappedBuffer
> +{
> +public:
> + MappedCamera3Buffer(const buffer_handle_t camera3buffer, int flags);
> +};
> +
> class CameraDevice : protected libcamera::Loggable
> {
> public:
> @@ -50,6 +57,7 @@ public:
>
> int facing() const { return facing_; }
> int orientation() const { return orientation_; }
> + unsigned int maxJpegBufferSize() const { return maxJpegBufferSize_; }
>
> void setCallbacks(const camera3_callback_ops_t *callbacks);
> const camera_metadata_t *getStaticMetadata();
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index 5c1f1d7da416..072edc9457bb 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -8,10 +8,14 @@
> #include "camera_stream.h"
>
> #include "camera_device.h"
> +#include "camera_metadata.h"
> #include "jpeg/encoder_libjpeg.h"
> +#include "jpeg/exif.h"
>
> using namespace libcamera;
>
> +LOG_DECLARE_CATEGORY(HAL);
> +
> CameraStream::CameraStream(CameraDevice *cameraDev,
> camera3_stream_t *androidStream,
> const libcamera::StreamConfiguration &cfg,
> @@ -35,3 +39,62 @@ int CameraStream::configure(const libcamera::StreamConfiguration &cfg)
>
> return 0;
> }
> +
> +int CameraStream::process(libcamera::FrameBuffer *source, MappedCamera3Buffer *dest,
> + CameraMetadata *metadata)
> +{
> + if (!encoder_)
> + return -EINVAL;
Maybe that's addressed by subsequent patches in this series, but down
the line I would envision process() being called for all CameraStream
instances, so if no processing is required, this should then return 0.
> +
> + /* Set EXIF metadata for various tags. */
> + Exif exif;
> + /* \todo Set Make and Model from external vendor tags. */
> + exif.setMake("libcamera");
> + exif.setModel("cameraModel");
> + exif.setOrientation(cameraDevice_->orientation());
> + exif.setSize(size_);
> + /*
> + * We set the frame's EXIF timestamp as the time of encode.
> + * Since the precision we need for EXIF timestamp is only one
> + * second, it is good enough.
> + */
> + exif.setTimestamp(std::time(nullptr));
> + if (exif.generate() != 0)
> + LOG(HAL, Error) << "Failed to generate valid EXIF data";
> +
> + int jpeg_size = encoder_->encode(source, dest->maps()[0], exif.data());
> + if (jpeg_size < 0) {
> + LOG(HAL, Error) << "Failed to encode stream image";
> + return jpeg_size;
> + }
> +
> + /*
> + * Fill in the JPEG blob header.
> + *
> + * The mapped size of the buffer is being returned as
> + * substantially larger than the requested JPEG_MAX_SIZE
> + * (which is referenced from maxJpegBufferSize_). Utilise
> + * this static size to ensure the correct offset of the blob is
> + * determined.
> + *
> + * \todo Investigate if the buffer size mismatch is an issue or
> + * expected behaviour.
> + */
> + uint8_t *resultPtr = dest->maps()[0].data() +
> + cameraDevice_->maxJpegBufferSize() -
> + sizeof(struct camera3_jpeg_blob);
> + auto *blob = reinterpret_cast<struct camera3_jpeg_blob *>(resultPtr);
> + blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;
> + blob->jpeg_size = jpeg_size;
> +
> + /* Update the JPEG result Metadata. */
> + metadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);
> +
> + const uint32_t jpeg_quality = 95;
> + metadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);
> +
> + const uint32_t jpeg_orientation = 0;
> + metadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);
> +
> + return 0;
> +}
> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
> index 2d71a50c78a4..dbcd1e53219f 100644
> --- a/src/android/camera_stream.h
> +++ b/src/android/camera_stream.h
> @@ -11,6 +11,7 @@
>
> #include <hardware/camera3.h>
>
> +#include <libcamera/buffer.h>
> #include <libcamera/camera.h>
> #include <libcamera/geometry.h>
> #include <libcamera/pixel_format.h>
> @@ -18,6 +19,8 @@
> #include "jpeg/encoder.h"
>
> class CameraDevice;
> +class CameraMetadata;
> +class MappedCamera3Buffer;
>
> class CameraStream
> {
> @@ -114,9 +117,11 @@ public:
> const libcamera::Size &size() const { return size_; }
> Type type() const { return type_; }
> unsigned int index() const { return index_; }
> - Encoder *encoder() const { return encoder_.get(); }
>
> int configure(const libcamera::StreamConfiguration &cfg);
> + int process(libcamera::FrameBuffer *source,
Can source be const ?
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> + MappedCamera3Buffer *dest,
> + CameraMetadata *metadata);
>
> private:
> CameraDevice *cameraDevice_;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list