| Message ID | 20200922094738.5327-2-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@ideasonboard.com> > Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > Signed-off-by: Jacopo Mondi <jacopo@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------| Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + * > + * > + * 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
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@ideasonboard.com> > > Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > Signed-off-by: Jacopo Mondi <jacopo@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@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
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@ideasonboard.com> > > > Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > Signed-off-by: Jacopo Mondi <jacopo@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@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
Hi Laurent, On Fri, Oct 02, 2020 at 04:58:21AM +0300, Laurent Pinchart wrote: > 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@ideasonboard.com> > > > > Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > > > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > > Signed-off-by: Jacopo Mondi <jacopo@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 :-) No worries, I've adopted what you have proposed in v4 which I sent to the list! Thanks j > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@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
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------| + * + * + * 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