[{"id":11161,"web_url":"https://patchwork.libcamera.org/comment/11161/","msgid":"<20200703150832.GB3396922@oden.dyn.berto.se>","date":"2020-07-03T15:08:32","subject":"Re: [libcamera-devel] [PATCH v3 6/8] android: camera_device:\n\tMaintain a vector of CameraStream","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Kieran,\n\nThanks for your work.\n\nOn 2020-07-03 13:39:17 +0100, Kieran Bingham wrote:\n> Introduce a vector storing a CameraStream to track and maintain\n> state between an Android stream (camera3_stream_t) and a libcamera\n> Stream.\n> \n> Only the index of the libcamera stream is stored, to facilitate identifying\n> the correct index for both the StreamConfiguration and Stream vectors.\n> \n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  src/android/camera_device.cpp | 22 ++++++++++++++++++++--\n>  src/android/camera_device.h   | 10 ++++++++++\n>  2 files changed, 30 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 4e77a92dbea5..28334751a26e 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -952,6 +952,20 @@ 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 * re-allcoation.\n> +\t */\n> +\tstreams_.clear();\n> +\tstreams_.reserve(stream_list->num_streams);\n> +\n> +\t/*\n> +\t * Track actually created streams, as there may not be a 1:1 mapping of\n> +\t * camera3 streams to libcamera streams.\n> +\t */\n> +\tunsigned int streamIndex = 0;\n> +\n>  \tfor (unsigned int i = 0; i < stream_list->num_streams; ++i) {\n>  \t\tcamera3_stream_t *stream = stream_list->streams[i];\n>  \n> @@ -974,6 +988,9 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>  \t\tstreamConfiguration.pixelFormat = format;\n>  \n>  \t\tconfig_->addConfiguration(streamConfiguration);\n> +\n> +\t\t/* Maintain internal state of all stream mappings. */\n> +\t\tstreams_[i].index = streamIndex++;\n>  \t}\n>  \n>  \tswitch (config_->validate()) {\n> @@ -991,10 +1008,11 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>  \n>  \tfor (unsigned int i = 0; i < stream_list->num_streams; ++i) {\n>  \t\tcamera3_stream_t *stream = stream_list->streams[i];\n> -\t\tStreamConfiguration &streamConfiguration = config_->at(i);\n> +\t\tCameraStream *cameraStream = &streams_[i];\n> +\t\tStreamConfiguration &cfg = config_->at(cameraStream->index);\n>  \n>  \t\t/* Use the bufferCount confirmed by the validation process. */\n> -\t\tstream->max_buffers = streamConfiguration.bufferCount;\n> +\t\tstream->max_buffers = cfg.bufferCount;\n>  \t}\n>  \n>  \t/*\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index d7834d94f439..8e306c1f6d8b 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -25,6 +25,15 @@\n>  \n>  class CameraMetadata;\n>  \n> +struct CameraStream {\n> +\t/*\n> +\t * The index represents the index for libcamera stream parameters. This\n> +\t * will differ from any index of the halStream, particularly for HAL\n> +\t * only streams such as MJPEG.\n> +\t */\n> +\tunsigned int index;\n> +};\n> +\n>  class CameraDevice : protected libcamera::Loggable\n>  {\n>  public:\n> @@ -90,6 +99,7 @@ private:\n>  \n>  \tstd::vector<Camera3StreamConfiguration> streamConfigurations_;\n>  \tstd::map<int, libcamera::PixelFormat> formatsMap_;\n> +\tstd::vector<CameraStream> streams_;\n>  \n>  \tint facing_;\n>  \tint orientation_;\n> -- \n> 2.25.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 56B7ABE905\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  3 Jul 2020 15:08:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DBF5560C5F;\n\tFri,  3 Jul 2020 17:08:35 +0200 (CEST)","from mail-lj1-x244.google.com (mail-lj1-x244.google.com\n\t[IPv6:2a00:1450:4864:20::244])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DF5A6603AD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  3 Jul 2020 17:08:34 +0200 (CEST)","by mail-lj1-x244.google.com with SMTP id f5so21509589ljj.10\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 03 Jul 2020 08:08:34 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\to1sm1314638ljj.82.2020.07.03.08.08.32\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 03 Jul 2020 08:08:33 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"PJL/8Ftk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=mRHGUZ/oIEwJ8QZYHYG1USRTcaJFiekGKFmHCNu4Sbc=;\n\tb=PJL/8FtktVJF1agUVIEhLhcYUutmlEeGBOusqVooRDrHnQ+bPt2kym8lwbOIqCPqqj\n\tB/F5ZbXR7oGHV6Wj/Zvwxb3OJcQtNY87bvr7/3DkZmAVetJ6/Lc3VvxC6JHekQVIS0vI\n\tk0LcQ24ThhEPiqERUU+HfBhiaOnj2FVLhdu9n7+9sskxzYj+Qv3FyhAuMmERXO1HDcWb\n\t/Le5LZHVsrEsgWaOO/QHe/ihjbJrydEkuLSLk+XJlPfQj/TQrKxVHAPrOxytQx0Je7rC\n\tku61KR9/SD4f+z86EGA0C2SMD2/867Ap8pxGO2yv99OLkI31VY8aLD7+kSOHZ/iSX3dy\n\tNzrg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=mRHGUZ/oIEwJ8QZYHYG1USRTcaJFiekGKFmHCNu4Sbc=;\n\tb=dJrWQcO5Cb8YtmkO3vaPURwajo8AIdojrOD6/Lt7SxQxk9z33w0akrJkx3RKB9LWfU\n\tiYQgQHNYzXIKTRnoVK25oc5tJ43CtK+RS+wqLV1HlwRt7ZxZH1IY0OdB8/EqbjITCsKF\n\txu/xSQKAVim/MS2JE+XgdzqTHaJ2ygFMcYoevNfnM4Wf8/wSI3r4GOV98a5kNtt/vO/n\n\tL0fcfD6/XAgk4NDV4Fe013RBV4ZkKCSXRjFsGk+CSiA79VBbuxukkNhHpPwODs4h/NzQ\n\tAV5hp5cJMlYSRks8qoU3Dnf4R7oIZ+nG43E7etPdB0U1qqxqtStZKlmrfODh2Kl7UPk1\n\t9Izw==","X-Gm-Message-State":"AOAM530qpoOmFqoXCySF1rWRV3p513jHMF0u/pplHxnGm3+sd96C7eQ0\n\t93kxb1MCZt36QsZwvZJP5Q04VQ==","X-Google-Smtp-Source":"ABdhPJxefDd14dTgCTjrAwhK1CUs+Nn1yO8g/19CSWHwfdz8wXHGAjG9iSUlnRrYe+G/nkH/GOdBOQ==","X-Received":"by 2002:a2e:6d02:: with SMTP id i2mr5875174ljc.309.1593788913951;\n\tFri, 03 Jul 2020 08:08:33 -0700 (PDT)","Date":"Fri, 3 Jul 2020 17:08:32 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20200703150832.GB3396922@oden.dyn.berto.se>","References":"<20200703123919.2223048-1-kieran.bingham@ideasonboard.com>\n\t<20200703123919.2223048-7-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200703123919.2223048-7-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 6/8] android: camera_device:\n\tMaintain a vector of CameraStream","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11191,"web_url":"https://patchwork.libcamera.org/comment/11191/","msgid":"<20200706082244.qymwq7ijtky6sl7d@uno.localdomain>","date":"2020-07-06T08:22:44","subject":"Re: [libcamera-devel] [PATCH v3 6/8] android: camera_device:\n\tMaintain a vector of CameraStream","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran\n   sorry, I'm going to be picky.\n\nOn Fri, Jul 03, 2020 at 01:39:17PM +0100, Kieran Bingham wrote:\n> Introduce a vector storing a CameraStream to track and maintain\n> state between an Android stream (camera3_stream_t) and a libcamera\n> Stream.\n>\n> Only the index of the libcamera stream is stored, to facilitate identifying\n> the correct index for both the StreamConfiguration and Stream vectors.\n>\n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  src/android/camera_device.cpp | 22 ++++++++++++++++++++--\n>  src/android/camera_device.h   | 10 ++++++++++\n>  2 files changed, 30 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 4e77a92dbea5..28334751a26e 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -952,6 +952,20 @@ 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 * re-allcoation.\n> +\t */\n> +\tstreams_.clear();\n> +\tstreams_.reserve(stream_list->num_streams);\n> +\n> +\t/*\n> +\t * Track actually created streams, as there may not be a 1:1 mapping of\n> +\t * camera3 streams to libcamera streams.\n> +\t */\n> +\tunsigned int streamIndex = 0;\n> +\n>  \tfor (unsigned int i = 0; i < stream_list->num_streams; ++i) {\n>  \t\tcamera3_stream_t *stream = stream_list->streams[i];\n>\n> @@ -974,6 +988,9 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>  \t\tstreamConfiguration.pixelFormat = format;\n>\n>  \t\tconfig_->addConfiguration(streamConfiguration);\n> +\n> +\t\t/* Maintain internal state of all stream mappings. */\n\nI don't want to be rude, but these are the comments I would prefer not\nto see in code, and I'm the first one with a tendency to over-comment\ntrivial operations (like recording an index) with comments that might\neven be misleading.  \"maintain the internal state of all stream\nmappings\" makes me look around for some kind of 'internal state' until\nI don't realize we're just recording an index for later use. I would\nprefer to know where the index is use and why, with a slightly longer:\n\n                /*\n                 * Record the index of the StreamConfiguration\n                 * associated to the camera3 stream. As not all\n                 * camera3 streams maps to a libcamera stream, the\n                 * index might be repeated.\n                 */\n\n> +\t\tstreams_[i].index = streamIndex++;\n>  \t}\n>\n>  \tswitch (config_->validate()) {\n> @@ -991,10 +1008,11 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>\n>  \tfor (unsigned int i = 0; i < stream_list->num_streams; ++i) {\n>  \t\tcamera3_stream_t *stream = stream_list->streams[i];\n> -\t\tStreamConfiguration &streamConfiguration = config_->at(i);\n> +\t\tCameraStream *cameraStream = &streams_[i];\n> +\t\tStreamConfiguration &cfg = config_->at(cameraStream->index);\n>\n>  \t\t/* Use the bufferCount confirmed by the validation process. */\n> -\t\tstream->max_buffers = streamConfiguration.bufferCount;\n> +\t\tstream->max_buffers = cfg.bufferCount;\n>  \t}\n>\n>  \t/*\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index d7834d94f439..8e306c1f6d8b 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -25,6 +25,15 @@\n>\n>  class CameraMetadata;\n>\n> +struct CameraStream {\n> +\t/*\n> +\t * The index represents the index for libcamera stream parameters. This\n\nWhat are \"stream parameters\" ?\n\n> +\t * will differ from any index of the halStream, particularly for HAL\n\nI'm not sure I get what \"from any index of the halStream\" means.\nI also think you removed 'halStream' from the patch\n\nI would:\n\"Index of the libcamera StreamConfiguration associated with an Android\nrequested stream. Not all streams requested by the Android framework\nare 1-to-1 mapped to a libcamera Stream as some of them, such as JPEG,\nmight be produced by the libcamera HAL by processing data produced by\nan existing libcamera Stream. This implies different Android stream\nmight be associated with the same libcamera StreamConfiguration index.\"\n\nFeel free to adjust if you think it's worth taking this in.\n\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n  j\n\n\n> +\t * only streams such as MJPEG.\n> +\t */\n> +\tunsigned int index;\n> +};\n> +\n>  class CameraDevice : protected libcamera::Loggable\n>  {\n>  public:\n> @@ -90,6 +99,7 @@ private:\n>\n>  \tstd::vector<Camera3StreamConfiguration> streamConfigurations_;\n>  \tstd::map<int, libcamera::PixelFormat> formatsMap_;\n> +\tstd::vector<CameraStream> streams_;\n>\n>  \tint facing_;\n>  \tint orientation_;\n> --\n> 2.25.1\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 AAA24BD790\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  6 Jul 2020 08:19:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 50E4960C59;\n\tMon,  6 Jul 2020 10:19:13 +0200 (CEST)","from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[217.70.183.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 49E8A603B1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  6 Jul 2020 10:19:12 +0200 (CEST)","from uno.localdomain (host-79-34-235-173.business.telecomitalia.it\n\t[79.34.235.173]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay3-d.mail.gandi.net (Postfix) with ESMTPSA id 3EB806000A;\n\tMon,  6 Jul 2020 08:19:10 +0000 (UTC)"],"X-Originating-IP":"79.34.235.173","Date":"Mon, 6 Jul 2020 10:22:44 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20200706082244.qymwq7ijtky6sl7d@uno.localdomain>","References":"<20200703123919.2223048-1-kieran.bingham@ideasonboard.com>\n\t<20200703123919.2223048-7-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200703123919.2223048-7-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 6/8] android: camera_device:\n\tMaintain a vector of CameraStream","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <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\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11217,"web_url":"https://patchwork.libcamera.org/comment/11217/","msgid":"<e14fcc5b-7494-fdb0-71a0-ad9e7efd8682@ideasonboard.com>","date":"2020-07-06T13:05:04","subject":"Re: [libcamera-devel] [PATCH v3 6/8] android: camera_device:\n\tMaintain a vector of CameraStream","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 06/07/2020 09:22, Jacopo Mondi wrote:\n> Hi Kieran\n>    sorry, I'm going to be picky.\n> \n> On Fri, Jul 03, 2020 at 01:39:17PM +0100, Kieran Bingham wrote:\n>> Introduce a vector storing a CameraStream to track and maintain\n>> state between an Android stream (camera3_stream_t) and a libcamera\n>> Stream.\n>>\n>> Only the index of the libcamera stream is stored, to facilitate identifying\n>> the correct index for both the StreamConfiguration and Stream vectors.\n>>\n>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>> ---\n>>  src/android/camera_device.cpp | 22 ++++++++++++++++++++--\n>>  src/android/camera_device.h   | 10 ++++++++++\n>>  2 files changed, 30 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n>> index 4e77a92dbea5..28334751a26e 100644\n>> --- a/src/android/camera_device.cpp\n>> +++ b/src/android/camera_device.cpp\n>> @@ -952,6 +952,20 @@ 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 * re-allcoation.\n>> +\t */\n>> +\tstreams_.clear();\n>> +\tstreams_.reserve(stream_list->num_streams);\n>> +\n>> +\t/*\n>> +\t * Track actually created streams, as there may not be a 1:1 mapping of\n>> +\t * camera3 streams to libcamera streams.\n>> +\t */\n>> +\tunsigned int streamIndex = 0;\n>> +\n>>  \tfor (unsigned int i = 0; i < stream_list->num_streams; ++i) {\n>>  \t\tcamera3_stream_t *stream = stream_list->streams[i];\n>>\n>> @@ -974,6 +988,9 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>>  \t\tstreamConfiguration.pixelFormat = format;\n>>\n>>  \t\tconfig_->addConfiguration(streamConfiguration);\n>> +\n>> +\t\t/* Maintain internal state of all stream mappings. */\n> \n> I don't want to be rude, but these are the comments I would prefer not\n> to see in code, and I'm the first one with a tendency to over-comment\n\n\nDon't worry - it's fine. I normally write comments before code, so\nsometimes indeed I'll end up with possibly inappropriate comments left\nthat I haven't yet removed.\n\nAnd in this series, these code blocks have bounced around so much in the\npatches they might think they're on a trampoline ;)\n\n\nIndeed, this code block started out with storing much more content and\ninternal state for JPEG, which has then been simplified to try to get\nthis multi-stream series separated...\n\n\n> trivial operations (like recording an index) with comments that might\n> even be misleading.  \"maintain the internal state of all stream\n> mappings\" makes me look around for some kind of 'internal state' until\n> I don't realize we're just recording an index for later use. I would\n> prefer to know where the index is use and why, with a slightly longer:\n\nIt's been simplified to a single index (for now).\n\n\n> \n>                 /*\n>                  * Record the index of the StreamConfiguration\n>                  * associated to the camera3 stream. As not all\n>                  * camera3 streams maps to a libcamera stream, the\n>                  * index might be repeated.\n>                  */\n\nIsn't that what's stated above at declaration of streamIndex?\n\nThe comment for /* maintain internal state of all stream mappings */\nhelps later on when there are several lines here, and it becomes a\ndistinct code block, but it has been simplified to a single line for\nthis patch.\n\nI'll just remove the line, there's no need to add a big block again. The\nsingle line was more of a separator...\n\n\nAnd in fact, I later found out I had to split the block anyway, as I can\nonly increment the index after adding the Configuration, where JPEG\nstreams that don't create a configuration break out of the loop earlier,\nso they have to update their internal streams_[i] structure before.\n\n\n>> +\t\tstreams_[i].index = streamIndex++;\n>>  \t}\n>>\n>>  \tswitch (config_->validate()) {\n>> @@ -991,10 +1008,11 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>>\n>>  \tfor (unsigned int i = 0; i < stream_list->num_streams; ++i) {\n>>  \t\tcamera3_stream_t *stream = stream_list->streams[i];\n>> -\t\tStreamConfiguration &streamConfiguration = config_->at(i);\n>> +\t\tCameraStream *cameraStream = &streams_[i];\n>> +\t\tStreamConfiguration &cfg = config_->at(cameraStream->index);\n>>\n>>  \t\t/* Use the bufferCount confirmed by the validation process. */\n>> -\t\tstream->max_buffers = streamConfiguration.bufferCount;\n>> +\t\tstream->max_buffers = cfg.bufferCount;\n>>  \t}\n>>\n>>  \t/*\n>> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n>> index d7834d94f439..8e306c1f6d8b 100644\n>> --- a/src/android/camera_device.h\n>> +++ b/src/android/camera_device.h\n>> @@ -25,6 +25,15 @@\n>>\n>>  class CameraMetadata;\n>>\n>> +struct CameraStream {\n>> +\t/*\n>> +\t * The index represents the index for libcamera stream parameters. This\n> \n> What are \"stream parameters\" ?\n\nStreamConfigurations, and Streams ... but I think actually the Stream*\nis obtained from the StreamConfiguration anyway, so really it's just the\nStreamConfigurations\n\n> \n>> +\t * will differ from any index of the halStream, particularly for HAL\n> \n> I'm not sure I get what \"from any index of the halStream\" means.\n> I also think you removed 'halStream' from the patch\n\nYes, I have now removed halStream, and I don't think I'll add it back\nlater any more.\n\n\n> I would:\n> \"Index of the libcamera StreamConfiguration associated with an Android\n> requested stream. Not all streams requested by the Android framework\n> are 1-to-1 mapped to a libcamera Stream as some of them, such as JPEG,\n> might be produced by the libcamera HAL by processing data produced by\n> an existing libcamera Stream. This implies different Android stream\n> might be associated with the same libcamera StreamConfiguration index.\"\n> \n> Feel free to adjust if you think it's worth taking this in.\n\nI've changed this to:\n\n /*\n  * The index of the libcamera StreamConfiguration as added during\n  * configureStreams(). A single libcamera Stream may be used to deliver\n  * one or more streams to the Android framework.\n  */\n\n> \n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks,\n\n\n> \n> Thanks\n>   j\n> \n> \n>> +\t * only streams such as MJPEG.\n>> +\t */\n>> +\tunsigned int index;\n>> +};\n>> +\n>>  class CameraDevice : protected libcamera::Loggable\n>>  {\n>>  public:\n>> @@ -90,6 +99,7 @@ private:\n>>\n>>  \tstd::vector<Camera3StreamConfiguration> streamConfigurations_;\n>>  \tstd::map<int, libcamera::PixelFormat> formatsMap_;\n>> +\tstd::vector<CameraStream> streams_;\n>>\n>>  \tint facing_;\n>>  \tint orientation_;\n>> --\n>> 2.25.1\n>>\n>> _______________________________________________\n>> libcamera-devel mailing list\n>> libcamera-devel@lists.libcamera.org\n>> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 6E466BD790\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  6 Jul 2020 13:05:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F1A4060E01;\n\tMon,  6 Jul 2020 15:05:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BCCF6603AA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  6 Jul 2020 15:05:07 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 38F77289;\n\tMon,  6 Jul 2020 15:05:06 +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=\"eqVq98+0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1594040707;\n\tbh=6EqJpZ9ZuojL6mIZA+ZrbTcMQidBQBGN4KZo+oA+AdQ=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=eqVq98+07Y4lO4EHSVR3wA40Bl1CJ0zqVs62zxMc4E+gMC5Hm0KMGnjdg+K0QAKBq\n\t8iPYDFH6Ddza0kK4HrCm+S6MHHBc7sB8USGaHZd1QWneScUvSXm1mNlw2UR+DAH8+u\n\tBPYrkn0AaQfMWutM2T+mHPNoJwMIpiv21widJVGY=","To":"Jacopo Mondi <jacopo@jmondi.org>","References":"<20200703123919.2223048-1-kieran.bingham@ideasonboard.com>\n\t<20200703123919.2223048-7-kieran.bingham@ideasonboard.com>\n\t<20200706082244.qymwq7ijtky6sl7d@uno.localdomain>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","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":"<e14fcc5b-7494-fdb0-71a0-ad9e7efd8682@ideasonboard.com>","Date":"Mon, 6 Jul 2020 14:05:04 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.8.0","MIME-Version":"1.0","In-Reply-To":"<20200706082244.qymwq7ijtky6sl7d@uno.localdomain>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v3 6/8] android: camera_device:\n\tMaintain a vector of CameraStream","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel <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\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11219,"web_url":"https://patchwork.libcamera.org/comment/11219/","msgid":"<20200706141630.7vt72jqbz56bgzc5@uno.localdomain>","date":"2020-07-06T14:16:30","subject":"Re: [libcamera-devel] [PATCH v3 6/8] android: camera_device:\n\tMaintain a vector of CameraStream","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"On Mon, Jul 06, 2020 at 02:05:04PM +0100, Kieran Bingham wrote:\n> Hi Jacopo,\n>\n> On 06/07/2020 09:22, Jacopo Mondi wrote:\n> > Hi Kieran\n> >    sorry, I'm going to be picky.\n> >\n> > On Fri, Jul 03, 2020 at 01:39:17PM +0100, Kieran Bingham wrote:\n> >> Introduce a vector storing a CameraStream to track and maintain\n> >> state between an Android stream (camera3_stream_t) and a libcamera\n> >> Stream.\n> >>\n> >> Only the index of the libcamera stream is stored, to facilitate identifying\n> >> the correct index for both the StreamConfiguration and Stream vectors.\n> >>\n> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> >> ---\n> >>  src/android/camera_device.cpp | 22 ++++++++++++++++++++--\n> >>  src/android/camera_device.h   | 10 ++++++++++\n> >>  2 files changed, 30 insertions(+), 2 deletions(-)\n> >>\n> >> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> >> index 4e77a92dbea5..28334751a26e 100644\n> >> --- a/src/android/camera_device.cpp\n> >> +++ b/src/android/camera_device.cpp\n> >> @@ -952,6 +952,20 @@ 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 * re-allcoation.\n> >> +\t */\n> >> +\tstreams_.clear();\n> >> +\tstreams_.reserve(stream_list->num_streams);\n> >> +\n> >> +\t/*\n> >> +\t * Track actually created streams, as there may not be a 1:1 mapping of\n> >> +\t * camera3 streams to libcamera streams.\n> >> +\t */\n> >> +\tunsigned int streamIndex = 0;\n> >> +\n> >>  \tfor (unsigned int i = 0; i < stream_list->num_streams; ++i) {\n> >>  \t\tcamera3_stream_t *stream = stream_list->streams[i];\n> >>\n> >> @@ -974,6 +988,9 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> >>  \t\tstreamConfiguration.pixelFormat = format;\n> >>\n> >>  \t\tconfig_->addConfiguration(streamConfiguration);\n> >> +\n> >> +\t\t/* Maintain internal state of all stream mappings. */\n> >\n> > I don't want to be rude, but these are the comments I would prefer not\n> > to see in code, and I'm the first one with a tendency to over-comment\n>\n>\n> Don't worry - it's fine. I normally write comments before code, so\n> sometimes indeed I'll end up with possibly inappropriate comments left\n> that I haven't yet removed.\n>\n> And in this series, these code blocks have bounced around so much in the\n> patches they might think they're on a trampoline ;)\n>\n>\n> Indeed, this code block started out with storing much more content and\n> internal state for JPEG, which has then been simplified to try to get\n> this multi-stream series separated...\n>\n>\n> > trivial operations (like recording an index) with comments that might\n> > even be misleading.  \"maintain the internal state of all stream\n> > mappings\" makes me look around for some kind of 'internal state' until\n> > I don't realize we're just recording an index for later use. I would\n> > prefer to know where the index is use and why, with a slightly longer:\n>\n> It's been simplified to a single index (for now).\n>\n>\n> >\n> >                 /*\n> >                  * Record the index of the StreamConfiguration\n> >                  * associated to the camera3 stream. As not all\n> >                  * camera3 streams maps to a libcamera stream, the\n> >                  * index might be repeated.\n> >                  */\n>\n> Isn't that what's stated above at declaration of streamIndex?\n>\n> The comment for /* maintain internal state of all stream mappings */\n> helps later on when there are several lines here, and it becomes a\n> distinct code block, but it has been simplified to a single line for\n> this patch.\n>\n> I'll just remove the line, there's no need to add a big block again. The\n> single line was more of a separator...\n>\n>\n> And in fact, I later found out I had to split the block anyway, as I can\n> only increment the index after adding the Configuration, where JPEG\n> streams that don't create a configuration break out of the loop earlier,\n> so they have to update their internal streams_[i] structure before.\n>\n>\n> >> +\t\tstreams_[i].index = streamIndex++;\n> >>  \t}\n> >>\n> >>  \tswitch (config_->validate()) {\n> >> @@ -991,10 +1008,11 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> >>\n> >>  \tfor (unsigned int i = 0; i < stream_list->num_streams; ++i) {\n> >>  \t\tcamera3_stream_t *stream = stream_list->streams[i];\n> >> -\t\tStreamConfiguration &streamConfiguration = config_->at(i);\n> >> +\t\tCameraStream *cameraStream = &streams_[i];\n> >> +\t\tStreamConfiguration &cfg = config_->at(cameraStream->index);\n> >>\n> >>  \t\t/* Use the bufferCount confirmed by the validation process. */\n> >> -\t\tstream->max_buffers = streamConfiguration.bufferCount;\n> >> +\t\tstream->max_buffers = cfg.bufferCount;\n> >>  \t}\n> >>\n> >>  \t/*\n> >> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> >> index d7834d94f439..8e306c1f6d8b 100644\n> >> --- a/src/android/camera_device.h\n> >> +++ b/src/android/camera_device.h\n> >> @@ -25,6 +25,15 @@\n> >>\n> >>  class CameraMetadata;\n> >>\n> >> +struct CameraStream {\n> >> +\t/*\n> >> +\t * The index represents the index for libcamera stream parameters. This\n> >\n> > What are \"stream parameters\" ?\n>\n> StreamConfigurations, and Streams ... but I think actually the Stream*\n> is obtained from the StreamConfiguration anyway, so really it's just the\n> StreamConfigurations\n>\n> >\n> >> +\t * will differ from any index of the halStream, particularly for HAL\n> >\n> > I'm not sure I get what \"from any index of the halStream\" means.\n> > I also think you removed 'halStream' from the patch\n>\n> Yes, I have now removed halStream, and I don't think I'll add it back\n> later any more.\n>\n>\n> > I would:\n> > \"Index of the libcamera StreamConfiguration associated with an Android\n> > requested stream. Not all streams requested by the Android framework\n> > are 1-to-1 mapped to a libcamera Stream as some of them, such as JPEG,\n> > might be produced by the libcamera HAL by processing data produced by\n> > an existing libcamera Stream. This implies different Android stream\n> > might be associated with the same libcamera StreamConfiguration index.\"\n> >\n> > Feel free to adjust if you think it's worth taking this in.\n>\n> I've changed this to:\n>\n>  /*\n>   * The index of the libcamera StreamConfiguration as added during\n>   * configureStreams(). A single libcamera Stream may be used to deliver\n>   * one or more streams to the Android framework.\n>   */\n\nSounds perfect!\nThanks\n\n>\n> >\n> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n>\n> Thanks,\n>\n>\n> >\n> > Thanks\n> >   j\n> >\n> >\n> >> +\t * only streams such as MJPEG.\n> >> +\t */\n> >> +\tunsigned int index;\n> >> +};\n> >> +\n> >>  class CameraDevice : protected libcamera::Loggable\n> >>  {\n> >>  public:\n> >> @@ -90,6 +99,7 @@ private:\n> >>\n> >>  \tstd::vector<Camera3StreamConfiguration> streamConfigurations_;\n> >>  \tstd::map<int, libcamera::PixelFormat> formatsMap_;\n> >> +\tstd::vector<CameraStream> streams_;\n> >>\n> >>  \tint facing_;\n> >>  \tint orientation_;\n> >> --\n> >> 2.25.1\n> >>\n> >> _______________________________________________\n> >> libcamera-devel mailing list\n> >> libcamera-devel@lists.libcamera.org\n> >> https://lists.libcamera.org/listinfo/libcamera-devel\n>\n> --\n> Regards\n> --\n> Kieran","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 D6C76BD792\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  6 Jul 2020 14:12:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7C68F60E01;\n\tMon,  6 Jul 2020 16:12:58 +0200 (CEST)","from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C1306603AA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  6 Jul 2020 16:12:57 +0200 (CEST)","from uno.localdomain (host-79-34-235-173.business.telecomitalia.it\n\t[79.34.235.173]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay10.mail.gandi.net (Postfix) with ESMTPSA id D63B8240013;\n\tMon,  6 Jul 2020 14:12:56 +0000 (UTC)"],"Date":"Mon, 6 Jul 2020 16:16:30 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20200706141630.7vt72jqbz56bgzc5@uno.localdomain>","References":"<20200703123919.2223048-1-kieran.bingham@ideasonboard.com>\n\t<20200703123919.2223048-7-kieran.bingham@ideasonboard.com>\n\t<20200706082244.qymwq7ijtky6sl7d@uno.localdomain>\n\t<e14fcc5b-7494-fdb0-71a0-ad9e7efd8682@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<e14fcc5b-7494-fdb0-71a0-ad9e7efd8682@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 6/8] android: camera_device:\n\tMaintain a vector of CameraStream","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <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\" <libcamera-devel-bounces@lists.libcamera.org>"}}]