[{"id":11093,"web_url":"https://patchwork.libcamera.org/comment/11093/","msgid":"<20200702232247.GF3158543@oden.dyn.berto.se>","date":"2020-07-02T23:22:47","subject":"Re: [libcamera-devel] [PATCH v2 3/9] android: camera_device:\n\tSupport multiple stream configurations","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-02 22:36:48 +0100, Kieran Bingham wrote:\n> Create an initial Camera Configuration using an empty role set, and\n> populate the StreamConfigurations manually from each of the streams\n> given by the Android camera3_stream_configuration_t stream_list.\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> src/android/camera_device.cpp | 51 +++++++++++++++++------------------\n> 1 file changed, 25 insertions(+), 26 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index b9031ff0c2a4..03dcdd520794 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -942,6 +942,16 @@ PixelFormat CameraDevice::toPixelFormat(int format)\n> */\n> int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> {\n> +\t/*\n> +\t * Generate an empty configuration, and construct a StreamConfiguration\n> +\t * for each camera3_stream to add to it.\n> +\t */\n> +\tconfig_ = camera_->generateConfiguration();\n> +\tif (!config_) {\n> +\t\tLOG(HAL, Error) << \"Failed to generate camera configuration\";\n> +\t\treturn -EINVAL;\n> +\t}\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> @@ -953,35 +963,18 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \t\t\t << \", height: \" << stream->height\n> \t\t\t << \", format: \" << utils::hex(stream->format)\n> \t\t\t << \" (\" << format.toString() << \")\";\n> -\t}\n> \n> -\t/* Only one stream is supported. */\n> -\tif (stream_list->num_streams != 1) {\n> -\t\tLOG(HAL, Error) << \"Only one stream supported\";\n> -\t\treturn -EINVAL;\n> -\t}\n> -\tcamera3_stream_t *camera3Stream = stream_list->streams[0];\n> +\t\tif (!format.isValid())\n> +\t\t\treturn -EINVAL;\n> \n> -\t/* Translate Android format code to libcamera pixel format. */\n> -\tPixelFormat format = toPixelFormat(camera3Stream->format);\n> -\tif (!format.isValid())\n> -\t\treturn -EINVAL;\n> +\t\tStreamConfiguration streamConfiguration;\n> \n> -\t/*\n> -\t * Hardcode viewfinder role, replacing the generated configuration\n> -\t * parameters with the ones requested by the Android framework.\n> -\t */\n> -\tStreamRoles roles = { StreamRole::Viewfinder };\n> -\tconfig_ = camera_->generateConfiguration(roles);\n> -\tif (!config_ || config_->empty()) {\n> -\t\tLOG(HAL, Error) << \"Failed to generate camera configuration\";\n> -\t\treturn -EINVAL;\n> -\t}\n> +\t\tstreamConfiguration.size.width = stream->width;\n> +\t\tstreamConfiguration.size.height = stream->height;\n> +\t\tstreamConfiguration.pixelFormat = format;\n\nThis is not related to this patch but blindly translating a HAL format \nusing a lookup table to a libcamera format will not work for RAW. I'm \njust curious if it is sufficient for JPEG?\n\n> \n> -\tStreamConfiguration *streamConfiguration = &config_->at(0);\n> -\tstreamConfiguration->size.width = camera3Stream->width;\n> -\tstreamConfiguration->size.height = camera3Stream->height;\n> -\tstreamConfiguration->pixelFormat = format;\n> +\t\tconfig_->addConfiguration(streamConfiguration);\n> +\t}\n> \n> \tswitch (config_->validate()) {\n> \tcase CameraConfiguration::Valid:\n> @@ -996,7 +989,13 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \t\treturn -EINVAL;\n> \t}\n> \n> -\tcamera3Stream->max_buffers = streamConfiguration->bufferCount;\n> +\tfor (unsigned int i = 0; i < stream_list->num_streams; ++i) {\n\nSmall nit, would it make sens to iterate over config_ instead of \nstream_list->num_streams? If we have a discrepancy we would end up with \nless streams then requested but if we use ->at(i) an exception.\n\nThis is a small thing that poped out at me, with or without this \nchanged,\n\nReviewed-by: Niklas Söderlund \n\n> +\t\tcamera3_stream_t *stream = stream_list->streams[i];\n> +\t\tStreamConfiguration &streamConfiguration = config_->at(i);\n> +\n> +\t\t/* Use the bufferCount confirmed by the validation process. */\n> +\t\tstream->max_buffers = streamConfiguration.bufferCount;\n> +\t}\n> \n> \t/*\n> \t * Once the CameraConfiguration has been adjusted/validated\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":"","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 03872BE905\n\tfor ;\n\tThu, 2 Jul 2020 23:22:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7277060C57;\n\tFri, 3 Jul 2020 01:22:50 +0200 (CEST)","from mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D20E5609C7\n\tfor ;\n\tFri, 3 Jul 2020 01:22:48 +0200 (CEST)","by mail-lj1-x241.google.com with SMTP id h22so27234499lji.9\n\tfor ;\n\tThu, 02 Jul 2020 16:22:48 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tq1sm3940957lfp.42.2020.07.02.16.22.47\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 02 Jul 2020 16:22:47 -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=\"D10CViuS\"; 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=co75XoUANdauiHGskNuSeBWTT+167y37NKiyBLGdsHM=;\n\tb=D10CViuSSpc7QtRIakJcLtoQDm1tNTY9fwKg5n9TayzW02ZN9x+cb3pCaXzA4GfhvV\n\tx/AobQ9Xu/0X0DQTH6x1bvxJ2W5yV/4OUgJ222l5qF7AD5+nSKuDMAvE72znnkev9Iwa\n\t9zFHdaFDUww3bwNkzs1JnAYZFMHcwoOktLD7716a8seJr7DbNUy5PIyYXV/9pNzLLjgs\n\t1TtxrkPKu4MFfHrOQdENZ0KKJTRuh8471rc4fHL+Vm/RAqQuL88jBQaY+VXBunmN8cXS\n\tp/m/RLKIs8dlGCTur1PLHLlocGcntxvHuaBpl66cZLrM2RXrIdAQRvulVpYfRMfftU9Z\n\t2yEg==","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=co75XoUANdauiHGskNuSeBWTT+167y37NKiyBLGdsHM=;\n\tb=eZRJh1nWzAOl7evyF+ua6n9TcYrWvdx/0R3Algscb098esnjkM1Efn4cTIUZTNEWVw\n\tYPpOwU89gH6b0atoRrqnT2ZWu8Z4OdR5ObWyHssUmjSHU8z8v+NJ0eG5M88gJ89YSdbl\n\tJJhNc+m+usLLs5z4BmkWfdb74R+SI8l1klMPGEM94WzTFaH5ZOAO0DRcxM9eZlYea7u0\n\tzJP9t8HsIxwI+X6wMrEPfBp65StliCE/phZtXZrAcFbRWQtoXgMbmAgrl1Q9S4PEuDiG\n\tsUE7IqhUzRhtXNCT3Wz3uykoCkgfWx8DBeu0n14oaF8q3l1v/3mdl2rx09w1GpmeJTO7\n\t4AQA==","X-Gm-Message-State":"AOAM530pCOWNGAxNFrJaDS+ICX9MW0lU/Outrk0Aap0SkBuewakilwhj\n\tTW5RZtwFO2F+0YRWnGsd+ffQFIeleDGe1w==","X-Google-Smtp-Source":"ABdhPJzr+DHKAHDL8uXji/ODhL1HCSjNqi1YCqR+54erExSV7tnOEJ5Dlujo8NBJrFZOFDoBCJREpw==","X-Received":"by 2002:a2e:974f:: with SMTP id f15mr858120ljj.193.1593732168197;\n\tThu, 02 Jul 2020 16:22:48 -0700 (PDT)","Date":"Fri, 3 Jul 2020 01:22:47 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Kieran Bingham ","Message-ID":"<20200702232247.GF3158543@oden.dyn.berto.se>","References":"<20200702213654.2129054-1-kieran.bingham@ideasonboard.com>\n\t<20200702213654.2129054-4-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200702213654.2129054-4-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 3/9] android: camera_device:\n\tSupport multiple stream configurations","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11101,"web_url":"https://patchwork.libcamera.org/comment/11101/","msgid":"<20200703002242.GB12562@pendragon.ideasonboard.com>","date":"2020-07-03T00:22:42","subject":"Re: [libcamera-devel] [PATCH v2 3/9] android: camera_device:\n\tSupport multiple stream configurations","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Thu, Jul 02, 2020 at 10:36:48PM +0100, Kieran Bingham wrote:\n> Create an initial Camera Configuration using an empty role set, and\n> populate the StreamConfigurations manually from each of the streams\n> given by the Android camera3_stream_configuration_t stream_list.\n> \n> Signed-off-by: Kieran Bingham \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/android/camera_device.cpp | 51 +++++++++++++++++------------------\n> 1 file changed, 25 insertions(+), 26 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index b9031ff0c2a4..03dcdd520794 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -942,6 +942,16 @@ PixelFormat CameraDevice::toPixelFormat(int format)\n> */\n> int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> {\n> +\t/*\n> +\t * Generate an empty configuration, and construct a StreamConfiguration\n> +\t * for each camera3_stream to add to it.\n> +\t */\n> +\tconfig_ = camera_->generateConfiguration();\n> +\tif (!config_) {\n> +\t\tLOG(HAL, Error) << \"Failed to generate camera configuration\";\n> +\t\treturn -EINVAL;\n> +\t}\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> @@ -953,35 +963,18 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \t\t\t << \", height: \" << stream->height\n> \t\t\t << \", format: \" << utils::hex(stream->format)\n> \t\t\t << \" (\" << format.toString() << \")\";\n> -\t}\n> \n> -\t/* Only one stream is supported. */\n> -\tif (stream_list->num_streams != 1) {\n> -\t\tLOG(HAL, Error) << \"Only one stream supported\";\n> -\t\treturn -EINVAL;\n> -\t}\n> -\tcamera3_stream_t *camera3Stream = stream_list->streams[0];\n> +\t\tif (!format.isValid())\n> +\t\t\treturn -EINVAL;\n> \n> -\t/* Translate Android format code to libcamera pixel format. */\n> -\tPixelFormat format = toPixelFormat(camera3Stream->format);\n> -\tif (!format.isValid())\n> -\t\treturn -EINVAL;\n> +\t\tStreamConfiguration streamConfiguration;\n> \n> -\t/*\n> -\t * Hardcode viewfinder role, replacing the generated configuration\n> -\t * parameters with the ones requested by the Android framework.\n> -\t */\n> -\tStreamRoles roles = { StreamRole::Viewfinder };\n> -\tconfig_ = camera_->generateConfiguration(roles);\n> -\tif (!config_ || config_->empty()) {\n> -\t\tLOG(HAL, Error) << \"Failed to generate camera configuration\";\n> -\t\treturn -EINVAL;\n> -\t}\n> +\t\tstreamConfiguration.size.width = stream->width;\n> +\t\tstreamConfiguration.size.height = stream->height;\n> +\t\tstreamConfiguration.pixelFormat = format;\n> \n> -\tStreamConfiguration *streamConfiguration = &config_->at(0);\n> -\tstreamConfiguration->size.width = camera3Stream->width;\n> -\tstreamConfiguration->size.height = camera3Stream->height;\n> -\tstreamConfiguration->pixelFormat = format;\n> +\t\tconfig_->addConfiguration(streamConfiguration);\n> +\t}\n> \n> \tswitch (config_->validate()) {\n> \tcase CameraConfiguration::Valid:\n> @@ -996,7 +989,13 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \t\treturn -EINVAL;\n> \t}\n> \n> -\tcamera3Stream->max_buffers = streamConfiguration->bufferCount;\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> +\n> +\t\t/* Use the bufferCount confirmed by the validation process. */\n> +\t\tstream->max_buffers = streamConfiguration.bufferCount;\n> +\t}\n> \n> \t/*\n> \t * Once the CameraConfiguration has been adjusted/validated","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 6B712BFFE2\n\tfor ;\n\tFri, 3 Jul 2020 00:22:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F3A1160C55;\n\tFri, 3 Jul 2020 02:22:48 +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 41D61609C7\n\tfor ;\n\tFri, 3 Jul 2020 02:22:47 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AF84D9CB;\n\tFri, 3 Jul 2020 02:22:46 +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=\"Ws3J1VTC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593735766;\n\tbh=BMuPQiodTp9LuWLhR7omykGuXAHeTYg/NF2RK6efpaQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Ws3J1VTCq+du6ffAEKYuji+mhBCxWzSZ7GFGdBpEvwKhA8p/v0X+Rv9mndu9gTZVZ\n\t1ALfHTuV8+n147V9uQzqK8uUVtkvieNu30yW2QufjDGwyTcFDi6g/zo6UW2vaOvv7B\n\tvKLhLTPAUlCUwJt9LrWWGNANc1TDFwUG/V612rf4=","Date":"Fri, 3 Jul 2020 03:22:42 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"<20200703002242.GB12562@pendragon.ideasonboard.com>","References":"<20200702213654.2129054-1-kieran.bingham@ideasonboard.com>\n\t<20200702213654.2129054-4-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200702213654.2129054-4-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 3/9] android: camera_device:\n\tSupport multiple stream configurations","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11122,"web_url":"https://patchwork.libcamera.org/comment/11122/","msgid":"<20200703090819.h33cpywfcbr3dpzn@uno.localdomain>","date":"2020-07-03T09:08:19","subject":"Re: [libcamera-devel] [PATCH v2 3/9] android: camera_device:\n\tSupport multiple stream configurations","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran\n\nOn Thu, Jul 02, 2020 at 10:36:48PM +0100, Kieran Bingham wrote:\n> Create an initial Camera Configuration using an empty role set, and\n> populate the StreamConfigurations manually from each of the streams\n> given by the Android camera3_stream_configuration_t stream_list.\n>\n> Signed-off-by: Kieran Bingham \n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> src/android/camera_device.cpp | 51 +++++++++++++++++------------------\n> 1 file changed, 25 insertions(+), 26 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index b9031ff0c2a4..03dcdd520794 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -942,6 +942,16 @@ PixelFormat CameraDevice::toPixelFormat(int format)\n> */\n> int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> {\n> +\t/*\n> +\t * Generate an empty configuration, and construct a StreamConfiguration\n> +\t * for each camera3_stream to add to it.\n> +\t */\n> +\tconfig_ = camera_->generateConfiguration();\n> +\tif (!config_) {\n> +\t\tLOG(HAL, Error) << \"Failed to generate camera configuration\";\n> +\t\treturn -EINVAL;\n> +\t}\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> @@ -953,35 +963,18 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \t\t\t << \", height: \" << stream->height\n> \t\t\t << \", format: \" << utils::hex(stream->format)\n> \t\t\t << \" (\" << format.toString() << \")\";\n> -\t}\n>\n> -\t/* Only one stream is supported. */\n> -\tif (stream_list->num_streams != 1) {\n> -\t\tLOG(HAL, Error) << \"Only one stream supported\";\n> -\t\treturn -EINVAL;\n> -\t}\n> -\tcamera3_stream_t *camera3Stream = stream_list->streams[0];\n> +\t\tif (!format.isValid())\n> +\t\t\treturn -EINVAL;\n>\n> -\t/* Translate Android format code to libcamera pixel format. */\n> -\tPixelFormat format = toPixelFormat(camera3Stream->format);\n> -\tif (!format.isValid())\n> -\t\treturn -EINVAL;\n> +\t\tStreamConfiguration streamConfiguration;\n>\n> -\t/*\n> -\t * Hardcode viewfinder role, replacing the generated configuration\n> -\t * parameters with the ones requested by the Android framework.\n> -\t */\n> -\tStreamRoles roles = { StreamRole::Viewfinder };\n> -\tconfig_ = camera_->generateConfiguration(roles);\n> -\tif (!config_ || config_->empty()) {\n> -\t\tLOG(HAL, Error) << \"Failed to generate camera configuration\";\n> -\t\treturn -EINVAL;\n> -\t}\n> +\t\tstreamConfiguration.size.width = stream->width;\n> +\t\tstreamConfiguration.size.height = stream->height;\n> +\t\tstreamConfiguration.pixelFormat = format;\n>\n> -\tStreamConfiguration *streamConfiguration = &config_->at(0);\n> -\tstreamConfiguration->size.width = camera3Stream->width;\n> -\tstreamConfiguration->size.height = camera3Stream->height;\n> -\tstreamConfiguration->pixelFormat = format;\n> +\t\tconfig_->addConfiguration(streamConfiguration);\n> +\t}\n>\n> \tswitch (config_->validate()) {\n> \tcase CameraConfiguration::Valid:\n> @@ -996,7 +989,13 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \t\treturn -EINVAL;\n> \t}\n>\n> -\tcamera3Stream->max_buffers = streamConfiguration->bufferCount;\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> +\n> +\t\t/* Use the bufferCount confirmed by the validation process. */\n> +\t\tstream->max_buffers = streamConfiguration.bufferCount;\n> +\t}\n>\n> \t/*\n> \t * Once the CameraConfiguration has been adjusted/validated\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":"","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 52956BFFE2\n\tfor ;\n\tFri, 3 Jul 2020 09:04:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EDD2C60C56;\n\tFri, 3 Jul 2020 11:04:49 +0200 (CEST)","from relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A127260C55\n\tfor ;\n\tFri, 3 Jul 2020 11:04:48 +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 relay12.mail.gandi.net (Postfix) with ESMTPSA id 86946200006;\n\tFri, 3 Jul 2020 09:04:47 +0000 (UTC)"],"Date":"Fri, 3 Jul 2020 11:08:19 +0200","From":"Jacopo Mondi ","To":"Kieran Bingham ","Message-ID":"<20200703090819.h33cpywfcbr3dpzn@uno.localdomain>","References":"<20200702213654.2129054-1-kieran.bingham@ideasonboard.com>\n\t<20200702213654.2129054-4-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200702213654.2129054-4-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 3/9] android: camera_device:\n\tSupport multiple stream configurations","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11127,"web_url":"https://patchwork.libcamera.org/comment/11127/","msgid":"<980814e7-9d41-89fc-6dba-291d4f8c4be3@ideasonboard.com>","date":"2020-07-03T09:30:17","subject":"Re: [libcamera-devel] [PATCH v2 3/9] android: camera_device:\n\tSupport multiple stream configurations","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Niklas,\n\nOn 03/07/2020 00:22, Niklas Söderlund wrote:\n> Hi Kieran,\n> \n> Thanks for your work.\n> \n> On 2020-07-02 22:36:48 +0100, Kieran Bingham wrote:\n>> Create an initial Camera Configuration using an empty role set, and\n>> populate the StreamConfigurations manually from each of the streams\n>> given by the Android camera3_stream_configuration_t stream_list.\n>>\n>> Signed-off-by: Kieran Bingham \n>> ---\n>> src/android/camera_device.cpp | 51 +++++++++++++++++------------------\n>> 1 file changed, 25 insertions(+), 26 deletions(-)\n>>\n>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n>> index b9031ff0c2a4..03dcdd520794 100644\n>> --- a/src/android/camera_device.cpp\n>> +++ b/src/android/camera_device.cpp\n>> @@ -942,6 +942,16 @@ PixelFormat CameraDevice::toPixelFormat(int format)\n>> */\n>> int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>> {\n>> +\t/*\n>> +\t * Generate an empty configuration, and construct a StreamConfiguration\n>> +\t * for each camera3_stream to add to it.\n>> +\t */\n>> +\tconfig_ = camera_->generateConfiguration();\n>> +\tif (!config_) {\n>> +\t\tLOG(HAL, Error) << \"Failed to generate camera configuration\";\n>> +\t\treturn -EINVAL;\n>> +\t}\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>> @@ -953,35 +963,18 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>> \t\t\t << \", height: \" << stream->height\n>> \t\t\t << \", format: \" << utils::hex(stream->format)\n>> \t\t\t << \" (\" << format.toString() << \")\";\n>> -\t}\n>> \n>> -\t/* Only one stream is supported. */\n>> -\tif (stream_list->num_streams != 1) {\n>> -\t\tLOG(HAL, Error) << \"Only one stream supported\";\n>> -\t\treturn -EINVAL;\n>> -\t}\n>> -\tcamera3_stream_t *camera3Stream = stream_list->streams[0];\n>> +\t\tif (!format.isValid())\n>> +\t\t\treturn -EINVAL;\n>> \n>> -\t/* Translate Android format code to libcamera pixel format. */\n>> -\tPixelFormat format = toPixelFormat(camera3Stream->format);\n>> -\tif (!format.isValid())\n>> -\t\treturn -EINVAL;\n>> +\t\tStreamConfiguration streamConfiguration;\n>> \n>> -\t/*\n>> -\t * Hardcode viewfinder role, replacing the generated configuration\n>> -\t * parameters with the ones requested by the Android framework.\n>> -\t */\n>> -\tStreamRoles roles = { StreamRole::Viewfinder };\n>> -\tconfig_ = camera_->generateConfiguration(roles);\n>> -\tif (!config_ || config_->empty()) {\n>> -\t\tLOG(HAL, Error) << \"Failed to generate camera configuration\";\n>> -\t\treturn -EINVAL;\n>> -\t}\n>> +\t\tstreamConfiguration.size.width = stream->width;\n>> +\t\tstreamConfiguration.size.height = stream->height;\n>> +\t\tstreamConfiguration.pixelFormat = format;\n> \n> This is not related to this patch but blindly translating a HAL format \n> using a lookup table to a libcamera format will not work for RAW. I'm \n> just curious if it is sufficient for JPEG?\n> \n\nNo - it is not, but that's not tackled by this patch.\n\nLets consider this patch as supporting multiple streams of types that\nare already supported ;-)\n\nI would expect this patch to now support multiple YUV streams.\n\n\n>> \n>> -\tStreamConfiguration *streamConfiguration = &config_->at(0);\n>> -\tstreamConfiguration->size.width = camera3Stream->width;\n>> -\tstreamConfiguration->size.height = camera3Stream->height;\n>> -\tstreamConfiguration->pixelFormat = format;\n>> +\t\tconfig_->addConfiguration(streamConfiguration);\n>> +\t}\n>> \n>> \tswitch (config_->validate()) {\n>> \tcase CameraConfiguration::Valid:\n>> @@ -996,7 +989,13 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>> \t\treturn -EINVAL;\n>> \t}\n>> \n>> -\tcamera3Stream->max_buffers = streamConfiguration->bufferCount;\n>> +\tfor (unsigned int i = 0; i < stream_list->num_streams; ++i) {\n> \n> Small nit, would it make sens to iterate over config_ instead of \n> stream_list->num_streams? If we have a discrepancy we would end up with \n> less streams then requested but if we use ->at(i) an exception.\n\nNo... or at least - not in the /very/ near future.\n\nThere will very soon be an explicit discrepency between the number of\ncamera3 streams and the number of libcamera streams.\n\nAdding a JPEG stream will not add a new stream, but we must still\nsupport it.\n\nLater I add a mapping which stores the correct libcamera index to\nreference for this stream (for which multiple camera3 streams might\nreference a single libcamera stream index).\n\nIf I iterate over config_ here, I would still need an index to map\n/back/ to the camera3_stream index, so either way an explicit for loop\nseems better here currently.\n\n\n\n> \n> This is a small thing that poped out at me, with or without this \n> changed,\n> \n> Reviewed-by: Niklas Söderlund \n\nThanks,\n\nI hope the above makes you happier with this path...\n\n\n> \n>> +\t\tcamera3_stream_t *stream = stream_list->streams[i];\n>> +\t\tStreamConfiguration &streamConfiguration = config_->at(i);\n>> +\n>> +\t\t/* Use the bufferCount confirmed by the validation process. */\n>> +\t\tstream->max_buffers = streamConfiguration.bufferCount;\n>> +\t}\n>> \n>> \t/*\n>> \t * Once the CameraConfiguration has been adjusted/validated\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>","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 323F5BFFE2\n\tfor ;\n\tFri, 3 Jul 2020 09:30:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BB6AE60C58;\n\tFri, 3 Jul 2020 11:30:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CFBC460C50\n\tfor ;\n\tFri, 3 Jul 2020 11:30:20 +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 52AEF51B;\n\tFri, 3 Jul 2020 11:30:20 +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=\"Ft6rVg9i\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593768620;\n\tbh=ESnZH4V+xTOaYXVbg7GLPbS8JuC4LOasyLmt2uptRa0=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=Ft6rVg9iKcZ/MU9uU9dyc4RD04xzGzgnnL8YPXPazP4YGHpFU+gr8fLRa41UPS9f8\n\tSXIzDog8WyJFS1VTLvTncD/J9XFAgq5DkocyCefRhTK1Zl7RKOp+M5pH5vg+d48rWV\n\tQppJSeElyy1Ll59zYhsl7ThhTlbbUjK82GA6j1S4=","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= ","References":"<20200702213654.2129054-1-kieran.bingham@ideasonboard.com>\n\t<20200702213654.2129054-4-kieran.bingham@ideasonboard.com>\n\t<20200702232247.GF3158543@oden.dyn.berto.se>","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":"<980814e7-9d41-89fc-6dba-291d4f8c4be3@ideasonboard.com>","Date":"Fri, 3 Jul 2020 10:30:17 +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":"<20200702232247.GF3158543@oden.dyn.berto.se>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v2 3/9] android: camera_device:\n\tSupport multiple stream configurations","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11137,"web_url":"https://patchwork.libcamera.org/comment/11137/","msgid":"<20200703101412.GB3373310@oden.dyn.berto.se>","date":"2020-07-03T10:14:12","subject":"Re: [libcamera-devel] [PATCH v2 3/9] android: camera_device:\n\tSupport multiple stream configurations","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Kieran,\n\nOn 2020-07-03 10:30:17 +0100, Kieran Bingham wrote:\n> Hi Niklas,\n> \n> On 03/07/2020 00:22, Niklas Söderlund wrote:\n> > Hi Kieran,\n> > \n> > Thanks for your work.\n> > \n> > On 2020-07-02 22:36:48 +0100, Kieran Bingham wrote:\n> >> Create an initial Camera Configuration using an empty role set, and\n> >> populate the StreamConfigurations manually from each of the streams\n> >> given by the Android camera3_stream_configuration_t stream_list.\n> >>\n> >> Signed-off-by: Kieran Bingham \n> >> ---\n> >> src/android/camera_device.cpp | 51 +++++++++++++++++------------------\n> >> 1 file changed, 25 insertions(+), 26 deletions(-)\n> >>\n> >> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> >> index b9031ff0c2a4..03dcdd520794 100644\n> >> --- a/src/android/camera_device.cpp\n> >> +++ b/src/android/camera_device.cpp\n> >> @@ -942,6 +942,16 @@ PixelFormat CameraDevice::toPixelFormat(int format)\n> >> */\n> >> int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> >> {\n> >> +\t/*\n> >> +\t * Generate an empty configuration, and construct a StreamConfiguration\n> >> +\t * for each camera3_stream to add to it.\n> >> +\t */\n> >> +\tconfig_ = camera_->generateConfiguration();\n> >> +\tif (!config_) {\n> >> +\t\tLOG(HAL, Error) << \"Failed to generate camera configuration\";\n> >> +\t\treturn -EINVAL;\n> >> +\t}\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> >> @@ -953,35 +963,18 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> >> \t\t\t << \", height: \" << stream->height\n> >> \t\t\t << \", format: \" << utils::hex(stream->format)\n> >> \t\t\t << \" (\" << format.toString() << \")\";\n> >> -\t}\n> >> \n> >> -\t/* Only one stream is supported. */\n> >> -\tif (stream_list->num_streams != 1) {\n> >> -\t\tLOG(HAL, Error) << \"Only one stream supported\";\n> >> -\t\treturn -EINVAL;\n> >> -\t}\n> >> -\tcamera3_stream_t *camera3Stream = stream_list->streams[0];\n> >> +\t\tif (!format.isValid())\n> >> +\t\t\treturn -EINVAL;\n> >> \n> >> -\t/* Translate Android format code to libcamera pixel format. */\n> >> -\tPixelFormat format = toPixelFormat(camera3Stream->format);\n> >> -\tif (!format.isValid())\n> >> -\t\treturn -EINVAL;\n> >> +\t\tStreamConfiguration streamConfiguration;\n> >> \n> >> -\t/*\n> >> -\t * Hardcode viewfinder role, replacing the generated configuration\n> >> -\t * parameters with the ones requested by the Android framework.\n> >> -\t */\n> >> -\tStreamRoles roles = { StreamRole::Viewfinder };\n> >> -\tconfig_ = camera_->generateConfiguration(roles);\n> >> -\tif (!config_ || config_->empty()) {\n> >> -\t\tLOG(HAL, Error) << \"Failed to generate camera configuration\";\n> >> -\t\treturn -EINVAL;\n> >> -\t}\n> >> +\t\tstreamConfiguration.size.width = stream->width;\n> >> +\t\tstreamConfiguration.size.height = stream->height;\n> >> +\t\tstreamConfiguration.pixelFormat = format;\n> > \n> > This is not related to this patch but blindly translating a HAL format \n> > using a lookup table to a libcamera format will not work for RAW. I'm \n> > just curious if it is sufficient for JPEG?\n> > \n> \n> No - it is not, but that's not tackled by this patch.\n> \n> Lets consider this patch as supporting multiple streams of types that\n> are already supported ;-)\n> \n> I would expect this patch to now support multiple YUV streams.\n> \n> \n> >> \n> >> -\tStreamConfiguration *streamConfiguration = &config_->at(0);\n> >> -\tstreamConfiguration->size.width = camera3Stream->width;\n> >> -\tstreamConfiguration->size.height = camera3Stream->height;\n> >> -\tstreamConfiguration->pixelFormat = format;\n> >> +\t\tconfig_->addConfiguration(streamConfiguration);\n> >> +\t}\n> >> \n> >> \tswitch (config_->validate()) {\n> >> \tcase CameraConfiguration::Valid:\n> >> @@ -996,7 +989,13 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> >> \t\treturn -EINVAL;\n> >> \t}\n> >> \n> >> -\tcamera3Stream->max_buffers = streamConfiguration->bufferCount;\n> >> +\tfor (unsigned int i = 0; i < stream_list->num_streams; ++i) {\n> > \n> > Small nit, would it make sens to iterate over config_ instead of \n> > stream_list->num_streams? If we have a discrepancy we would end up with \n> > less streams then requested but if we use ->at(i) an exception.\n> \n> No... or at least - not in the /very/ near future.\n> \n> There will very soon be an explicit discrepency between the number of\n> camera3 streams and the number of libcamera streams.\n> \n> Adding a JPEG stream will not add a new stream, but we must still\n> support it.\n> \n> Later I add a mapping which stores the correct libcamera index to\n> reference for this stream (for which multiple camera3 streams might\n> reference a single libcamera stream index).\n> \n> If I iterate over config_ here, I would still need an index to map\n> /back/ to the camera3_stream index, so either way an explicit for loop\n> seems better here currently.\n> \n> \n> \n> > \n> > This is a small thing that poped out at me, with or without this \n> > changed,\n> > \n> > Reviewed-by: Niklas Söderlund \n> \n> Thanks,\n> \n> I hope the above makes you happier with this path...\n\nIt does, thanks for the explanation.\n\n> \n> \n> > \n> >> +\t\tcamera3_stream_t *stream = stream_list->streams[i];\n> >> +\t\tStreamConfiguration &streamConfiguration = config_->at(i);\n> >> +\n> >> +\t\t/* Use the bufferCount confirmed by the validation process. */\n> >> +\t\tstream->max_buffers = streamConfiguration.bufferCount;\n> >> +\t}\n> >> \n> >> \t/*\n> >> \t * Once the CameraConfiguration has been adjusted/validated\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> -- \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 8754CBE905\n\tfor ;\n\tFri, 3 Jul 2020 10:14:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5496560C57;\n\tFri, 3 Jul 2020 12:14:16 +0200 (CEST)","from mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7758960C53\n\tfor ;\n\tFri, 3 Jul 2020 12:14:14 +0200 (CEST)","by mail-lj1-x241.google.com with SMTP id e8so2029114ljb.0\n\tfor ;\n\tFri, 03 Jul 2020 03:14:14 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\t144sm4484469lfm.87.2020.07.03.03.14.12\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 03 Jul 2020 03:14:13 -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=\"YxEqb8w6\"; 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=+BUqxAzXNcCiD1QxsJnBs3O7BDD36yFAdsWX+totKvI=;\n\tb=YxEqb8w6qdV3XjjRuG3YD8MrQMVDE4z3CBwY30gqQQSBweEuDyA76B1n6XHpHDMgRz\n\tZwnC0yPHW3jXkrEQ1mN2Bmn6RyccJuGpGlEUt0dPmA4gaYZSaSA9vZHwBeTUhFW7xC+k\n\tfhQkSH+E8P04mad3rTbPpz5pX3xMcFutG0x7QBGg1fzUvmPlt7+JeQOcVBinHjIQLRno\n\tF3THl93cjHZyCIWwqaT10IN5qEWZFXwwrH4ZdQlaJQJO9Zsu7k1468bDf5gDbpMDSSmm\n\tuDj54gUEgqbLZ8Ar8N4b05cR2EVf+bz5e0Q3XvLNPr7TvS9Tn25oAN60JjPQwRm9tqZU\n\tMt7w==","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=+BUqxAzXNcCiD1QxsJnBs3O7BDD36yFAdsWX+totKvI=;\n\tb=cBFAxZu4PHGi+DpfQNKF4Y9TatD7Py69KMNGaPm/DwMxYNbxE3d4tCe5yfaY3jwiLO\n\tsuioBw+rZ+7ULjxqJflP225vcwoufkrA5Iu2BtnHrBiqEF3HIGFD5IG1odOW6+uyi0T+\n\tCEh+JMNzHWiPFlKTPiB/HDFiGb26uI+5Fsu22/WHxE9zaKWONnbgspnXNgGGQ/Df7p2G\n\tT7vF/PsfEZ4lC1oGOjCQ2tVNG5NSbe4+sYKeLEvboxKr51JpcNfUz8uxUNUoWGuXAFYT\n\tnAiMhh2AUHj8i0SOAfs/6QdkE5A7LCPpS03Xf/lee3/kyR/m2H9+T5YpVFZ/NLseODE8\n\tIm1A==","X-Gm-Message-State":"AOAM532vMGBlIWwuzBeMbD1hA3H0tMDtRUT4+gm9/zVoBa/aNE4auStC\n\t4XcSuLO6dFAtBupKVfgrEvPQ9ZezhDSF0Q==","X-Google-Smtp-Source":"ABdhPJzPmMhwnYjnbbM/trAFbXyTAD/bXTZDrONDEMgy+U1/aTiXI3kn9OhlIGqSHK+UsNvAdpvI8Q==","X-Received":"by 2002:a2e:b5d0:: with SMTP id\n\tg16mr18250872ljn.246.1593771253701; \n\tFri, 03 Jul 2020 03:14:13 -0700 (PDT)","Date":"Fri, 3 Jul 2020 12:14:12 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Kieran Bingham ","Message-ID":"<20200703101412.GB3373310@oden.dyn.berto.se>","References":"<20200702213654.2129054-1-kieran.bingham@ideasonboard.com>\n\t<20200702213654.2129054-4-kieran.bingham@ideasonboard.com>\n\t<20200702232247.GF3158543@oden.dyn.berto.se>\n\t<980814e7-9d41-89fc-6dba-291d4f8c4be3@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<980814e7-9d41-89fc-6dba-291d4f8c4be3@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 3/9] android: camera_device:\n\tSupport multiple stream configurations","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]