[{"id":12751,"web_url":"https://patchwork.libcamera.org/comment/12751/","msgid":"<16a5ad69-67a6-3099-5766-2db44006ed56@ideasonboard.com>","date":"2020-09-24T16:27:14","subject":"Re: [libcamera-devel] [PATCH v3 6/8] android: camera_device: Add\n\tmethods to get and return buffers","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 22/09/2020 10:47, Jacopo Mondi wrote:\n> Add two methods to the CameraDevice class to retrieve and return\n> frame buffers associated to a stream from the memory pool reserved\n> in libcamera.\n> \n> Protect accessing the vector of FrameBufer pointers with a\n\ns/FrameBufer/FrameBuffer/\n\n> per-pool mutex in the get and return buffer methods.\n> \n> Reviewed-by: Niklas Söderlund \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_device.cpp | 34 +++++++++++++++++++++++++++++++++-\n> src/android/camera_device.h | 11 +++++++++--\n> 2 files changed, 42 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index f96ea7321a67..6ed56ff57dab 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -8,6 +8,7 @@\n> #include \"camera_device.h\"\n> #include \"camera_ops.h\"\n> \n> +#include \n> #include \n> #include \n> #include \n> @@ -1400,11 +1401,42 @@ int CameraDevice::allocateBuffersPool(Stream *stream)\n> \t * the HAL.\n> \t */\n> \tfor (const auto &frameBuffer : allocator_.buffers(stream))\n> -\t\tbufferPool_[stream].push_back(frameBuffer.get());\n> +\t\tbufferPool_[stream].buffers.push_back(frameBuffer.get());\n> \n> \treturn 0;\n> }\n> \n> +libcamera::FrameBuffer *CameraDevice::getBuffer(libcamera::Stream *stream)\n> +{\n> +\tif (bufferPool_.find(stream) == bufferPool_.end())\n> +\t\treturn nullptr;\n> +\n> +\tBufferPool *pool = &bufferPool_[stream];\n> +\tstd::lock_guard locker(pool->mutex);\n> +\n> +\tif (pool->buffers.empty()) {\n> +\t\tLOG(HAL, Error) << \"Buffer underrun\";\n> +\t\treturn nullptr;\n> +\t}\n> +\n> +\tFrameBuffer *buffer = pool->buffers.front();\n> +\tpool->buffers.erase(pool->buffers.begin());\n> +\n\nThis feels quite complicated\n\nA buffer pool with multiple sets of buffers which are specific to each\nstream\n\nAs I've already suggested moving this stuff to be in a CameraStream:\n\nHow about making the BufferPool (with it's key attribute being the\nlocked container) a class with a getBuffer() and a putBuffer() which\nensures the lock is held in those calls.\n\nThen the individual buffer pools keep the lock (I guess the locking is\nrequired because this can all happen asynchronously in events).\n\n> +\treturn buffer;\n> +}\n> +\n> +void CameraDevice::returnBuffer(libcamera::Stream *stream,\n> +\t\t\t\tlibcamera::FrameBuffer *buffer)\n> +{\n> +\tif (bufferPool_.find(stream) == bufferPool_.end())\n> +\t\treturn;\n> +\n> +\tBufferPool *pool = &bufferPool_[stream];\n\nOh I misread earlier - it's a map of pools by stream, not a pool mapping\nmultiple streams of buffers.\n\n\nFor this series in general, I'd say - define the buffer /pool storage\nyou want, add it - then allocate the buffers and put them there.\n\nThis series has added them, then changed the storage along the way which\nhas been confusing to follow I think.\n\n\n\n> +\tstd::lock_guard locker(pool->mutex);\n> +\n> +\tpool->buffers.push_back(buffer);\n> +}\n> +\n> int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Request)\n> {\n> \tif (!camera3Request->num_output_buffers) {\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index 4cef34c01a49..5addffdc070a 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -9,6 +9,7 @@\n> \n> #include \n> #include \n> +#include \n> #include \n> #include \n> \n> @@ -166,8 +167,11 @@ protected:\n> \tstd::string logPrefix() const override;\n> \n> private:\n> -\tusing FrameBufferPool = std::map -\t\t\t\t\t std::vector>;\n> +\tstruct BufferPool {\n\nAha - my beloved BufferPool is coming back ;-)\n\n> +\t\tstd::mutex mutex;\n> +\t\tstd::vector buffers;\n> +\t};\n> +\tusing FrameBufferPool = std::map;\n\nI still see this (a BufferPool if required) as better handled inside the\nCameraStream object.\n\nI'm trying to picture in my head how the iterations might change, but I\nguess when filling a request, you would cycle each android stream and\ndeliver all given buffers to the CameraStreams.\n\nThen cycle all CameraStreams and ask if they have any buffer to add to\nthe current request.\n\nSend request...\n\nAnd on request complete, let the magic* complete everything.\n\n\n* Insert magic pixie dust where required.\n\nHrm ... but all of that is possibly quite different to your\n'internal/mapped/direct' concepts ... so I'm not quite sure yet anyway.\n\n\n\n> \n> \tCameraDevice(unsigned int id, const std::shared_ptr &camera);\n> \n> @@ -198,6 +202,9 @@ private:\n> \tstd::tuple calculateStaticMetadataSize();\n> \tlibcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer);\n> \tint allocateBuffersPool(libcamera::Stream *stream);\n> +\tlibcamera::FrameBuffer *getBuffer(libcamera::Stream *stream);\n> +\tvoid returnBuffer(libcamera::Stream *stream,\n> +\t\t\t libcamera::FrameBuffer *buffer);\n> \n> \tvoid notifyShutter(uint32_t frameNumber, uint64_t timestamp);\n> \tvoid notifyError(uint32_t frameNumber, camera3_stream_t *stream);\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 8A613C3B5B\n\tfor ;\n\tThu, 24 Sep 2020 16:27:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5B12962FEE;\n\tThu, 24 Sep 2020 18:27:20 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A555B60363\n\tfor ;\n\tThu, 24 Sep 2020 18:27:18 +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 1EA952FD;\n\tThu, 24 Sep 2020 18:27:17 +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=\"KE9gUQZf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1600964838;\n\tbh=a/WizOT/5bwb9DDJy2vjXnoqQAw64JdYClPxW+1Omfk=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=KE9gUQZfXcdpV+ZcQItuRrGNeRTL+h7037uufa4hwwa6kkisFI9tm5e8JJUFvBbNb\n\tYtVy+pdDY5rnra3P0C5Cyw15EGU6EOgce5d+AJHoL3gi7DkiyUD3Ui7z+TLnZbAcO6\n\tnKP/VDJsQHLKPOEkcO1I38ZPnIBpga/UHU/VNXfw=","To":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","References":"<20200922094738.5327-1-jacopo@jmondi.org>\n\t<20200922094738.5327-7-jacopo@jmondi.org>","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":"<16a5ad69-67a6-3099-5766-2db44006ed56@ideasonboard.com>","Date":"Thu, 24 Sep 2020 17:27:14 +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":"<20200922094738.5327-7-jacopo@jmondi.org>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v3 6/8] android: camera_device: Add\n\tmethods to get and return buffers","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":"hanlinchen@chromium.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12766,"web_url":"https://patchwork.libcamera.org/comment/12766/","msgid":"<20200925111032.gnw4b2tzhfaoups7@uno.localdomain>","date":"2020-09-25T11:10:32","subject":"Re: [libcamera-devel] [PATCH v3 6/8] android: camera_device: Add\n\tmethods to get and return buffers","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran,\n\nOn Thu, Sep 24, 2020 at 05:27:14PM +0100, Kieran Bingham wrote:\n> Hi Jacopo,\n>\n> On 22/09/2020 10:47, Jacopo Mondi wrote:\n> > Add two methods to the CameraDevice class to retrieve and return\n> > frame buffers associated to a stream from the memory pool reserved\n> > in libcamera.\n> >\n> > Protect accessing the vector of FrameBufer pointers with a\n>\n> s/FrameBufer/FrameBuffer/\n>\n> > per-pool mutex in the get and return buffer methods.\n> >\n> > Reviewed-by: Niklas Söderlund \n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/android/camera_device.cpp | 34 +++++++++++++++++++++++++++++++++-\n> > src/android/camera_device.h | 11 +++++++++--\n> > 2 files changed, 42 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index f96ea7321a67..6ed56ff57dab 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -8,6 +8,7 @@\n> > #include \"camera_device.h\"\n> > #include \"camera_ops.h\"\n> >\n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -1400,11 +1401,42 @@ int CameraDevice::allocateBuffersPool(Stream *stream)\n> > \t * the HAL.\n> > \t */\n> > \tfor (const auto &frameBuffer : allocator_.buffers(stream))\n> > -\t\tbufferPool_[stream].push_back(frameBuffer.get());\n> > +\t\tbufferPool_[stream].buffers.push_back(frameBuffer.get());\n> >\n> > \treturn 0;\n> > }\n> >\n> > +libcamera::FrameBuffer *CameraDevice::getBuffer(libcamera::Stream *stream)\n> > +{\n> > +\tif (bufferPool_.find(stream) == bufferPool_.end())\n> > +\t\treturn nullptr;\n> > +\n> > +\tBufferPool *pool = &bufferPool_[stream];\n> > +\tstd::lock_guard locker(pool->mutex);\n> > +\n> > +\tif (pool->buffers.empty()) {\n> > +\t\tLOG(HAL, Error) << \"Buffer underrun\";\n> > +\t\treturn nullptr;\n> > +\t}\n> > +\n> > +\tFrameBuffer *buffer = pool->buffers.front();\n> > +\tpool->buffers.erase(pool->buffers.begin());\n> > +\n>\n> This feels quite complicated\n\nJust to note that if you move this to CameraStream, you will save the\nfirst 3 lines only. And the first two are already a paranoid check as\nthat condition shall never happen.\n\n>\n> A buffer pool with multiple sets of buffers which are specific to each\n> stream\n>\n> As I've already suggested moving this stuff to be in a CameraStream:\n>\n> How about making the BufferPool (with it's key attribute being the\n> locked container) a class with a getBuffer() and a putBuffer() which\n> ensures the lock is held in those calls.\n>\n> Then the individual buffer pools keep the lock (I guess the locking is\n> required because this can all happen asynchronously in events).\n>\n\nLocking is require because you can have a request to complete while\nanother one is queued. Might this happen from different threads of\nexecution ? I am not a 100% sure but it mostly depends on the\nthreading model of the camera stack, and we have no guarantees that\nit's single threaded.\n\n> > +\treturn buffer;\n> > +}\n> > +\n> > +void CameraDevice::returnBuffer(libcamera::Stream *stream,\n> > +\t\t\t\tlibcamera::FrameBuffer *buffer)\n> > +{\n> > +\tif (bufferPool_.find(stream) == bufferPool_.end())\n> > +\t\treturn;\n> > +\n> > +\tBufferPool *pool = &bufferPool_[stream];\n>\n> Oh I misread earlier - it's a map of pools by stream, not a pool mapping\n> multiple streams of buffers.\n>\n>\n> For this series in general, I'd say - define the buffer /pool storage\n> you want, add it - then allocate the buffers and put them there.\n>\n> This series has added them, then changed the storage along the way which\n> has been confusing to follow I think.\n\nWhen it was added there was no lock required. As soon as it's need I\nadded it. Adding before using it makes any sense ?\n\n>\n>\n>\n> > +\tstd::lock_guard locker(pool->mutex);\n> > +\n> > +\tpool->buffers.push_back(buffer);\n> > +}\n> > +\n> > int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Request)\n> > {\n> > \tif (!camera3Request->num_output_buffers) {\n> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > index 4cef34c01a49..5addffdc070a 100644\n> > --- a/src/android/camera_device.h\n> > +++ b/src/android/camera_device.h\n> > @@ -9,6 +9,7 @@\n> >\n> > #include \n> > #include \n> > +#include \n> > #include \n> > #include \n> >\n> > @@ -166,8 +167,11 @@ protected:\n> > \tstd::string logPrefix() const override;\n> >\n> > private:\n> > -\tusing FrameBufferPool = std::map > -\t\t\t\t\t std::vector>;\n> > +\tstruct BufferPool {\n>\n> Aha - my beloved BufferPool is coming back ;-)\n>\n> > +\t\tstd::mutex mutex;\n> > +\t\tstd::vector buffers;\n> > +\t};\n> > +\tusing FrameBufferPool = std::map;\n>\n> I still see this (a BufferPool if required) as better handled inside the\n> CameraStream object.\n>\n> I'm trying to picture in my head how the iterations might change, but I\n> guess when filling a request, you would cycle each android stream and\n> deliver all given buffers to the CameraStreams.\n>\n> Then cycle all CameraStreams and ask if they have any buffer to add to\n> the current request.\n>\n> Send request...\n>\n> And on request complete, let the magic* complete everything.\n>\n>\n> * Insert magic pixie dust where required.\n>\n> Hrm ... but all of that is possibly quite different to your\n> 'internal/mapped/direct' concepts ... so I'm not quite sure yet anyway.\n\n\nSo I tried moving the buffer pool to the CameraStream, but to me there\nare two constraints here:\n\n- the FrameBufferAllocator is constructed with a Camera and indexes\n buffers by Stream -> it belongs to the CameraDevice which also\n manages the lifetime of the allocated FrameBuffer\n- I don't want resource allocation/free in one class and the logic to\n get/return buffers in one other as it seems a layer violation to me\n\nI tried moving allocation/release of buffers as well as get/retrurn as\nyou suggested to the CameraStream class. What happens is that:\n- The CameraStream constructor has now 7 parameters -> it's a signal\n something is wrong imho\n- I want to release buffers for the Stream handled by the CameraStream\n in the CameraStream::~CameraStream() destructor. If I add a\n destructor the class implicit copy contructor gets deleted. If I add\n it as a default it fails as the Encoder virtual base class is not\n copy constructible. If I add a copy constructor I should rework how\n the encoder is handled, as right now is a unique_ptr<> and moving it\n from 'other' to 'this' would invalidate 'other'. If the CameraStream\n class is not copy constructable, we cannot streams_.reserve() and\n all the house of cards built on the assumption streams_ doesn't get\n relocated falls down.\n\nAll in all, I don't think it's worth at this point. CameraStream will\nbe reworked, currently there's no clear separation between what's\nAndroid and what's libcamera (I'm referring to sizes and format) and\nthe constructor has already too many parameters sign that we're\nduplicating information between CameraDevice and CameraStream.\n\nFor this series, I won't go to that extent. Maybe there's some easy\nroute I haven't identified yet.\n\n>\n>\n>\n> >\n> > \tCameraDevice(unsigned int id, const std::shared_ptr &camera);\n> >\n> > @@ -198,6 +202,9 @@ private:\n> > \tstd::tuple calculateStaticMetadataSize();\n> > \tlibcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer);\n> > \tint allocateBuffersPool(libcamera::Stream *stream);\n> > +\tlibcamera::FrameBuffer *getBuffer(libcamera::Stream *stream);\n> > +\tvoid returnBuffer(libcamera::Stream *stream,\n> > +\t\t\t libcamera::FrameBuffer *buffer);\n> >\n> > \tvoid notifyShutter(uint32_t frameNumber, uint64_t timestamp);\n> > \tvoid notifyError(uint32_t frameNumber, camera3_stream_t *stream);\n> >\n>\n> --\n> Regards\n> --\n> Kieran","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 53475C3B5B\n\tfor ;\n\tFri, 25 Sep 2020 11:06:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E911C63021;\n\tFri, 25 Sep 2020 13:06:40 +0200 (CEST)","from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B767A62FD8\n\tfor ;\n\tFri, 25 Sep 2020 13:06:39 +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 relay4-d.mail.gandi.net (Postfix) with ESMTPSA id BB5ECE000B;\n\tFri, 25 Sep 2020 11:06:38 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Fri, 25 Sep 2020 13:10:32 +0200","From":"Jacopo Mondi ","To":"Kieran Bingham ","Message-ID":"<20200925111032.gnw4b2tzhfaoups7@uno.localdomain>","References":"<20200922094738.5327-1-jacopo@jmondi.org>\n\t<20200922094738.5327-7-jacopo@jmondi.org>\n\t<16a5ad69-67a6-3099-5766-2db44006ed56@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<16a5ad69-67a6-3099-5766-2db44006ed56@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 6/8] android: camera_device: Add\n\tmethods to get and return buffers","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":"hanlinchen@chromium.org, libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]