[libcamera-devel] [PATCH v3 1/8] android: camera_device: Add CameraStream::Type
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Oct 2 03:58:21 CEST 2020
Hi Jacopo,
On Wed, Sep 30, 2020 at 11:14:43AM +0200, Jacopo Mondi wrote:
> 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.
Ah I get it now. I don't mind keeping it, but I think the arrows should
be explained then. I was confused :-)
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> Thanks
>
> > > + *
> > > + *
> > > + * 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