[{"id":23209,"web_url":"https://patchwork.libcamera.org/comment/23209/","msgid":"<90b73de3-b831-6b61-7b4c-02db614ea062@ideasonboard.com>","date":"2022-05-29T09:44:14","subject":"Re: [libcamera-devel] [PATCH v2 1/5] android: camera_stream: Add\n\tsourceStream","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch\n\nOn 5/27/22 11:34, Paul Elder via libcamera-devel wrote:\n> From: Hirokazu Honda \n>\n> Add to the CameraStream class a sourceStream field, which for streams\n> of type Mapped contains a reference to the stream which produces the\n> actual image data.\n\n\nI think some re-pharsing would be nice to better comprehend the changes.\n\n\"Add a sourceStream field to the CameraStream class, meant to contain a \nreference\nof a direct stream (which produces actual image data) for \nCameraStream::Mapped\nstream types.\"\n\n>\n> The sourceStream of mapped streams will be used in later patches to make\n> sure for each Mapped stream at least one libcamera::Stream is queued to\n> the libcamera::Camera.\n>\n> Signed-off-by: Hirokazu Honda \n> Signed-off-by: Jacopo Mondi \n> Signed-off-by: Paul Elder \n>\n> ---\n> Changes in v2:\n> - fix whitespace\n> ---\n> src/android/camera_device.cpp | 8 +++++++-\n> src/android/camera_stream.cpp | 6 ++++--\n> src/android/camera_stream.h | 6 +++++-\n> 3 files changed, 16 insertions(+), 4 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 8e804d4d..20599665 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -681,10 +681,16 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \tfor (const auto &streamConfig : streamConfigs) {\n> \t\tconfig->addConfiguration(streamConfig.config);\n> \n> +\t\tCameraStream *sourceStream = nullptr;\n> \t\tfor (auto &stream : streamConfig.streams) {\n> \t\t\tstreams_.emplace_back(this, config.get(), stream.type,\n> -\t\t\t\t\t stream.stream, config->size() - 1);\n> +\t\t\t\t\t stream.stream, sourceStream,\n> +\t\t\t\t\t config->size() - 1);\n> \t\t\tstream.stream->priv = static_cast(&streams_.back());\n> +\n> +\t\t\t/* Mapped streams are always associated with a Direct one. */\n> +\t\t\tif (stream.type == CameraStream::Type::Direct)\n\n\nThis doesn't look right to me, why are we setting sourceStream field for \na direct stream type?\n\nShouldn't be it something like:\n\n+            if (stream.type == CameraStream::Type::Mapped)\n+                      sourceStream = ....\n\ninstead?\n\n> +\t\t\t\tsourceStream = &streams_.back();\n> \t\t}\n> \t}\n> \n> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> index 94f1884c..154e088e 100644\n> --- a/src/android/camera_stream.cpp\n> +++ b/src/android/camera_stream.cpp\n> @@ -52,9 +52,11 @@ LOG_DECLARE_CATEGORY(HAL)\n> \n> CameraStream::CameraStream(CameraDevice *const cameraDevice,\n> \t\t\t CameraConfiguration *config, Type type,\n> -\t\t\t camera3_stream_t *camera3Stream, unsigned int index)\n> +\t\t\t camera3_stream_t *camera3Stream,\n> +\t\t\t CameraStream *const sourceStream, unsigned int index)\n> \t: cameraDevice_(cameraDevice), config_(config), type_(type),\n> -\t camera3Stream_(camera3Stream), index_(index)\n> +\t camera3Stream_(camera3Stream), sourceStream_(sourceStream),\n> +\t index_(index)\n> {\n> }\n> \n> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> index f9504462..4c5078b2 100644\n> --- a/src/android/camera_stream.h\n> +++ b/src/android/camera_stream.h\n> @@ -114,7 +114,9 @@ public:\n> \t};\n> \tCameraStream(CameraDevice *const cameraDevice,\n> \t\t libcamera::CameraConfiguration *config, Type type,\n> -\t\t camera3_stream_t *camera3Stream, unsigned int index);\n> +\t\t camera3_stream_t *camera3Stream,\n> +\t\t CameraStream *const sourceStream,\n> +\t\t unsigned int index);\n> \tCameraStream(CameraStream &&other);\n> \t~CameraStream();\n> \n> @@ -122,6 +124,7 @@ public:\n> \tcamera3_stream_t *camera3Stream() const { return camera3Stream_; }\n> \tconst libcamera::StreamConfiguration &configuration() const;\n> \tlibcamera::Stream *stream() const;\n> +\tCameraStream *sourceStream() const { return sourceStream_; }\n> \n> \tint configure();\n> \tint process(Camera3RequestDescriptor::StreamBuffer *streamBuffer);\n> @@ -167,6 +170,7 @@ private:\n> \tconst libcamera::CameraConfiguration *config_;\n> \tconst Type type_;\n> \tcamera3_stream_t *camera3Stream_;\n> +\tCameraStream *const sourceStream_;\n> \tconst unsigned int index_;\n> \n> \tstd::unique_ptr allocator_;","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 69034BD160\n\tfor ;\n\tSun, 29 May 2022 09:44:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9CDF265633;\n\tSun, 29 May 2022 11:44:19 +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 05D7960413\n\tfor ;\n\tSun, 29 May 2022 11:44:18 +0200 (CEST)","from [192.168.1.68] (235.red-81-36-186.dynamicip.rima-tde.net\n\t[81.36.186.235])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 864686DC;\n\tSun, 29 May 2022 11:44:17 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1653817459;\n\tbh=7mFQETLPJqGMq7r+h/rbi/lI2wD3C7lESO1lbnNzxoE=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=qPLFFAl8tkybK7As6SLVDnJ6REFSrS7h/J2/0Wi87hHZZpXDDjsUQCEt71WjHwE/T\n\tSmQDeBPLwzLkggycVCZFN62hs+PVX4wDqBrUM+wzdoi8Gwd3pWS52jaVDEkT5CPd3l\n\t28QDmiDm89uXB1uTKyHTRzNFRhfsf5yEqWGQLeTc108uQRyr5hXKQL93gXi6vI5TuY\n\tcvgPQxushIrIvmAjPaBMzgDKGd7tzTng6vcJfWyz62nP8sH+zVjag3JuQDe+5l0RFF\n\tRvVWh+bXp1VEKjImTc3+6l+u+A56MG06RTOjsbTlJxVN9qJv7yMlLJbQ+J6jvd2mmN\n\tJp6wDmCqfb+4w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1653817457;\n\tbh=7mFQETLPJqGMq7r+h/rbi/lI2wD3C7lESO1lbnNzxoE=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=FQycSedHqYDiAVRBo2eMGn8jf6Zgjp8JRefmM6b+a8/guI52Yl12QRgvmgaMNmwkX\n\ta8BzcSaANvSOQd5fpuWuHYQxbrWITzbF18hprgP6E3QFiy9UNZp1qPgf5oCvHlX2Ku\n\tan2vuqTR0xDlLkMvqA6LmCK+rVR2WLP+0QWvSLck="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"FQycSedH\"; dkim-atps=neutral","Message-ID":"<90b73de3-b831-6b61-7b4c-02db614ea062@ideasonboard.com>","Date":"Sun, 29 May 2022 11:44:14 +0200","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220527093440.953377-1-paul.elder@ideasonboard.com>\n\t<20220527093440.953377-2-paul.elder@ideasonboard.com>","In-Reply-To":"<20220527093440.953377-2-paul.elder@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 1/5] android: camera_stream: Add\n\tsourceStream","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","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23241,"web_url":"https://patchwork.libcamera.org/comment/23241/","msgid":"<20220530095225.ed7myp4litabsbg4@uno.localdomain>","date":"2022-05-30T09:52:25","subject":"Re: [libcamera-devel] [PATCH v2 1/5] android: camera_stream: Add\n\tsourceStream","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Umang,\n\nOn Sun, May 29, 2022 at 11:44:14AM +0200, Umang Jain via libcamera-devel wrote:\n> Hi Paul,\n>\n> Thank you for the patch\n>\n> On 5/27/22 11:34, Paul Elder via libcamera-devel wrote:\n> > From: Hirokazu Honda \n> >\n> > Add to the CameraStream class a sourceStream field, which for streams\n> > of type Mapped contains a reference to the stream which produces the\n> > actual image data.\n>\n>\n> I think some re-pharsing would be nice to better comprehend the changes.\n>\n> \"Add a sourceStream field to the CameraStream class, meant to contain a\n> reference\n> of a direct stream (which produces actual image data) for\n> CameraStream::Mapped\n> stream types.\"\n>\n> >\n> > The sourceStream of mapped streams will be used in later patches to make\n> > sure for each Mapped stream at least one libcamera::Stream is queued to\n> > the libcamera::Camera.\n> >\n> > Signed-off-by: Hirokazu Honda \n> > Signed-off-by: Jacopo Mondi \n> > Signed-off-by: Paul Elder \n> >\n> > ---\n> > Changes in v2:\n> > - fix whitespace\n> > ---\n> > src/android/camera_device.cpp | 8 +++++++-\n> > src/android/camera_stream.cpp | 6 ++++--\n> > src/android/camera_stream.h | 6 +++++-\n> > 3 files changed, 16 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 8e804d4d..20599665 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -681,10 +681,16 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > \tfor (const auto &streamConfig : streamConfigs) {\n> > \t\tconfig->addConfiguration(streamConfig.config);\n> > +\t\tCameraStream *sourceStream = nullptr;\n> > \t\tfor (auto &stream : streamConfig.streams) {\n> > \t\t\tstreams_.emplace_back(this, config.get(), stream.type,\n> > -\t\t\t\t\t stream.stream, config->size() - 1);\n> > +\t\t\t\t\t stream.stream, sourceStream,\n> > +\t\t\t\t\t config->size() - 1);\n> > \t\t\tstream.stream->priv = static_cast(&streams_.back());\n> > +\n> > +\t\t\t/* Mapped streams are always associated with a Direct one. */\n> > +\t\t\tif (stream.type == CameraStream::Type::Direct)\n>\n>\n> This doesn't look right to me, why are we setting sourceStream field for a\n> direct stream type?\n>\n> Shouldn't be it something like:\n>\n> +            if (stream.type == CameraStream::Type::Mapped)\n> +                      sourceStream = ....\n>\n> instead?\n>\n\nI think it's correct the way it is.\n\nWe populate sourceStream for streams of type Mapped, and initialze it\nwhen the corresponding Direct stream is first found in the\nstreamConfig.streams list.\n\n\n> > +\t\t\t\tsourceStream = &streams_.back();\n> > \t\t}\n> > \t}\n> > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> > index 94f1884c..154e088e 100644\n> > --- a/src/android/camera_stream.cpp\n> > +++ b/src/android/camera_stream.cpp\n> > @@ -52,9 +52,11 @@ LOG_DECLARE_CATEGORY(HAL)\n> > CameraStream::CameraStream(CameraDevice *const cameraDevice,\n> > \t\t\t CameraConfiguration *config, Type type,\n> > -\t\t\t camera3_stream_t *camera3Stream, unsigned int index)\n> > +\t\t\t camera3_stream_t *camera3Stream,\n> > +\t\t\t CameraStream *const sourceStream, unsigned int index)\n> > \t: cameraDevice_(cameraDevice), config_(config), type_(type),\n> > -\t camera3Stream_(camera3Stream), index_(index)\n> > +\t camera3Stream_(camera3Stream), sourceStream_(sourceStream),\n> > +\t index_(index)\n> > {\n> > }\n> > diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> > index f9504462..4c5078b2 100644\n> > --- a/src/android/camera_stream.h\n> > +++ b/src/android/camera_stream.h\n> > @@ -114,7 +114,9 @@ public:\n> > \t};\n> > \tCameraStream(CameraDevice *const cameraDevice,\n> > \t\t libcamera::CameraConfiguration *config, Type type,\n> > -\t\t camera3_stream_t *camera3Stream, unsigned int index);\n> > +\t\t camera3_stream_t *camera3Stream,\n> > +\t\t CameraStream *const sourceStream,\n> > +\t\t unsigned int index);\n> > \tCameraStream(CameraStream &&other);\n> > \t~CameraStream();\n> > @@ -122,6 +124,7 @@ public:\n> > \tcamera3_stream_t *camera3Stream() const { return camera3Stream_; }\n> > \tconst libcamera::StreamConfiguration &configuration() const;\n> > \tlibcamera::Stream *stream() const;\n> > +\tCameraStream *sourceStream() const { return sourceStream_; }\n> > \tint configure();\n> > \tint process(Camera3RequestDescriptor::StreamBuffer *streamBuffer);\n> > @@ -167,6 +170,7 @@ private:\n> > \tconst libcamera::CameraConfiguration *config_;\n> > \tconst Type type_;\n> > \tcamera3_stream_t *camera3Stream_;\n> > +\tCameraStream *const sourceStream_;\n> > \tconst unsigned int index_;\n> > \tstd::unique_ptr allocator_;","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 59F12BD161\n\tfor ;\n\tMon, 30 May 2022 09:52:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B08CF61FB7;\n\tMon, 30 May 2022 11:52:29 +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 452BE6040B\n\tfor ;\n\tMon, 30 May 2022 11:52:28 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 43918E0004;\n\tMon, 30 May 2022 09:52:26 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1653904349;\n\tbh=CCiy52DWsjpo+IE51MahVDan8ODnagvvmpbFiG6HniA=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=WfhiMgpImirbf97V7NnXeUF8lmdFq2KKNVSHpNv2/t1DzytMJpdApGWIhLox+N2YA\n\tXFZ3mo/FUex9OUyGp8+0QMr9MLbB0Em9eDyDKX1lMtRUK+aVa/Lr+KP4YnoQL15Bo3\n\t0H6BX9iCdt8MRRKBsMvCijY94Xu23TXYk0A9+vcDW8qlVQva6gkrIfa4A34gYPSjzl\n\tbM+ne6dBqdru3dnP4jSejOoDjgpObJ5Y/XjR99VNvkqeVz5CkSKPYxwpMfWIHJei5W\n\t4RHzQbyv5wxf/Aux4DwzFiOGDNqFGD1nfKSj3srVE1q++cl6bnacncmAZVjA73Rm9x\n\ttF74ahLcgf+ZQ==","Date":"Mon, 30 May 2022 11:52:25 +0200","To":"Umang Jain ","Message-ID":"<20220530095225.ed7myp4litabsbg4@uno.localdomain>","References":"<20220527093440.953377-1-paul.elder@ideasonboard.com>\n\t<20220527093440.953377-2-paul.elder@ideasonboard.com>\n\t<90b73de3-b831-6b61-7b4c-02db614ea062@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<90b73de3-b831-6b61-7b4c-02db614ea062@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 1/5] android: camera_stream: Add\n\tsourceStream","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23248,"web_url":"https://patchwork.libcamera.org/comment/23248/","msgid":"","date":"2022-05-30T10:37:14","subject":"Re: [libcamera-devel] [PATCH v2 1/5] android: camera_stream: Add\n\tsourceStream","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Jacopo\n\nOn 5/30/22 11:52, Jacopo Mondi wrote:\n> Hi Umang,\n>\n> On Sun, May 29, 2022 at 11:44:14AM +0200, Umang Jain via libcamera-devel wrote:\n>> Hi Paul,\n>>\n>> Thank you for the patch\n>>\n>> On 5/27/22 11:34, Paul Elder via libcamera-devel wrote:\n>>> From: Hirokazu Honda \n>>>\n>>> Add to the CameraStream class a sourceStream field, which for streams\n>>> of type Mapped contains a reference to the stream which produces the\n>>> actual image data.\n>>\n>> I think some re-pharsing would be nice to better comprehend the changes.\n>>\n>> \"Add a sourceStream field to the CameraStream class, meant to contain a\n>> reference\n>> of a direct stream (which produces actual image data) for\n>> CameraStream::Mapped\n>> stream types.\"\n>>\n>>> The sourceStream of mapped streams will be used in later patches to make\n>>> sure for each Mapped stream at least one libcamera::Stream is queued to\n>>> the libcamera::Camera.\n>>>\n>>> Signed-off-by: Hirokazu Honda \n>>> Signed-off-by: Jacopo Mondi \n>>> Signed-off-by: Paul Elder \n>>>\n>>> ---\n>>> Changes in v2:\n>>> - fix whitespace\n>>> ---\n>>> src/android/camera_device.cpp | 8 +++++++-\n>>> src/android/camera_stream.cpp | 6 ++++--\n>>> src/android/camera_stream.h | 6 +++++-\n>>> 3 files changed, 16 insertions(+), 4 deletions(-)\n>>>\n>>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n>>> index 8e804d4d..20599665 100644\n>>> --- a/src/android/camera_device.cpp\n>>> +++ b/src/android/camera_device.cpp\n>>> @@ -681,10 +681,16 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>>> \tfor (const auto &streamConfig : streamConfigs) {\n>>> \t\tconfig->addConfiguration(streamConfig.config);\n>>> +\t\tCameraStream *sourceStream = nullptr;\n>>> \t\tfor (auto &stream : streamConfig.streams) {\n>>> \t\t\tstreams_.emplace_back(this, config.get(), stream.type,\n>>> -\t\t\t\t\t stream.stream, config->size() - 1);\n>>> +\t\t\t\t\t stream.stream, sourceStream,\n>>> +\t\t\t\t\t config->size() - 1);\n>>> \t\t\tstream.stream->priv = static_cast(&streams_.back());\n>>> +\n>>> +\t\t\t/* Mapped streams are always associated with a Direct one. */\n>>> +\t\t\tif (stream.type == CameraStream::Type::Direct)\n>>\n>> This doesn't look right to me, why are we setting sourceStream field for a\n>> direct stream type?\n>>\n>> Shouldn't be it something like:\n>>\n>> +            if (stream.type == CameraStream::Type::Mapped)\n>> +                      sourceStream = ....\n>>\n>> instead?\n>>\n> I think it's correct the way it is.\n>\n> We populate sourceStream for streams of type Mapped, and initialze it\n> when the corresponding Direct stream is first found in the\n> streamConfig.streams list.\n\n\nAh yeah, I got it now. I failed to see the sourceStream is a single \npointer for the entire inner loop. It's setting the sourceStream on a \ndirect stream and then on next iteration (probably which is a mapped \nstream), the sourceStream is not-null and hence gets emplace_back corretly.\n\nThanks for clarifying,\n\nWith the commit message be re-worded a bit,\n\nReviewed-by: Umang Jain \n\n>\n>\n>>> +\t\t\t\tsourceStream = &streams_.back();\n>>> \t\t}\n>>> \t}\n>>> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n>>> index 94f1884c..154e088e 100644\n>>> --- a/src/android/camera_stream.cpp\n>>> +++ b/src/android/camera_stream.cpp\n>>> @@ -52,9 +52,11 @@ LOG_DECLARE_CATEGORY(HAL)\n>>> CameraStream::CameraStream(CameraDevice *const cameraDevice,\n>>> \t\t\t CameraConfiguration *config, Type type,\n>>> -\t\t\t camera3_stream_t *camera3Stream, unsigned int index)\n>>> +\t\t\t camera3_stream_t *camera3Stream,\n>>> +\t\t\t CameraStream *const sourceStream, unsigned int index)\n>>> \t: cameraDevice_(cameraDevice), config_(config), type_(type),\n>>> -\t camera3Stream_(camera3Stream), index_(index)\n>>> +\t camera3Stream_(camera3Stream), sourceStream_(sourceStream),\n>>> +\t index_(index)\n>>> {\n>>> }\n>>> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n>>> index f9504462..4c5078b2 100644\n>>> --- a/src/android/camera_stream.h\n>>> +++ b/src/android/camera_stream.h\n>>> @@ -114,7 +114,9 @@ public:\n>>> \t};\n>>> \tCameraStream(CameraDevice *const cameraDevice,\n>>> \t\t libcamera::CameraConfiguration *config, Type type,\n>>> -\t\t camera3_stream_t *camera3Stream, unsigned int index);\n>>> +\t\t camera3_stream_t *camera3Stream,\n>>> +\t\t CameraStream *const sourceStream,\n>>> +\t\t unsigned int index);\n>>> \tCameraStream(CameraStream &&other);\n>>> \t~CameraStream();\n>>> @@ -122,6 +124,7 @@ public:\n>>> \tcamera3_stream_t *camera3Stream() const { return camera3Stream_; }\n>>> \tconst libcamera::StreamConfiguration &configuration() const;\n>>> \tlibcamera::Stream *stream() const;\n>>> +\tCameraStream *sourceStream() const { return sourceStream_; }\n>>> \tint configure();\n>>> \tint process(Camera3RequestDescriptor::StreamBuffer *streamBuffer);\n>>> @@ -167,6 +170,7 @@ private:\n>>> \tconst libcamera::CameraConfiguration *config_;\n>>> \tconst Type type_;\n>>> \tcamera3_stream_t *camera3Stream_;\n>>> +\tCameraStream *const sourceStream_;\n>>> \tconst unsigned int index_;\n>>> \tstd::unique_ptr allocator_;","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 BB8C0BD161\n\tfor ;\n\tMon, 30 May 2022 10:37:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5B1B365639;\n\tMon, 30 May 2022 12:37: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 1F83C65634\n\tfor ;\n\tMon, 30 May 2022 12:37:18 +0200 (CEST)","from [192.168.1.68] (235.red-81-36-186.dynamicip.rima-tde.net\n\t[81.36.186.235])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 93B79B90;\n\tMon, 30 May 2022 12:37:17 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1653907040;\n\tbh=Th4QcbxV3DSntRdQ/QFp8QbN4IqRDcYnX15y3aMsCy4=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=EHaSJlUxYXnG3OKRZgWTOYHOK661gIROb2fBhCW3yMJ1QsD6Pl7rFYugts2q6MYl3\n\tTK6dba+SGbnzLbW802KJSzGTEJlQzJ4rmaVKJDUsYRt4If/uFT9jqQjeFW8VQtUlmN\n\tTuhM8PtQhRV2ztjmIoxfMlMx8VyXFqBPvcM/ocQZSzL0IexKJLU1PYn0l4QNxGMNAC\n\t++p6KNFl66UQPrLw5d5fzM6u0z0JajbQyjCJu1gV/R7dgU3bwrtz+ptowK8TetGGKM\n\tSBP72WOMJ4LfbigccBWfP0cAjjTT2ahUiSp32Gte9WFIMI4XPIU70x/AhhRxmn4llM\n\t+o2rRMo07E9MA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1653907037;\n\tbh=Th4QcbxV3DSntRdQ/QFp8QbN4IqRDcYnX15y3aMsCy4=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=M0vwkSdOCHC9AC/oyJwI7KmFapA57wDipKZ7U+I/YFTUf8yuJTk5HgujOs2zmIqjL\n\tXUx/VFI8rLaXVcKt/CyCOMWjg1pb3LRJCVK/94X0k53Tum1W09XVYSyMnr/cD9ibIQ\n\tOlwxzg64go4LaZRZEYOpsRO5SHwBg+etUe9MDOX4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"M0vwkSdO\"; dkim-atps=neutral","Message-ID":"","Date":"Mon, 30 May 2022 12:37:14 +0200","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Jacopo Mondi ","References":"<20220527093440.953377-1-paul.elder@ideasonboard.com>\n\t<20220527093440.953377-2-paul.elder@ideasonboard.com>\n\t<90b73de3-b831-6b61-7b4c-02db614ea062@ideasonboard.com>\n\t<20220530095225.ed7myp4litabsbg4@uno.localdomain>","In-Reply-To":"<20220530095225.ed7myp4litabsbg4@uno.localdomain>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 1/5] android: camera_stream: Add\n\tsourceStream","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","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23286,"web_url":"https://patchwork.libcamera.org/comment/23286/","msgid":"","date":"2022-06-02T07:52:39","subject":"Re: [libcamera-devel] [PATCH v2 1/5] android: camera_stream: Add\n\tsourceStream","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Mon, May 30, 2022 at 12:37:14PM +0200, Umang Jain via libcamera-devel wrote:\n> On 5/30/22 11:52, Jacopo Mondi wrote:\n> > On Sun, May 29, 2022 at 11:44:14AM +0200, Umang Jain via libcamera-devel wrote:\n> >> On 5/27/22 11:34, Paul Elder via libcamera-devel wrote:\n> >>> From: Hirokazu Honda \n> >>>\n> >>> Add to the CameraStream class a sourceStream field, which for streams\n> >>> of type Mapped contains a reference to the stream which produces the\n> >>> actual image data.\n> >>\n> >> I think some re-pharsing would be nice to better comprehend the changes.\n> >>\n> >> \"Add a sourceStream field to the CameraStream class, meant to contain a\n> >> reference\n> >> of a direct stream (which produces actual image data) for\n> >> CameraStream::Mapped\n> >> stream types.\"\n> >>\n> >>> The sourceStream of mapped streams will be used in later patches to make\n> >>> sure for each Mapped stream at least one libcamera::Stream is queued to\n> >>> the libcamera::Camera.\n> >>>\n> >>> Signed-off-by: Hirokazu Honda \n> >>> Signed-off-by: Jacopo Mondi \n> >>> Signed-off-by: Paul Elder \n> >>>\n> >>> ---\n> >>> Changes in v2:\n> >>> - fix whitespace\n> >>> ---\n> >>> src/android/camera_device.cpp | 8 +++++++-\n> >>> src/android/camera_stream.cpp | 6 ++++--\n> >>> src/android/camera_stream.h | 6 +++++-\n> >>> 3 files changed, 16 insertions(+), 4 deletions(-)\n> >>>\n> >>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> >>> index 8e804d4d..20599665 100644\n> >>> --- a/src/android/camera_device.cpp\n> >>> +++ b/src/android/camera_device.cpp\n> >>> @@ -681,10 +681,16 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> >>> \tfor (const auto &streamConfig : streamConfigs) {\n> >>> \t\tconfig->addConfiguration(streamConfig.config);\n> >>> + \t\tCameraStream *sourceStream = nullptr;\n> >>> \t\tfor (auto &stream : streamConfig.streams) {\n> >>> \t\t\tstreams_.emplace_back(this, config.get(), stream.type,\n> >>> - \t\t\t\t\t stream.stream, config->size() - 1);\n> >>> + \t\t\t\t\t stream.stream, sourceStream,\n> >>> + \t\t\t\t\t config->size() - 1);\n> >>> \t\t\tstream.stream->priv = static_cast(&streams_.back());\n> >>> +\n> >>> + \t\t\t/* Mapped streams are always associated with a Direct one. */\n> >>> + \t\t\tif (stream.type == CameraStream::Type::Direct)\n> >>\n> >> This doesn't look right to me, why are we setting sourceStream field for a\n> >> direct stream type?\n> >>\n> >> Shouldn't be it something like:\n> >>\n> >> +            if (stream.type == CameraStream::Type::Mapped)\n> >> +                      sourceStream = ....\n> >>\n> >> instead?\n> >>\n> > I think it's correct the way it is.\n> >\n> > We populate sourceStream for streams of type Mapped, and initialze it\n> > when the corresponding Direct stream is first found in the\n> > streamConfig.streams list.\n> \n> Ah yeah, I got it now. I failed to see the sourceStream is a single \n> pointer for the entire inner loop. It's setting the sourceStream on a \n> direct stream and then on next iteration (probably which is a mapped \n> stream), the sourceStream is not-null and hence gets emplace_back corretly.\n\nI think this deserved to be captured in the comment, especially given\nthat we rely on the direct stream being present in the streams list\nbefore any mapped stream.\n\n\t\t\t/*\n\t\t\t * The streamConfig.streams vector contains as its first\n\t\t\t * element a Direct (or Internal) stream, and then an\n\t\t\t * optional set Mapped streams derived from the\n\t\t\t * Direct streams. Cache the Direct stream pointer, to\n\t\t\t * be used when constructing the subsequent mapped\n\t\t\t * streams.\n\t\t\t */\n\n\n> Thanks for clarifying,\n> \n> With the commit message be re-worded a bit,\n> \n> Reviewed-by: Umang Jain \n> \n> >>> + \t\t\t\tsourceStream = &streams_.back();\n> >>> \t\t}\n> >>> \t}\n> >>> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> >>> index 94f1884c..154e088e 100644\n> >>> --- a/src/android/camera_stream.cpp\n> >>> +++ b/src/android/camera_stream.cpp\n> >>> @@ -52,9 +52,11 @@ LOG_DECLARE_CATEGORY(HAL)\n> >>> CameraStream::CameraStream(CameraDevice *const cameraDevice,\n> >>> \t\t\t CameraConfiguration *config, Type type,\n> >>> - \t\t\t camera3_stream_t *camera3Stream, unsigned int index)\n> >>> + \t\t\t camera3_stream_t *camera3Stream,\n> >>> + \t\t\t CameraStream *const sourceStream, unsigned int index)\n> >>> \t: cameraDevice_(cameraDevice), config_(config), type_(type),\n> >>> - \t camera3Stream_(camera3Stream), index_(index)\n> >>> + \t camera3Stream_(camera3Stream), sourceStream_(sourceStream),\n> >>> + \t index_(index)\n> >>> {\n> >>> }\n> >>> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> >>> index f9504462..4c5078b2 100644\n> >>> --- a/src/android/camera_stream.h\n> >>> +++ b/src/android/camera_stream.h\n> >>> @@ -114,7 +114,9 @@ public:\n> >>> \t};\n> >>> \tCameraStream(CameraDevice *const cameraDevice,\n> >>> \t\t libcamera::CameraConfiguration *config, Type type,\n> >>> - \t\t camera3_stream_t *camera3Stream, unsigned int index);\n> >>> + \t\t camera3_stream_t *camera3Stream,\n> >>> + \t\t CameraStream *const sourceStream,\n> >>> + \t\t unsigned int index);\n> >>> \tCameraStream(CameraStream &&other);\n> >>> \t~CameraStream();\n> >>> @@ -122,6 +124,7 @@ public:\n> >>> \tcamera3_stream_t *camera3Stream() const { return camera3Stream_; }\n> >>> \tconst libcamera::StreamConfiguration &configuration() const;\n> >>> \tlibcamera::Stream *stream() const;\n> >>> + \tCameraStream *sourceStream() const { return sourceStream_; }\n> >>> \tint configure();\n> >>> \tint process(Camera3RequestDescriptor::StreamBuffer *streamBuffer);\n> >>> @@ -167,6 +170,7 @@ private:\n> >>> \tconst libcamera::CameraConfiguration *config_;\n> >>> \tconst Type type_;\n> >>> \tcamera3_stream_t *camera3Stream_;\n> >>> + \tCameraStream *const sourceStream_;\n\nNot a candidate for this patch, but it's time to document this class.\n\nReviewed-by: Laurent Pinchart \n\n> >>> \tconst unsigned int index_;\n> >>> \tstd::unique_ptr allocator_;","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 AB755BD161\n\tfor ;\n\tThu, 2 Jun 2022 07:52:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EA4066563B;\n\tThu, 2 Jun 2022 09:52:45 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7F38865633\n\tfor ;\n\tThu, 2 Jun 2022 09:52:44 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(lmontsouris-659-1-41-236.w92-154.abo.wanadoo.fr [92.154.76.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DE4806BD;\n\tThu, 2 Jun 2022 09:52:43 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1654156365;\n\tbh=xvjAOJgzdq3lIh4ID2dgR6RmJUDDR6SlV8V6rkk5UNg=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=ySmYLv0BRVj5UpRdnba3UsfJ2Amh/J7f0y3ZTyR5i75wLjS7HRr0tL5/JK7urFJIi\n\tHGDEOExWZ/5XImiU6us+I8GLhbDhxkwuAEkk9Jn/UoDIAUoD4ucQ0gDXKrpypO51ai\n\tcbVTO9fKqNOb/SLB+NZIQvob/sgoEWw8SAkhTeQK72/3byVw0NG5VtG3lQEIjKY2cl\n\tvV7ifREBOC8cKNbdFO1ORaC+UyNX5ubNnjphbe2Vgmdncl/eRTouMS6GegT6MtCarb\n\tnFzm/RIJBWUX1iGF5s9L1ag6owDHFlPUCzS5qGsl+ERfL9MWQ9dGCE9iuhhKsKETYY\n\tHleEw+EZq7nBw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1654156364;\n\tbh=xvjAOJgzdq3lIh4ID2dgR6RmJUDDR6SlV8V6rkk5UNg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GpnHBLeG4F+sS6ZRVmWpcz0QIIi4QVALgch5oYOR1aIVbznKTk3Ob44MiO6SWEykZ\n\tNop/BFaenZeEvJF097SfsaqJKjTQxNq60njRnbB973jJnvnyIMP65PlnyLduT4kwmY\n\tAfvM+qhDVqiJ68HdHB6Eh9jLqXRId0iYSURVKHxk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"GpnHBLeG\"; dkim-atps=neutral","Date":"Thu, 2 Jun 2022 10:52:39 +0300","To":"Umang Jain ","Message-ID":"","References":"<20220527093440.953377-1-paul.elder@ideasonboard.com>\n\t<20220527093440.953377-2-paul.elder@ideasonboard.com>\n\t<90b73de3-b831-6b61-7b4c-02db614ea062@ideasonboard.com>\n\t<20220530095225.ed7myp4litabsbg4@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 1/5] android: camera_stream: Add\n\tsourceStream","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]