[{"id":647,"web_url":"https://patchwork.libcamera.org/comment/647/","msgid":"<20190127231744.GC5154@pendragon.ideasonboard.com>","date":"2019-01-27T23:17:44","subject":"Re: [libcamera-devel] [PATCH v3 4/6] libcamera: camera: extend\n\tcamera object to support 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:06AM +0100, Niklas Söderlund wrote:\n> A camera consist of one or more video streams origination from the same\n\ns/consist/consists/\n\n> video source. The different streams could for example have access to\n> different hardware blocks in the video pipeline and therefor be able to\n> process the video source in different ways.\n> \n> All static information describing each streams needs to be recorded at\n\nInformation is plural, so s/needs/need/\n\n> camera creation. After a camera is created an application can retrieve\n> the static information about its stream at any time.\n> \n> Update all pipeline handlers to register one stream per camera, this\n> might be extended in the future for some of the pipelines.\n\nMaybe s/might/will/ :-)\n\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> include/libcamera/camera.h | 10 ++++-\n> src/libcamera/camera.cpp | 55 ++++++++++++++++++++++++++--\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +-\n> src/libcamera/pipeline/uvcvideo.cpp | 4 +-\n> src/libcamera/pipeline/vimc.cpp | 4 +-\n> 5 files changed, 69 insertions(+), 8 deletions(-)\n> \n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 7e358f8c0aa093cf..786d4d7d66bed5b2 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -15,12 +15,14 @@\n> namespace libcamera {\n> \n> class PipelineHandler;\n> +class Stream;\n> \n> class Camera final\n> {\n> public:\n> \tstatic std::shared_ptr create(PipelineHandler *pipe,\n> -\t\t\t\t\t const std::string &name);\n> +\t\t\t\t\t const std::string &name,\n> +\t\t\t\t\t const std::vector &streams);\n> \n> \tCamera(const Camera &) = delete;\n> \tvoid operator=(const Camera &) = delete;\n> @@ -32,6 +34,8 @@ public:\n> \tint acquire();\n> \tvoid release();\n> \n> +\tstd::vector streams() const;\n\nDo we need to return a copy, or could we return a const\nstd::vector & ?\n\n> +\n> private:\n> \tCamera(PipelineHandler *pipe, const std::string &name);\n> \t~Camera();\n> @@ -39,10 +43,14 @@ private:\n> \tfriend class PipelineHandler;\n> \tvoid disconnect();\n> \n> +\tbool haveStreamID(unsigned int id) const;\n\nI'd name this hasStreamID, or possibly even hasStreamId.\n\n> +\n> \tstd::shared_ptr pipe_;\n> \tstd::string name_;\n> +\tstd::vector streams_;\n> \n> \tbool acquired_;\n> +\tbool disconnected_;\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 500976b237bcbd2d..3b2c00d0a4bb45d1 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -6,6 +6,7 @@\n> */\n> \n> #include \n> +#include \n> \n> #include \"log.h\"\n> #include \"pipeline_handler.h\"\n> @@ -56,13 +57,15 @@ LOG_DECLARE_CATEGORY(Camera)\n> * \\brief Create a camera instance\n> * \\param[in] name The name of the camera device\n> * \\param[in] pipe The pipeline handler responsible for the camera device\n> + * \\param[in] streams Array of streams the camera shall provide\n\ns/shall provide/provides/ ?\n\n> *\n> * The caller is responsible for guaranteeing unicity of the camera name.\n> *\n> * \\return A shared pointer to the newly created camera object\n> */\n> std::shared_ptr Camera::create(PipelineHandler *pipe,\n> -\t\t\t\t const std::string &name)\n> +\t\t\t\t const std::string &name,\n> +\t\t\t\t const std::vector &streams)\n> {\n> \tstruct Allocator : std::allocator {\n> \t\tvoid construct(void *p, PipelineHandler *pipe,\n> @@ -76,7 +79,19 @@ std::shared_ptr Camera::create(PipelineHandler *pipe,\n> \t\t}\n> \t};\n> \n> -\treturn std::allocate_shared(Allocator(), pipe, name);\n> +\tstd::shared_ptr camera =\n> +\t\tstd::allocate_shared(Allocator(), pipe, name);\n> +\n> +\tfor (const Stream &stream : streams) {\n> +\t\tif (camera->haveStreamID(stream.id())) {\n> +\t\t\tLOG(Camera, Error) << \"Duplication of stream ID\";\n> +\t\t\tcamera.reset();\n> +\t\t\tbreak;\n> +\t\t}\n\nI think we can do better. Instead of returning a null pointer (which by\nthe way you don't check for in the pipeline handlers, so I expect\nthird-party pipeline handlers not to handle that error correctly\neither), we should make an API that can't be used incorrectly. For\ninstance you could pass a vector of streams without any ID assign, and\nassign IDs incrementally starting from 0 in this function. The\nhaveStreamID() method won't be needed anymore.\n\n> +\t\tcamera->streams_.push_back(stream);\n> +\t}\n> +\n> +\treturn camera;\n> }\n> \n> /**\n> @@ -102,7 +117,8 @@ const std::string &Camera::name() const\n> */\n> \n> Camera::Camera(PipelineHandler *pipe, const std::string &name)\n> -\t: pipe_(pipe->shared_from_this()), name_(name), acquired_(false)\n> +\t: pipe_(pipe->shared_from_this()), name_(name), acquired_(false),\n> +\t disconnected_(false)\n> {\n> }\n> \n> @@ -125,10 +141,24 @@ void Camera::disconnect()\n> {\n> \tLOG(Camera, Debug) << \"Disconnecting camera \" << name_;\n> \n> -\t/** \\todo Block API calls when they will be implemented. */\n> +\tdisconnected_ = true;\n> \tdisconnected.emit(this);\n> }\n> \n> +/**\n> + * \\brief Check if camera have a stream ID\n> + * \\param[in] id Stream ID to check for\n> + * \\return ture if stream ID exists, else false\n> + */\n> +bool Camera::haveStreamID(unsigned int id) const\n> +{\n> +\tfor (const Stream &stream : streams_)\n> +\t\tif (stream.id() == id)\n> +\t\t\treturn true;\n> +\n> +\treturn false;\n> +}\n> +\n> /**\n> * \\brief Acquire the camera device for exclusive access\n> *\n> @@ -164,4 +194,21 @@ void Camera::release()\n> \tacquired_ = false;\n> }\n> \n> +/**\n> + * \\brief Retrieve all the camera's stream information\n> + *\n> + * Retrieve all of the camera's static stream information. The static\n> + * information describes among other things how many streams the camera support\n\nLet's wrap documentation at 80 columns by default if not very\nimpractical.\n\n> + * and each streams capabilities.\n> + *\n> + * \\return An array of all the camera's streams or an empty list on error.\n> + */\n> +std::vector Camera::streams() const\n> +{\n> +\tif (disconnected_)\n> +\t\treturn std::vector{};\n\nAs this is static information I'm not sure we should fail the call.\nCould there be cases where the application might want to get information\nfrom the streams to clean up when the camera gets disconnected ?\n\n> +\n> +\treturn streams_;\n> +}\n> +\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index d74655d037728feb..dbb2a89163c36cbc 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -9,6 +9,7 @@\n> #include \n> \n> #include \n> +#include \n> \n> #include \"device_enumerator.h\"\n> #include \"log.h\"\n> @@ -197,7 +198,8 @@ void PipelineHandlerIPU3::registerCameras()\n> \t\t\tcontinue;\n> \n> \t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> -\t\tstd::shared_ptr camera = Camera::create(this, cameraName);\n> +\t\tstd::vector streams{ Stream(0) };\n> +\t\tstd::shared_ptr camera = Camera::create(this, cameraName, streams);\n\nI think this will become a bit cumbersome to use when the Stream class\nwill gain more fields, but we can improve the API then.\n\n> \n> \t\t/*\n> \t\t * If V4L2 device creation fails, the Camera instance won't be\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index c51e8bc1f2c2bf25..bc4a8d7236be589d 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -6,6 +6,7 @@\n> */\n> \n> #include \n> +#include \n> \n> #include \"device_enumerator.h\"\n> #include \"media_device.h\"\n> @@ -64,7 +65,8 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> \t\treturn false;\n> \t}\n> \n> -\tstd::shared_ptr camera = Camera::create(this, media_->model());\n> +\tstd::vector streams{ Stream(0) };\n> +\tstd::shared_ptr camera = Camera::create(this, media_->model(), streams);\n> \tregisterCamera(std::move(camera));\n> \thotplugMediaDevice(media_.get());\n> \n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index f58a97d51619515d..c426a953aea1b3dd 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -6,6 +6,7 @@\n> */\n> \n> #include \n> +#include \n> \n> #include \"device_enumerator.h\"\n> #include \"media_device.h\"\n> @@ -56,7 +57,8 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)\n> \n> \tmedia_->acquire();\n> \n> -\tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\");\n> +\tstd::vector streams{ Stream(0) };\n> +\tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\", streams);\n> \tregisterCamera(std::move(camera));\n> \n> \treturn true;","headers":{"Return-Path":"","Received":["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 D405F60C78\n\tfor ;\n\tMon, 28 Jan 2019 00:17:51 +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 57ACC85;\n\tMon, 28 Jan 2019 00:17:49 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548631071;\n\tbh=DP6lrZTABiXLpEKPhOdtZvjgQXu8IXe3ZLnFsDdXaOY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=EDQ4SVroOCMuTpcJtwIeVjdTA09H/cc3u8AZmbTbC+zRz/3Im950t19EEj9XI1knp\n\tzzPxRYKrKuejQFTeWAbmkJRj0XFchEcdr8cmbPBtp4Ub5iXSn9X99fumehK2TyRI2U\n\tOf0nF4DWIfEsJaVNEKPqEVnSN4Y0N3XmQMYwpnuc=","Date":"Mon, 28 Jan 2019 01:17:44 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190127231744.GC5154@pendragon.ideasonboard.com>","References":"<20190127002208.18913-1-niklas.soderlund@ragnatech.se>\n\t<20190127002208.18913-5-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-5-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 4/6] libcamera: camera: extend\n\tcamera object to support 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":"Sun, 27 Jan 2019 23:17:52 -0000"}},{"id":654,"web_url":"https://patchwork.libcamera.org/comment/654/","msgid":"","date":"2019-01-28T11:34:23","subject":"Re: [libcamera-devel] [PATCH v3 4/6] libcamera: camera: extend\n\tcamera object to support streams","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Niklas,\n\nOn 27/01/2019 00:22, Niklas Söderlund wrote:\n> A camera consist of one or more video streams origination from the same\n\nA camera *consists* of one or more video streams *originating* from ...\n\n> video source. The different streams could for example have access to\n> different hardware blocks in the video pipeline and therefor be able to\n\ntherefore\n\n> process the video source in different ways.\n> \n> All static information describing each streams needs to be recorded at\n\n\ndescribing each stream\n\n\n> camera creation. After a camera is created an application can retrieve\n> the static information about its stream at any time.\n> \n> Update all pipeline handlers to register one stream per camera, this\n> might be extended in the future for some of the pipelines.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> include/libcamera/camera.h | 10 ++++-\n> src/libcamera/camera.cpp | 55 ++++++++++++++++++++++++++--\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +-\n> src/libcamera/pipeline/uvcvideo.cpp | 4 +-\n> src/libcamera/pipeline/vimc.cpp | 4 +-\n> 5 files changed, 69 insertions(+), 8 deletions(-)\n> \n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 7e358f8c0aa093cf..786d4d7d66bed5b2 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -15,12 +15,14 @@\n> namespace libcamera {\n> \n> class PipelineHandler;\n> +class Stream;\n> \n> class Camera final\n> {\n> public:\n> \tstatic std::shared_ptr create(PipelineHandler *pipe,\n> -\t\t\t\t\t const std::string &name);\n> +\t\t\t\t\t const std::string &name,\n> +\t\t\t\t\t const std::vector &streams);\n> \n> \tCamera(const Camera &) = delete;\n> \tvoid operator=(const Camera &) = delete;\n> @@ -32,6 +34,8 @@ public:\n> \tint acquire();\n> \tvoid release();\n> \n> +\tstd::vector streams() const;\n> +\n> private:\n> \tCamera(PipelineHandler *pipe, const std::string &name);\n> \t~Camera();\n> @@ -39,10 +43,14 @@ private:\n> \tfriend class PipelineHandler;\n> \tvoid disconnect();\n> \n> +\tbool haveStreamID(unsigned int id) const;\n\nhave? should this be 'hasStreamID()' ?\n\nLooking at the implementation, Yes - I think so.\n\n> +\n> \tstd::shared_ptr pipe_;\n> \tstd::string name_;\n> +\tstd::vector streams_;\n> \n> \tbool acquired_;\n> +\tbool disconnected_;\n\nIs this related to streams?\n\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 500976b237bcbd2d..3b2c00d0a4bb45d1 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -6,6 +6,7 @@\n> */\n> \n> #include \n> +#include \n> \n> #include \"log.h\"\n> #include \"pipeline_handler.h\"\n> @@ -56,13 +57,15 @@ LOG_DECLARE_CATEGORY(Camera)\n> * \\brief Create a camera instance\n> * \\param[in] name The name of the camera device\n> * \\param[in] pipe The pipeline handler responsible for the camera device\n> + * \\param[in] streams Array of streams the camera shall provide\n> *\n> * The caller is responsible for guaranteeing unicity of the camera name.\n> *\n> * \\return A shared pointer to the newly created camera object\n> */\n> std::shared_ptr Camera::create(PipelineHandler *pipe,\n> -\t\t\t\t const std::string &name)\n> +\t\t\t\t const std::string &name,\n> +\t\t\t\t const std::vector &streams)\n> {\n> \tstruct Allocator : std::allocator {\n> \t\tvoid construct(void *p, PipelineHandler *pipe,\n> @@ -76,7 +79,19 @@ std::shared_ptr Camera::create(PipelineHandler *pipe,\n> \t\t}\n> \t};\n> \n> -\treturn std::allocate_shared(Allocator(), pipe, name);\n> +\tstd::shared_ptr camera =\n> +\t\tstd::allocate_shared(Allocator(), pipe, name);\n> +\n> +\tfor (const Stream &stream : streams) {\n> +\t\tif (camera->haveStreamID(stream.id())) {\n> +\t\t\tLOG(Camera, Error) << \"Duplication of stream ID\";\n\nWhat about the pipeline adding a stream to a camera instead, and the ID\nbeing allocated by the camera device?\n\nOr does this make things the wrong way around?\n\nI guess it would also have to be a function which only the Pipeline was\nable to call somehow...\n\n(we wouldn't want a public Camera API that could arbitrarily add streams!)..\n\n\n> +\t\t\tcamera.reset();\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcamera->streams_.push_back(stream);\n> +\t}\n> +\n> +\treturn camera;\n> }\n> \n> /**\n> @@ -102,7 +117,8 @@ const std::string &Camera::name() const\n> */\n> \n> Camera::Camera(PipelineHandler *pipe, const std::string &name)\n> -\t: pipe_(pipe->shared_from_this()), name_(name), acquired_(false)\n> +\t: pipe_(pipe->shared_from_this()), name_(name), acquired_(false),\n> +\t disconnected_(false)\n> {\n> }\n> \n> @@ -125,10 +141,24 @@ void Camera::disconnect()\n> {\n> \tLOG(Camera, Debug) << \"Disconnecting camera \" << name_;\n> \n> -\t/** \\todo Block API calls when they will be implemented. */\n> +\tdisconnected_ = true;\n> \tdisconnected.emit(this);\n> }\n> \n> +/**\n> + * \\brief Check if camera have a stream ID\n\nCheck if the camera has a particular stream ID\n\n\n> + * \\param[in] id Stream ID to check for\n> + * \\return ture if stream ID exists, else false\n\ns/ture/True/\n\n\n> + */\n> +bool Camera::haveStreamID(unsigned int id) const\n> +{\n> +\tfor (const Stream &stream : streams_)\n> +\t\tif (stream.id() == id)\n> +\t\t\treturn true;\n> +\n> +\treturn false;\n> +}\n> +\n> /**\n> * \\brief Acquire the camera device for exclusive access\n> *\n> @@ -164,4 +194,21 @@ void Camera::release()\n> \tacquired_ = false;\n> }\n> \n> +/**\n> + * \\brief Retrieve all the camera's stream information\n> + *\n> + * Retrieve all of the camera's static stream information. The static\n> + * information describes among other things how many streams the camera support\n> + * and each streams capabilities.\n> + *\n> + * \\return An array of all the camera's streams or an empty list on error.\n\nIs if(disconnected) an error?\n\nI'd say if a camera is disconnected it has no streams ... I don't think\nthat's an error?\n\nPerhaps:\n\n\\return An array of all the camera's streams valid streams.\n\n\n> + */\n> +std::vector Camera::streams() const\n> +{\n> +\tif (disconnected_)\n> +\t\treturn std::vector{};\n> +\n> +\treturn streams_;\n> +}\n> +\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index d74655d037728feb..dbb2a89163c36cbc 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -9,6 +9,7 @@\n> #include \n> \n> #include \n> +#include \n> \n> #include \"device_enumerator.h\"\n> #include \"log.h\"\n> @@ -197,7 +198,8 @@ void PipelineHandlerIPU3::registerCameras()\n> \t\t\tcontinue;\n> \n> \t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> -\t\tstd::shared_ptr camera = Camera::create(this, cameraName);\n> +\t\tstd::vector streams{ Stream(0) };\n> +\t\tstd::shared_ptr camera = Camera::create(this, cameraName, streams);\n> \n> \t\t/*\n> \t\t * If V4L2 device creation fails, the Camera instance won't be\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index c51e8bc1f2c2bf25..bc4a8d7236be589d 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -6,6 +6,7 @@\n> */\n> \n> #include \n> +#include \n> \n> #include \"device_enumerator.h\"\n> #include \"media_device.h\"\n> @@ -64,7 +65,8 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> \t\treturn false;\n> \t}\n> \n> -\tstd::shared_ptr camera = Camera::create(this, media_->model());\n> +\tstd::vector streams{ Stream(0) };\n> +\tstd::shared_ptr camera = Camera::create(this, media_->model(), streams);\n> \tregisterCamera(std::move(camera));\n> \thotplugMediaDevice(media_.get());\n> \n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index f58a97d51619515d..c426a953aea1b3dd 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -6,6 +6,7 @@\n> */\n> \n> #include \n> +#include \n> \n> #include \"device_enumerator.h\"\n> #include \"media_device.h\"\n> @@ -56,7 +57,8 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)\n> \n> \tmedia_->acquire();\n> \n> -\tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\");\n> +\tstd::vector streams{ Stream(0) };\n> +\tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\", streams);\n\nOut of interest, UVC exposes two video device nodes. One for the video\ndata, and one for 'meta' data.\n\nWould we consider this meta data to be a second stream?\n\n\n> \tregisterCamera(std::move(camera));\n> \n> \treturn true;\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 4002F60C6A\n\tfor ;\n\tMon, 28 Jan 2019 12:34:26 +0100 (CET)","from [192.168.0.21]\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 93AD385;\n\tMon, 28 Jan 2019 12:34:25 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548675265;\n\tbh=MCso33Ii1I2hHFqbq1INro3e+zzc/AHYHalrFT6aoUU=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=hr6IwHAio1xQm2hEJyE0nT9vsPvKd68TTgTLgw+7L2YaNDanNhZ2zFzh7OTaJbPLn\n\taG0cieQlte0E8mNwLV8o5FNIo4IA/oFB5n+qF+qBNghJ6fJr8mhj9UFSv4RrDbF2Ut\n\tGRdBZSFi0VvaZWSUMyXPF1rg+U+/wORRBdJEF+XQ=","Reply-To":"kieran.bingham@ideasonboard.com","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20190127002208.18913-1-niklas.soderlund@ragnatech.se>\n\t<20190127002208.18913-5-niklas.soderlund@ragnatech.se>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","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\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"","Date":"Mon, 28 Jan 2019 11:34:23 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.2.1","MIME-Version":"1.0","In-Reply-To":"<20190127002208.18913-5-niklas.soderlund@ragnatech.se>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v3 4/6] libcamera: camera: extend\n\tcamera object to support 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 11:34:26 -0000"}},{"id":666,"web_url":"https://patchwork.libcamera.org/comment/666/","msgid":"<20190128230015.GB26790@bigcity.dyn.berto.se>","date":"2019-01-28T23:00:15","subject":"Re: [libcamera-devel] [PATCH v3 4/6] libcamera: camera: extend\n\tcamera object to support streams","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 feedback.\n\nOn 2019-01-28 11:34:23 +0000, Kieran Bingham wrote:\n> Hi Niklas,\n> \n> On 27/01/2019 00:22, Niklas Söderlund wrote:\n> > A camera consist of one or more video streams origination from the same\n> \n> A camera *consists* of one or more video streams *originating* from ...\n> \n> > video source. The different streams could for example have access to\n> > different hardware blocks in the video pipeline and therefor be able to\n> \n> therefore\n> \n> > process the video source in different ways.\n> > \n> > All static information describing each streams needs to be recorded at\n> \n> \n> describing each stream\n> \n\nThanks!\n\n> \n> > camera creation. After a camera is created an application can retrieve\n> > the static information about its stream at any time.\n> > \n> > Update all pipeline handlers to register one stream per camera, this\n> > might be extended in the future for some of the pipelines.\n> > \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > include/libcamera/camera.h | 10 ++++-\n> > src/libcamera/camera.cpp | 55 ++++++++++++++++++++++++++--\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +-\n> > src/libcamera/pipeline/uvcvideo.cpp | 4 +-\n> > src/libcamera/pipeline/vimc.cpp | 4 +-\n> > 5 files changed, 69 insertions(+), 8 deletions(-)\n> > \n> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > index 7e358f8c0aa093cf..786d4d7d66bed5b2 100644\n> > --- a/include/libcamera/camera.h\n> > +++ b/include/libcamera/camera.h\n> > @@ -15,12 +15,14 @@\n> > namespace libcamera {\n> > \n> > class PipelineHandler;\n> > +class Stream;\n> > \n> > class Camera final\n> > {\n> > public:\n> > \tstatic std::shared_ptr create(PipelineHandler *pipe,\n> > -\t\t\t\t\t const std::string &name);\n> > +\t\t\t\t\t const std::string &name,\n> > +\t\t\t\t\t const std::vector &streams);\n> > \n> > \tCamera(const Camera &) = delete;\n> > \tvoid operator=(const Camera &) = delete;\n> > @@ -32,6 +34,8 @@ public:\n> > \tint acquire();\n> > \tvoid release();\n> > \n> > +\tstd::vector streams() const;\n> > +\n> > private:\n> > \tCamera(PipelineHandler *pipe, const std::string &name);\n> > \t~Camera();\n> > @@ -39,10 +43,14 @@ private:\n> > \tfriend class PipelineHandler;\n> > \tvoid disconnect();\n> > \n> > +\tbool haveStreamID(unsigned int id) const;\n> \n> have? should this be 'hasStreamID()' ?\n> \n> Looking at the implementation, Yes - I think so.\n\nI have renamed this hasStreamId merging your and Laurents comments on \nthis.\n\n> \n> > +\n> > \tstd::shared_ptr pipe_;\n> > \tstd::string name_;\n> > +\tstd::vector streams_;\n> > \n> > \tbool acquired_;\n> > +\tbool disconnected_;\n> \n> Is this related to streams?\n\nYes, if a camera is disconnected operations should not be forwarded to \nthe pipeline handler. Laurent thinks streams information should still be \navailable after a disconnect so I have moved this to the first patch who \nadds operations which needs a connected camera.\n\n> \n> > };\n> > \n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index 500976b237bcbd2d..3b2c00d0a4bb45d1 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -6,6 +6,7 @@\n> > */\n> > \n> > #include \n> > +#include \n> > \n> > #include \"log.h\"\n> > #include \"pipeline_handler.h\"\n> > @@ -56,13 +57,15 @@ LOG_DECLARE_CATEGORY(Camera)\n> > * \\brief Create a camera instance\n> > * \\param[in] name The name of the camera device\n> > * \\param[in] pipe The pipeline handler responsible for the camera device\n> > + * \\param[in] streams Array of streams the camera shall provide\n> > *\n> > * The caller is responsible for guaranteeing unicity of the camera name.\n> > *\n> > * \\return A shared pointer to the newly created camera object\n> > */\n> > std::shared_ptr Camera::create(PipelineHandler *pipe,\n> > -\t\t\t\t const std::string &name)\n> > +\t\t\t\t const std::string &name,\n> > +\t\t\t\t const std::vector &streams)\n> > {\n> > \tstruct Allocator : std::allocator {\n> > \t\tvoid construct(void *p, PipelineHandler *pipe,\n> > @@ -76,7 +79,19 @@ std::shared_ptr Camera::create(PipelineHandler *pipe,\n> > \t\t}\n> > \t};\n> > \n> > -\treturn std::allocate_shared(Allocator(), pipe, name);\n> > +\tstd::shared_ptr camera =\n> > +\t\tstd::allocate_shared(Allocator(), pipe, name);\n> > +\n> > +\tfor (const Stream &stream : streams) {\n> > +\t\tif (camera->haveStreamID(stream.id())) {\n> > +\t\t\tLOG(Camera, Error) << \"Duplication of stream ID\";\n> \n> What about the pipeline adding a stream to a camera instead, and the ID\n> being allocated by the camera device?\n\nFor the ID to have any real meaning the pipeline needs to assign the id \nas it is the real user of it. When configuring a camera the pipeline \nhandler needs to know which streams are to be used and it currently do \nthis by the use of the id. This might change in the future as the API \nevolve.\n\n> \n> Or does this make things the wrong way around?\n> \n> I guess it would also have to be a function which only the Pipeline was\n> able to call somehow...\n> \n> (we wouldn't want a public Camera API that could arbitrarily add streams!)..\n\n:-)\n\n> \n> \n> > +\t\t\tcamera.reset();\n> > +\t\t\tbreak;\n> > +\t\t}\n> > +\t\tcamera->streams_.push_back(stream);\n> > +\t}\n> > +\n> > +\treturn camera;\n> > }\n> > \n> > /**\n> > @@ -102,7 +117,8 @@ const std::string &Camera::name() const\n> > */\n> > \n> > Camera::Camera(PipelineHandler *pipe, const std::string &name)\n> > -\t: pipe_(pipe->shared_from_this()), name_(name), acquired_(false)\n> > +\t: pipe_(pipe->shared_from_this()), name_(name), acquired_(false),\n> > +\t disconnected_(false)\n> > {\n> > }\n> > \n> > @@ -125,10 +141,24 @@ void Camera::disconnect()\n> > {\n> > \tLOG(Camera, Debug) << \"Disconnecting camera \" << name_;\n> > \n> > -\t/** \\todo Block API calls when they will be implemented. */\n> > +\tdisconnected_ = true;\n> > \tdisconnected.emit(this);\n> > }\n> > \n> > +/**\n> > + * \\brief Check if camera have a stream ID\n> \n> Check if the camera has a particular stream ID\n> \n> \n> > + * \\param[in] id Stream ID to check for\n> > + * \\return ture if stream ID exists, else false\n> \n> s/ture/True/\n\nThanks.\n\n> \n> \n> > + */\n> > +bool Camera::haveStreamID(unsigned int id) const\n> > +{\n> > +\tfor (const Stream &stream : streams_)\n> > +\t\tif (stream.id() == id)\n> > +\t\t\treturn true;\n> > +\n> > +\treturn false;\n> > +}\n> > +\n> > /**\n> > * \\brief Acquire the camera device for exclusive access\n> > *\n> > @@ -164,4 +194,21 @@ void Camera::release()\n> > \tacquired_ = false;\n> > }\n> > \n> > +/**\n> > + * \\brief Retrieve all the camera's stream information\n> > + *\n> > + * Retrieve all of the camera's static stream information. The static\n> > + * information describes among other things how many streams the camera support\n> > + * and each streams capabilities.\n> > + *\n> > + * \\return An array of all the camera's streams or an empty list on error.\n> \n> Is if(disconnected) an error?\n> \n> I'd say if a camera is disconnected it has no streams ... I don't think\n> that's an error?\n> \n> Perhaps:\n> \n> \\return An array of all the camera's streams valid streams.\n\nMerging your and Laurents comment a disconnected camera will keep \nexposing the static stream information so I have update this to:\n\n\\return An array of all the camera's streams.\n\n> \n> \n> > + */\n> > +std::vector Camera::streams() const\n> > +{\n> > +\tif (disconnected_)\n> > +\t\treturn std::vector{};\n> > +\n> > +\treturn streams_;\n> > +}\n> > +\n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index d74655d037728feb..dbb2a89163c36cbc 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -9,6 +9,7 @@\n> > #include \n> > \n> > #include \n> > +#include \n> > \n> > #include \"device_enumerator.h\"\n> > #include \"log.h\"\n> > @@ -197,7 +198,8 @@ void PipelineHandlerIPU3::registerCameras()\n> > \t\t\tcontinue;\n> > \n> > \t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> > -\t\tstd::shared_ptr camera = Camera::create(this, cameraName);\n> > +\t\tstd::vector streams{ Stream(0) };\n> > +\t\tstd::shared_ptr camera = Camera::create(this, cameraName, streams);\n> > \n> > \t\t/*\n> > \t\t * If V4L2 device creation fails, the Camera instance won't be\n> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> > index c51e8bc1f2c2bf25..bc4a8d7236be589d 100644\n> > --- a/src/libcamera/pipeline/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> > @@ -6,6 +6,7 @@\n> > */\n> > \n> > #include \n> > +#include \n> > \n> > #include \"device_enumerator.h\"\n> > #include \"media_device.h\"\n> > @@ -64,7 +65,8 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> > \t\treturn false;\n> > \t}\n> > \n> > -\tstd::shared_ptr camera = Camera::create(this, media_->model());\n> > +\tstd::vector streams{ Stream(0) };\n> > +\tstd::shared_ptr camera = Camera::create(this, media_->model(), streams);\n> > \tregisterCamera(std::move(camera));\n> > \thotplugMediaDevice(media_.get());\n> > \n> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> > index f58a97d51619515d..c426a953aea1b3dd 100644\n> > --- a/src/libcamera/pipeline/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc.cpp\n> > @@ -6,6 +6,7 @@\n> > */\n> > \n> > #include \n> > +#include \n> > \n> > #include \"device_enumerator.h\"\n> > #include \"media_device.h\"\n> > @@ -56,7 +57,8 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)\n> > \n> > \tmedia_->acquire();\n> > \n> > -\tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\");\n> > +\tstd::vector streams{ Stream(0) };\n> > +\tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\", streams);\n> \n> Out of interest, UVC exposes two video device nodes. One for the video\n> data, and one for 'meta' data.\n> \n> Would we consider this meta data to be a second stream?\n\nPossibly, to be honest I don't know what the second video node is used \nfor. If it carries meta data maybe it should be used by a 3A sometime in \nthe future :-)\n\n> \n> \n> > \tregisterCamera(std::move(camera));\n> > \n> > \treturn true;\n> > \n> \n> -- \n> Regards\n> --\n> Kieran","headers":{"Return-Path":"","Received":["from mail-lf1-x144.google.com (mail-lf1-x144.google.com\n\t[IPv6:2a00:1450:4864:20::144])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BD52260B1B\n\tfor ;\n\tTue, 29 Jan 2019 00:00:17 +0100 (CET)","by mail-lf1-x144.google.com with SMTP id f5so13136501lfc.13\n\tfor ;\n\tMon, 28 Jan 2019 15:00:17 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tr26-v6sm3225901lji.25.2019.01.28.15.00.16\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tMon, 28 Jan 2019 15:00:16 -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=T32wuTt46ZkQR/lu3ixyaisA9oj2csBmJ+AR+S+xS24=;\n\tb=kPV8j45j5yr+Htb6nFXEHBnf4JLSsuDnQ+6hFxSGgxUh4ReWv1UQq3Sa2qor1stKVQ\n\tGLvpxBwMAmgRcYjUvquMDTh0IGLaBnr4v06jOvuLU4gR0j2Ob85/ZGpUh2dSAOUhRQ/Z\n\twl4u+2E+eHX64EN/5joxETm4KxODXBRItqC9d7oNLJXybN/e8y3kB0gUp1nVMY7nJ4zR\n\tdX4/y6mlnrfv8E78PrrXR4VnMv8H4IdR132f9CChaCY1nb5c8ph8u8aeVmNt7W9Yxfq1\n\tOJ7kBhKNTfg8b6dNKJFA0y3noMP5kzSVTBwI0/yi0Re6fHVlvIRlosaEYAuBhY0wUqWR\n\tX3mg==","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=T32wuTt46ZkQR/lu3ixyaisA9oj2csBmJ+AR+S+xS24=;\n\tb=nI534PqwaGa7Dgouwj/Aez+FDv2h1oJaVmIwx+vtjtar+PEnm1L8odSKG+K7zqanJj\n\tcbUt5A/X7RqS6ZWRugXsx22R/qf+LDrWbOTPxq9Q72QlsWa5UKw9HvHnO7alrrbQiS2W\n\tvqG4NK12/3FvetEvBmbG5Zygp9VoHDrkZI9MwdMpIy+ZBcDlfVkC7Eg8LdNVYqw9kKzA\n\tVTZeVHXuFDlwhQkd39bpirXhIA8auoFGx5NXzvRqmR8du/lkPlihqtlAkHvCIT6NVJSW\n\tJvG4fgkgHLkrqaRLKhROC63YPuSZ6WCkBFh+9/UKwFbKdqR1axyk2xBgvyCm5SNRh6fu\n\tT+Lw==","X-Gm-Message-State":"AJcUukfz7XVODmktqpPE01AJuSeiKhWxeet7XoOKBzrxfC69Tko6R74E\n\tVCnrY+VIfjQpHE0Bou5TONWNT1NOHyE=","X-Google-Smtp-Source":"ALg8bN7wB/2SyZ7Oh4Y6ZGCSik3PGXbWwjwzgnkpm6JKd6KoNQZKBYBAl7/BnihhF9+mbhC/g93Zhw==","X-Received":"by 2002:a19:1019:: with SMTP id\n\tf25mr19083653lfi.54.1548716416936; \n\tMon, 28 Jan 2019 15:00:16 -0800 (PST)","Date":"Tue, 29 Jan 2019 00:00:15 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190128230015.GB26790@bigcity.dyn.berto.se>","References":"<20190127002208.18913-1-niklas.soderlund@ragnatech.se>\n\t<20190127002208.18913-5-niklas.soderlund@ragnatech.se>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 4/6] libcamera: camera: extend\n\tcamera object to support 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 23:00:18 -0000"}},{"id":667,"web_url":"https://patchwork.libcamera.org/comment/667/","msgid":"<20190129012441.GC26790@bigcity.dyn.berto.se>","date":"2019-01-29T01:24:41","subject":"Re: [libcamera-devel] [PATCH v3 4/6] libcamera: camera: extend\n\tcamera object to support streams","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Kieran,\n\nScratch all comments regarding the stream id in my previous reply. I \nhave remodeled this to get rid of the stream ids completely.\n\nOn 2019-01-29 00:00:15 +0100, Niklas Söderlund wrote:\n> Hi Kieran,\n> \n> Thanks for your feedback.\n> \n> On 2019-01-28 11:34:23 +0000, Kieran Bingham wrote:\n> > Hi Niklas,\n> > \n> > On 27/01/2019 00:22, Niklas Söderlund wrote:\n> > > A camera consist of one or more video streams origination from the same\n> > \n> > A camera *consists* of one or more video streams *originating* from ...\n> > \n> > > video source. The different streams could for example have access to\n> > > different hardware blocks in the video pipeline and therefor be able to\n> > \n> > therefore\n> > \n> > > process the video source in different ways.\n> > > \n> > > All static information describing each streams needs to be recorded at\n> > \n> > \n> > describing each stream\n> > \n> \n> Thanks!\n> \n> > \n> > > camera creation. After a camera is created an application can retrieve\n> > > the static information about its stream at any time.\n> > > \n> > > Update all pipeline handlers to register one stream per camera, this\n> > > might be extended in the future for some of the pipelines.\n> > > \n> > > Signed-off-by: Niklas Söderlund \n> > > ---\n> > > include/libcamera/camera.h | 10 ++++-\n> > > src/libcamera/camera.cpp | 55 ++++++++++++++++++++++++++--\n> > > src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +-\n> > > src/libcamera/pipeline/uvcvideo.cpp | 4 +-\n> > > src/libcamera/pipeline/vimc.cpp | 4 +-\n> > > 5 files changed, 69 insertions(+), 8 deletions(-)\n> > > \n> > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > > index 7e358f8c0aa093cf..786d4d7d66bed5b2 100644\n> > > --- a/include/libcamera/camera.h\n> > > +++ b/include/libcamera/camera.h\n> > > @@ -15,12 +15,14 @@\n> > > namespace libcamera {\n> > > \n> > > class PipelineHandler;\n> > > +class Stream;\n> > > \n> > > class Camera final\n> > > {\n> > > public:\n> > > \tstatic std::shared_ptr create(PipelineHandler *pipe,\n> > > -\t\t\t\t\t const std::string &name);\n> > > +\t\t\t\t\t const std::string &name,\n> > > +\t\t\t\t\t const std::vector &streams);\n> > > \n> > > \tCamera(const Camera &) = delete;\n> > > \tvoid operator=(const Camera &) = delete;\n> > > @@ -32,6 +34,8 @@ public:\n> > > \tint acquire();\n> > > \tvoid release();\n> > > \n> > > +\tstd::vector streams() const;\n> > > +\n> > > private:\n> > > \tCamera(PipelineHandler *pipe, const std::string &name);\n> > > \t~Camera();\n> > > @@ -39,10 +43,14 @@ private:\n> > > \tfriend class PipelineHandler;\n> > > \tvoid disconnect();\n> > > \n> > > +\tbool haveStreamID(unsigned int id) const;\n> > \n> > have? should this be 'hasStreamID()' ?\n> > \n> > Looking at the implementation, Yes - I think so.\n> \n> I have renamed this hasStreamId merging your and Laurents comments on \n> this.\n> \n> > \n> > > +\n> > > \tstd::shared_ptr pipe_;\n> > > \tstd::string name_;\n> > > +\tstd::vector streams_;\n> > > \n> > > \tbool acquired_;\n> > > +\tbool disconnected_;\n> > \n> > Is this related to streams?\n> \n> Yes, if a camera is disconnected operations should not be forwarded to \n> the pipeline handler. Laurent thinks streams information should still be \n> available after a disconnect so I have moved this to the first patch who \n> adds operations which needs a connected camera.\n> \n> > \n> > > };\n> > > \n> > > } /* namespace libcamera */\n> > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > index 500976b237bcbd2d..3b2c00d0a4bb45d1 100644\n> > > --- a/src/libcamera/camera.cpp\n> > > +++ b/src/libcamera/camera.cpp\n> > > @@ -6,6 +6,7 @@\n> > > */\n> > > \n> > > #include \n> > > +#include \n> > > \n> > > #include \"log.h\"\n> > > #include \"pipeline_handler.h\"\n> > > @@ -56,13 +57,15 @@ LOG_DECLARE_CATEGORY(Camera)\n> > > * \\brief Create a camera instance\n> > > * \\param[in] name The name of the camera device\n> > > * \\param[in] pipe The pipeline handler responsible for the camera device\n> > > + * \\param[in] streams Array of streams the camera shall provide\n> > > *\n> > > * The caller is responsible for guaranteeing unicity of the camera name.\n> > > *\n> > > * \\return A shared pointer to the newly created camera object\n> > > */\n> > > std::shared_ptr Camera::create(PipelineHandler *pipe,\n> > > -\t\t\t\t const std::string &name)\n> > > +\t\t\t\t const std::string &name,\n> > > +\t\t\t\t const std::vector &streams)\n> > > {\n> > > \tstruct Allocator : std::allocator {\n> > > \t\tvoid construct(void *p, PipelineHandler *pipe,\n> > > @@ -76,7 +79,19 @@ std::shared_ptr Camera::create(PipelineHandler *pipe,\n> > > \t\t}\n> > > \t};\n> > > \n> > > -\treturn std::allocate_shared(Allocator(), pipe, name);\n> > > +\tstd::shared_ptr camera =\n> > > +\t\tstd::allocate_shared(Allocator(), pipe, name);\n> > > +\n> > > +\tfor (const Stream &stream : streams) {\n> > > +\t\tif (camera->haveStreamID(stream.id())) {\n> > > +\t\t\tLOG(Camera, Error) << \"Duplication of stream ID\";\n> > \n> > What about the pipeline adding a stream to a camera instead, and the ID\n> > being allocated by the camera device?\n> \n> For the ID to have any real meaning the pipeline needs to assign the id \n> as it is the real user of it. When configuring a camera the pipeline \n> handler needs to know which streams are to be used and it currently do \n> this by the use of the id. This might change in the future as the API \n> evolve.\n> \n> > \n> > Or does this make things the wrong way around?\n> > \n> > I guess it would also have to be a function which only the Pipeline was\n> > able to call somehow...\n> > \n> > (we wouldn't want a public Camera API that could arbitrarily add streams!)..\n> \n> :-)\n> \n> > \n> > \n> > > +\t\t\tcamera.reset();\n> > > +\t\t\tbreak;\n> > > +\t\t}\n> > > +\t\tcamera->streams_.push_back(stream);\n> > > +\t}\n> > > +\n> > > +\treturn camera;\n> > > }\n> > > \n> > > /**\n> > > @@ -102,7 +117,8 @@ const std::string &Camera::name() const\n> > > */\n> > > \n> > > Camera::Camera(PipelineHandler *pipe, const std::string &name)\n> > > -\t: pipe_(pipe->shared_from_this()), name_(name), acquired_(false)\n> > > +\t: pipe_(pipe->shared_from_this()), name_(name), acquired_(false),\n> > > +\t disconnected_(false)\n> > > {\n> > > }\n> > > \n> > > @@ -125,10 +141,24 @@ void Camera::disconnect()\n> > > {\n> > > \tLOG(Camera, Debug) << \"Disconnecting camera \" << name_;\n> > > \n> > > -\t/** \\todo Block API calls when they will be implemented. */\n> > > +\tdisconnected_ = true;\n> > > \tdisconnected.emit(this);\n> > > }\n> > > \n> > > +/**\n> > > + * \\brief Check if camera have a stream ID\n> > \n> > Check if the camera has a particular stream ID\n> > \n> > \n> > > + * \\param[in] id Stream ID to check for\n> > > + * \\return ture if stream ID exists, else false\n> > \n> > s/ture/True/\n> \n> Thanks.\n> \n> > \n> > \n> > > + */\n> > > +bool Camera::haveStreamID(unsigned int id) const\n> > > +{\n> > > +\tfor (const Stream &stream : streams_)\n> > > +\t\tif (stream.id() == id)\n> > > +\t\t\treturn true;\n> > > +\n> > > +\treturn false;\n> > > +}\n> > > +\n> > > /**\n> > > * \\brief Acquire the camera device for exclusive access\n> > > *\n> > > @@ -164,4 +194,21 @@ void Camera::release()\n> > > \tacquired_ = false;\n> > > }\n> > > \n> > > +/**\n> > > + * \\brief Retrieve all the camera's stream information\n> > > + *\n> > > + * Retrieve all of the camera's static stream information. The static\n> > > + * information describes among other things how many streams the camera support\n> > > + * and each streams capabilities.\n> > > + *\n> > > + * \\return An array of all the camera's streams or an empty list on error.\n> > \n> > Is if(disconnected) an error?\n> > \n> > I'd say if a camera is disconnected it has no streams ... I don't think\n> > that's an error?\n> > \n> > Perhaps:\n> > \n> > \\return An array of all the camera's streams valid streams.\n> \n> Merging your and Laurents comment a disconnected camera will keep \n> exposing the static stream information so I have update this to:\n> \n> \\return An array of all the camera's streams.\n> \n> > \n> > \n> > > + */\n> > > +std::vector Camera::streams() const\n> > > +{\n> > > +\tif (disconnected_)\n> > > +\t\treturn std::vector{};\n> > > +\n> > > +\treturn streams_;\n> > > +}\n> > > +\n> > > } /* namespace libcamera */\n> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > index d74655d037728feb..dbb2a89163c36cbc 100644\n> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > @@ -9,6 +9,7 @@\n> > > #include \n> > > \n> > > #include \n> > > +#include \n> > > \n> > > #include \"device_enumerator.h\"\n> > > #include \"log.h\"\n> > > @@ -197,7 +198,8 @@ void PipelineHandlerIPU3::registerCameras()\n> > > \t\t\tcontinue;\n> > > \n> > > \t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> > > -\t\tstd::shared_ptr camera = Camera::create(this, cameraName);\n> > > +\t\tstd::vector streams{ Stream(0) };\n> > > +\t\tstd::shared_ptr camera = Camera::create(this, cameraName, streams);\n> > > \n> > > \t\t/*\n> > > \t\t * If V4L2 device creation fails, the Camera instance won't be\n> > > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> > > index c51e8bc1f2c2bf25..bc4a8d7236be589d 100644\n> > > --- a/src/libcamera/pipeline/uvcvideo.cpp\n> > > +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> > > @@ -6,6 +6,7 @@\n> > > */\n> > > \n> > > #include \n> > > +#include \n> > > \n> > > #include \"device_enumerator.h\"\n> > > #include \"media_device.h\"\n> > > @@ -64,7 +65,8 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> > > \t\treturn false;\n> > > \t}\n> > > \n> > > -\tstd::shared_ptr camera = Camera::create(this, media_->model());\n> > > +\tstd::vector streams{ Stream(0) };\n> > > +\tstd::shared_ptr camera = Camera::create(this, media_->model(), streams);\n> > > \tregisterCamera(std::move(camera));\n> > > \thotplugMediaDevice(media_.get());\n> > > \n> > > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> > > index f58a97d51619515d..c426a953aea1b3dd 100644\n> > > --- a/src/libcamera/pipeline/vimc.cpp\n> > > +++ b/src/libcamera/pipeline/vimc.cpp\n> > > @@ -6,6 +6,7 @@\n> > > */\n> > > \n> > > #include \n> > > +#include \n> > > \n> > > #include \"device_enumerator.h\"\n> > > #include \"media_device.h\"\n> > > @@ -56,7 +57,8 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)\n> > > \n> > > \tmedia_->acquire();\n> > > \n> > > -\tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\");\n> > > +\tstd::vector streams{ Stream(0) };\n> > > +\tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\", streams);\n> > \n> > Out of interest, UVC exposes two video device nodes. One for the video\n> > data, and one for 'meta' data.\n> > \n> > Would we consider this meta data to be a second stream?\n> \n> Possibly, to be honest I don't know what the second video node is used \n> for. If it carries meta data maybe it should be used by a 3A sometime in \n> the future :-)\n> \n> > \n> > \n> > > \tregisterCamera(std::move(camera));\n> > > \n> > > \treturn true;\n> > > \n> > \n> > -- \n> > Regards\n> > --\n> > Kieran\n> \n> -- \n> Regards,\n> Niklas Söderlund","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 46B6360C78\n\tfor ;\n\tTue, 29 Jan 2019 02:24:44 +0100 (CET)","by mail-lj1-x241.google.com with SMTP id k19-v6so15988825lji.11\n\tfor ;\n\tMon, 28 Jan 2019 17:24:44 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\ta20-v6sm3400461ljf.28.2019.01.28.17.24.41\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tMon, 28 Jan 2019 17:24:42 -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=kS3vL6reTHulh8Onu/N6xO63TVyKgUk6bicMT65gO38=;\n\tb=xt4KHPTOkEBs+xJ+6ac1HJsnHZgrlEVwpMIjXW4mBuSeArvLwBnwfhAJ9Pj0bt5thg\n\tDXuF2cQWSq18IvkIdtQzOzTzFeQs3SQFMuXk8nrt/dgZGS/wxPw0/pah2c48+1db2n94\n\t7gBWG3P3yLEBh3JlnEe0tusuBp3CIyZgatdBNsUk3vKmHpuxTsKlw+cqEqWsgQ9jqB2B\n\tExMdQg5h+pi9n7zMP+EaJO7o7IXLMkdZB/IcHtmoJfmJdx0wcoxbD3KVdRkyW5ka8Wkg\n\tQ5ND8pigAWRTN2ZNpWJMvUJHgmwX0IMSwv26W2NNNwDz6+0poL4sYMWNQ6lLyJ23u8lB\n\tNrEA==","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=kS3vL6reTHulh8Onu/N6xO63TVyKgUk6bicMT65gO38=;\n\tb=fp4NaAfdlQ7nqaeDGeO8HBLdlfWE76EEl0nmGHv2A1HqWBbecyjLV6NNzkkHHX/eMM\n\t0Jus7rM5gpO5fLfATYsxtNZGOrNBp+1c96saDNa7zLoOzJ8u+kAca3f0E7crnIJjnP25\n\tFc0zvUk0eT8FYopNa6Jky9dY75aWd+6TigF2XGnWEOZrlfUlw9C/A1/QgLcMHxlno9x5\n\t0QF+uobArBr3Iiwln/1md/jgn9VJOd3addi32kYrnNRwyZMw3zcZ8wrGFaDOLnrRn8GH\n\tCIW2KXNqA7yCEGMjDqV/xgeVYaiKaP3Cp0ZjkyhSO1zDCV0F3blk90ZNGol9LOvR6omo\n\tXRSA==","X-Gm-Message-State":"AJcUukdQNIop+FtNjaZ6rdDQ885BYc77e0yqoK/PuUGDei82PCtkMi8T\n\tikGtzLmG/y/DMdFZQTTAHdnIXQ==","X-Google-Smtp-Source":"ALg8bN5/UEzM0vW5gY1CedDte06iJRUe3zeGfizIsy2dXr+ISLTJdKIEnsDXB3oZd4gCSrMhl8/ftQ==","X-Received":"by 2002:a2e:e02:: with SMTP id\n\t2-v6mr18980475ljo.10.1548725083129; \n\tMon, 28 Jan 2019 17:24:43 -0800 (PST)","Date":"Tue, 29 Jan 2019 02:24:41 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190129012441.GC26790@bigcity.dyn.berto.se>","References":"<20190127002208.18913-1-niklas.soderlund@ragnatech.se>\n\t<20190127002208.18913-5-niklas.soderlund@ragnatech.se>\n\t\n\t<20190128230015.GB26790@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190128230015.GB26790@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 4/6] libcamera: camera: extend\n\tcamera object to support 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:24:44 -0000"}},{"id":668,"web_url":"https://patchwork.libcamera.org/comment/668/","msgid":"<20190129013520.GD26790@bigcity.dyn.berto.se>","date":"2019-01-29T01:35:21","subject":"Re: [libcamera-devel] [PATCH v3 4/6] libcamera: camera: extend\n\tcamera object to support 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 01:17:44 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Sun, Jan 27, 2019 at 01:22:06AM +0100, Niklas Söderlund wrote:\n> > A camera consist of one or more video streams origination from the same\n> \n> s/consist/consists/\n> \n> > video source. The different streams could for example have access to\n> > different hardware blocks in the video pipeline and therefor be able to\n> > process the video source in different ways.\n> > \n> > All static information describing each streams needs to be recorded at\n> \n> Information is plural, so s/needs/need/\n> \n> > camera creation. After a camera is created an application can retrieve\n> > the static information about its stream at any time.\n> > \n> > Update all pipeline handlers to register one stream per camera, this\n> > might be extended in the future for some of the pipelines.\n> \n> Maybe s/might/will/ :-)\n\nThanks!\n\n> \n> > \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > include/libcamera/camera.h | 10 ++++-\n> > src/libcamera/camera.cpp | 55 ++++++++++++++++++++++++++--\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +-\n> > src/libcamera/pipeline/uvcvideo.cpp | 4 +-\n> > src/libcamera/pipeline/vimc.cpp | 4 +-\n> > 5 files changed, 69 insertions(+), 8 deletions(-)\n> > \n> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > index 7e358f8c0aa093cf..786d4d7d66bed5b2 100644\n> > --- a/include/libcamera/camera.h\n> > +++ b/include/libcamera/camera.h\n> > @@ -15,12 +15,14 @@\n> > namespace libcamera {\n> > \n> > class PipelineHandler;\n> > +class Stream;\n> > \n> > class Camera final\n> > {\n> > public:\n> > \tstatic std::shared_ptr create(PipelineHandler *pipe,\n> > -\t\t\t\t\t const std::string &name);\n> > +\t\t\t\t\t const std::string &name,\n> > +\t\t\t\t\t const std::vector &streams);\n> > \n> > \tCamera(const Camera &) = delete;\n> > \tvoid operator=(const Camera &) = delete;\n> > @@ -32,6 +34,8 @@ public:\n> > \tint acquire();\n> > \tvoid release();\n> > \n> > +\tstd::vector streams() const;\n> \n> Do we need to return a copy, or could we return a const\n> std::vector & ?\n\nI think we need to return a copy to make life easier for the \napplication. Consider the use-case\n\nstd::vector streams = camera->streams();\n\nfor (Stream *stream : streams)\n if (stream->isViewfinder())\n streams.erase(stream);\n\nstd::map config = camera->streamConfiguration(streams);\n\n\nI use Stream * instead of Stream in this example as it is what will be \nused in v4.\n\n> \n> > +\n> > private:\n> > \tCamera(PipelineHandler *pipe, const std::string &name);\n> > \t~Camera();\n> > @@ -39,10 +43,14 @@ private:\n> > \tfriend class PipelineHandler;\n> > \tvoid disconnect();\n> > \n> > +\tbool haveStreamID(unsigned int id) const;\n> \n> I'd name this hasStreamID, or possibly even hasStreamId.\n\nThis is removed in v4.\n\n> \n> > +\n> > \tstd::shared_ptr pipe_;\n> > \tstd::string name_;\n> > +\tstd::vector streams_;\n> > \n> > \tbool acquired_;\n> > +\tbool disconnected_;\n> > };\n> > \n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index 500976b237bcbd2d..3b2c00d0a4bb45d1 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -6,6 +6,7 @@\n> > */\n> > \n> > #include \n> > +#include \n> > \n> > #include \"log.h\"\n> > #include \"pipeline_handler.h\"\n> > @@ -56,13 +57,15 @@ LOG_DECLARE_CATEGORY(Camera)\n> > * \\brief Create a camera instance\n> > * \\param[in] name The name of the camera device\n> > * \\param[in] pipe The pipeline handler responsible for the camera device\n> > + * \\param[in] streams Array of streams the camera shall provide\n> \n> s/shall provide/provides/ ?\n\nThanks.\n\n> \n> > *\n> > * The caller is responsible for guaranteeing unicity of the camera name.\n> > *\n> > * \\return A shared pointer to the newly created camera object\n> > */\n> > std::shared_ptr Camera::create(PipelineHandler *pipe,\n> > -\t\t\t\t const std::string &name)\n> > +\t\t\t\t const std::string &name,\n> > +\t\t\t\t const std::vector &streams)\n> > {\n> > \tstruct Allocator : std::allocator {\n> > \t\tvoid construct(void *p, PipelineHandler *pipe,\n> > @@ -76,7 +79,19 @@ std::shared_ptr Camera::create(PipelineHandler *pipe,\n> > \t\t}\n> > \t};\n> > \n> > -\treturn std::allocate_shared(Allocator(), pipe, name);\n> > +\tstd::shared_ptr camera =\n> > +\t\tstd::allocate_shared(Allocator(), pipe, name);\n> > +\n> > +\tfor (const Stream &stream : streams) {\n> > +\t\tif (camera->haveStreamID(stream.id())) {\n> > +\t\t\tLOG(Camera, Error) << \"Duplication of stream ID\";\n> > +\t\t\tcamera.reset();\n> > +\t\t\tbreak;\n> > +\t\t}\n> \n> I think we can do better. Instead of returning a null pointer (which by\n> the way you don't check for in the pipeline handlers, so I expect\n> third-party pipeline handlers not to handle that error correctly\n> either), we should make an API that can't be used incorrectly. For\n> instance you could pass a vector of streams without any ID assign, and\n> assign IDs incrementally starting from 0 in this function. The\n> haveStreamID() method won't be needed anymore.\n\nGood point. This is removed in v4 for a design which makes the issue \nimpossible. Thanks for motivating me to have another go at this!\n\n> \n> > +\t\tcamera->streams_.push_back(stream);\n> > +\t}\n> > +\n> > +\treturn camera;\n> > }\n> > \n> > /**\n> > @@ -102,7 +117,8 @@ const std::string &Camera::name() const\n> > */\n> > \n> > Camera::Camera(PipelineHandler *pipe, const std::string &name)\n> > -\t: pipe_(pipe->shared_from_this()), name_(name), acquired_(false)\n> > +\t: pipe_(pipe->shared_from_this()), name_(name), acquired_(false),\n> > +\t disconnected_(false)\n> > {\n> > }\n> > \n> > @@ -125,10 +141,24 @@ void Camera::disconnect()\n> > {\n> > \tLOG(Camera, Debug) << \"Disconnecting camera \" << name_;\n> > \n> > -\t/** \\todo Block API calls when they will be implemented. */\n> > +\tdisconnected_ = true;\n> > \tdisconnected.emit(this);\n> > }\n> > \n> > +/**\n> > + * \\brief Check if camera have a stream ID\n> > + * \\param[in] id Stream ID to check for\n> > + * \\return ture if stream ID exists, else false\n> > + */\n> > +bool Camera::haveStreamID(unsigned int id) const\n> > +{\n> > +\tfor (const Stream &stream : streams_)\n> > +\t\tif (stream.id() == id)\n> > +\t\t\treturn true;\n> > +\n> > +\treturn false;\n> > +}\n> > +\n> > /**\n> > * \\brief Acquire the camera device for exclusive access\n> > *\n> > @@ -164,4 +194,21 @@ void Camera::release()\n> > \tacquired_ = false;\n> > }\n> > \n> > +/**\n> > + * \\brief Retrieve all the camera's stream information\n> > + *\n> > + * Retrieve all of the camera's static stream information. The static\n> > + * information describes among other things how many streams the camera support\n> \n> Let's wrap documentation at 80 columns by default if not very\n> impractical.\n\nIn my editor it's exactly 80 columns wide. I do agree it looks odd \nanyhow and have changed it for style.\n\n> \n> > + * and each streams capabilities.\n> > + *\n> > + * \\return An array of all the camera's streams or an empty list on error.\n> > + */\n> > +std::vector Camera::streams() const\n> > +{\n> > +\tif (disconnected_)\n> > +\t\treturn std::vector{};\n> \n> As this is static information I'm not sure we should fail the call.\n> Could there be cases where the application might want to get information\n> from the streams to clean up when the camera gets disconnected ?\n\nI have keep this for v4 as in the new design the Stream objects are \nowned by the pipeline handler. We should always have a pipeline handler \nas we keep a shard_ptr<> to it. So I'm open to discuss the removal of \nthis check in v4.\n\n> \n> > +\n> > +\treturn streams_;\n> > +}\n> > +\n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index d74655d037728feb..dbb2a89163c36cbc 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -9,6 +9,7 @@\n> > #include \n> > \n> > #include \n> > +#include \n> > \n> > #include \"device_enumerator.h\"\n> > #include \"log.h\"\n> > @@ -197,7 +198,8 @@ void PipelineHandlerIPU3::registerCameras()\n> > \t\t\tcontinue;\n> > \n> > \t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> > -\t\tstd::shared_ptr camera = Camera::create(this, cameraName);\n> > +\t\tstd::vector streams{ Stream(0) };\n> > +\t\tstd::shared_ptr camera = Camera::create(this, cameraName, streams);\n> \n> I think this will become a bit cumbersome to use when the Stream class\n> will gain more fields, but we can improve the API then.\n\nLets improve the API over time.\n\n> \n> > \n> > \t\t/*\n> > \t\t * If V4L2 device creation fails, the Camera instance won't be\n> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> > index c51e8bc1f2c2bf25..bc4a8d7236be589d 100644\n> > --- a/src/libcamera/pipeline/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> > @@ -6,6 +6,7 @@\n> > */\n> > \n> > #include \n> > +#include \n> > \n> > #include \"device_enumerator.h\"\n> > #include \"media_device.h\"\n> > @@ -64,7 +65,8 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> > \t\treturn false;\n> > \t}\n> > \n> > -\tstd::shared_ptr camera = Camera::create(this, media_->model());\n> > +\tstd::vector streams{ Stream(0) };\n> > +\tstd::shared_ptr camera = Camera::create(this, media_->model(), streams);\n> > \tregisterCamera(std::move(camera));\n> > \thotplugMediaDevice(media_.get());\n> > \n> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> > index f58a97d51619515d..c426a953aea1b3dd 100644\n> > --- a/src/libcamera/pipeline/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc.cpp\n> > @@ -6,6 +6,7 @@\n> > */\n> > \n> > #include \n> > +#include \n> > \n> > #include \"device_enumerator.h\"\n> > #include \"media_device.h\"\n> > @@ -56,7 +57,8 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)\n> > \n> > \tmedia_->acquire();\n> > \n> > -\tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\");\n> > +\tstd::vector streams{ Stream(0) };\n> > +\tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\", streams);\n> > \tregisterCamera(std::move(camera));\n> > \n> > \treturn true;\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5FD2E60C78\n\tfor ;\n\tTue, 29 Jan 2019 02:35:24 +0100 (CET)","by mail-lj1-x242.google.com with SMTP id u89-v6so16083513lje.1\n\tfor ;\n\tMon, 28 Jan 2019 17:35:24 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\td10sm3300773lfa.71.2019.01.28.17.35.22\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tMon, 28 Jan 2019 17:35:22 -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=5euBB/CQgUs1XA9eKVR/p26JkFoRveVExI6wIyQPYd8=;\n\tb=SBPLavYITWs6sR0LFCjLmhVjMdN64QRawvVPLFBUFwL3BW+Gh2/saO99RpGYRV9Dwq\n\tvj6i4ZE/VPc2SR47tdqo99oS3zw9RhLWDqpN7m9p8XxTI+WcVxiTrzc5+3BtAPjc4pTf\n\td0svUNClKW5dz3fR8SRhi62sY2MsdgkjrvfkiydK66qOe39l3MhteOFVirci00vmBXr8\n\tsWFGjKr0thxf22MDkciY+/qlq7zCif8dtshsFm9P7XijEukfQMRVvpDCO9CKq2jPYBWx\n\tGGvwcga/7z6DXdtSeSJN+vREz4ZB7ywyejX1zReQHtj+VzHOpdXVyR5nWWQDUaVclyVj\n\tWa+w==","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=5euBB/CQgUs1XA9eKVR/p26JkFoRveVExI6wIyQPYd8=;\n\tb=mBeVntTqzb5h8LUMiqXmGzW+XbEpcxLsPWIl3YSZXSAd1wbSWTkvPbzq7yoB/FWwB2\n\tmKwmrM6hiWhPp5GxLrIK6dntA8I/aqMlVprro8QtXklwCXOjaDC6meYNPjILNZvE/D8D\n\t2RSEVyCdFJx6beQ2EK4zxZWOlKhvgE7TDclukMf949yLsdu+vQEeH2UfPhJagxu7rIXy\n\tiwUiIia3PTkwXwBn3EEJ/63CDpETeigsn7EmjqKJdfF+5G4dPooZASszcJIjv4MBG6h1\n\t9I4/O1wsCgj4FqnKHPGVkcVblzE4a5hWQ6l5zLUsW12WZHP8WnQFyK+wPIFTaPtTrE8N\n\t7Inw==","X-Gm-Message-State":"AJcUukcdbEa3eGNSkXXR993kdx182UUuf0ZILi79HQOTUNHkgBHmE6Gp\n\tEKtX4EdhV7pU3Dv7bFXb/Cy8QA==","X-Google-Smtp-Source":"ALg8bN7PNvXZWlZGqxDGJwkBqD3o2+Ah/BEaLz8XH7OASH1CaeabGZ7bklUq8tUKsgchX14KM4JPfA==","X-Received":"by 2002:a2e:9849:: with SMTP id\n\te9-v6mr18991773ljj.9.1548725723508; \n\tMon, 28 Jan 2019 17:35:23 -0800 (PST)","Date":"Tue, 29 Jan 2019 02:35:21 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190129013520.GD26790@bigcity.dyn.berto.se>","References":"<20190127002208.18913-1-niklas.soderlund@ragnatech.se>\n\t<20190127002208.18913-5-niklas.soderlund@ragnatech.se>\n\t<20190127231744.GC5154@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":"<20190127231744.GC5154@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 4/6] libcamera: camera: extend\n\tcamera object to support 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:35:24 -0000"}},{"id":691,"web_url":"https://patchwork.libcamera.org/comment/691/","msgid":"<20190130001635.GB6632@pendragon.ideasonboard.com>","date":"2019-01-30T00:16:35","subject":"Re: [libcamera-devel] [PATCH v3 4/6] libcamera: camera: extend\n\tcamera object to support streams","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, Jan 29, 2019 at 02:35:21AM +0100, Niklas Söderlund wrote:\n> On 2019-01-28 01:17:44 +0200, Laurent Pinchart wrote:\n> > On Sun, Jan 27, 2019 at 01:22:06AM +0100, Niklas Söderlund wrote:\n> > > A camera consist of one or more video streams origination from the same\n> > \n> > s/consist/consists/\n> > \n> > > video source. The different streams could for example have access to\n> > > different hardware blocks in the video pipeline and therefor be able to\n> > > process the video source in different ways.\n> > > \n> > > All static information describing each streams needs to be recorded at\n> > \n> > Information is plural, so s/needs/need/\n> > \n> > > camera creation. After a camera is created an application can retrieve\n> > > the static information about its stream at any time.\n> > > \n> > > Update all pipeline handlers to register one stream per camera, this\n> > > might be extended in the future for some of the pipelines.\n> > \n> > Maybe s/might/will/ :-)\n> \n> Thanks!\n> \n> > > \n> > > Signed-off-by: Niklas Söderlund \n> > > ---\n> > > include/libcamera/camera.h | 10 ++++-\n> > > src/libcamera/camera.cpp | 55 ++++++++++++++++++++++++++--\n> > > src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +-\n> > > src/libcamera/pipeline/uvcvideo.cpp | 4 +-\n> > > src/libcamera/pipeline/vimc.cpp | 4 +-\n> > > 5 files changed, 69 insertions(+), 8 deletions(-)\n> > > \n> > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > > index 7e358f8c0aa093cf..786d4d7d66bed5b2 100644\n> > > --- a/include/libcamera/camera.h\n> > > +++ b/include/libcamera/camera.h\n> > > @@ -15,12 +15,14 @@\n> > > namespace libcamera {\n> > > \n> > > class PipelineHandler;\n> > > +class Stream;\n> > > \n> > > class Camera final\n> > > {\n> > > public:\n> > > \tstatic std::shared_ptr create(PipelineHandler *pipe,\n> > > -\t\t\t\t\t const std::string &name);\n> > > +\t\t\t\t\t const std::string &name,\n> > > +\t\t\t\t\t const std::vector &streams);\n> > > \n> > > \tCamera(const Camera &) = delete;\n> > > \tvoid operator=(const Camera &) = delete;\n> > > @@ -32,6 +34,8 @@ public:\n> > > \tint acquire();\n> > > \tvoid release();\n> > > \n> > > +\tstd::vector streams() const;\n> > \n> > Do we need to return a copy, or could we return a const\n> > std::vector & ?\n> \n> I think we need to return a copy to make life easier for the \n> application. Consider the use-case\n> \n> std::vector streams = camera->streams();\n> \n> for (Stream *stream : streams)\n> if (stream->isViewfinder())\n> streams.erase(stream);\n> \n> std::map config = camera->streamConfiguration(streams);\n\nYou can still do this with\n\nconst std::vector &Camera::Streams() const\n\nas the line\n\n\tstd::vector streams = camera->streams();\n\nwill create a copy of the vector, while\n\n\tconst std::vector &streams = camera->streams();\n\ncould be used if the application needs read-only inspection.\n\n> I use Stream * instead of Stream in this example as it is what will be \n> used in v4.\n> \n> > > +\n> > > private:\n> > > \tCamera(PipelineHandler *pipe, const std::string &name);\n> > > \t~Camera();\n> > > @@ -39,10 +43,14 @@ private:\n> > > \tfriend class PipelineHandler;\n> > > \tvoid disconnect();\n> > > \n> > > +\tbool haveStreamID(unsigned int id) const;\n> > \n> > I'd name this hasStreamID, or possibly even hasStreamId.\n> \n> This is removed in v4.\n> \n> > > +\n> > > \tstd::shared_ptr pipe_;\n> > > \tstd::string name_;\n> > > +\tstd::vector streams_;\n> > > \n> > > \tbool acquired_;\n> > > +\tbool disconnected_;\n> > > };\n> > > \n> > > } /* namespace libcamera */\n> > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > index 500976b237bcbd2d..3b2c00d0a4bb45d1 100644\n> > > --- a/src/libcamera/camera.cpp\n> > > +++ b/src/libcamera/camera.cpp\n> > > @@ -6,6 +6,7 @@\n> > > */\n> > > \n> > > #include \n> > > +#include \n> > > \n> > > #include \"log.h\"\n> > > #include \"pipeline_handler.h\"\n> > > @@ -56,13 +57,15 @@ LOG_DECLARE_CATEGORY(Camera)\n> > > * \\brief Create a camera instance\n> > > * \\param[in] name The name of the camera device\n> > > * \\param[in] pipe The pipeline handler responsible for the camera device\n> > > + * \\param[in] streams Array of streams the camera shall provide\n> > \n> > s/shall provide/provides/ ?\n> \n> Thanks.\n> \n> > > *\n> > > * The caller is responsible for guaranteeing unicity of the camera name.\n> > > *\n> > > * \\return A shared pointer to the newly created camera object\n> > > */\n> > > std::shared_ptr Camera::create(PipelineHandler *pipe,\n> > > -\t\t\t\t const std::string &name)\n> > > +\t\t\t\t const std::string &name,\n> > > +\t\t\t\t const std::vector &streams)\n> > > {\n> > > \tstruct Allocator : std::allocator {\n> > > \t\tvoid construct(void *p, PipelineHandler *pipe,\n> > > @@ -76,7 +79,19 @@ std::shared_ptr Camera::create(PipelineHandler *pipe,\n> > > \t\t}\n> > > \t};\n> > > \n> > > -\treturn std::allocate_shared(Allocator(), pipe, name);\n> > > +\tstd::shared_ptr camera =\n> > > +\t\tstd::allocate_shared(Allocator(), pipe, name);\n> > > +\n> > > +\tfor (const Stream &stream : streams) {\n> > > +\t\tif (camera->haveStreamID(stream.id())) {\n> > > +\t\t\tLOG(Camera, Error) << \"Duplication of stream ID\";\n> > > +\t\t\tcamera.reset();\n> > > +\t\t\tbreak;\n> > > +\t\t}\n> > \n> > I think we can do better. Instead of returning a null pointer (which by\n> > the way you don't check for in the pipeline handlers, so I expect\n> > third-party pipeline handlers not to handle that error correctly\n> > either), we should make an API that can't be used incorrectly. For\n> > instance you could pass a vector of streams without any ID assign, and\n> > assign IDs incrementally starting from 0 in this function. The\n> > haveStreamID() method won't be needed anymore.\n> \n> Good point. This is removed in v4 for a design which makes the issue \n> impossible. Thanks for motivating me to have another go at this!\n> \n> > > +\t\tcamera->streams_.push_back(stream);\n> > > +\t}\n> > > +\n> > > +\treturn camera;\n> > > }\n> > > \n> > > /**\n> > > @@ -102,7 +117,8 @@ const std::string &Camera::name() const\n> > > */\n> > > \n> > > Camera::Camera(PipelineHandler *pipe, const std::string &name)\n> > > -\t: pipe_(pipe->shared_from_this()), name_(name), acquired_(false)\n> > > +\t: pipe_(pipe->shared_from_this()), name_(name), acquired_(false),\n> > > +\t disconnected_(false)\n> > > {\n> > > }\n> > > \n> > > @@ -125,10 +141,24 @@ void Camera::disconnect()\n> > > {\n> > > \tLOG(Camera, Debug) << \"Disconnecting camera \" << name_;\n> > > \n> > > -\t/** \\todo Block API calls when they will be implemented. */\n> > > +\tdisconnected_ = true;\n> > > \tdisconnected.emit(this);\n> > > }\n> > > \n> > > +/**\n> > > + * \\brief Check if camera have a stream ID\n> > > + * \\param[in] id Stream ID to check for\n> > > + * \\return ture if stream ID exists, else false\n> > > + */\n> > > +bool Camera::haveStreamID(unsigned int id) const\n> > > +{\n> > > +\tfor (const Stream &stream : streams_)\n> > > +\t\tif (stream.id() == id)\n> > > +\t\t\treturn true;\n> > > +\n> > > +\treturn false;\n> > > +}\n> > > +\n> > > /**\n> > > * \\brief Acquire the camera device for exclusive access\n> > > *\n> > > @@ -164,4 +194,21 @@ void Camera::release()\n> > > \tacquired_ = false;\n> > > }\n> > > \n> > > +/**\n> > > + * \\brief Retrieve all the camera's stream information\n> > > + *\n> > > + * Retrieve all of the camera's static stream information. The static\n> > > + * information describes among other things how many streams the camera support\n> > \n> > Let's wrap documentation at 80 columns by default if not very\n> > impractical.\n> \n> In my editor it's exactly 80 columns wide. I do agree it looks odd \n> anyhow and have changed it for style.\n> \n> > > + * and each streams capabilities.\n> > > + *\n> > > + * \\return An array of all the camera's streams or an empty list on error.\n> > > + */\n> > > +std::vector Camera::streams() const\n> > > +{\n> > > +\tif (disconnected_)\n> > > +\t\treturn std::vector{};\n> > \n> > As this is static information I'm not sure we should fail the call.\n> > Could there be cases where the application might want to get information\n> > from the streams to clean up when the camera gets disconnected ?\n> \n> I have keep this for v4 as in the new design the Stream objects are \n> owned by the pipeline handler. We should always have a pipeline handler \n> as we keep a shard_ptr<> to it. So I'm open to discuss the removal of \n> this check in v4.\n> \n> > > +\n> > > +\treturn streams_;\n> > > +}\n> > > +\n> > > } /* namespace libcamera */\n> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > index d74655d037728feb..dbb2a89163c36cbc 100644\n> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > @@ -9,6 +9,7 @@\n> > > #include \n> > > \n> > > #include \n> > > +#include \n> > > \n> > > #include \"device_enumerator.h\"\n> > > #include \"log.h\"\n> > > @@ -197,7 +198,8 @@ void PipelineHandlerIPU3::registerCameras()\n> > > \t\t\tcontinue;\n> > > \n> > > \t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> > > -\t\tstd::shared_ptr camera = Camera::create(this, cameraName);\n> > > +\t\tstd::vector streams{ Stream(0) };\n> > > +\t\tstd::shared_ptr camera = Camera::create(this, cameraName, streams);\n> > \n> > I think this will become a bit cumbersome to use when the Stream class\n> > will gain more fields, but we can improve the API then.\n> \n> Lets improve the API over time.\n> \n> > > \n> > > \t\t/*\n> > > \t\t * If V4L2 device creation fails, the Camera instance won't be\n> > > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> > > index c51e8bc1f2c2bf25..bc4a8d7236be589d 100644\n> > > --- a/src/libcamera/pipeline/uvcvideo.cpp\n> > > +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> > > @@ -6,6 +6,7 @@\n> > > */\n> > > \n> > > #include \n> > > +#include \n> > > \n> > > #include \"device_enumerator.h\"\n> > > #include \"media_device.h\"\n> > > @@ -64,7 +65,8 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> > > \t\treturn false;\n> > > \t}\n> > > \n> > > -\tstd::shared_ptr camera = Camera::create(this, media_->model());\n> > > +\tstd::vector streams{ Stream(0) };\n> > > +\tstd::shared_ptr camera = Camera::create(this, media_->model(), streams);\n> > > \tregisterCamera(std::move(camera));\n> > > \thotplugMediaDevice(media_.get());\n> > > \n> > > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> > > index f58a97d51619515d..c426a953aea1b3dd 100644\n> > > --- a/src/libcamera/pipeline/vimc.cpp\n> > > +++ b/src/libcamera/pipeline/vimc.cpp\n> > > @@ -6,6 +6,7 @@\n> > > */\n> > > \n> > > #include \n> > > +#include \n> > > \n> > > #include \"device_enumerator.h\"\n> > > #include \"media_device.h\"\n> > > @@ -56,7 +57,8 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)\n> > > \n> > > \tmedia_->acquire();\n> > > \n> > > -\tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\");\n> > > +\tstd::vector streams{ Stream(0) };\n> > > +\tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\", streams);\n> > > \tregisterCamera(std::move(camera));\n> > > \n> > > \treturn true;","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 3790A60DB8\n\tfor ;\n\tWed, 30 Jan 2019 01:16:38 +0100 (CET)","from pendragon.ideasonboard.com (85-76-73-0-nat.elisa-mobile.fi\n\t[85.76.73.0])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 53FE141;\n\tWed, 30 Jan 2019 01:16:36 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548807397;\n\tbh=bRIT8L2PqL6FvrWj2ENCOC/4C5kVGysyi73siEH7vvw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pJ/n5uBgF41sUhQWNPhELOWk8kViuGJZ1ZFtwTe46dLDCLpQI6T4+69dTA3v77jCq\n\tKOdAXqGM2pC11Ak0et+UxbBrOFWh9fz2HqBF4S/zqYrvSiHy3O8ldQGyyibVZc285K\n\tlpLQw0VsLdERhoJQoM3xdhcXh9OTgTzYozobMpyY=","Date":"Wed, 30 Jan 2019 02:16:35 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190130001635.GB6632@pendragon.ideasonboard.com>","References":"<20190127002208.18913-1-niklas.soderlund@ragnatech.se>\n\t<20190127002208.18913-5-niklas.soderlund@ragnatech.se>\n\t<20190127231744.GC5154@pendragon.ideasonboard.com>\n\t<20190129013520.GD26790@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190129013520.GD26790@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 4/6] libcamera: camera: extend\n\tcamera object to support 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":"Wed, 30 Jan 2019 00:16:38 -0000"}},{"id":699,"web_url":"https://patchwork.libcamera.org/comment/699/","msgid":"<20190130113432.GG19527@bigcity.dyn.berto.se>","date":"2019-01-30T11:34:32","subject":"Re: [libcamera-devel] [PATCH v3 4/6] libcamera: camera: extend\n\tcamera object to support 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\nOn 2019-01-30 02:16:35 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> On Tue, Jan 29, 2019 at 02:35:21AM +0100, Niklas Söderlund wrote:\n> > On 2019-01-28 01:17:44 +0200, Laurent Pinchart wrote:\n> > > On Sun, Jan 27, 2019 at 01:22:06AM +0100, Niklas Söderlund wrote:\n> > > > A camera consist of one or more video streams origination from the same\n> > > \n> > > s/consist/consists/\n> > > \n> > > > video source. The different streams could for example have access to\n> > > > different hardware blocks in the video pipeline and therefor be able to\n> > > > process the video source in different ways.\n> > > > \n> > > > All static information describing each streams needs to be recorded at\n> > > \n> > > Information is plural, so s/needs/need/\n> > > \n> > > > camera creation. After a camera is created an application can retrieve\n> > > > the static information about its stream at any time.\n> > > > \n> > > > Update all pipeline handlers to register one stream per camera, this\n> > > > might be extended in the future for some of the pipelines.\n> > > \n> > > Maybe s/might/will/ :-)\n> > \n> > Thanks!\n> > \n> > > > \n> > > > Signed-off-by: Niklas Söderlund \n> > > > ---\n> > > > include/libcamera/camera.h | 10 ++++-\n> > > > src/libcamera/camera.cpp | 55 ++++++++++++++++++++++++++--\n> > > > src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +-\n> > > > src/libcamera/pipeline/uvcvideo.cpp | 4 +-\n> > > > src/libcamera/pipeline/vimc.cpp | 4 +-\n> > > > 5 files changed, 69 insertions(+), 8 deletions(-)\n> > > > \n> > > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > > > index 7e358f8c0aa093cf..786d4d7d66bed5b2 100644\n> > > > --- a/include/libcamera/camera.h\n> > > > +++ b/include/libcamera/camera.h\n> > > > @@ -15,12 +15,14 @@\n> > > > namespace libcamera {\n> > > > \n> > > > class PipelineHandler;\n> > > > +class Stream;\n> > > > \n> > > > class Camera final\n> > > > {\n> > > > public:\n> > > > \tstatic std::shared_ptr create(PipelineHandler *pipe,\n> > > > -\t\t\t\t\t const std::string &name);\n> > > > +\t\t\t\t\t const std::string &name,\n> > > > +\t\t\t\t\t const std::vector &streams);\n> > > > \n> > > > \tCamera(const Camera &) = delete;\n> > > > \tvoid operator=(const Camera &) = delete;\n> > > > @@ -32,6 +34,8 @@ public:\n> > > > \tint acquire();\n> > > > \tvoid release();\n> > > > \n> > > > +\tstd::vector streams() const;\n> > > \n> > > Do we need to return a copy, or could we return a const\n> > > std::vector & ?\n> > \n> > I think we need to return a copy to make life easier for the \n> > application. Consider the use-case\n> > \n> > std::vector streams = camera->streams();\n> > \n> > for (Stream *stream : streams)\n> > if (stream->isViewfinder())\n> > streams.erase(stream);\n> > \n> > std::map config = camera->streamConfiguration(streams);\n> \n> You can still do this with\n> \n> const std::vector &Camera::Streams() const\n> \n> as the line\n> \n> \tstd::vector streams = camera->streams();\n> \n> will create a copy of the vector, while\n> \n> \tconst std::vector &streams = camera->streams();\n> \n> could be used if the application needs read-only inspection.\n\nYou are correct, will make this const in v5.\n\n> \n> > I use Stream * instead of Stream in this example as it is what will be \n> > used in v4.\n> > \n> > > > +\n> > > > private:\n> > > > \tCamera(PipelineHandler *pipe, const std::string &name);\n> > > > \t~Camera();\n> > > > @@ -39,10 +43,14 @@ private:\n> > > > \tfriend class PipelineHandler;\n> > > > \tvoid disconnect();\n> > > > \n> > > > +\tbool haveStreamID(unsigned int id) const;\n> > > \n> > > I'd name this hasStreamID, or possibly even hasStreamId.\n> > \n> > This is removed in v4.\n> > \n> > > > +\n> > > > \tstd::shared_ptr pipe_;\n> > > > \tstd::string name_;\n> > > > +\tstd::vector streams_;\n> > > > \n> > > > \tbool acquired_;\n> > > > +\tbool disconnected_;\n> > > > };\n> > > > \n> > > > } /* namespace libcamera */\n> > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > > index 500976b237bcbd2d..3b2c00d0a4bb45d1 100644\n> > > > --- a/src/libcamera/camera.cpp\n> > > > +++ b/src/libcamera/camera.cpp\n> > > > @@ -6,6 +6,7 @@\n> > > > */\n> > > > \n> > > > #include \n> > > > +#include \n> > > > \n> > > > #include \"log.h\"\n> > > > #include \"pipeline_handler.h\"\n> > > > @@ -56,13 +57,15 @@ LOG_DECLARE_CATEGORY(Camera)\n> > > > * \\brief Create a camera instance\n> > > > * \\param[in] name The name of the camera device\n> > > > * \\param[in] pipe The pipeline handler responsible for the camera device\n> > > > + * \\param[in] streams Array of streams the camera shall provide\n> > > \n> > > s/shall provide/provides/ ?\n> > \n> > Thanks.\n> > \n> > > > *\n> > > > * The caller is responsible for guaranteeing unicity of the camera name.\n> > > > *\n> > > > * \\return A shared pointer to the newly created camera object\n> > > > */\n> > > > std::shared_ptr Camera::create(PipelineHandler *pipe,\n> > > > -\t\t\t\t const std::string &name)\n> > > > +\t\t\t\t const std::string &name,\n> > > > +\t\t\t\t const std::vector &streams)\n> > > > {\n> > > > \tstruct Allocator : std::allocator {\n> > > > \t\tvoid construct(void *p, PipelineHandler *pipe,\n> > > > @@ -76,7 +79,19 @@ std::shared_ptr Camera::create(PipelineHandler *pipe,\n> > > > \t\t}\n> > > > \t};\n> > > > \n> > > > -\treturn std::allocate_shared(Allocator(), pipe, name);\n> > > > +\tstd::shared_ptr camera =\n> > > > +\t\tstd::allocate_shared(Allocator(), pipe, name);\n> > > > +\n> > > > +\tfor (const Stream &stream : streams) {\n> > > > +\t\tif (camera->haveStreamID(stream.id())) {\n> > > > +\t\t\tLOG(Camera, Error) << \"Duplication of stream ID\";\n> > > > +\t\t\tcamera.reset();\n> > > > +\t\t\tbreak;\n> > > > +\t\t}\n> > > \n> > > I think we can do better. Instead of returning a null pointer (which by\n> > > the way you don't check for in the pipeline handlers, so I expect\n> > > third-party pipeline handlers not to handle that error correctly\n> > > either), we should make an API that can't be used incorrectly. For\n> > > instance you could pass a vector of streams without any ID assign, and\n> > > assign IDs incrementally starting from 0 in this function. The\n> > > haveStreamID() method won't be needed anymore.\n> > \n> > Good point. This is removed in v4 for a design which makes the issue \n> > impossible. Thanks for motivating me to have another go at this!\n> > \n> > > > +\t\tcamera->streams_.push_back(stream);\n> > > > +\t}\n> > > > +\n> > > > +\treturn camera;\n> > > > }\n> > > > \n> > > > /**\n> > > > @@ -102,7 +117,8 @@ const std::string &Camera::name() const\n> > > > */\n> > > > \n> > > > Camera::Camera(PipelineHandler *pipe, const std::string &name)\n> > > > -\t: pipe_(pipe->shared_from_this()), name_(name), acquired_(false)\n> > > > +\t: pipe_(pipe->shared_from_this()), name_(name), acquired_(false),\n> > > > +\t disconnected_(false)\n> > > > {\n> > > > }\n> > > > \n> > > > @@ -125,10 +141,24 @@ void Camera::disconnect()\n> > > > {\n> > > > \tLOG(Camera, Debug) << \"Disconnecting camera \" << name_;\n> > > > \n> > > > -\t/** \\todo Block API calls when they will be implemented. */\n> > > > +\tdisconnected_ = true;\n> > > > \tdisconnected.emit(this);\n> > > > }\n> > > > \n> > > > +/**\n> > > > + * \\brief Check if camera have a stream ID\n> > > > + * \\param[in] id Stream ID to check for\n> > > > + * \\return ture if stream ID exists, else false\n> > > > + */\n> > > > +bool Camera::haveStreamID(unsigned int id) const\n> > > > +{\n> > > > +\tfor (const Stream &stream : streams_)\n> > > > +\t\tif (stream.id() == id)\n> > > > +\t\t\treturn true;\n> > > > +\n> > > > +\treturn false;\n> > > > +}\n> > > > +\n> > > > /**\n> > > > * \\brief Acquire the camera device for exclusive access\n> > > > *\n> > > > @@ -164,4 +194,21 @@ void Camera::release()\n> > > > \tacquired_ = false;\n> > > > }\n> > > > \n> > > > +/**\n> > > > + * \\brief Retrieve all the camera's stream information\n> > > > + *\n> > > > + * Retrieve all of the camera's static stream information. The static\n> > > > + * information describes among other things how many streams the camera support\n> > > \n> > > Let's wrap documentation at 80 columns by default if not very\n> > > impractical.\n> > \n> > In my editor it's exactly 80 columns wide. I do agree it looks odd \n> > anyhow and have changed it for style.\n> > \n> > > > + * and each streams capabilities.\n> > > > + *\n> > > > + * \\return An array of all the camera's streams or an empty list on error.\n> > > > + */\n> > > > +std::vector Camera::streams() const\n> > > > +{\n> > > > +\tif (disconnected_)\n> > > > +\t\treturn std::vector{};\n> > > \n> > > As this is static information I'm not sure we should fail the call.\n> > > Could there be cases where the application might want to get information\n> > > from the streams to clean up when the camera gets disconnected ?\n> > \n> > I have keep this for v4 as in the new design the Stream objects are \n> > owned by the pipeline handler. We should always have a pipeline handler \n> > as we keep a shard_ptr<> to it. So I'm open to discuss the removal of \n> > this check in v4.\n> > \n> > > > +\n> > > > +\treturn streams_;\n> > > > +}\n> > > > +\n> > > > } /* namespace libcamera */\n> > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > index d74655d037728feb..dbb2a89163c36cbc 100644\n> > > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > @@ -9,6 +9,7 @@\n> > > > #include \n> > > > \n> > > > #include \n> > > > +#include \n> > > > \n> > > > #include \"device_enumerator.h\"\n> > > > #include \"log.h\"\n> > > > @@ -197,7 +198,8 @@ void PipelineHandlerIPU3::registerCameras()\n> > > > \t\t\tcontinue;\n> > > > \n> > > > \t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> > > > -\t\tstd::shared_ptr camera = Camera::create(this, cameraName);\n> > > > +\t\tstd::vector streams{ Stream(0) };\n> > > > +\t\tstd::shared_ptr camera = Camera::create(this, cameraName, streams);\n> > > \n> > > I think this will become a bit cumbersome to use when the Stream class\n> > > will gain more fields, but we can improve the API then.\n> > \n> > Lets improve the API over time.\n> > \n> > > > \n> > > > \t\t/*\n> > > > \t\t * If V4L2 device creation fails, the Camera instance won't be\n> > > > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> > > > index c51e8bc1f2c2bf25..bc4a8d7236be589d 100644\n> > > > --- a/src/libcamera/pipeline/uvcvideo.cpp\n> > > > +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> > > > @@ -6,6 +6,7 @@\n> > > > */\n> > > > \n> > > > #include \n> > > > +#include \n> > > > \n> > > > #include \"device_enumerator.h\"\n> > > > #include \"media_device.h\"\n> > > > @@ -64,7 +65,8 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> > > > \t\treturn false;\n> > > > \t}\n> > > > \n> > > > -\tstd::shared_ptr camera = Camera::create(this, media_->model());\n> > > > +\tstd::vector streams{ Stream(0) };\n> > > > +\tstd::shared_ptr camera = Camera::create(this, media_->model(), streams);\n> > > > \tregisterCamera(std::move(camera));\n> > > > \thotplugMediaDevice(media_.get());\n> > > > \n> > > > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> > > > index f58a97d51619515d..c426a953aea1b3dd 100644\n> > > > --- a/src/libcamera/pipeline/vimc.cpp\n> > > > +++ b/src/libcamera/pipeline/vimc.cpp\n> > > > @@ -6,6 +6,7 @@\n> > > > */\n> > > > \n> > > > #include \n> > > > +#include \n> > > > \n> > > > #include \"device_enumerator.h\"\n> > > > #include \"media_device.h\"\n> > > > @@ -56,7 +57,8 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)\n> > > > \n> > > > \tmedia_->acquire();\n> > > > \n> > > > -\tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\");\n> > > > +\tstd::vector streams{ Stream(0) };\n> > > > +\tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\", streams);\n> > > > \tregisterCamera(std::move(camera));\n> > > > \n> > > > \treturn true;\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lj1-x22e.google.com (mail-lj1-x22e.google.com\n\t[IPv6:2a00:1450:4864:20::22e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8039C60B2D\n\tfor ;\n\tWed, 30 Jan 2019 12:34:35 +0100 (CET)","by mail-lj1-x22e.google.com with SMTP id k15-v6so20386827ljc.8\n\tfor ;\n\tWed, 30 Jan 2019 03:34:35 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\ta62sm249376lfa.37.2019.01.30.03.34.33\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tWed, 30 Jan 2019 03:34:33 -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=2TeLFfnqCqGSz+p3JFfG1yATbqC0e6KeNprj3HlDsGY=;\n\tb=wjKux+g8kPoSBCfM0TH6SF70VFZkUx92Mmj8yaPKiF5mkumXJAo5I1yX0Q8RNYF5OI\n\tz3wBP2PXFhy/8bK4wsj268jur4wmr79iX8f3/+z1aXbJ3Sj8JBFWqivZZZzK+JJitN6Y\n\tBm8swTNcHvktTTsNqxC1YcrPp9vL9I5yoMhKjKhH4zOFkDymAPUlLgF4oe2oMF2Q3M8T\n\tZQ8o80QyZ2rONtuYuaDMnTOjv+G8K9T1JhgygdmQy1ri38CztwOOzFqS+3+dZWO79dtl\n\tVzC78igtJu8Y2U9sHKC+b8AUYSnoIcsUFygCe5nd8dKukkYttkxj49xgztobceClUs5C\n\teQfQ==","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=2TeLFfnqCqGSz+p3JFfG1yATbqC0e6KeNprj3HlDsGY=;\n\tb=WikhG2GZ0yQmezUuPGNK3jB5Ohm1lQVLDDnRTkmfOi+SiVvyPScAeYKDH3Ch+9FuX1\n\tWvSduZUgWj1JnDgrA5F2pVHzH2YPEvWUQyZPzOJKI+o16l70QWjOVTbl1ifyZaqPbawN\n\tZU63lexLmYNdACPMHvIdxYs5BVON39TBK8Tboq1Ioe9URcPWdZlyR13WqzeP4vQtwcAT\n\tgLIluFEwy0PhkyvXVNZtmffuadMhCbAyVI0YGsmXSVV1DYAZmLCnTh1ND9El/J+Ti6LG\n\tNcNdmZCMPN37IqVV5YuayyggTwtsK7xQGy8V5pdlJpUNXg5lObm+WMUQW5BGsDZLZQY0\n\tfAkA==","X-Gm-Message-State":"AJcUukeCD0Pwu+vuS++vMjExmkf3NjuIWeKIWDnLmJ8jpZ9EQDifS7h/\n\tc8hAbvrhNWHdCklzfKzaFUdHHA==","X-Google-Smtp-Source":"ALg8bN6wKfrWAZCREHx9Xd+cF9tZsWmWE7HqWYfWDaRMPygrWavFpw5h5Asf9YXe9Jqe6bTtEF9eLw==","X-Received":"by 2002:a2e:9e16:: with SMTP id\n\te22-v6mr24308591ljk.4.1548848074351; \n\tWed, 30 Jan 2019 03:34:34 -0800 (PST)","Date":"Wed, 30 Jan 2019 12:34:32 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190130113432.GG19527@bigcity.dyn.berto.se>","References":"<20190127002208.18913-1-niklas.soderlund@ragnatech.se>\n\t<20190127002208.18913-5-niklas.soderlund@ragnatech.se>\n\t<20190127231744.GC5154@pendragon.ideasonboard.com>\n\t<20190129013520.GD26790@bigcity.dyn.berto.se>\n\t<20190130001635.GB6632@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":"<20190130001635.GB6632@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 4/6] libcamera: camera: extend\n\tcamera object to support 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":"Wed, 30 Jan 2019 11:34:35 -0000"}}]