[libcamera-devel] [PATCH 07/15] android: camera_stream: Fetch format and size from configuration
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Oct 5 14:15:22 CEST 2020
Hi Jacopo,
Thank you for the patch.
On Mon, Oct 05, 2020 at 01:46:41PM +0300, Laurent Pinchart wrote:
> From: Jacopo Mondi <jacopo at jmondi.org>
>
> Fetch the format and size of the libcamera::StreamConfiguration
> associated with a CameraStream by accessing the configuration by
> index.
>
> This removes the need to store the libcamera stream format and sizes
> as class members and avoid duplicating information that might get out
> of sync.
>
> It also allows to remove the StreamConfiguration from the constructor
> parameters list, as it can be identified by its index. While at it,
> re-order the constructor parameters order.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> src/android/camera_device.cpp | 8 +++-----
> src/android/camera_stream.cpp | 23 ++++++++++++++---------
> src/android/camera_stream.h | 18 ++++++------------
> 3 files changed, 23 insertions(+), 26 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 4f8f3e5790ca..adaec54dbfdb 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1206,9 +1206,8 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
> streamConfiguration.pixelFormat = format;
>
> config_->addConfiguration(streamConfiguration);
> - unsigned int index = config_->size() - 1;
> - streams_.emplace_back(this, stream, streamConfiguration,
> - CameraStream::Type::Direct, index);
> + streams_.emplace_back(this, CameraStream::Type::Direct,
> + stream, config_->size() - 1);
> stream->priv = static_cast<void *>(&streams_.back());
> }
>
> @@ -1262,8 +1261,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
> index = config_->size() - 1;
> }
>
> - StreamConfiguration &cfg = config_->at(index);
> - streams_.emplace_back(this, jpegStream, cfg, type, index);
> + streams_.emplace_back(this, type, jpegStream, index);
> jpegStream->priv = static_cast<void *>(&streams_.back());
> }
>
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index 250f0ab0a3b4..6e7419ae31b8 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -16,18 +16,13 @@ using namespace libcamera;
>
> LOG_DECLARE_CATEGORY(HAL);
>
> -CameraStream::CameraStream(CameraDevice *cameraDev,
> - camera3_stream_t *androidStream,
> - const libcamera::StreamConfiguration &cfg,
> - Type type, unsigned int index)
> - : cameraDevice_(cameraDev), androidStream_(androidStream), type_(type),
> +CameraStream::CameraStream(CameraDevice *cameraDev, Type type,
> + camera3_stream_t *androidStream, unsigned int index)
> + : cameraDevice_(cameraDev), type_(type), androidStream_(androidStream),
> index_(index), encoder_(nullptr)
> {
> config_ = cameraDevice_->cameraConfiguration();
>
> - format_ = cfg.pixelFormat;
> - size_ = cfg.size;
> -
> if (type_ == Type::Internal || type_ == Type::Mapped)
> encoder_.reset(new EncoderLibJpeg);
> }
> @@ -42,6 +37,16 @@ Stream *CameraStream::stream() const
> return streamConfiguration().stream();
> }
>
> +const PixelFormat &CameraStream::format() const
> +{
> + return streamConfiguration().pixelFormat;
> +}
> +
> +const Size &CameraStream::size() const
> +{
> + return streamConfiguration().size;
> +}
> +
> int CameraStream::configure(const libcamera::StreamConfiguration &cfg)
> {
> if (encoder_)
> @@ -62,7 +67,7 @@ int CameraStream::process(libcamera::FrameBuffer *source, MappedCamera3Buffer *d
> exif.setMake("libcamera");
> exif.setModel("cameraModel");
> exif.setOrientation(cameraDevice_->orientation());
> - exif.setSize(size_);
> + 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
> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
> index fa295a69404f..b67b4c0ca0b3 100644
> --- a/src/android/camera_stream.h
> +++ b/src/android/camera_stream.h
> @@ -107,18 +107,16 @@ public:
> Internal,
> Mapped,
> };
> - CameraStream(CameraDevice *cameraDev,
> - camera3_stream_t *androidStream,
> - const libcamera::StreamConfiguration &cfg,
> - Type type, unsigned int index);
> + CameraStream(CameraDevice *cameraDev, Type type,
> + camera3_stream_t *androidStream, unsigned int index);
>
> int outputFormat() const { return androidStream_->format; }
> - const libcamera::PixelFormat &format() const { return format_; }
> - const libcamera::Size &size() const { return size_; }
> Type type() const { return type_; }
>
> const libcamera::StreamConfiguration &streamConfiguration() const;
> libcamera::Stream *stream() const;
> + const libcamera::PixelFormat &format() const;
> + const libcamera::Size &size() const;
This is a bit ambiguous. These functions return the format and size of
the libcamera stream, which may differ from the format and size of the
Android stream. I wonder if we shouldn't use
->streamConfiguration().pixelFormat (and the same for size) in the
caller instead, to make it more explicit.
>
> int configure(const libcamera::StreamConfiguration &cfg);
> int process(libcamera::FrameBuffer *source,
> @@ -127,13 +125,8 @@ public:
>
> private:
> CameraDevice *cameraDevice_;
> - libcamera::CameraConfiguration *config_;
> - camera3_stream_t *androidStream_;
> Type type_;
> -
> - /* Libcamera facing format and sizes. */
> - libcamera::PixelFormat format_;
> - libcamera::Size size_;
> + camera3_stream_t *androidStream_;
>
> /*
> * The index of the libcamera StreamConfiguration as added during
> @@ -141,6 +134,7 @@ private:
> * one or more streams to the Android framework.
> */
> unsigned int index_;
> + libcamera::CameraConfiguration *config_;
> std::unique_ptr<Encoder> encoder_;
> };
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list