[{"id":13027,"web_url":"https://patchwork.libcamera.org/comment/13027/","msgid":"<20201007015320.GE30598@pendragon.ideasonboard.com>","date":"2020-10-07T01:53:20","subject":"Re: [libcamera-devel] [PATCH v2 10/13] android: camera_stream:\n\tCreate buffer poll","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 Tue, Oct 06, 2020 at 04:44:29PM +0200, Jacopo Mondi wrote:\n> Add a FrameBufferAllocator class member to the CameraStream class.\n> The allocator is constructed for CameraStream instances that needs\n> internal allocation and automatically deleted.\n> \n> Allocate FrameBuffers using the allocator_ class member in the\n> CameraStream class at CameraStream::configure() time and add two\n> methods to the CameraStream class to get and put FrameBuffer pointers\n> from the pool of allocated buffers. As buffer allocation can take place\n> only after the Camera has been configured, move the CameraStream\n> configuration loop in the CameraDevice class after camera_->configure()\n> call.\n> \n> The newly created pool will be used to provide buffers to CameraStream\n> that need to provide memory to libcamera where to deliver frames.\n> \n> Reviewed-by: Kieran Bingham \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_device.cpp | 22 +++++++++---------\n> src/android/camera_stream.cpp | 43 +++++++++++++++++++++++++++++++++++\n> src/android/camera_stream.h | 9 ++++++++\n> 3 files changed, 63 insertions(+), 11 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 1e2cbeeb92d1..ecdc0922e90b 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1282,6 +1282,17 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \t\treturn -EINVAL;\n> \t}\n> \n> +\t/*\n> +\t * Once the CameraConfiguration has been adjusted/validated\n> +\t * it can be applied to the camera.\n> +\t */\n> +\tint ret = camera_->configure(config_.get());\n> +\tif (ret) {\n> +\t\tLOG(HAL, Error) << \"Failed to configure camera '\"\n> +\t\t\t\t<< camera_->id() << \"'\";\n> +\t\treturn ret;\n> +\t}\n> +\n> \t/*\n> \t * Configure the HAL CameraStream instances using the associated\n> \t * StreamConfiguration and set the number of required buffers in\n> @@ -1295,17 +1306,6 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \t\t}\n> \t}\n> \n> -\t/*\n> -\t * Once the CameraConfiguration has been adjusted/validated\n> -\t * it can be applied to the camera.\n> -\t */\n> -\tint ret = camera_->configure(config_.get());\n> -\tif (ret) {\n> -\t\tLOG(HAL, Error) << \"Failed to configure camera '\"\n> -\t\t\t\t<< camera_->id() << \"'\";\n> -\t\treturn ret;\n> -\t}\n> -\n> \treturn 0;\n> }\n> \n> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> index f899be4fe007..eac1480530f8 100644\n> --- a/src/android/camera_stream.cpp\n> +++ b/src/android/camera_stream.cpp\n> @@ -26,6 +26,11 @@ CameraStream::CameraStream(CameraDevice *cameraDevice, Type type,\n> \n> \tif (type_ == Type::Internal || type_ == Type::Mapped)\n> \t\tencoder_ = std::make_unique();\n> +\n> +\tif (type == Type::Internal) {\n> +\t\tallocator_ = std::make_unique(cameraDevice_->camera());\n> +\t\tmutex_ = std::make_unique();\n\nThis is a bit annoying. It comes from std::vector::reserve()\nrequiring T to be MoveInsertable, even if no move will actually take\nplace as we always reserve entries starting from an empty vector (but\nthe compiler can't known that).\n\nI think it would be worth using std::list instead of std::vector to\nstore the CameraStream instances in CameraDevice, and embedding the\nmutex. You'll need to drop the reserve() call as std::list doesn't have\nthat (and update the comment before it accordingly).\n\nActually I can post the diff as I made the modifications to ensure it\nwould work :-)\n\ndiff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\nindex edac9f28ab67..5a466629b78b 100644\n--- a/src/android/camera_device.cpp\n+++ b/src/android/camera_device.cpp\n@@ -1165,13 +1165,8 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n \t\treturn -EINVAL;\n \t}\n\n-\t/*\n-\t * Clear and remove any existing configuration from previous calls, and\n-\t * ensure the required entries are available without further\n-\t * reallocation.\n-\t */\n+\t/* Clear and remove any existing configuration from previous calls. */\n \tstreams_.clear();\n-\tstreams_.reserve(stream_list->num_streams);\n\n \t/* First handle all non-MJPEG streams. */\n \tcamera3_stream_t *jpegStream = nullptr;\ndiff --git a/src/android/camera_device.h b/src/android/camera_device.h\nindex b4b32f77a29a..d1412d8d5fb8 100644\n--- a/src/android/camera_device.h\n+++ b/src/android/camera_device.h\n@@ -7,6 +7,7 @@\n #ifndef __ANDROID_CAMERA_DEVICE_H__\n #define __ANDROID_CAMERA_DEVICE_H__\n\n+#include \n #include \n #include \n #include \n@@ -121,7 +122,7 @@ private:\n\n \tstd::vector streamConfigurations_;\n \tstd::map formatsMap_;\n-\tstd::vector streams_;\n+\tstd::list streams_;\n\n \tint facing_;\n \tint orientation_;\ndiff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\nindex eac1480530f8..707c4a5e1077 100644\n--- a/src/android/camera_stream.cpp\n+++ b/src/android/camera_stream.cpp\n@@ -27,10 +27,8 @@ CameraStream::CameraStream(CameraDevice *cameraDevice, Type type,\n \tif (type_ == Type::Internal || type_ == Type::Mapped)\n \t\tencoder_ = std::make_unique();\n\n-\tif (type == Type::Internal) {\n+\tif (type == Type::Internal)\n \t\tallocator_ = std::make_unique(cameraDevice_->camera());\n-\t\tmutex_ = std::make_unique();\n-\t}\n }\n\n const StreamConfiguration &CameraStream::configuration() const\n@@ -130,7 +128,7 @@ FrameBuffer *CameraStream::getBuffer()\n \tif (!allocator_)\n \t\treturn nullptr;\n\n-\tstd::lock_guard locker(*mutex_);\n+\tstd::lock_guard locker(mutex_);\n\n \tif (buffers_.empty()) {\n \t\tLOG(HAL, Error) << \"Buffer underrun\";\n@@ -148,7 +146,7 @@ void CameraStream::putBuffer(libcamera::FrameBuffer *buffer)\n \tif (!allocator_)\n \t\treturn;\n\n-\tstd::lock_guard locker(*mutex_);\n+\tstd::lock_guard locker(mutex_);\n\n \tbuffers_.push_back(buffer);\n }\ndiff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\nindex f929e8260ae3..a177a99a2ea1 100644\n--- a/src/android/camera_stream.h\n+++ b/src/android/camera_stream.h\n@@ -138,7 +138,7 @@ private:\n \tstd::unique_ptr encoder_;\n \tstd::unique_ptr allocator_;\n \tstd::vector buffers_;\n-\tstd::unique_ptr mutex_;\n+\tstd::mutex mutex_;\n };\n\n #endif /* __ANDROID_CAMERA_STREAM__ */\n\nThis change can go on top or be integrated in the series (and there's no\nneed to report just for this), up to you.\n\nReviewed-by: Laurent Pinchart \n\n> +\t}\n> }\n> \n> const StreamConfiguration &CameraStream::configuration() const\n> @@ -46,6 +51,16 @@ int CameraStream::configure()\n> \t\t\treturn ret;\n> \t}\n> \n> +\tif (allocator_) {\n> +\t\tint ret = allocator_->allocate(stream());\n> +\t\tif (ret < 0)\n> +\t\t\treturn ret;\n> +\n> +\t\t/* Save a pointer to the reserved frame buffers */\n> +\t\tfor (const auto &frameBuffer : allocator_->buffers(stream()))\n> +\t\t\tbuffers_.push_back(frameBuffer.get());\n> +\t}\n> +\n> \tcamera3Stream_->max_buffers = configuration().bufferCount;\n> \n> \treturn 0;\n> @@ -109,3 +124,31 @@ int CameraStream::process(const libcamera::FrameBuffer &source,\n> \n> \treturn 0;\n> }\n> +\n> +FrameBuffer *CameraStream::getBuffer()\n> +{\n> +\tif (!allocator_)\n> +\t\treturn nullptr;\n> +\n> +\tstd::lock_guard locker(*mutex_);\n> +\n> +\tif (buffers_.empty()) {\n> +\t\tLOG(HAL, Error) << \"Buffer underrun\";\n> +\t\treturn nullptr;\n> +\t}\n> +\n> +\tFrameBuffer *buffer = buffers_.back();\n> +\tbuffers_.pop_back();\n> +\n> +\treturn buffer;\n> +}\n> +\n> +void CameraStream::putBuffer(libcamera::FrameBuffer *buffer)\n> +{\n> +\tif (!allocator_)\n> +\t\treturn;\n> +\n> +\tstd::lock_guard locker(*mutex_);\n> +\n> +\tbuffers_.push_back(buffer);\n> +}\n> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> index 4c51f0fb3393..f929e8260ae3 100644\n> --- a/src/android/camera_stream.h\n> +++ b/src/android/camera_stream.h\n> @@ -8,11 +8,14 @@\n> #define __ANDROID_CAMERA_STREAM_H__\n> \n> #include \n> +#include \n> +#include \n> \n> #include \n> \n> #include \n> #include \n> +#include \n> #include \n> #include \n> \n> @@ -117,6 +120,8 @@ public:\n> \tint configure();\n> \tint process(const libcamera::FrameBuffer &source,\n> \t\t MappedCamera3Buffer *dest, CameraMetadata *metadata);\n> +\tlibcamera::FrameBuffer *getBuffer();\n> +\tvoid putBuffer(libcamera::FrameBuffer *buffer);\n> \n> private:\n> \tCameraDevice *cameraDevice_;\n> @@ -129,7 +134,11 @@ private:\n> \t * one or more streams to the Android framework.\n> \t */\n> \tunsigned int index_;\n> +\n> \tstd::unique_ptr encoder_;\n> +\tstd::unique_ptr allocator_;\n> +\tstd::vector buffers_;\n> +\tstd::unique_ptr mutex_;\n> };\n> \n> #endif /* __ANDROID_CAMERA_STREAM__ */","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 A427FBEEE0\n\tfor ;\n\tWed, 7 Oct 2020 01:54:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2DCB263C1B;\n\tWed, 7 Oct 2020 03:54:05 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7302063BD5\n\tfor ;\n\tWed, 7 Oct 2020 03:54:03 +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 E014D59E;\n\tWed, 7 Oct 2020 03:54:02 +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=\"DaH/7M4N\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1602035643;\n\tbh=Fk+ZznwaSsTFAFDgpqLEFn4ST0KDLwCmzoXmK24BZ2c=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=DaH/7M4NSwIbKueqJ5NCZLN8fOdnaWtxszaBgXcsui3DXNqRyyUpcALphapeSrJpL\n\tu42UHF+IpMgor9Dvu54q83VO3iWRjsl8RUsjZ71OZ7IkGxk0N4TBtc1uFK9WjeOV0F\n\tg375Y5MrHTMiTggQpaBS+s5pNbz8qg21/D+LBS7M=","Date":"Wed, 7 Oct 2020 04:53:20 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20201007015320.GE30598@pendragon.ideasonboard.com>","References":"<20201006144432.22908-1-jacopo@jmondi.org>\n\t<20201006144432.22908-11-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201006144432.22908-11-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 10/13] android: camera_stream:\n\tCreate buffer poll","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13031,"web_url":"https://patchwork.libcamera.org/comment/13031/","msgid":"<20201007080532.h33ufwpl72kvi2ii@uno.localdomain>","date":"2020-10-07T08:05:32","subject":"Re: [libcamera-devel] [PATCH v2 10/13] android: camera_stream:\n\tCreate buffer poll","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Wed, Oct 07, 2020 at 04:53:20AM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Tue, Oct 06, 2020 at 04:44:29PM +0200, Jacopo Mondi wrote:\n> > Add a FrameBufferAllocator class member to the CameraStream class.\n> > The allocator is constructed for CameraStream instances that needs\n> > internal allocation and automatically deleted.\n> >\n> > Allocate FrameBuffers using the allocator_ class member in the\n> > CameraStream class at CameraStream::configure() time and add two\n> > methods to the CameraStream class to get and put FrameBuffer pointers\n> > from the pool of allocated buffers. As buffer allocation can take place\n> > only after the Camera has been configured, move the CameraStream\n> > configuration loop in the CameraDevice class after camera_->configure()\n> > call.\n> >\n> > The newly created pool will be used to provide buffers to CameraStream\n> > that need to provide memory to libcamera where to deliver frames.\n> >\n> > Reviewed-by: Kieran Bingham \n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/android/camera_device.cpp | 22 +++++++++---------\n> > src/android/camera_stream.cpp | 43 +++++++++++++++++++++++++++++++++++\n> > src/android/camera_stream.h | 9 ++++++++\n> > 3 files changed, 63 insertions(+), 11 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 1e2cbeeb92d1..ecdc0922e90b 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -1282,6 +1282,17 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > \t\treturn -EINVAL;\n> > \t}\n> >\n> > +\t/*\n> > +\t * Once the CameraConfiguration has been adjusted/validated\n> > +\t * it can be applied to the camera.\n> > +\t */\n> > +\tint ret = camera_->configure(config_.get());\n> > +\tif (ret) {\n> > +\t\tLOG(HAL, Error) << \"Failed to configure camera '\"\n> > +\t\t\t\t<< camera_->id() << \"'\";\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > \t/*\n> > \t * Configure the HAL CameraStream instances using the associated\n> > \t * StreamConfiguration and set the number of required buffers in\n> > @@ -1295,17 +1306,6 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > \t\t}\n> > \t}\n> >\n> > -\t/*\n> > -\t * Once the CameraConfiguration has been adjusted/validated\n> > -\t * it can be applied to the camera.\n> > -\t */\n> > -\tint ret = camera_->configure(config_.get());\n> > -\tif (ret) {\n> > -\t\tLOG(HAL, Error) << \"Failed to configure camera '\"\n> > -\t\t\t\t<< camera_->id() << \"'\";\n> > -\t\treturn ret;\n> > -\t}\n> > -\n> > \treturn 0;\n> > }\n> >\n> > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> > index f899be4fe007..eac1480530f8 100644\n> > --- a/src/android/camera_stream.cpp\n> > +++ b/src/android/camera_stream.cpp\n> > @@ -26,6 +26,11 @@ CameraStream::CameraStream(CameraDevice *cameraDevice, Type type,\n> >\n> > \tif (type_ == Type::Internal || type_ == Type::Mapped)\n> > \t\tencoder_ = std::make_unique();\n> > +\n> > +\tif (type == Type::Internal) {\n> > +\t\tallocator_ = std::make_unique(cameraDevice_->camera());\n> > +\t\tmutex_ = std::make_unique();\n>\n> This is a bit annoying. It comes from std::vector::reserve()\n> requiring T to be MoveInsertable, even if no move will actually take\n> place as we always reserve entries starting from an empty vector (but\n> the compiler can't known that).\n>\n> I think it would be worth using std::list instead of std::vector to\n> store the CameraStream instances in CameraDevice, and embedding the\n> mutex. You'll need to drop the reserve() call as std::list doesn't have\n> that (and update the comment before it accordingly).\n>\n> Actually I can post the diff as I made the modifications to ensure it\n> would work :-)\n\nWe can indeed take this on top, but I wonder how that works as\nstd::list documentation reports:\n\"T must meet the requirements of CopyAssignable and CopyConstructible.\"\n\nAnd CameraStream is not CopyConstructable as it contains unique_ptr<>\ninstances...\n\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index edac9f28ab67..5a466629b78b 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1165,13 +1165,8 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \t\treturn -EINVAL;\n> \t}\n>\n> -\t/*\n> -\t * Clear and remove any existing configuration from previous calls, and\n> -\t * ensure the required entries are available without further\n> -\t * reallocation.\n> -\t */\n> +\t/* Clear and remove any existing configuration from previous calls. */\n> \tstreams_.clear();\n> -\tstreams_.reserve(stream_list->num_streams);\n>\n> \t/* First handle all non-MJPEG streams. */\n> \tcamera3_stream_t *jpegStream = nullptr;\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index b4b32f77a29a..d1412d8d5fb8 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -7,6 +7,7 @@\n> #ifndef __ANDROID_CAMERA_DEVICE_H__\n> #define __ANDROID_CAMERA_DEVICE_H__\n>\n> +#include \n> #include \n> #include \n> #include \n> @@ -121,7 +122,7 @@ private:\n>\n> \tstd::vector streamConfigurations_;\n> \tstd::map formatsMap_;\n> -\tstd::vector streams_;\n> +\tstd::list streams_;\n>\n> \tint facing_;\n> \tint orientation_;\n> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> index eac1480530f8..707c4a5e1077 100644\n> --- a/src/android/camera_stream.cpp\n> +++ b/src/android/camera_stream.cpp\n> @@ -27,10 +27,8 @@ CameraStream::CameraStream(CameraDevice *cameraDevice, Type type,\n> \tif (type_ == Type::Internal || type_ == Type::Mapped)\n> \t\tencoder_ = std::make_unique();\n>\n> -\tif (type == Type::Internal) {\n> +\tif (type == Type::Internal)\n> \t\tallocator_ = std::make_unique(cameraDevice_->camera());\n> -\t\tmutex_ = std::make_unique();\n> -\t}\n> }\n>\n> const StreamConfiguration &CameraStream::configuration() const\n> @@ -130,7 +128,7 @@ FrameBuffer *CameraStream::getBuffer()\n> \tif (!allocator_)\n> \t\treturn nullptr;\n>\n> -\tstd::lock_guard locker(*mutex_);\n> +\tstd::lock_guard locker(mutex_);\n>\n> \tif (buffers_.empty()) {\n> \t\tLOG(HAL, Error) << \"Buffer underrun\";\n> @@ -148,7 +146,7 @@ void CameraStream::putBuffer(libcamera::FrameBuffer *buffer)\n> \tif (!allocator_)\n> \t\treturn;\n>\n> -\tstd::lock_guard locker(*mutex_);\n> +\tstd::lock_guard locker(mutex_);\n>\n> \tbuffers_.push_back(buffer);\n> }\n> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> index f929e8260ae3..a177a99a2ea1 100644\n> --- a/src/android/camera_stream.h\n> +++ b/src/android/camera_stream.h\n> @@ -138,7 +138,7 @@ private:\n> \tstd::unique_ptr encoder_;\n> \tstd::unique_ptr allocator_;\n> \tstd::vector buffers_;\n> -\tstd::unique_ptr mutex_;\n> +\tstd::mutex mutex_;\n> };\n>\n> #endif /* __ANDROID_CAMERA_STREAM__ */\n>\n> This change can go on top or be integrated in the series (and there's no\n> need to report just for this), up to you.\n>\n> Reviewed-by: Laurent Pinchart \n>\n> > +\t}\n> > }\n> >\n> > const StreamConfiguration &CameraStream::configuration() const\n> > @@ -46,6 +51,16 @@ int CameraStream::configure()\n> > \t\t\treturn ret;\n> > \t}\n> >\n> > +\tif (allocator_) {\n> > +\t\tint ret = allocator_->allocate(stream());\n> > +\t\tif (ret < 0)\n> > +\t\t\treturn ret;\n> > +\n> > +\t\t/* Save a pointer to the reserved frame buffers */\n> > +\t\tfor (const auto &frameBuffer : allocator_->buffers(stream()))\n> > +\t\t\tbuffers_.push_back(frameBuffer.get());\n> > +\t}\n> > +\n> > \tcamera3Stream_->max_buffers = configuration().bufferCount;\n> >\n> > \treturn 0;\n> > @@ -109,3 +124,31 @@ int CameraStream::process(const libcamera::FrameBuffer &source,\n> >\n> > \treturn 0;\n> > }\n> > +\n> > +FrameBuffer *CameraStream::getBuffer()\n> > +{\n> > +\tif (!allocator_)\n> > +\t\treturn nullptr;\n> > +\n> > +\tstd::lock_guard locker(*mutex_);\n> > +\n> > +\tif (buffers_.empty()) {\n> > +\t\tLOG(HAL, Error) << \"Buffer underrun\";\n> > +\t\treturn nullptr;\n> > +\t}\n> > +\n> > +\tFrameBuffer *buffer = buffers_.back();\n> > +\tbuffers_.pop_back();\n> > +\n> > +\treturn buffer;\n> > +}\n> > +\n> > +void CameraStream::putBuffer(libcamera::FrameBuffer *buffer)\n> > +{\n> > +\tif (!allocator_)\n> > +\t\treturn;\n> > +\n> > +\tstd::lock_guard locker(*mutex_);\n> > +\n> > +\tbuffers_.push_back(buffer);\n> > +}\n> > diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> > index 4c51f0fb3393..f929e8260ae3 100644\n> > --- a/src/android/camera_stream.h\n> > +++ b/src/android/camera_stream.h\n> > @@ -8,11 +8,14 @@\n> > #define __ANDROID_CAMERA_STREAM_H__\n> >\n> > #include \n> > +#include \n> > +#include \n> >\n> > #include \n> >\n> > #include \n> > #include \n> > +#include \n> > #include \n> > #include \n> >\n> > @@ -117,6 +120,8 @@ public:\n> > \tint configure();\n> > \tint process(const libcamera::FrameBuffer &source,\n> > \t\t MappedCamera3Buffer *dest, CameraMetadata *metadata);\n> > +\tlibcamera::FrameBuffer *getBuffer();\n> > +\tvoid putBuffer(libcamera::FrameBuffer *buffer);\n> >\n> > private:\n> > \tCameraDevice *cameraDevice_;\n> > @@ -129,7 +134,11 @@ private:\n> > \t * one or more streams to the Android framework.\n> > \t */\n> > \tunsigned int index_;\n> > +\n> > \tstd::unique_ptr encoder_;\n> > +\tstd::unique_ptr allocator_;\n> > +\tstd::vector buffers_;\n> > +\tstd::unique_ptr mutex_;\n> > };\n> >\n> > #endif /* __ANDROID_CAMERA_STREAM__ */\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 DFA6CBEEDF\n\tfor ;\n\tWed, 7 Oct 2020 08:01:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5332C63BE4;\n\tWed, 7 Oct 2020 10:01:36 +0200 (CEST)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 004BE63B27\n\tfor ;\n\tWed, 7 Oct 2020 10:01:34 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id 0F658100013;\n\tWed, 7 Oct 2020 08:01:32 +0000 (UTC)"],"Date":"Wed, 7 Oct 2020 10:05:32 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20201007080532.h33ufwpl72kvi2ii@uno.localdomain>","References":"<20201006144432.22908-1-jacopo@jmondi.org>\n\t<20201006144432.22908-11-jacopo@jmondi.org>\n\t<20201007015320.GE30598@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201007015320.GE30598@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 10/13] android: camera_stream:\n\tCreate buffer poll","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13038,"web_url":"https://patchwork.libcamera.org/comment/13038/","msgid":"<39746d4f-52d9-2a82-578d-ab2e11d64719@ideasonboard.com>","date":"2020-10-07T09:31:28","subject":"Re: [libcamera-devel] [PATCH v2 10/13] android: camera_stream:\n\tCreate buffer poll","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo\n\nin $SUBJECT s/poll/pool/\n--\nKieran\n\n\nOn 07/10/2020 09:05, Jacopo Mondi wrote:\n> Hi Laurent,\n> \n> On Wed, Oct 07, 2020 at 04:53:20AM +0300, Laurent Pinchart wrote:\n>> Hi Jacopo,\n>>\n>> Thank you for the patch.\n>>\n>> On Tue, Oct 06, 2020 at 04:44:29PM +0200, Jacopo Mondi wrote:\n>>> Add a FrameBufferAllocator class member to the CameraStream class.\n>>> The allocator is constructed for CameraStream instances that needs\n>>> internal allocation and automatically deleted.\n>>>\n>>> Allocate FrameBuffers using the allocator_ class member in the\n>>> CameraStream class at CameraStream::configure() time and add two\n>>> methods to the CameraStream class to get and put FrameBuffer pointers\n>>> from the pool of allocated buffers. As buffer allocation can take place\n>>> only after the Camera has been configured, move the CameraStream\n>>> configuration loop in the CameraDevice class after camera_->configure()\n>>> call.\n>>>\n>>> The newly created pool will be used to provide buffers to CameraStream\n>>> that need to provide memory to libcamera where to deliver frames.\n>>>\n>>> Reviewed-by: Kieran Bingham \n>>> Signed-off-by: Jacopo Mondi \n>>> ---\n>>> src/android/camera_device.cpp | 22 +++++++++---------\n>>> src/android/camera_stream.cpp | 43 +++++++++++++++++++++++++++++++++++\n>>> src/android/camera_stream.h | 9 ++++++++\n>>> 3 files changed, 63 insertions(+), 11 deletions(-)\n>>>\n>>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n>>> index 1e2cbeeb92d1..ecdc0922e90b 100644\n>>> --- a/src/android/camera_device.cpp\n>>> +++ b/src/android/camera_device.cpp\n>>> @@ -1282,6 +1282,17 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>>> \t\treturn -EINVAL;\n>>> \t}\n>>>\n>>> +\t/*\n>>> +\t * Once the CameraConfiguration has been adjusted/validated\n>>> +\t * it can be applied to the camera.\n>>> +\t */\n>>> +\tint ret = camera_->configure(config_.get());\n>>> +\tif (ret) {\n>>> +\t\tLOG(HAL, Error) << \"Failed to configure camera '\"\n>>> +\t\t\t\t<< camera_->id() << \"'\";\n>>> +\t\treturn ret;\n>>> +\t}\n>>> +\n>>> \t/*\n>>> \t * Configure the HAL CameraStream instances using the associated\n>>> \t * StreamConfiguration and set the number of required buffers in\n>>> @@ -1295,17 +1306,6 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>>> \t\t}\n>>> \t}\n>>>\n>>> -\t/*\n>>> -\t * Once the CameraConfiguration has been adjusted/validated\n>>> -\t * it can be applied to the camera.\n>>> -\t */\n>>> -\tint ret = camera_->configure(config_.get());\n>>> -\tif (ret) {\n>>> -\t\tLOG(HAL, Error) << \"Failed to configure camera '\"\n>>> -\t\t\t\t<< camera_->id() << \"'\";\n>>> -\t\treturn ret;\n>>> -\t}\n>>> -\n>>> \treturn 0;\n>>> }\n>>>\n>>> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n>>> index f899be4fe007..eac1480530f8 100644\n>>> --- a/src/android/camera_stream.cpp\n>>> +++ b/src/android/camera_stream.cpp\n>>> @@ -26,6 +26,11 @@ CameraStream::CameraStream(CameraDevice *cameraDevice, Type type,\n>>>\n>>> \tif (type_ == Type::Internal || type_ == Type::Mapped)\n>>> \t\tencoder_ = std::make_unique();\n>>> +\n>>> +\tif (type == Type::Internal) {\n>>> +\t\tallocator_ = std::make_unique(cameraDevice_->camera());\n>>> +\t\tmutex_ = std::make_unique();\n>>\n>> This is a bit annoying. It comes from std::vector::reserve()\n>> requiring T to be MoveInsertable, even if no move will actually take\n>> place as we always reserve entries starting from an empty vector (but\n>> the compiler can't known that).\n>>\n>> I think it would be worth using std::list instead of std::vector to\n>> store the CameraStream instances in CameraDevice, and embedding the\n>> mutex. You'll need to drop the reserve() call as std::list doesn't have\n>> that (and update the comment before it accordingly).\n>>\n>> Actually I can post the diff as I made the modifications to ensure it\n>> would work :-)\n> \n> We can indeed take this on top, but I wonder how that works as\n> std::list documentation reports:\n> \"T must meet the requirements of CopyAssignable and CopyConstructible.\"\n> \n> And CameraStream is not CopyConstructable as it contains unique_ptr<>\n> instances...\n> \n>>\n>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n>> index edac9f28ab67..5a466629b78b 100644\n>> --- a/src/android/camera_device.cpp\n>> +++ b/src/android/camera_device.cpp\n>> @@ -1165,13 +1165,8 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>> \t\treturn -EINVAL;\n>> \t}\n>>\n>> -\t/*\n>> -\t * Clear and remove any existing configuration from previous calls, and\n>> -\t * ensure the required entries are available without further\n>> -\t * reallocation.\n>> -\t */\n>> +\t/* Clear and remove any existing configuration from previous calls. */\n>> \tstreams_.clear();\n>> -\tstreams_.reserve(stream_list->num_streams);\n>>\n>> \t/* First handle all non-MJPEG streams. */\n>> \tcamera3_stream_t *jpegStream = nullptr;\n>> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n>> index b4b32f77a29a..d1412d8d5fb8 100644\n>> --- a/src/android/camera_device.h\n>> +++ b/src/android/camera_device.h\n>> @@ -7,6 +7,7 @@\n>> #ifndef __ANDROID_CAMERA_DEVICE_H__\n>> #define __ANDROID_CAMERA_DEVICE_H__\n>>\n>> +#include \n>> #include \n>> #include \n>> #include \n>> @@ -121,7 +122,7 @@ private:\n>>\n>> \tstd::vector streamConfigurations_;\n>> \tstd::map formatsMap_;\n>> -\tstd::vector streams_;\n>> +\tstd::list streams_;\n>>\n>> \tint facing_;\n>> \tint orientation_;\n>> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n>> index eac1480530f8..707c4a5e1077 100644\n>> --- a/src/android/camera_stream.cpp\n>> +++ b/src/android/camera_stream.cpp\n>> @@ -27,10 +27,8 @@ CameraStream::CameraStream(CameraDevice *cameraDevice, Type type,\n>> \tif (type_ == Type::Internal || type_ == Type::Mapped)\n>> \t\tencoder_ = std::make_unique();\n>>\n>> -\tif (type == Type::Internal) {\n>> +\tif (type == Type::Internal)\n>> \t\tallocator_ = std::make_unique(cameraDevice_->camera());\n>> -\t\tmutex_ = std::make_unique();\n>> -\t}\n>> }\n>>\n>> const StreamConfiguration &CameraStream::configuration() const\n>> @@ -130,7 +128,7 @@ FrameBuffer *CameraStream::getBuffer()\n>> \tif (!allocator_)\n>> \t\treturn nullptr;\n>>\n>> -\tstd::lock_guard locker(*mutex_);\n>> +\tstd::lock_guard locker(mutex_);\n>>\n>> \tif (buffers_.empty()) {\n>> \t\tLOG(HAL, Error) << \"Buffer underrun\";\n>> @@ -148,7 +146,7 @@ void CameraStream::putBuffer(libcamera::FrameBuffer *buffer)\n>> \tif (!allocator_)\n>> \t\treturn;\n>>\n>> -\tstd::lock_guard locker(*mutex_);\n>> +\tstd::lock_guard locker(mutex_);\n>>\n>> \tbuffers_.push_back(buffer);\n>> }\n>> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n>> index f929e8260ae3..a177a99a2ea1 100644\n>> --- a/src/android/camera_stream.h\n>> +++ b/src/android/camera_stream.h\n>> @@ -138,7 +138,7 @@ private:\n>> \tstd::unique_ptr encoder_;\n>> \tstd::unique_ptr allocator_;\n>> \tstd::vector buffers_;\n>> -\tstd::unique_ptr mutex_;\n>> +\tstd::mutex mutex_;\n>> };\n>>\n>> #endif /* __ANDROID_CAMERA_STREAM__ */\n>>\n>> This change can go on top or be integrated in the series (and there's no\n>> need to report just for this), up to you.\n>>\n>> Reviewed-by: Laurent Pinchart \n>>\n>>> +\t}\n>>> }\n>>>\n>>> const StreamConfiguration &CameraStream::configuration() const\n>>> @@ -46,6 +51,16 @@ int CameraStream::configure()\n>>> \t\t\treturn ret;\n>>> \t}\n>>>\n>>> +\tif (allocator_) {\n>>> +\t\tint ret = allocator_->allocate(stream());\n>>> +\t\tif (ret < 0)\n>>> +\t\t\treturn ret;\n>>> +\n>>> +\t\t/* Save a pointer to the reserved frame buffers */\n>>> +\t\tfor (const auto &frameBuffer : allocator_->buffers(stream()))\n>>> +\t\t\tbuffers_.push_back(frameBuffer.get());\n>>> +\t}\n>>> +\n>>> \tcamera3Stream_->max_buffers = configuration().bufferCount;\n>>>\n>>> \treturn 0;\n>>> @@ -109,3 +124,31 @@ int CameraStream::process(const libcamera::FrameBuffer &source,\n>>>\n>>> \treturn 0;\n>>> }\n>>> +\n>>> +FrameBuffer *CameraStream::getBuffer()\n>>> +{\n>>> +\tif (!allocator_)\n>>> +\t\treturn nullptr;\n>>> +\n>>> +\tstd::lock_guard locker(*mutex_);\n>>> +\n>>> +\tif (buffers_.empty()) {\n>>> +\t\tLOG(HAL, Error) << \"Buffer underrun\";\n>>> +\t\treturn nullptr;\n>>> +\t}\n>>> +\n>>> +\tFrameBuffer *buffer = buffers_.back();\n>>> +\tbuffers_.pop_back();\n>>> +\n>>> +\treturn buffer;\n>>> +}\n>>> +\n>>> +void CameraStream::putBuffer(libcamera::FrameBuffer *buffer)\n>>> +{\n>>> +\tif (!allocator_)\n>>> +\t\treturn;\n>>> +\n>>> +\tstd::lock_guard locker(*mutex_);\n>>> +\n>>> +\tbuffers_.push_back(buffer);\n>>> +}\n>>> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n>>> index 4c51f0fb3393..f929e8260ae3 100644\n>>> --- a/src/android/camera_stream.h\n>>> +++ b/src/android/camera_stream.h\n>>> @@ -8,11 +8,14 @@\n>>> #define __ANDROID_CAMERA_STREAM_H__\n>>>\n>>> #include \n>>> +#include \n>>> +#include \n>>>\n>>> #include \n>>>\n>>> #include \n>>> #include \n>>> +#include \n>>> #include \n>>> #include \n>>>\n>>> @@ -117,6 +120,8 @@ public:\n>>> \tint configure();\n>>> \tint process(const libcamera::FrameBuffer &source,\n>>> \t\t MappedCamera3Buffer *dest, CameraMetadata *metadata);\n>>> +\tlibcamera::FrameBuffer *getBuffer();\n>>> +\tvoid putBuffer(libcamera::FrameBuffer *buffer);\n>>>\n>>> private:\n>>> \tCameraDevice *cameraDevice_;\n>>> @@ -129,7 +134,11 @@ private:\n>>> \t * one or more streams to the Android framework.\n>>> \t */\n>>> \tunsigned int index_;\n>>> +\n>>> \tstd::unique_ptr encoder_;\n>>> +\tstd::unique_ptr allocator_;\n>>> +\tstd::vector buffers_;\n>>> +\tstd::unique_ptr mutex_;\n>>> };\n>>>\n>>> #endif /* __ANDROID_CAMERA_STREAM__ */\n>>\n>> --\n>> Regards,\n>>\n>> Laurent Pinchart\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel\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 6F8BDBEEE0\n\tfor ;\n\tWed, 7 Oct 2020 09:31:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0884463BE4;\n\tWed, 7 Oct 2020 11:31:34 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0513B6035F\n\tfor ;\n\tWed, 7 Oct 2020 11:31:32 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 502229DA;\n\tWed, 7 Oct 2020 11:31:31 +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=\"uWh2vdax\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1602063091;\n\tbh=GwlAh2pD7GUL8DjDJDN39ibBUvUBXEtKbIDPo58qAo0=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=uWh2vdaxxiT+CRC5K2s5uUaTLyE2dl7aImWDB+roGIw6NO+I771hgnaXu6HlZvL4r\n\tHIk4D0fQM1DcQwXEVQ3BYPN6xmajfMSOrnipKJn5V7ngJEKEDnvFgAl6IdWf8WpwLW\n\tnM+cueKmXyXtLb7rrmMKPERi7xzxP42WrUxKalyU=","To":"Jacopo Mondi ,\n\tLaurent Pinchart ","References":"<20201006144432.22908-1-jacopo@jmondi.org>\n\t<20201006144432.22908-11-jacopo@jmondi.org>\n\t<20201007015320.GE30598@pendragon.ideasonboard.com>\n\t<20201007080532.h33ufwpl72kvi2ii@uno.localdomain>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<39746d4f-52d9-2a82-578d-ab2e11d64719@ideasonboard.com>","Date":"Wed, 7 Oct 2020 10:31:28 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20201007080532.h33ufwpl72kvi2ii@uno.localdomain>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v2 10/13] android: camera_stream:\n\tCreate buffer poll","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","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13064,"web_url":"https://patchwork.libcamera.org/comment/13064/","msgid":"<20201007131802.GE3937@pendragon.ideasonboard.com>","date":"2020-10-07T13:18:02","subject":"Re: [libcamera-devel] [PATCH v2 10/13] android: camera_stream:\n\tCreate buffer poll","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, Oct 07, 2020 at 10:05:32AM +0200, Jacopo Mondi wrote:\n> On Wed, Oct 07, 2020 at 04:53:20AM +0300, Laurent Pinchart wrote:\n> > On Tue, Oct 06, 2020 at 04:44:29PM +0200, Jacopo Mondi wrote:\n> > > Add a FrameBufferAllocator class member to the CameraStream class.\n> > > The allocator is constructed for CameraStream instances that needs\n> > > internal allocation and automatically deleted.\n> > >\n> > > Allocate FrameBuffers using the allocator_ class member in the\n> > > CameraStream class at CameraStream::configure() time and add two\n> > > methods to the CameraStream class to get and put FrameBuffer pointers\n> > > from the pool of allocated buffers. As buffer allocation can take place\n> > > only after the Camera has been configured, move the CameraStream\n> > > configuration loop in the CameraDevice class after camera_->configure()\n> > > call.\n> > >\n> > > The newly created pool will be used to provide buffers to CameraStream\n> > > that need to provide memory to libcamera where to deliver frames.\n> > >\n> > > Reviewed-by: Kieran Bingham \n> > > Signed-off-by: Jacopo Mondi \n> > > ---\n> > > src/android/camera_device.cpp | 22 +++++++++---------\n> > > src/android/camera_stream.cpp | 43 +++++++++++++++++++++++++++++++++++\n> > > src/android/camera_stream.h | 9 ++++++++\n> > > 3 files changed, 63 insertions(+), 11 deletions(-)\n> > >\n> > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > index 1e2cbeeb92d1..ecdc0922e90b 100644\n> > > --- a/src/android/camera_device.cpp\n> > > +++ b/src/android/camera_device.cpp\n> > > @@ -1282,6 +1282,17 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > > \t\treturn -EINVAL;\n> > > \t}\n> > >\n> > > +\t/*\n> > > +\t * Once the CameraConfiguration has been adjusted/validated\n> > > +\t * it can be applied to the camera.\n> > > +\t */\n> > > +\tint ret = camera_->configure(config_.get());\n> > > +\tif (ret) {\n> > > +\t\tLOG(HAL, Error) << \"Failed to configure camera '\"\n> > > +\t\t\t\t<< camera_->id() << \"'\";\n> > > +\t\treturn ret;\n> > > +\t}\n> > > +\n> > > \t/*\n> > > \t * Configure the HAL CameraStream instances using the associated\n> > > \t * StreamConfiguration and set the number of required buffers in\n> > > @@ -1295,17 +1306,6 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > > \t\t}\n> > > \t}\n> > >\n> > > -\t/*\n> > > -\t * Once the CameraConfiguration has been adjusted/validated\n> > > -\t * it can be applied to the camera.\n> > > -\t */\n> > > -\tint ret = camera_->configure(config_.get());\n> > > -\tif (ret) {\n> > > -\t\tLOG(HAL, Error) << \"Failed to configure camera '\"\n> > > -\t\t\t\t<< camera_->id() << \"'\";\n> > > -\t\treturn ret;\n> > > -\t}\n> > > -\n> > > \treturn 0;\n> > > }\n> > >\n> > > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> > > index f899be4fe007..eac1480530f8 100644\n> > > --- a/src/android/camera_stream.cpp\n> > > +++ b/src/android/camera_stream.cpp\n> > > @@ -26,6 +26,11 @@ CameraStream::CameraStream(CameraDevice *cameraDevice, Type type,\n> > >\n> > > \tif (type_ == Type::Internal || type_ == Type::Mapped)\n> > > \t\tencoder_ = std::make_unique();\n> > > +\n> > > +\tif (type == Type::Internal) {\n> > > +\t\tallocator_ = std::make_unique(cameraDevice_->camera());\n> > > +\t\tmutex_ = std::make_unique();\n> >\n> > This is a bit annoying. It comes from std::vector::reserve()\n> > requiring T to be MoveInsertable, even if no move will actually take\n> > place as we always reserve entries starting from an empty vector (but\n> > the compiler can't known that).\n> >\n> > I think it would be worth using std::list instead of std::vector to\n> > store the CameraStream instances in CameraDevice, and embedding the\n> > mutex. You'll need to drop the reserve() call as std::list doesn't have\n> > that (and update the comment before it accordingly).\n> >\n> > Actually I can post the diff as I made the modifications to ensure it\n> > would work :-)\n> \n> We can indeed take this on top, but I wonder how that works as\n> std::list documentation reports:\n> \"T must meet the requirements of CopyAssignable and CopyConstructible.\"\n\nThat's until C++11. Starting at C++17, it states\n\n\"The requirements that are imposed on the elements depend on the actual\noperations performed on the container. Generally, it is required that\nelement type meets the requirements of Erasable, but many member\nfunctions impose stricter requirements. This container (but not its\nmembers) can be instantiated with an incomplete element type if the\nallocator satisfies the allocator completeness requirements.\"\n\nstd::list::push_back() requires CopyInsertable or MoveInsertable\n(depending on which overload is used), but std::list::emplace_back()\nonly requires EmplaceConstructible.\n\n> And CameraStream is not CopyConstructable as it contains unique_ptr<>\n> instances...\n> \n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index edac9f28ab67..5a466629b78b 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -1165,13 +1165,8 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > \t\treturn -EINVAL;\n> > \t}\n> >\n> > -\t/*\n> > -\t * Clear and remove any existing configuration from previous calls, and\n> > -\t * ensure the required entries are available without further\n> > -\t * reallocation.\n> > -\t */\n> > +\t/* Clear and remove any existing configuration from previous calls. */\n> > \tstreams_.clear();\n> > -\tstreams_.reserve(stream_list->num_streams);\n> >\n> > \t/* First handle all non-MJPEG streams. */\n> > \tcamera3_stream_t *jpegStream = nullptr;\n> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > index b4b32f77a29a..d1412d8d5fb8 100644\n> > --- a/src/android/camera_device.h\n> > +++ b/src/android/camera_device.h\n> > @@ -7,6 +7,7 @@\n> > #ifndef __ANDROID_CAMERA_DEVICE_H__\n> > #define __ANDROID_CAMERA_DEVICE_H__\n> >\n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -121,7 +122,7 @@ private:\n> >\n> > \tstd::vector streamConfigurations_;\n> > \tstd::map formatsMap_;\n> > -\tstd::vector streams_;\n> > +\tstd::list streams_;\n> >\n> > \tint facing_;\n> > \tint orientation_;\n> > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> > index eac1480530f8..707c4a5e1077 100644\n> > --- a/src/android/camera_stream.cpp\n> > +++ b/src/android/camera_stream.cpp\n> > @@ -27,10 +27,8 @@ CameraStream::CameraStream(CameraDevice *cameraDevice, Type type,\n> > \tif (type_ == Type::Internal || type_ == Type::Mapped)\n> > \t\tencoder_ = std::make_unique();\n> >\n> > -\tif (type == Type::Internal) {\n> > +\tif (type == Type::Internal)\n> > \t\tallocator_ = std::make_unique(cameraDevice_->camera());\n> > -\t\tmutex_ = std::make_unique();\n> > -\t}\n> > }\n> >\n> > const StreamConfiguration &CameraStream::configuration() const\n> > @@ -130,7 +128,7 @@ FrameBuffer *CameraStream::getBuffer()\n> > \tif (!allocator_)\n> > \t\treturn nullptr;\n> >\n> > -\tstd::lock_guard locker(*mutex_);\n> > +\tstd::lock_guard locker(mutex_);\n> >\n> > \tif (buffers_.empty()) {\n> > \t\tLOG(HAL, Error) << \"Buffer underrun\";\n> > @@ -148,7 +146,7 @@ void CameraStream::putBuffer(libcamera::FrameBuffer *buffer)\n> > \tif (!allocator_)\n> > \t\treturn;\n> >\n> > -\tstd::lock_guard locker(*mutex_);\n> > +\tstd::lock_guard locker(mutex_);\n> >\n> > \tbuffers_.push_back(buffer);\n> > }\n> > diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> > index f929e8260ae3..a177a99a2ea1 100644\n> > --- a/src/android/camera_stream.h\n> > +++ b/src/android/camera_stream.h\n> > @@ -138,7 +138,7 @@ private:\n> > \tstd::unique_ptr encoder_;\n> > \tstd::unique_ptr allocator_;\n> > \tstd::vector buffers_;\n> > -\tstd::unique_ptr mutex_;\n> > +\tstd::mutex mutex_;\n> > };\n> >\n> > #endif /* __ANDROID_CAMERA_STREAM__ */\n> >\n> > This change can go on top or be integrated in the series (and there's no\n> > need to report just for this), up to you.\n> >\n> > Reviewed-by: Laurent Pinchart \n> >\n> > > +\t}\n> > > }\n> > >\n> > > const StreamConfiguration &CameraStream::configuration() const\n> > > @@ -46,6 +51,16 @@ int CameraStream::configure()\n> > > \t\t\treturn ret;\n> > > \t}\n> > >\n> > > +\tif (allocator_) {\n> > > +\t\tint ret = allocator_->allocate(stream());\n> > > +\t\tif (ret < 0)\n> > > +\t\t\treturn ret;\n> > > +\n> > > +\t\t/* Save a pointer to the reserved frame buffers */\n> > > +\t\tfor (const auto &frameBuffer : allocator_->buffers(stream()))\n> > > +\t\t\tbuffers_.push_back(frameBuffer.get());\n> > > +\t}\n> > > +\n> > > \tcamera3Stream_->max_buffers = configuration().bufferCount;\n> > >\n> > > \treturn 0;\n> > > @@ -109,3 +124,31 @@ int CameraStream::process(const libcamera::FrameBuffer &source,\n> > >\n> > > \treturn 0;\n> > > }\n> > > +\n> > > +FrameBuffer *CameraStream::getBuffer()\n> > > +{\n> > > +\tif (!allocator_)\n> > > +\t\treturn nullptr;\n> > > +\n> > > +\tstd::lock_guard locker(*mutex_);\n> > > +\n> > > +\tif (buffers_.empty()) {\n> > > +\t\tLOG(HAL, Error) << \"Buffer underrun\";\n> > > +\t\treturn nullptr;\n> > > +\t}\n> > > +\n> > > +\tFrameBuffer *buffer = buffers_.back();\n> > > +\tbuffers_.pop_back();\n> > > +\n> > > +\treturn buffer;\n> > > +}\n> > > +\n> > > +void CameraStream::putBuffer(libcamera::FrameBuffer *buffer)\n> > > +{\n> > > +\tif (!allocator_)\n> > > +\t\treturn;\n> > > +\n> > > +\tstd::lock_guard locker(*mutex_);\n> > > +\n> > > +\tbuffers_.push_back(buffer);\n> > > +}\n> > > diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> > > index 4c51f0fb3393..f929e8260ae3 100644\n> > > --- a/src/android/camera_stream.h\n> > > +++ b/src/android/camera_stream.h\n> > > @@ -8,11 +8,14 @@\n> > > #define __ANDROID_CAMERA_STREAM_H__\n> > >\n> > > #include \n> > > +#include \n> > > +#include \n> > >\n> > > #include \n> > >\n> > > #include \n> > > #include \n> > > +#include \n> > > #include \n> > > #include \n> > >\n> > > @@ -117,6 +120,8 @@ public:\n> > > \tint configure();\n> > > \tint process(const libcamera::FrameBuffer &source,\n> > > \t\t MappedCamera3Buffer *dest, CameraMetadata *metadata);\n> > > +\tlibcamera::FrameBuffer *getBuffer();\n> > > +\tvoid putBuffer(libcamera::FrameBuffer *buffer);\n> > >\n> > > private:\n> > > \tCameraDevice *cameraDevice_;\n> > > @@ -129,7 +134,11 @@ private:\n> > > \t * one or more streams to the Android framework.\n> > > \t */\n> > > \tunsigned int index_;\n> > > +\n> > > \tstd::unique_ptr encoder_;\n> > > +\tstd::unique_ptr allocator_;\n> > > +\tstd::vector buffers_;\n> > > +\tstd::unique_ptr mutex_;\n> > > };\n> > >\n> > > #endif /* __ANDROID_CAMERA_STREAM__ */","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 8DA1ABEEE0\n\tfor ;\n\tWed, 7 Oct 2020 13:18:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 54A26605BE;\n\tWed, 7 Oct 2020 15:18:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C73B560580\n\tfor ;\n\tWed, 7 Oct 2020 15:18:44 +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 54AD29DA;\n\tWed, 7 Oct 2020 15:18:44 +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=\"e+m3B3Yi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1602076724;\n\tbh=QNhGYeQdx2naan1A7bzK/gAl8F4Rv4zTSjk5oxqWkq8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=e+m3B3Yi/2VmboCyLVvPZbQ5CAJq8BsmFRsiPgfA3Q/CnqG5fydoriGJoCuMvSENy\n\tdWFFybBVdTWoshbJ4Hi4usNIOiNSCCwzR8KT/Rn0wmLt1323SEaBf1S+Bm+3lnBJ3p\n\tR5RTAGxnhAAwbtH4xFukYurkJDiGVg/ssTdcBZ0c=","Date":"Wed, 7 Oct 2020 16:18:02 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20201007131802.GE3937@pendragon.ideasonboard.com>","References":"<20201006144432.22908-1-jacopo@jmondi.org>\n\t<20201006144432.22908-11-jacopo@jmondi.org>\n\t<20201007015320.GE30598@pendragon.ideasonboard.com>\n\t<20201007080532.h33ufwpl72kvi2ii@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201007080532.h33ufwpl72kvi2ii@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2 10/13] android: camera_stream:\n\tCreate buffer poll","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13071,"web_url":"https://patchwork.libcamera.org/comment/13071/","msgid":"<20201007133853.qs3xd232knqeshfs@uno.localdomain>","date":"2020-10-07T13:38:53","subject":"Re: [libcamera-devel] [PATCH v2 10/13] android: camera_stream:\n\tCreate buffer poll","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\nOn Wed, Oct 07, 2020 at 04:18:02PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Wed, Oct 07, 2020 at 10:05:32AM +0200, Jacopo Mondi wrote:\n> > On Wed, Oct 07, 2020 at 04:53:20AM +0300, Laurent Pinchart wrote:\n> > > On Tue, Oct 06, 2020 at 04:44:29PM +0200, Jacopo Mondi wrote:\n\n[snip]\n\n> > > >\n> > > > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> > > > index f899be4fe007..eac1480530f8 100644\n> > > > --- a/src/android/camera_stream.cpp\n> > > > +++ b/src/android/camera_stream.cpp\n> > > > @@ -26,6 +26,11 @@ CameraStream::CameraStream(CameraDevice *cameraDevice, Type type,\n> > > >\n> > > > \tif (type_ == Type::Internal || type_ == Type::Mapped)\n> > > > \t\tencoder_ = std::make_unique();\n> > > > +\n> > > > +\tif (type == Type::Internal) {\n> > > > +\t\tallocator_ = std::make_unique(cameraDevice_->camera());\n> > > > +\t\tmutex_ = std::make_unique();\n> > >\n> > > This is a bit annoying. It comes from std::vector::reserve()\n> > > requiring T to be MoveInsertable, even if no move will actually take\n> > > place as we always reserve entries starting from an empty vector (but\n> > > the compiler can't known that).\n> > >\n> > > I think it would be worth using std::list instead of std::vector to\n> > > store the CameraStream instances in CameraDevice, and embedding the\n> > > mutex. You'll need to drop the reserve() call as std::list doesn't have\n> > > that (and update the comment before it accordingly).\n> > >\n> > > Actually I can post the diff as I made the modifications to ensure it\n> > > would work :-)\n> >\n> > We can indeed take this on top, but I wonder how that works as\n> > std::list documentation reports:\n> > \"T must meet the requirements of CopyAssignable and CopyConstructible.\"\n>\n> That's until C++11. Starting at C++17, it states\n>\n> \"The requirements that are imposed on the elements depend on the actual\n> operations performed on the container. Generally, it is required that\n> element type meets the requirements of Erasable, but many member\n> functions impose stricter requirements. This container (but not its\n> members) can be instantiated with an incomplete element type if the\n> allocator satisfies the allocator completeness requirements.\"\n>\n> std::list::push_back() requires CopyInsertable or MoveInsertable\n> (depending on which overload is used), but std::list::emplace_back()\n> only requires EmplaceConstructible.\n>\n\nAh, thanks for the details.\n\nLast attempt of push back: using std::list we lose random access,\nwhich we don't need at the moment as we only cycle on them or retrieve\nthem by pointer from the camera3 streams, but if we need to do so in\nfuture we will have to backtrack.\n\n\n> > And CameraStream is not CopyConstructable as it contains unique_ptr<>\n> > instances...\n> >\n> > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > index edac9f28ab67..5a466629b78b 100644\n> > > --- a/src/android/camera_device.cpp\n> > > +++ b/src/android/camera_device.cpp\n> > > @@ -1165,13 +1165,8 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > > \t\treturn -EINVAL;\n> > > \t}\n> > >\n> > > -\t/*\n> > > -\t * Clear and remove any existing configuration from previous calls, and\n> > > -\t * ensure the required entries are available without further\n> > > -\t * reallocation.\n> > > -\t */\n> > > +\t/* Clear and remove any existing configuration from previous calls. */\n> > > \tstreams_.clear();\n> > > -\tstreams_.reserve(stream_list->num_streams);\n> > >\n> > > \t/* First handle all non-MJPEG streams. */\n> > > \tcamera3_stream_t *jpegStream = nullptr;\n> > > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > > index b4b32f77a29a..d1412d8d5fb8 100644\n> > > --- a/src/android/camera_device.h\n> > > +++ b/src/android/camera_device.h\n> > > @@ -7,6 +7,7 @@\n> > > #ifndef __ANDROID_CAMERA_DEVICE_H__\n> > > #define __ANDROID_CAMERA_DEVICE_H__\n> > >\n> > > +#include \n> > > #include \n> > > #include \n> > > #include \n> > > @@ -121,7 +122,7 @@ private:\n> > >\n> > > \tstd::vector streamConfigurations_;\n> > > \tstd::map formatsMap_;\n> > > -\tstd::vector streams_;\n> > > +\tstd::list streams_;\n> > >\n> > > \tint facing_;\n> > > \tint orientation_;\n> > > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> > > index eac1480530f8..707c4a5e1077 100644\n> > > --- a/src/android/camera_stream.cpp\n> > > +++ b/src/android/camera_stream.cpp\n> > > @@ -27,10 +27,8 @@ CameraStream::CameraStream(CameraDevice *cameraDevice, Type type,\n> > > \tif (type_ == Type::Internal || type_ == Type::Mapped)\n> > > \t\tencoder_ = std::make_unique();\n> > >\n> > > -\tif (type == Type::Internal) {\n> > > +\tif (type == Type::Internal)\n> > > \t\tallocator_ = std::make_unique(cameraDevice_->camera());\n> > > -\t\tmutex_ = std::make_unique();\n> > > -\t}\n> > > }\n> > >\n> > > const StreamConfiguration &CameraStream::configuration() const\n> > > @@ -130,7 +128,7 @@ FrameBuffer *CameraStream::getBuffer()\n> > > \tif (!allocator_)\n> > > \t\treturn nullptr;\n> > >\n> > > -\tstd::lock_guard locker(*mutex_);\n> > > +\tstd::lock_guard locker(mutex_);\n> > >\n> > > \tif (buffers_.empty()) {\n> > > \t\tLOG(HAL, Error) << \"Buffer underrun\";\n> > > @@ -148,7 +146,7 @@ void CameraStream::putBuffer(libcamera::FrameBuffer *buffer)\n> > > \tif (!allocator_)\n> > > \t\treturn;\n> > >\n> > > -\tstd::lock_guard locker(*mutex_);\n> > > +\tstd::lock_guard locker(mutex_);\n> > >\n> > > \tbuffers_.push_back(buffer);\n> > > }\n> > > diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> > > index f929e8260ae3..a177a99a2ea1 100644\n> > > --- a/src/android/camera_stream.h\n> > > +++ b/src/android/camera_stream.h\n> > > @@ -138,7 +138,7 @@ private:\n> > > \tstd::unique_ptr encoder_;\n> > > \tstd::unique_ptr allocator_;\n> > > \tstd::vector buffers_;\n> > > -\tstd::unique_ptr mutex_;\n> > > +\tstd::mutex mutex_;\n> > > };\n> > >\n> > > #endif /* __ANDROID_CAMERA_STREAM__ */\n> > >\n> > > This change can go on top or be integrated in the series (and there's no\n> > > need to report just for this), up to you.\n> > >\n> > > Reviewed-by: Laurent Pinchart \n> > >\n> > > > +\t}\n> > > > }\n> > > >\n> > > > const StreamConfiguration &CameraStream::configuration() const\n> > > > @@ -46,6 +51,16 @@ int CameraStream::configure()\n> > > > \t\t\treturn ret;\n> > > > \t}\n> > > >\n> > > > +\tif (allocator_) {\n> > > > +\t\tint ret = allocator_->allocate(stream());\n> > > > +\t\tif (ret < 0)\n> > > > +\t\t\treturn ret;\n> > > > +\n> > > > +\t\t/* Save a pointer to the reserved frame buffers */\n> > > > +\t\tfor (const auto &frameBuffer : allocator_->buffers(stream()))\n> > > > +\t\t\tbuffers_.push_back(frameBuffer.get());\n> > > > +\t}\n> > > > +\n> > > > \tcamera3Stream_->max_buffers = configuration().bufferCount;\n> > > >\n> > > > \treturn 0;\n> > > > @@ -109,3 +124,31 @@ int CameraStream::process(const libcamera::FrameBuffer &source,\n> > > >\n> > > > \treturn 0;\n> > > > }\n> > > > +\n> > > > +FrameBuffer *CameraStream::getBuffer()\n> > > > +{\n> > > > +\tif (!allocator_)\n> > > > +\t\treturn nullptr;\n> > > > +\n> > > > +\tstd::lock_guard locker(*mutex_);\n> > > > +\n> > > > +\tif (buffers_.empty()) {\n> > > > +\t\tLOG(HAL, Error) << \"Buffer underrun\";\n> > > > +\t\treturn nullptr;\n> > > > +\t}\n> > > > +\n> > > > +\tFrameBuffer *buffer = buffers_.back();\n> > > > +\tbuffers_.pop_back();\n> > > > +\n> > > > +\treturn buffer;\n> > > > +}\n> > > > +\n> > > > +void CameraStream::putBuffer(libcamera::FrameBuffer *buffer)\n> > > > +{\n> > > > +\tif (!allocator_)\n> > > > +\t\treturn;\n> > > > +\n> > > > +\tstd::lock_guard locker(*mutex_);\n> > > > +\n> > > > +\tbuffers_.push_back(buffer);\n> > > > +}\n> > > > diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> > > > index 4c51f0fb3393..f929e8260ae3 100644\n> > > > --- a/src/android/camera_stream.h\n> > > > +++ b/src/android/camera_stream.h\n> > > > @@ -8,11 +8,14 @@\n> > > > #define __ANDROID_CAMERA_STREAM_H__\n> > > >\n> > > > #include \n> > > > +#include \n> > > > +#include \n> > > >\n> > > > #include \n> > > >\n> > > > #include \n> > > > #include \n> > > > +#include \n> > > > #include \n> > > > #include \n> > > >\n> > > > @@ -117,6 +120,8 @@ public:\n> > > > \tint configure();\n> > > > \tint process(const libcamera::FrameBuffer &source,\n> > > > \t\t MappedCamera3Buffer *dest, CameraMetadata *metadata);\n> > > > +\tlibcamera::FrameBuffer *getBuffer();\n> > > > +\tvoid putBuffer(libcamera::FrameBuffer *buffer);\n> > > >\n> > > > private:\n> > > > \tCameraDevice *cameraDevice_;\n> > > > @@ -129,7 +134,11 @@ private:\n> > > > \t * one or more streams to the Android framework.\n> > > > \t */\n> > > > \tunsigned int index_;\n> > > > +\n> > > > \tstd::unique_ptr encoder_;\n> > > > +\tstd::unique_ptr allocator_;\n> > > > +\tstd::vector buffers_;\n> > > > +\tstd::unique_ptr mutex_;\n> > > > };\n> > > >\n> > > > #endif /* __ANDROID_CAMERA_STREAM__ */\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 8672DBEEDF\n\tfor ;\n\tWed, 7 Oct 2020 13:34:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 282C6605D1;\n\tWed, 7 Oct 2020 15:34:54 +0200 (CEST)","from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BD0D0605BE\n\tfor ;\n\tWed, 7 Oct 2020 15:34:53 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 423971C001D;\n\tWed, 7 Oct 2020 13:34:52 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 7 Oct 2020 15:38:53 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20201007133853.qs3xd232knqeshfs@uno.localdomain>","References":"<20201006144432.22908-1-jacopo@jmondi.org>\n\t<20201006144432.22908-11-jacopo@jmondi.org>\n\t<20201007015320.GE30598@pendragon.ideasonboard.com>\n\t<20201007080532.h33ufwpl72kvi2ii@uno.localdomain>\n\t<20201007131802.GE3937@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201007131802.GE3937@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 10/13] android: camera_stream:\n\tCreate buffer poll","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13072,"web_url":"https://patchwork.libcamera.org/comment/13072/","msgid":"<20201007133559.GK3937@pendragon.ideasonboard.com>","date":"2020-10-07T13:35:59","subject":"Re: [libcamera-devel] [PATCH v2 10/13] android: camera_stream:\n\tCreate buffer poll","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, Oct 07, 2020 at 03:38:53PM +0200, Jacopo Mondi wrote:\n> On Wed, Oct 07, 2020 at 04:18:02PM +0300, Laurent Pinchart wrote:\n> > On Wed, Oct 07, 2020 at 10:05:32AM +0200, Jacopo Mondi wrote:\n> > > On Wed, Oct 07, 2020 at 04:53:20AM +0300, Laurent Pinchart wrote:\n> > > > On Tue, Oct 06, 2020 at 04:44:29PM +0200, Jacopo Mondi wrote:\n> \n> [snip]\n> \n> > > > >\n> > > > > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> > > > > index f899be4fe007..eac1480530f8 100644\n> > > > > --- a/src/android/camera_stream.cpp\n> > > > > +++ b/src/android/camera_stream.cpp\n> > > > > @@ -26,6 +26,11 @@ CameraStream::CameraStream(CameraDevice *cameraDevice, Type type,\n> > > > >\n> > > > > \tif (type_ == Type::Internal || type_ == Type::Mapped)\n> > > > > \t\tencoder_ = std::make_unique();\n> > > > > +\n> > > > > +\tif (type == Type::Internal) {\n> > > > > +\t\tallocator_ = std::make_unique(cameraDevice_->camera());\n> > > > > +\t\tmutex_ = std::make_unique();\n> > > >\n> > > > This is a bit annoying. It comes from std::vector::reserve()\n> > > > requiring T to be MoveInsertable, even if no move will actually take\n> > > > place as we always reserve entries starting from an empty vector (but\n> > > > the compiler can't known that).\n> > > >\n> > > > I think it would be worth using std::list instead of std::vector to\n> > > > store the CameraStream instances in CameraDevice, and embedding the\n> > > > mutex. You'll need to drop the reserve() call as std::list doesn't have\n> > > > that (and update the comment before it accordingly).\n> > > >\n> > > > Actually I can post the diff as I made the modifications to ensure it\n> > > > would work :-)\n> > >\n> > > We can indeed take this on top, but I wonder how that works as\n> > > std::list documentation reports:\n> > > \"T must meet the requirements of CopyAssignable and CopyConstructible.\"\n> >\n> > That's until C++11. Starting at C++17, it states\n> >\n> > \"The requirements that are imposed on the elements depend on the actual\n> > operations performed on the container. Generally, it is required that\n> > element type meets the requirements of Erasable, but many member\n> > functions impose stricter requirements. This container (but not its\n> > members) can be instantiated with an incomplete element type if the\n> > allocator satisfies the allocator completeness requirements.\"\n> >\n> > std::list::push_back() requires CopyInsertable or MoveInsertable\n> > (depending on which overload is used), but std::list::emplace_back()\n> > only requires EmplaceConstructible.\n> \n> Ah, thanks for the details.\n> \n> Last attempt of push back: using std::list we lose random access,\n> which we don't need at the moment as we only cycle on them or retrieve\n> them by pointer from the camera3 streams, but if we need to do so in\n> future we will have to backtrack.\n\nThat's a good point, even though I'm not sure we'll need random access.\nI'll let you decide which (potential) issue you like best, and pick\nvector or list accordingly :-)\n\n> > > And CameraStream is not CopyConstructable as it contains unique_ptr<>\n> > > instances...\n> > >\n> > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > > index edac9f28ab67..5a466629b78b 100644\n> > > > --- a/src/android/camera_device.cpp\n> > > > +++ b/src/android/camera_device.cpp\n> > > > @@ -1165,13 +1165,8 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > > > \t\treturn -EINVAL;\n> > > > \t}\n> > > >\n> > > > -\t/*\n> > > > -\t * Clear and remove any existing configuration from previous calls, and\n> > > > -\t * ensure the required entries are available without further\n> > > > -\t * reallocation.\n> > > > -\t */\n> > > > +\t/* Clear and remove any existing configuration from previous calls. */\n> > > > \tstreams_.clear();\n> > > > -\tstreams_.reserve(stream_list->num_streams);\n> > > >\n> > > > \t/* First handle all non-MJPEG streams. */\n> > > > \tcamera3_stream_t *jpegStream = nullptr;\n> > > > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > > > index b4b32f77a29a..d1412d8d5fb8 100644\n> > > > --- a/src/android/camera_device.h\n> > > > +++ b/src/android/camera_device.h\n> > > > @@ -7,6 +7,7 @@\n> > > > #ifndef __ANDROID_CAMERA_DEVICE_H__\n> > > > #define __ANDROID_CAMERA_DEVICE_H__\n> > > >\n> > > > +#include \n> > > > #include \n> > > > #include \n> > > > #include \n> > > > @@ -121,7 +122,7 @@ private:\n> > > >\n> > > > \tstd::vector streamConfigurations_;\n> > > > \tstd::map formatsMap_;\n> > > > -\tstd::vector streams_;\n> > > > +\tstd::list streams_;\n> > > >\n> > > > \tint facing_;\n> > > > \tint orientation_;\n> > > > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> > > > index eac1480530f8..707c4a5e1077 100644\n> > > > --- a/src/android/camera_stream.cpp\n> > > > +++ b/src/android/camera_stream.cpp\n> > > > @@ -27,10 +27,8 @@ CameraStream::CameraStream(CameraDevice *cameraDevice, Type type,\n> > > > \tif (type_ == Type::Internal || type_ == Type::Mapped)\n> > > > \t\tencoder_ = std::make_unique();\n> > > >\n> > > > -\tif (type == Type::Internal) {\n> > > > +\tif (type == Type::Internal)\n> > > > \t\tallocator_ = std::make_unique(cameraDevice_->camera());\n> > > > -\t\tmutex_ = std::make_unique();\n> > > > -\t}\n> > > > }\n> > > >\n> > > > const StreamConfiguration &CameraStream::configuration() const\n> > > > @@ -130,7 +128,7 @@ FrameBuffer *CameraStream::getBuffer()\n> > > > \tif (!allocator_)\n> > > > \t\treturn nullptr;\n> > > >\n> > > > -\tstd::lock_guard locker(*mutex_);\n> > > > +\tstd::lock_guard locker(mutex_);\n> > > >\n> > > > \tif (buffers_.empty()) {\n> > > > \t\tLOG(HAL, Error) << \"Buffer underrun\";\n> > > > @@ -148,7 +146,7 @@ void CameraStream::putBuffer(libcamera::FrameBuffer *buffer)\n> > > > \tif (!allocator_)\n> > > > \t\treturn;\n> > > >\n> > > > -\tstd::lock_guard locker(*mutex_);\n> > > > +\tstd::lock_guard locker(mutex_);\n> > > >\n> > > > \tbuffers_.push_back(buffer);\n> > > > }\n> > > > diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> > > > index f929e8260ae3..a177a99a2ea1 100644\n> > > > --- a/src/android/camera_stream.h\n> > > > +++ b/src/android/camera_stream.h\n> > > > @@ -138,7 +138,7 @@ private:\n> > > > \tstd::unique_ptr encoder_;\n> > > > \tstd::unique_ptr allocator_;\n> > > > \tstd::vector buffers_;\n> > > > -\tstd::unique_ptr mutex_;\n> > > > +\tstd::mutex mutex_;\n> > > > };\n> > > >\n> > > > #endif /* __ANDROID_CAMERA_STREAM__ */\n> > > >\n> > > > This change can go on top or be integrated in the series (and there's no\n> > > > need to report just for this), up to you.\n> > > >\n> > > > Reviewed-by: Laurent Pinchart \n> > > >\n> > > > > +\t}\n> > > > > }\n> > > > >\n> > > > > const StreamConfiguration &CameraStream::configuration() const\n> > > > > @@ -46,6 +51,16 @@ int CameraStream::configure()\n> > > > > \t\t\treturn ret;\n> > > > > \t}\n> > > > >\n> > > > > +\tif (allocator_) {\n> > > > > +\t\tint ret = allocator_->allocate(stream());\n> > > > > +\t\tif (ret < 0)\n> > > > > +\t\t\treturn ret;\n> > > > > +\n> > > > > +\t\t/* Save a pointer to the reserved frame buffers */\n> > > > > +\t\tfor (const auto &frameBuffer : allocator_->buffers(stream()))\n> > > > > +\t\t\tbuffers_.push_back(frameBuffer.get());\n> > > > > +\t}\n> > > > > +\n> > > > > \tcamera3Stream_->max_buffers = configuration().bufferCount;\n> > > > >\n> > > > > \treturn 0;\n> > > > > @@ -109,3 +124,31 @@ int CameraStream::process(const libcamera::FrameBuffer &source,\n> > > > >\n> > > > > \treturn 0;\n> > > > > }\n> > > > > +\n> > > > > +FrameBuffer *CameraStream::getBuffer()\n> > > > > +{\n> > > > > +\tif (!allocator_)\n> > > > > +\t\treturn nullptr;\n> > > > > +\n> > > > > +\tstd::lock_guard locker(*mutex_);\n> > > > > +\n> > > > > +\tif (buffers_.empty()) {\n> > > > > +\t\tLOG(HAL, Error) << \"Buffer underrun\";\n> > > > > +\t\treturn nullptr;\n> > > > > +\t}\n> > > > > +\n> > > > > +\tFrameBuffer *buffer = buffers_.back();\n> > > > > +\tbuffers_.pop_back();\n> > > > > +\n> > > > > +\treturn buffer;\n> > > > > +}\n> > > > > +\n> > > > > +void CameraStream::putBuffer(libcamera::FrameBuffer *buffer)\n> > > > > +{\n> > > > > +\tif (!allocator_)\n> > > > > +\t\treturn;\n> > > > > +\n> > > > > +\tstd::lock_guard locker(*mutex_);\n> > > > > +\n> > > > > +\tbuffers_.push_back(buffer);\n> > > > > +}\n> > > > > diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> > > > > index 4c51f0fb3393..f929e8260ae3 100644\n> > > > > --- a/src/android/camera_stream.h\n> > > > > +++ b/src/android/camera_stream.h\n> > > > > @@ -8,11 +8,14 @@\n> > > > > #define __ANDROID_CAMERA_STREAM_H__\n> > > > >\n> > > > > #include \n> > > > > +#include \n> > > > > +#include \n> > > > >\n> > > > > #include \n> > > > >\n> > > > > #include \n> > > > > #include \n> > > > > +#include \n> > > > > #include \n> > > > > #include \n> > > > >\n> > > > > @@ -117,6 +120,8 @@ public:\n> > > > > \tint configure();\n> > > > > \tint process(const libcamera::FrameBuffer &source,\n> > > > > \t\t MappedCamera3Buffer *dest, CameraMetadata *metadata);\n> > > > > +\tlibcamera::FrameBuffer *getBuffer();\n> > > > > +\tvoid putBuffer(libcamera::FrameBuffer *buffer);\n> > > > >\n> > > > > private:\n> > > > > \tCameraDevice *cameraDevice_;\n> > > > > @@ -129,7 +134,11 @@ private:\n> > > > > \t * one or more streams to the Android framework.\n> > > > > \t */\n> > > > > \tunsigned int index_;\n> > > > > +\n> > > > > \tstd::unique_ptr encoder_;\n> > > > > +\tstd::unique_ptr allocator_;\n> > > > > +\tstd::vector buffers_;\n> > > > > +\tstd::unique_ptr mutex_;\n> > > > > };\n> > > > >\n> > > > > #endif /* __ANDROID_CAMERA_STREAM__ */","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 1E403BEEDF\n\tfor ;\n\tWed, 7 Oct 2020 13:36:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A9FAB605CF;\n\tWed, 7 Oct 2020 15:36:42 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 981D2605BE\n\tfor ;\n\tWed, 7 Oct 2020 15:36:41 +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 071E09DA;\n\tWed, 7 Oct 2020 15:36:40 +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=\"U42g07wV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1602077801;\n\tbh=hyMmg4wDEfATszYiXsoqaLYHMLzKHW4+400NdSBFazc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=U42g07wVomloEJtlY9BphxiTsEeeCV3vLoMhA+UsuXPq2KkArmzp+FxmdbR2uZ7Ua\n\tHaPPPRUslexfpETjiUVCR3OwwivEIwJq38m8Am6BW3Wga5KvvfmMSeX5o1pstn4t+g\n\tgLhb1iwh8hQ6wVtAz/U74zD8u1ouQwPgbqRf3BrI=","Date":"Wed, 7 Oct 2020 16:35:59 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20201007133559.GK3937@pendragon.ideasonboard.com>","References":"<20201006144432.22908-1-jacopo@jmondi.org>\n\t<20201006144432.22908-11-jacopo@jmondi.org>\n\t<20201007015320.GE30598@pendragon.ideasonboard.com>\n\t<20201007080532.h33ufwpl72kvi2ii@uno.localdomain>\n\t<20201007131802.GE3937@pendragon.ideasonboard.com>\n\t<20201007133853.qs3xd232knqeshfs@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201007133853.qs3xd232knqeshfs@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2 10/13] android: camera_stream:\n\tCreate buffer poll","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]