[libcamera-devel] [PATCH v3 1/8] android: camera_device: Add CameraStream::Type
Jacopo Mondi
jacopo at jmondi.org
Wed Sep 30 11:14:43 CEST 2020
Hi Laurent,
On Tue, Sep 29, 2020 at 04:28:24AM +0300, Laurent Pinchart wrote:
> On Tue, Sep 22, 2020 at 11:47:31AM +0200, Jacopo Mondi wrote:
> > Define the CameraStream::Type enumeration and assign it to
> > each CameraStream instance at construction time.
> >
> > The CameraStream type will be used to decide if memory needs to be
> > allocated on its behalf or if the stream is backed by memory externally
> > allocated by the Android framework.
> >
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> > src/android/camera_device.cpp | 14 ++++--
> > src/android/camera_device.h | 87 ++++++++++++++++++++++++++++++++++-
> > 2 files changed, 96 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 70d77a17ef43..e4ffbc02c2da 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -169,9 +169,11 @@ MappedCamera3Buffer::MappedCamera3Buffer(const buffer_handle_t camera3buffer,
> > }
> > }
> >
> > -CameraStream::CameraStream(PixelFormat format, Size size,
> > +CameraStream::CameraStream(PixelFormat format, Size size, Type type,
> > unsigned int index, Encoder *encoder)
> > - : format_(format), size_(size), index_(index), encoder_(encoder)
> > +
> > + : format_(format), size_(size), type_(type), index_(index),
> > + encoder_(encoder)
> > {
> > }
> >
> > @@ -1222,12 +1224,14 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
> >
> > config_->addConfiguration(streamConfiguration);
> > unsigned int index = config_->size() - 1;
> > - streams_.emplace_back(format, size, index);
> > + streams_.emplace_back(format, size, CameraStream::Type::Direct,
> > + index);
> > stream->priv = static_cast<void *>(&streams_.back());
> > }
> >
> > /* Now handle the MJPEG streams, adding a new stream if required. */
> > if (jpegStream) {
> > + CameraStream::Type type;
> > int index = -1;
> >
> > /* Search for a compatible stream in the non-JPEG ones. */
> > @@ -1245,6 +1249,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
> > LOG(HAL, Info)
> > << "Android JPEG stream mapped to libcamera stream " << i;
> >
> > + type = CameraStream::Type::Mapped;
> > index = i;
> > break;
> > }
> > @@ -1269,6 +1274,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
> > LOG(HAL, Info) << "Adding " << streamConfiguration.toString()
> > << " for MJPEG support";
> >
> > + type = CameraStream::Type::Internal;
> > config_->addConfiguration(streamConfiguration);
> > index = config_->size() - 1;
> > }
> > @@ -1287,7 +1293,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
> > return ret;
> > }
> >
> > - streams_.emplace_back(formats::MJPEG, cfg.size, index, encoder);
> > + streams_.emplace_back(formats::MJPEG, cfg.size, type, index, encoder);
> > jpegStream->priv = static_cast<void *>(&streams_.back());
> > }
> >
> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> > index 1837748d2efc..9a9406cc257c 100644
> > --- a/src/android/camera_device.h
> > +++ b/src/android/camera_device.h
> > @@ -30,17 +30,102 @@ class CameraMetadata;
> > class CameraStream
> > {
> > public:
> > + /*
> > + * Enumeration of CameraStream types.
> > + *
> > + * A camera stream associates an Android stream to a libcamera stream.
> > + * This enumeration describes how the two streams are associated and how
> > + * and where data produced from libcamera are delivered to the
> > + * Android framework.
> > + *
> > + * Direct:
> > + *
> > + * The Android stream is directly mapped onto a libcamera stream: frames
> > + * are delivered by the library directly in the memory location
> > + * specified by the Android stream (buffer_handle_t->data) and provided
> > + * to the framework as they are. The Android stream characteristics are
> > + * directly translated to the libcamera stream configuration.
> > + *
> > + * +-----+ +-----+
> > + * | A | | L |
> > + * +-----+ +-----+
> > + * | |
> > + * V V
> > + * +-----+ +------+
> > + * | B |<---------------| FB |
> > + * +-----+ +------+
> > + *
> > + *
> > + * Internal:
> > + *
> > + * Data for the Android stream is produced by processing a libcamera
> > + * stream created by the HAL for that purpose. The libcamera stream
> > + * needs to be supplied with intermediate buffers where the library
> > + * delivers frames to be processed and then provided to the framework.
> > + * The libcamera stream configuration is not a direct translation of the
> > + * Android stream characteristics, but it describes the format and size
> > + * required for the processing procedure to produce frames in the
> > + * Android required format.
> > + *
> > + * +-----+ +-----+ +-----+
> > + * | A | | L |--->| FB |
> > + * +-----+ +-----+ +-----+
> > + * | | |
> > + * V V |
> > + * +-----+ +------+ |
> > + * | B | | B |<----+
> > + * +-----+ +------+
> > + * ^ |
> > + * |-------Processing------|
>
> Maybe there's something I don't get here, but isn't the processing from
> FB to B, not from B to B ? Shouldn't it look like this ?
>
> * +-----+ +-----+
> * | A | | L |
> * +-----+ +-----+
> * | |
> * V V
> * +-----+ +------+
> * | B | | FB |
> * +-----+ +------+
> * ^ |
> * |------Processing------|
Well, as we haven't really formalized what the arrow means at all, and
this diagram is just for reference, I guess we can play it as we like
the most. The original meant that that libcamera creates a framebuffer
that wraps a buffer allocated by libcamera. I can remove the right
part of the drawing if it's confusing.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Thanks
j
>
> > + *
> > + *
> > + * Mapped:
> > + *
> > + * Data for the Android stream is produced by processing a libcamera
> > + * stream associated with another CameraStream. Mapped camera streams do
> > + * not need any memory to be reserved for them as they process data
> > + * produced by libcamera for a different stream whose format and size
> > + * is compatible with the processing procedure requirements to produce
> > + * frames in the Android required format.
> > + *
> > + * +-----+ +-----+ +-----+
> > + * | A | | A' | | L |
> > + * +-----+ +-----+ +-----+
> > + * | | |
> > + * V V V
> > + * +-----+ +-----+ +------+
> > + * | B | | B' |<---------| FB |
> > + * +-----+ +-----+ +------+
> > + * ^ |
> > + * |--Processing--|
> > + *
> > + *
> > + * --------------------------------------------------------------------
> > + * A = Android stream
> > + * L = libcamera stream
> > + * B = memory buffer
> > + * FB = libcamera FrameBuffer
> > + * "Processing" = Frame processing procedure (Encoding, scaling etc)
> > + */
> > + enum class Type {
> > + Direct,
> > + Internal,
> > + Mapped,
> > + };
> > +
> > CameraStream(libcamera::PixelFormat format, libcamera::Size size,
> > - unsigned int index, Encoder *encoder = nullptr);
> > + Type type, unsigned int index, Encoder *encoder = nullptr);
> >
> > const libcamera::PixelFormat &format() const { return format_; }
> > const libcamera::Size &size() const { return size_; }
> > + Type type() const { return type_; }
> > unsigned int index() const { return index_; }
> > Encoder *encoder() const { return encoder_.get(); }
> >
> > private:
> > libcamera::PixelFormat format_;
> > libcamera::Size size_;
> > + Type type_;
> > /*
> > * The index of the libcamera StreamConfiguration as added during
> > * configureStreams(). A single libcamera Stream may be used to deliver
>
> --
> Regards,
>
> Laurent Pinchart
More information about the libcamera-devel
mailing list