[{"id":4846,"web_url":"https://patchwork.libcamera.org/comment/4846/","msgid":"<4d43c87f-ab35-158a-69ab-0d37e21ead72@ideasonboard.com>","date":"2020-05-19T09:24:38","subject":"Re: [libcamera-devel] [PATCH 1/8] cam: Pass stream roles to Capture\n\tclass","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 19/05/2020 04:24, Laurent Pinchart wrote:\n> The stream roles will be needed in the Capture class to verify the\n> configuration. Store they in the CamApp class and pass them to the\n\ns/they/them/\n\nOther than that,\n\nReviewed-by: Kieran Bingham \n\n> Capture class constructor.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> src/cam/capture.cpp | 5 +++--\n> src/cam/capture.h | 4 +++-\n> src/cam/main.cpp | 9 +++++----\n> 3 files changed, 11 insertions(+), 7 deletions(-)\n> \n> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n> index 55fa2dabcee9..b7e06bcc9463 100644\n> --- a/src/cam/capture.cpp\n> +++ b/src/cam/capture.cpp\n> @@ -16,8 +16,9 @@\n> \n> using namespace libcamera;\n> \n> -Capture::Capture(std::shared_ptr camera, CameraConfiguration *config)\n> -\t: camera_(camera), config_(config), writer_(nullptr)\n> +Capture::Capture(std::shared_ptr camera, CameraConfiguration *config,\n> +\t\t const StreamRoles &roles)\n> +\t: camera_(camera), config_(config), roles_(roles), writer_(nullptr)\n> {\n> }\n> \n> diff --git a/src/cam/capture.h b/src/cam/capture.h\n> index 9bca5661070e..c0e697b831fb 100644\n> --- a/src/cam/capture.h\n> +++ b/src/cam/capture.h\n> @@ -24,7 +24,8 @@ class Capture\n> {\n> public:\n> \tCapture(std::shared_ptr camera,\n> -\t\tlibcamera::CameraConfiguration *config);\n> +\t\tlibcamera::CameraConfiguration *config,\n> +\t\tconst libcamera::StreamRoles &roles);\n> \n> \tint run(EventLoop *loop, const OptionsParser::Options &options);\n> private:\n> @@ -35,6 +36,7 @@ private:\n> \n> \tstd::shared_ptr camera_;\n> \tlibcamera::CameraConfiguration *config_;\n> +\tlibcamera::StreamRoles roles_;\n> \n> \tstd::map streamName_;\n> \tBufferWriter *writer_;\n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index 2512fe9da782..cdd29d500202 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -47,6 +47,7 @@ private:\n> \tOptionsParser::Options options_;\n> \tCameraManager *cm_;\n> \tstd::shared_ptr camera_;\n> +\tStreamRoles roles_;\n> \tstd::unique_ptr config_;\n> \tEventLoop *loop_;\n> };\n> @@ -194,10 +195,10 @@ int CamApp::parseOptions(int argc, char *argv[])\n> \n> int CamApp::prepareConfig()\n> {\n> -\tStreamRoles roles = StreamKeyValueParser::roles(options_[OptStream]);\n> +\troles_ = StreamKeyValueParser::roles(options_[OptStream]);\n> \n> -\tconfig_ = camera_->generateConfiguration(roles);\n> -\tif (!config_ || config_->size() != roles.size()) {\n> +\tconfig_ = camera_->generateConfiguration(roles_);\n> +\tif (!config_ || config_->size() != roles_.size()) {\n> \t\tstd::cerr << \"Failed to get default stream configuration\"\n> \t\t\t << std::endl;\n> \t\treturn -EINVAL;\n> @@ -326,7 +327,7 @@ int CamApp::run()\n> \t}\n> \n> \tif (options_.isSet(OptCapture)) {\n> -\t\tCapture capture(camera_, config_.get());\n> +\t\tCapture capture(camera_, config_.get(), roles_);\n> \t\treturn capture.run(loop_, options_);\n> \t}\n> \n>","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 582A3603D8\n\tfor ;\n\tTue, 19 May 2020 11:24:45 +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 6BC7030C;\n\tTue, 19 May 2020 11:24:44 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"IWfb/Q9u\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1589880284;\n\tbh=7GLdky1ZZj82QEF9k6sDfPzcis8FNjJ9o1kR1ZRqgek=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=IWfb/Q9uoNmlwE9F4P9MJqqq2E5X9duMuVD644pmp4g9ZZ2y67/azuL4l8CaWotIY\n\tMlEub7auVORlObWHk+YbmGQOvq5PLuYBnDau+BzEZe2cn31Z5F6Wv7litbnUJ4gM7Z\n\t5wdcGhOOWqWTLZ1CKmUbwokpP4CmLMULsKV4yXp4=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200519032505.17307-1-laurent.pinchart@ideasonboard.com>\n\t<20200519032505.17307-2-laurent.pinchart@ideasonboard.com>","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":"<4d43c87f-ab35-158a-69ab-0d37e21ead72@ideasonboard.com>","Date":"Tue, 19 May 2020 10:24:38 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.7.0","MIME-Version":"1.0","In-Reply-To":"<20200519032505.17307-2-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 1/8] cam: Pass stream roles to Capture\n\tclass","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","X-List-Received-Date":"Tue, 19 May 2020 09:24:45 -0000"}},{"id":4859,"web_url":"https://patchwork.libcamera.org/comment/4859/","msgid":"<20200519142750.GL470768@oden.dyn.berto.se>","date":"2020-05-19T14:27:50","subject":"Re: [libcamera-devel] [PATCH 1/8] cam: Pass stream roles to Capture\n\tclass","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2020-05-19 06:24:58 +0300, Laurent Pinchart wrote:\n> The stream roles will be needed in the Capture class to verify the\n> configuration. Store they in the CamApp class and pass them to the\n> Capture class constructor.\n\nThe patch itself is fine but I wonder if this is the right way. We have \npreviously endeavored to limit the spread of the roles past the \ngenerateConfiguration() as they somewhat lose there meaning after that, \nright?\n\nEven if the user requested a viewfinder and video stream they are only \nhints to the pipeline hander is \"free\" to return any type of camera \nconfiguration, even one with only 1 stream even if more where requested \nin the role hints.\n\nFor this reason I wonder if instead the check for the combination of the \nnew option -D and that the stream role hint should be set to viewfinder \nshould be moved and be validated right after we have parsed the command \nline options?\n\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> src/cam/capture.cpp | 5 +++--\n> src/cam/capture.h | 4 +++-\n> src/cam/main.cpp | 9 +++++----\n> 3 files changed, 11 insertions(+), 7 deletions(-)\n> \n> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n> index 55fa2dabcee9..b7e06bcc9463 100644\n> --- a/src/cam/capture.cpp\n> +++ b/src/cam/capture.cpp\n> @@ -16,8 +16,9 @@\n> \n> using namespace libcamera;\n> \n> -Capture::Capture(std::shared_ptr camera, CameraConfiguration *config)\n> -\t: camera_(camera), config_(config), writer_(nullptr)\n> +Capture::Capture(std::shared_ptr camera, CameraConfiguration *config,\n> +\t\t const StreamRoles &roles)\n> +\t: camera_(camera), config_(config), roles_(roles), writer_(nullptr)\n> {\n> }\n> \n> diff --git a/src/cam/capture.h b/src/cam/capture.h\n> index 9bca5661070e..c0e697b831fb 100644\n> --- a/src/cam/capture.h\n> +++ b/src/cam/capture.h\n> @@ -24,7 +24,8 @@ class Capture\n> {\n> public:\n> \tCapture(std::shared_ptr camera,\n> -\t\tlibcamera::CameraConfiguration *config);\n> +\t\tlibcamera::CameraConfiguration *config,\n> +\t\tconst libcamera::StreamRoles &roles);\n> \n> \tint run(EventLoop *loop, const OptionsParser::Options &options);\n> private:\n> @@ -35,6 +36,7 @@ private:\n> \n> \tstd::shared_ptr camera_;\n> \tlibcamera::CameraConfiguration *config_;\n> +\tlibcamera::StreamRoles roles_;\n> \n> \tstd::map streamName_;\n> \tBufferWriter *writer_;\n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index 2512fe9da782..cdd29d500202 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -47,6 +47,7 @@ private:\n> \tOptionsParser::Options options_;\n> \tCameraManager *cm_;\n> \tstd::shared_ptr camera_;\n> +\tStreamRoles roles_;\n> \tstd::unique_ptr config_;\n> \tEventLoop *loop_;\n> };\n> @@ -194,10 +195,10 @@ int CamApp::parseOptions(int argc, char *argv[])\n> \n> int CamApp::prepareConfig()\n> {\n> -\tStreamRoles roles = StreamKeyValueParser::roles(options_[OptStream]);\n> +\troles_ = StreamKeyValueParser::roles(options_[OptStream]);\n> \n> -\tconfig_ = camera_->generateConfiguration(roles);\n> -\tif (!config_ || config_->size() != roles.size()) {\n> +\tconfig_ = camera_->generateConfiguration(roles_);\n> +\tif (!config_ || config_->size() != roles_.size()) {\n> \t\tstd::cerr << \"Failed to get default stream configuration\"\n> \t\t\t << std::endl;\n> \t\treturn -EINVAL;\n> @@ -326,7 +327,7 @@ int CamApp::run()\n> \t}\n> \n> \tif (options_.isSet(OptCapture)) {\n> -\t\tCapture capture(camera_, config_.get());\n> +\t\tCapture capture(camera_, config_.get(), roles_);\n> \t\treturn capture.run(loop_, options_);\n> \t}\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["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 70BC0603D9\n\tfor ;\n\tTue, 19 May 2020 16:27:52 +0200 (CEST)","by mail-lj1-x241.google.com with SMTP id q2so1759400ljm.10\n\tfor ;\n\tTue, 19 May 2020 07:27:52 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\th84sm9158106lfd.88.2020.05.19.07.27.51\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 19 May 2020 07:27:51 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com header.b=\"zVqpRQlT\"; \n\tdkim-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=RpSBk1WFBH+DQZedW3q9oGOWudJK3/gcMI9dWG6PfAE=;\n\tb=zVqpRQlTGp2p/RO/b/MT3k33NfRJhP1/b/mG9SLAm/4Rh6dxbWK8vs4HhzDCttki+q\n\ts0JVmQU7NCzVffP78nhZUwapa0nOPF7iL+WifuLxQEUcpYfLvFppTJkKwHGMgZIvJ6tl\n\tOOEJwHgX0X//RqqsLOCblBmxJTBMeZNDtb80E3JBQl1eNIO8Zn3vR4PlbkcsGhkayHAX\n\tTfYkt9QaNtPT0Bf3bwaHCwnOo9jeuAXkYmduulmp5gMFUdcVELyrbt3YOPNyrVTxXE3i\n\taO8EdaFVaxSe9g7AJu76UaJiQLWs8T2RSKz6pBCHShXorwN67ZbUrRMR4ErYEKHczH/7\n\tbEjg==","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=RpSBk1WFBH+DQZedW3q9oGOWudJK3/gcMI9dWG6PfAE=;\n\tb=lXj/X9UQ+uf1bKBenJa6A9gqcRIqZJks9zksdiVrURUkq/mUNm+s8sDSAJl300jq5Z\n\tR0tZCQ/FHOpvneFcFcNeRUFQXJwyJlzUl/kqgUFmCCow9OFMPzLtXNaInAr1+rB9yy4Q\n\tp4hqh3Z0xpekNGihipBaiHGIV4l4AQ6zSKB4L6BnHt6IF/vbBz28SsG/qLZev6YYqBOa\n\tfZsgXaxH6WnY41VdEC0SMcGASLRuIeOIIxP/JFWYOawwi6LllVH/RTkgGEncSxXDCZgd\n\tcJDr3k0fTUPqiAMVGlmLjS0VNXeidj1dBUybuxZvzo2zY8o6CQOl57pzmfuBIvxpGtt2\n\tV9mw==","X-Gm-Message-State":"AOAM530yAl9X4CPd9vWQuMETUCHtCDTzycvRZVus/PrusrfQHbZ/kN8W\n\tqDddHmeWWKTh/jNylRg3rrwV/29BqLsuXA==","X-Google-Smtp-Source":"ABdhPJzmeWGPRhi2l/Wo8h1ELkm1NrL/ayBHX+rh5SO/KFk5/l/tcwcNo2Fk4F7Z3mNPXwUPr7Vlbw==","X-Received":"by 2002:a2e:a169:: with SMTP id\n\tu9mr14144483ljl.144.1589898471735; \n\tTue, 19 May 2020 07:27:51 -0700 (PDT)","Date":"Tue, 19 May 2020 16:27:50 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200519142750.GL470768@oden.dyn.berto.se>","References":"<20200519032505.17307-1-laurent.pinchart@ideasonboard.com>\n\t<20200519032505.17307-2-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200519032505.17307-2-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/8] cam: Pass stream roles to Capture\n\tclass","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","X-List-Received-Date":"Tue, 19 May 2020 14:27:52 -0000"}},{"id":4871,"web_url":"https://patchwork.libcamera.org/comment/4871/","msgid":"<20200519223753.GH3820@pendragon.ideasonboard.com>","date":"2020-05-19T22:37:53","subject":"Re: [libcamera-devel] [PATCH 1/8] cam: Pass stream roles to Capture\n\tclass","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Tue, May 19, 2020 at 04:27:50PM +0200, Niklas Söderlund wrote:\n> On 2020-05-19 06:24:58 +0300, Laurent Pinchart wrote:\n> > The stream roles will be needed in the Capture class to verify the\n> > configuration. Store they in the CamApp class and pass them to the\n> > Capture class constructor.\n> \n> The patch itself is fine but I wonder if this is the right way. We have \n> previously endeavored to limit the spread of the roles past the \n> generateConfiguration() as they somewhat lose there meaning after that, \n> right?\n> \n> Even if the user requested a viewfinder and video stream they are only \n> hints to the pipeline hander is \"free\" to return any type of camera \n> configuration, even one with only 1 stream even if more where requested \n> in the role hints.\n> \n> For this reason I wonder if instead the check for the combination of the \n> new option -D and that the stream role hint should be set to viewfinder \n> should be moved and be validated right after we have parsed the command \n> line options?\n\nI'll do so.\n\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/cam/capture.cpp | 5 +++--\n> > src/cam/capture.h | 4 +++-\n> > src/cam/main.cpp | 9 +++++----\n> > 3 files changed, 11 insertions(+), 7 deletions(-)\n> > \n> > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n> > index 55fa2dabcee9..b7e06bcc9463 100644\n> > --- a/src/cam/capture.cpp\n> > +++ b/src/cam/capture.cpp\n> > @@ -16,8 +16,9 @@\n> > \n> > using namespace libcamera;\n> > \n> > -Capture::Capture(std::shared_ptr camera, CameraConfiguration *config)\n> > -\t: camera_(camera), config_(config), writer_(nullptr)\n> > +Capture::Capture(std::shared_ptr camera, CameraConfiguration *config,\n> > +\t\t const StreamRoles &roles)\n> > +\t: camera_(camera), config_(config), roles_(roles), writer_(nullptr)\n> > {\n> > }\n> > \n> > diff --git a/src/cam/capture.h b/src/cam/capture.h\n> > index 9bca5661070e..c0e697b831fb 100644\n> > --- a/src/cam/capture.h\n> > +++ b/src/cam/capture.h\n> > @@ -24,7 +24,8 @@ class Capture\n> > {\n> > public:\n> > \tCapture(std::shared_ptr camera,\n> > -\t\tlibcamera::CameraConfiguration *config);\n> > +\t\tlibcamera::CameraConfiguration *config,\n> > +\t\tconst libcamera::StreamRoles &roles);\n> > \n> > \tint run(EventLoop *loop, const OptionsParser::Options &options);\n> > private:\n> > @@ -35,6 +36,7 @@ private:\n> > \n> > \tstd::shared_ptr camera_;\n> > \tlibcamera::CameraConfiguration *config_;\n> > +\tlibcamera::StreamRoles roles_;\n> > \n> > \tstd::map streamName_;\n> > \tBufferWriter *writer_;\n> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > index 2512fe9da782..cdd29d500202 100644\n> > --- a/src/cam/main.cpp\n> > +++ b/src/cam/main.cpp\n> > @@ -47,6 +47,7 @@ private:\n> > \tOptionsParser::Options options_;\n> > \tCameraManager *cm_;\n> > \tstd::shared_ptr camera_;\n> > +\tStreamRoles roles_;\n> > \tstd::unique_ptr config_;\n> > \tEventLoop *loop_;\n> > };\n> > @@ -194,10 +195,10 @@ int CamApp::parseOptions(int argc, char *argv[])\n> > \n> > int CamApp::prepareConfig()\n> > {\n> > -\tStreamRoles roles = StreamKeyValueParser::roles(options_[OptStream]);\n> > +\troles_ = StreamKeyValueParser::roles(options_[OptStream]);\n> > \n> > -\tconfig_ = camera_->generateConfiguration(roles);\n> > -\tif (!config_ || config_->size() != roles.size()) {\n> > +\tconfig_ = camera_->generateConfiguration(roles_);\n> > +\tif (!config_ || config_->size() != roles_.size()) {\n> > \t\tstd::cerr << \"Failed to get default stream configuration\"\n> > \t\t\t << std::endl;\n> > \t\treturn -EINVAL;\n> > @@ -326,7 +327,7 @@ int CamApp::run()\n> > \t}\n> > \n> > \tif (options_.isSet(OptCapture)) {\n> > -\t\tCapture capture(camera_, config_.get());\n> > +\t\tCapture capture(camera_, config_.get(), roles_);\n> > \t\treturn capture.run(loop_, options_);\n> > \t}\n> >","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AE9A3603DB\n\tfor ;\n\tWed, 20 May 2020 00:38:06 +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 C280630C;\n\tWed, 20 May 2020 00:38:05 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"r83DYG1d\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1589927886;\n\tbh=qnjrXj2E8ohfV9a/1XQFeqlCAFA+Q3LoysJpkqSidgo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=r83DYG1dwPts59JgCR1fa94dLbz3AhAoEo6Wweb0nebKd5n1ypEgqLOXGvOSjN5DG\n\tibM4t47PWEjQ75i2Eyaz0ERnZBnupgGGDUgN4WaAHOos8xrbBBM4LxRVa/VQ1Dq6t6\n\tdgjTe5188c22am6m38TSWf9bfMiFNTVUi1kfTnG0=","Date":"Wed, 20 May 2020 01:37:53 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200519223753.GH3820@pendragon.ideasonboard.com>","References":"<20200519032505.17307-1-laurent.pinchart@ideasonboard.com>\n\t<20200519032505.17307-2-laurent.pinchart@ideasonboard.com>\n\t<20200519142750.GL470768@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200519142750.GL470768@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH 1/8] cam: Pass stream roles to Capture\n\tclass","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","X-List-Received-Date":"Tue, 19 May 2020 22:38:06 -0000"}}]