[{"id":649,"web_url":"https://patchwork.libcamera.org/comment/649/","msgid":"<20190128003523.GE5154@pendragon.ideasonboard.com>","date":"2019-01-28T00:35:23","subject":"Re: [libcamera-devel] [PATCH v3 6/6] libcamera: camera: extend\n\tcamera object to support configuration of streams","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Sun, Jan 27, 2019 at 01:22:08AM +0100, Niklas Söderlund wrote:\n> Extend the camera to support reading and configuring of formats for\n\ns/of formats/formats/\n\n> groups of streams. The implementation in the Camera are minimalistic as\n> the heavy lifting are done by the pipeline handler implementations.\n> \n> The most important functionality the camera provides in this context is\n> validation of data structures passed to it from the application and\n> access control to the pipeline handler.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> include/libcamera/camera.h | 4 +++\n> src/libcamera/camera.cpp | 68 ++++++++++++++++++++++++++++++++++++++\n> 2 files changed, 72 insertions(+)\n> \n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 786d4d7d66bed5b2..798fee2487940208 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -7,6 +7,7 @@\n> #ifndef __LIBCAMERA_CAMERA_H__\n> #define __LIBCAMERA_CAMERA_H__\n> \n> +#include \n> #include \n> #include \n> \n> @@ -16,6 +17,7 @@ namespace libcamera {\n> \n> class PipelineHandler;\n> class Stream;\n> +class StreamConfiguration;\n> \n> class Camera final\n> {\n> @@ -35,6 +37,8 @@ public:\n> \tvoid release();\n> \n> \tstd::vector streams() const;\n> +\tstd::map streamConfiguration(const std::vector &streams) const;\n> +\tint configureStreams(std::map &config);\n> \n> private:\n> \tCamera(PipelineHandler *pipe, const std::string &name);\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 3b2c00d0a4bb45d1..e41143972b0b537a 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -211,4 +211,72 @@ std::vector Camera::streams() const\n> \treturn streams_;\n> }\n> \n> +/**\n> + * \\brief Retrieve a group of stream configurations\n> + * \\param[in] streams A map of stream IDs and configurations to setup\n> + *\n> + * Retrieve the camera's configuration for a specified group of streams. The\n> + * caller can specify which of the camera's streams to retrieve configuration\n> + * from by populating \\a streams.\n> + *\n> + * The easiest way to populate the array of streams to fetch configuration from\n> + * is to first retrieve the camera's full array of stream by with streams() and\n> + * then potentially trim it down to only contain the streams the caller\n> + * are interested in.\n> + *\n> + * \\return A map of successfully retrieved stream IDs and configurations or an\n> + * empty list on error.\n> + */\n> +std::map\n> +Camera::streamConfiguration(const std::vector &streams) const\n> +{\n> +\tstd::map config;\n> +\n> +\tif (disconnected_ || !streams.size())\n> +\t\treturn config;\n> +\n> +\tfor (const Stream &stream : streams)\n> +\t\tif (!haveStreamID(stream.id()))\n> +\t\t\treturn config;\n> +\n> +\treturn pipe_->streamConfiguration(this, streams);\n> +}\n> +\n> +/**\n> + * \\brief Configure the camera's streams prior to capture\n> + * \\param[in] config A map of stream IDs and configurations to setup\n> + *\n> + * Prior to starting capture, the camera must be configured to select a\n> + * group of streams to be involved in the capture and their configuration.\n> + * The caller specify which streams are to be involved and their configuration\n\ns/specify/specifies/\n\n> + * by populating \\a config.\n> + *\n> + * The easiest way to populate the array of config is to fetch an initial\n> + * configuration from the camera with streamConfiguration() and then change the\n> + * parameters to fit the callers need and once all the streams parameters are\n\ns/callers need/caller's needs/\n\n> + * configured hand that over to configureStreams() to actually setup the camera.\n> + *\n> + * Exclusive access to the camera shall be ensured by a call to acquire() prior\n> + * to calling this function, otherwise an -EACCES error is be returned.\n\nYou should document that -ENODEV will be returned in case the camera is\ndisconnected, and -EINVAL if the configuration isn't valid.\n\n> + *\n> + * \\return 0 on success or a negative error code on error.\n> + */\n> +int Camera::configureStreams(std::map &config)\n> +{\n> +\tif (disconnected_)\n> +\t\treturn -ENODEV;\n> +\n> +\tif (!acquired_)\n> +\t\treturn -EACCES;\n> +\n> +\tif (!config.size())\n> +\t\treturn -EINVAL;\n> +\n> +\tfor (const auto &pair : config)\n> +\t\tif (!haveStreamID(pair.first))\n> +\t\t\treturn -EINVAL;\n\nCould we make this error impossible by indexing the map on a Stream\npointer ?\n\n> +\treturn pipe_->configureStreams(this, config);\n> +}\n> +\n> } /* namespace libcamera */","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 261B060C78\n\tfor ;\n\tMon, 28 Jan 2019 01:35:27 +0100 (CET)","from pendragon.ideasonboard.com (85-76-73-155-nat.elisa-mobile.fi\n\t[85.76.73.155])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C5F8685;\n\tMon, 28 Jan 2019 01:35:25 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548635726;\n\tbh=eDH0DCGntke2+/c0MDkjrK3MMBIzFS/+tK0URP87Ppc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=faaOlBEf0BYU/PDoM74tBxSKhNnPjkKjiXsvt93REy27Gignhm87/5RsWmliKAvR6\n\tpoG6YVb1A/bLZib2vNU6qjSYnZdAE5lQtaTxhNRnwFBgrWjjfMvaSqijAAtEPcuuun\n\tc1suWDNdUl4AsSJjFdr3nAsP1fbJpXPriMF8hT4s=","Date":"Mon, 28 Jan 2019 02:35:23 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190128003523.GE5154@pendragon.ideasonboard.com>","References":"<20190127002208.18913-1-niklas.soderlund@ragnatech.se>\n\t<20190127002208.18913-7-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190127002208.18913-7-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 6/6] libcamera: camera: extend\n\tcamera object to support configuration of streams","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 28 Jan 2019 00:35:27 -0000"}},{"id":670,"web_url":"https://patchwork.libcamera.org/comment/670/","msgid":"<20190129015552.GF26790@bigcity.dyn.berto.se>","date":"2019-01-29T01:55:53","subject":"Re: [libcamera-devel] [PATCH v3 6/6] libcamera: camera: extend\n\tcamera object to support configuration of streams","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 feedback.\n\nOn 2019-01-28 02:35:23 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Sun, Jan 27, 2019 at 01:22:08AM +0100, Niklas Söderlund wrote:\n> > Extend the camera to support reading and configuring of formats for\n> \n> s/of formats/formats/\n\nThanks.\n\n> \n> > groups of streams. The implementation in the Camera are minimalistic as\n> > the heavy lifting are done by the pipeline handler implementations.\n> > \n> > The most important functionality the camera provides in this context is\n> > validation of data structures passed to it from the application and\n> > access control to the pipeline handler.\n> > \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > include/libcamera/camera.h | 4 +++\n> > src/libcamera/camera.cpp | 68 ++++++++++++++++++++++++++++++++++++++\n> > 2 files changed, 72 insertions(+)\n> > \n> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > index 786d4d7d66bed5b2..798fee2487940208 100644\n> > --- a/include/libcamera/camera.h\n> > +++ b/include/libcamera/camera.h\n> > @@ -7,6 +7,7 @@\n> > #ifndef __LIBCAMERA_CAMERA_H__\n> > #define __LIBCAMERA_CAMERA_H__\n> > \n> > +#include \n> > #include \n> > #include \n> > \n> > @@ -16,6 +17,7 @@ namespace libcamera {\n> > \n> > class PipelineHandler;\n> > class Stream;\n> > +class StreamConfiguration;\n> > \n> > class Camera final\n> > {\n> > @@ -35,6 +37,8 @@ public:\n> > \tvoid release();\n> > \n> > \tstd::vector streams() const;\n> > +\tstd::map streamConfiguration(const std::vector &streams) const;\n> > +\tint configureStreams(std::map &config);\n> > \n> > private:\n> > \tCamera(PipelineHandler *pipe, const std::string &name);\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index 3b2c00d0a4bb45d1..e41143972b0b537a 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -211,4 +211,72 @@ std::vector Camera::streams() const\n> > \treturn streams_;\n> > }\n> > \n> > +/**\n> > + * \\brief Retrieve a group of stream configurations\n> > + * \\param[in] streams A map of stream IDs and configurations to setup\n> > + *\n> > + * Retrieve the camera's configuration for a specified group of streams. The\n> > + * caller can specify which of the camera's streams to retrieve configuration\n> > + * from by populating \\a streams.\n> > + *\n> > + * The easiest way to populate the array of streams to fetch configuration from\n> > + * is to first retrieve the camera's full array of stream by with streams() and\n> > + * then potentially trim it down to only contain the streams the caller\n> > + * are interested in.\n> > + *\n> > + * \\return A map of successfully retrieved stream IDs and configurations or an\n> > + * empty list on error.\n> > + */\n> > +std::map\n> > +Camera::streamConfiguration(const std::vector &streams) const\n> > +{\n> > +\tstd::map config;\n> > +\n> > +\tif (disconnected_ || !streams.size())\n> > +\t\treturn config;\n> > +\n> > +\tfor (const Stream &stream : streams)\n> > +\t\tif (!haveStreamID(stream.id()))\n> > +\t\t\treturn config;\n> > +\n> > +\treturn pipe_->streamConfiguration(this, streams);\n> > +}\n> > +\n> > +/**\n> > + * \\brief Configure the camera's streams prior to capture\n> > + * \\param[in] config A map of stream IDs and configurations to setup\n> > + *\n> > + * Prior to starting capture, the camera must be configured to select a\n> > + * group of streams to be involved in the capture and their configuration.\n> > + * The caller specify which streams are to be involved and their configuration\n> \n> s/specify/specifies/\n> \n> > + * by populating \\a config.\n> > + *\n> > + * The easiest way to populate the array of config is to fetch an initial\n> > + * configuration from the camera with streamConfiguration() and then change the\n> > + * parameters to fit the callers need and once all the streams parameters are\n> \n> s/callers need/caller's needs/\n> \n> > + * configured hand that over to configureStreams() to actually setup the camera.\n> > + *\n> > + * Exclusive access to the camera shall be ensured by a call to acquire() prior\n> > + * to calling this function, otherwise an -EACCES error is be returned.\n> \n> You should document that -ENODEV will be returned in case the camera is\n> disconnected, and -EINVAL if the configuration isn't valid.\n\nGood point.\n\n> \n> > + *\n> > + * \\return 0 on success or a negative error code on error.\n> > + */\n> > +int Camera::configureStreams(std::map &config)\n> > +{\n> > +\tif (disconnected_)\n> > +\t\treturn -ENODEV;\n> > +\n> > +\tif (!acquired_)\n> > +\t\treturn -EACCES;\n> > +\n> > +\tif (!config.size())\n> > +\t\treturn -EINVAL;\n> > +\n> > +\tfor (const auto &pair : config)\n> > +\t\tif (!haveStreamID(pair.first))\n> > +\t\t\treturn -EINVAL;\n> \n> Could we make this error impossible by indexing the map on a Stream\n> pointer ?\n\nI had a try at this for v4, it looks much better. Thanks for the \nsuggestion.\n\n> \n> > +\treturn pipe_->configureStreams(this, config);\n> > +}\n> > +\n> > } /* namespace libcamera */\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lf1-x142.google.com (mail-lf1-x142.google.com\n\t[IPv6:2a00:1450:4864:20::142])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 57FD060C78\n\tfor ;\n\tTue, 29 Jan 2019 02:55:55 +0100 (CET)","by mail-lf1-x142.google.com with SMTP id a8so13407568lfk.5\n\tfor ;\n\tMon, 28 Jan 2019 17:55:55 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\th203sm3375107lfe.44.2019.01.28.17.55.53\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tMon, 28 Jan 2019 17:55:53 -0800 (PST)"],"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\t:user-agent; bh=/Dm8cgHeS+dJ6gz8Up4wXARSKyJBX2gRhdYdtxBQmDA=;\n\tb=It5TC/VACVfs/oVyUIZVXTQYKALeOqZ1kotPPB5J0GZ9R3DFDpp+/1mvbPsQIxYEXq\n\tLNqi1VtzoHVMbNX0SDm9hHL0lnDb8z9GL1b9GGZnsZO+MQ5WHUwsPXHsJJMcKS0uBOA3\n\tNDYadlV3t+UTdCuTQu3gpsxbO8cQ6xNBEOPf+F0nNhSM5hYrtNQj/vJxJZ0iN8ePAZGK\n\tWuKGILjcQbgpxw6T3bQvcGBov5pCr1TUfGsik1BEz6So5hmhinKWZcQ50QAVaFNSRPv8\n\t0XMqXfLXyfUJ8lDbwaQB8w1EmhRaZyQ4935k1a5u6PVUo0tWzIYSfH/WC2R3tTSRdga9\n\t8z8Q==","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:user-agent;\n\tbh=/Dm8cgHeS+dJ6gz8Up4wXARSKyJBX2gRhdYdtxBQmDA=;\n\tb=X6mJoz+fW0xK3ZE3ulESumy9cxNcy+4GPQbUsqKk3cwaqhaIDOr/dAA9D7ZznD8VL/\n\tUYmLPsmIZPyWIXoUFmYmG7XCd7pPEOLdZ0MxZknurTlDFBjgR+Bbl51pEDkwGxKEPO81\n\tnR4Du9OPbjis+5ugMqHiGY/5kw5xe34tlwzcLa6pXwCDAzZX7fY6r0FkIAVO7pnhhK7H\n\t9E6eQTfKVDZpyZSKvUphulNYRyVugQ4Ss7oK6YKvUAxJErEphla+7IqTylUjQDXcTHLO\n\tGZ2WRmqAQdbHl3pWRguPxJxJsLLL+eSFzsLDgZNSglELGo9tp4JGixINdaI85Pvshpsm\n\taLmw==","X-Gm-Message-State":"AJcUukdkzH7bmyjanKTcGOHIyAW2omheRmllmY+yy9F5gU3A1+xikUo8\n\tJprIgZ383UgZUu/3whA/sOYEYA==","X-Google-Smtp-Source":"ALg8bN5pkJspc+MnWqaMzEoixlk3AmfvgE/k0ri+n5sOjBaDm7TwY/atxt6l170qvJj6iKR/R3uaUA==","X-Received":"by 2002:a19:7919:: with SMTP id\n\tu25mr19051004lfc.18.1548726954522; \n\tMon, 28 Jan 2019 17:55:54 -0800 (PST)","Date":"Tue, 29 Jan 2019 02:55:53 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190129015552.GF26790@bigcity.dyn.berto.se>","References":"<20190127002208.18913-1-niklas.soderlund@ragnatech.se>\n\t<20190127002208.18913-7-niklas.soderlund@ragnatech.se>\n\t<20190128003523.GE5154@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190128003523.GE5154@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 6/6] libcamera: camera: extend\n\tcamera object to support configuration of streams","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 29 Jan 2019 01:55:55 -0000"}}]