[{"id":12976,"web_url":"https://patchwork.libcamera.org/comment/12976/","msgid":"<20201005121522.GI3931@pendragon.ideasonboard.com>","date":"2020-10-05T12:15:22","subject":"Re: [libcamera-devel] [PATCH 07/15] android: camera_stream: Fetch\n\tformat and size from configuration","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Mon, Oct 05, 2020 at 01:46:41PM +0300, Laurent Pinchart wrote:\n> From: Jacopo Mondi \n> \n> Fetch the format and size of the libcamera::StreamConfiguration\n> associated with a CameraStream by accessing the configuration by\n> index.\n> \n> This removes the need to store the libcamera stream format and sizes\n> as class members and avoid duplicating information that might get out\n> of sync.\n> \n> It also allows to remove the StreamConfiguration from the constructor\n> parameters list, as it can be identified by its index. While at it,\n> re-order the constructor parameters order.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_device.cpp | 8 +++-----\n> src/android/camera_stream.cpp | 23 ++++++++++++++---------\n> src/android/camera_stream.h | 18 ++++++------------\n> 3 files changed, 23 insertions(+), 26 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 4f8f3e5790ca..adaec54dbfdb 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1206,9 +1206,8 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \t\tstreamConfiguration.pixelFormat = format;\n> \n> \t\tconfig_->addConfiguration(streamConfiguration);\n> -\t\tunsigned int index = config_->size() - 1;\n> -\t\tstreams_.emplace_back(this, stream, streamConfiguration,\n> -\t\t\t\t CameraStream::Type::Direct, index);\n> +\t\tstreams_.emplace_back(this, CameraStream::Type::Direct,\n> +\t\t\t\t stream, config_->size() - 1);\n> \t\tstream->priv = static_cast(&streams_.back());\n> \t}\n> \n> @@ -1262,8 +1261,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \t\t\tindex = config_->size() - 1;\n> \t\t}\n> \n> -\t\tStreamConfiguration &cfg = config_->at(index);\n> -\t\tstreams_.emplace_back(this, jpegStream, cfg, type, index);\n> +\t\tstreams_.emplace_back(this, type, jpegStream, index);\n> \t\tjpegStream->priv = static_cast(&streams_.back());\n> \t}\n> \n> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> index 250f0ab0a3b4..6e7419ae31b8 100644\n> --- a/src/android/camera_stream.cpp\n> +++ b/src/android/camera_stream.cpp\n> @@ -16,18 +16,13 @@ using namespace libcamera;\n> \n> LOG_DECLARE_CATEGORY(HAL);\n> \n> -CameraStream::CameraStream(CameraDevice *cameraDev,\n> -\t\t\t camera3_stream_t *androidStream,\n> -\t\t\t const libcamera::StreamConfiguration &cfg,\n> -\t\t\t Type type, unsigned int index)\n> -\t: cameraDevice_(cameraDev), androidStream_(androidStream), type_(type),\n> +CameraStream::CameraStream(CameraDevice *cameraDev, Type type,\n> +\t\t\t camera3_stream_t *androidStream, unsigned int index)\n> +\t: cameraDevice_(cameraDev), type_(type), androidStream_(androidStream),\n> \t index_(index), encoder_(nullptr)\n> {\n> \tconfig_ = cameraDevice_->cameraConfiguration();\n> \n> -\tformat_ = cfg.pixelFormat;\n> -\tsize_ = cfg.size;\n> -\n> \tif (type_ == Type::Internal || type_ == Type::Mapped)\n> \t\tencoder_.reset(new EncoderLibJpeg);\n> }\n> @@ -42,6 +37,16 @@ Stream *CameraStream::stream() const\n> \treturn streamConfiguration().stream();\n> }\n> \n> +const PixelFormat &CameraStream::format() const\n> +{\n> +\treturn streamConfiguration().pixelFormat;\n> +}\n> +\n> +const Size &CameraStream::size() const\n> +{\n> +\treturn streamConfiguration().size;\n> +}\n> +\n> int CameraStream::configure(const libcamera::StreamConfiguration &cfg)\n> {\n> \tif (encoder_)\n> @@ -62,7 +67,7 @@ int CameraStream::process(libcamera::FrameBuffer *source, MappedCamera3Buffer *d\n> \texif.setMake(\"libcamera\");\n> \texif.setModel(\"cameraModel\");\n> \texif.setOrientation(cameraDevice_->orientation());\n> -\texif.setSize(size_);\n> +\texif.setSize(size());\n> \t/*\n> \t * We set the frame's EXIF timestamp as the time of encode.\n> \t * Since the precision we need for EXIF timestamp is only one\n> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> index fa295a69404f..b67b4c0ca0b3 100644\n> --- a/src/android/camera_stream.h\n> +++ b/src/android/camera_stream.h\n> @@ -107,18 +107,16 @@ public:\n> \t\tInternal,\n> \t\tMapped,\n> \t};\n> -\tCameraStream(CameraDevice *cameraDev,\n> -\t\t camera3_stream_t *androidStream,\n> -\t\t const libcamera::StreamConfiguration &cfg,\n> -\t\t Type type, unsigned int index);\n> +\tCameraStream(CameraDevice *cameraDev, Type type,\n> +\t\t camera3_stream_t *androidStream, unsigned int index);\n> \n> \tint outputFormat() const { return androidStream_->format; }\n> -\tconst libcamera::PixelFormat &format() const { return format_; }\n> -\tconst libcamera::Size &size() const { return size_; }\n> \tType type() const { return type_; }\n> \n> \tconst libcamera::StreamConfiguration &streamConfiguration() const;\n> \tlibcamera::Stream *stream() const;\n> +\tconst libcamera::PixelFormat &format() const;\n> +\tconst libcamera::Size &size() const;\n\nThis is a bit ambiguous. These functions return the format and size of\nthe libcamera stream, which may differ from the format and size of the\nAndroid stream. I wonder if we shouldn't use\n->streamConfiguration().pixelFormat (and the same for size) in the\ncaller instead, to make it more explicit.\n\n> \n> \tint configure(const libcamera::StreamConfiguration &cfg);\n> \tint process(libcamera::FrameBuffer *source,\n> @@ -127,13 +125,8 @@ public:\n> \n> private:\n> \tCameraDevice *cameraDevice_;\n> -\tlibcamera::CameraConfiguration *config_;\n> -\tcamera3_stream_t *androidStream_;\n> \tType type_;\n> -\n> -\t/* Libcamera facing format and sizes. */\n> -\tlibcamera::PixelFormat format_;\n> -\tlibcamera::Size size_;\n> +\tcamera3_stream_t *androidStream_;\n> \n> \t/*\n> \t * The index of the libcamera StreamConfiguration as added during\n> @@ -141,6 +134,7 @@ private:\n> \t * one or more streams to the Android framework.\n> \t */\n> \tunsigned int index_;\n> +\tlibcamera::CameraConfiguration *config_;\n> \tstd::unique_ptr encoder_;\n> };\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id CDFE7C3B5D\n\tfor ;\n\tMon, 5 Oct 2020 12:16:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5FAF263BE4;\n\tMon, 5 Oct 2020 14:16:03 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5D92263BD5\n\tfor ;\n\tMon, 5 Oct 2020 14:16:01 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C85B23B\n\tfor ;\n\tMon, 5 Oct 2020 14:16:00 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Q+bkEN4b\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1601900161;\n\tbh=doVnFm4R8MzAXDURa8YbVTaeqkqu4fiCpX5wMlTEYFg=;\n\th=Date:From:To:Subject:References:In-Reply-To:From;\n\tb=Q+bkEN4bmvi2+awr+hR/vkWkjSkV9rS811GufIsBsFA33tvjCNz3RFoB4Phm9+/uU\n\tq4GdADuJohwn1ohC3fvvBma9w3+CJOsy70EAkm3G7rMGk7Gx1j/9TyPxJBZpVcwCXY\n\tRzPXB37Jkf3RtUcGq5Ap/OwMFHQi40qgbtPTXvNI=","Date":"Mon, 5 Oct 2020 15:15:22 +0300","From":"Laurent Pinchart ","To":"libcamera-devel@lists.libcamera.org","Message-ID":"<20201005121522.GI3931@pendragon.ideasonboard.com>","References":"<20201005104649.10812-1-laurent.pinchart@ideasonboard.com>\n\t<20201005104649.10812-8-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201005104649.10812-8-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 07/15] android: camera_stream: Fetch\n\tformat and size from configuration","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]