[{"id":621,"web_url":"https://patchwork.libcamera.org/comment/621/","msgid":"<20190126092005.GC4596@pendragon.ideasonboard.com>","date":"2019-01-26T09:20:05","subject":"Re: [libcamera-devel] [PATCH v2 3/7] libcamera: pipelines: add\n\tmethod to retrieve 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 Fri, Jan 25, 2019 at 04:33:36PM +0100, Niklas Söderlund wrote:\n> A Camera object needs to be able to inquire its responsible\n> PipelineHandler for which streams it supports. Add and implement such an\n> interface to the PipelineHandler base class and all pipeline handler\n> implementations.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> src/libcamera/include/pipeline_handler.h | 3 +++\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 12 ++++++++++++\n> src/libcamera/pipeline/uvcvideo.cpp | 12 ++++++++++++\n> src/libcamera/pipeline/vimc.cpp | 12 ++++++++++++\n> src/libcamera/pipeline_handler.cpp | 12 ++++++++++++\n> 5 files changed, 51 insertions(+)\n> \n> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> index 0db84217089e692a..f92cc5b5fa5e777b 100644\n> --- a/src/libcamera/include/pipeline_handler.h\n> +++ b/src/libcamera/include/pipeline_handler.h\n> @@ -17,6 +17,7 @@ namespace libcamera {\n> class CameraManager;\n> class DeviceEnumerator;\n> class MediaDevice;\n> +class Stream;\n> \n> class Camera;\n> class CameraData\n> @@ -38,6 +39,8 @@ public:\n> \tPipelineHandler(CameraManager *manager);\n> \tvirtual ~PipelineHandler();\n> \n> +\tvirtual std::vector streams(const Camera *camera) const = 0;\n> +\n\nI thought pipeline handlers would create streams when they create the\ncamera, instead of on-demand at runtime. Wouldn't that be simpler for\npipeline handlers ? I envision the stream objects to be static for the\nlifetime of the camera, so creating them on demand each time would be\nmore error prone as pipeline handlers could make mistakes and return\ndifferent streams each time.\n\n> \tvirtual bool match(DeviceEnumerator *enumerator) = 0;\n> \n> protected:\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index d74655d037728feb..5c35c7a53b9347a3 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> @@ -27,6 +28,8 @@ public:\n> \tPipelineHandlerIPU3(CameraManager *manager);\n> \t~PipelineHandlerIPU3();\n> \n> +\tstd::vector streams(const Camera *camera) const;\n> +\n> \tbool match(DeviceEnumerator *enumerator);\n> \n> private:\n> @@ -60,6 +63,15 @@ PipelineHandlerIPU3::~PipelineHandlerIPU3()\n> \t\timgu_->release();\n> }\n> \n> +std::vector PipelineHandlerIPU3::streams(const Camera *camera) const\n> +{\n> +\tstd::vector streams;\n> +\n> +\tstreams.push_back(Stream(0));\n> +\n> +\treturn streams;\n> +}\n> +\n> bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> {\n> \tDeviceMatch cio2_dm(\"ipu3-cio2\");\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index c51e8bc1f2c2bf25..e1f023245b8e63dd 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> @@ -20,6 +21,8 @@ public:\n> \tPipelineHandlerUVC(CameraManager *manager);\n> \t~PipelineHandlerUVC();\n> \n> +\tstd::vector streams(const Camera *camera) const;\n> +\n> \tbool match(DeviceEnumerator *enumerator);\n> \n> private:\n> @@ -41,6 +44,15 @@ PipelineHandlerUVC::~PipelineHandlerUVC()\n> \t\tmedia_->release();\n> }\n> \n> +std::vector PipelineHandlerUVC::streams(const Camera *camera) const\n> +{\n> +\tstd::vector streams;\n> +\n> +\tstreams.push_back(Stream(0));\n> +\n> +\treturn streams;\n> +}\n> +\n> bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> {\n> \tDeviceMatch dm(\"uvcvideo\");\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index f58a97d51619515d..a96ec9f431361a32 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> @@ -19,6 +20,8 @@ public:\n> \tPipeHandlerVimc(CameraManager *manager);\n> \t~PipeHandlerVimc();\n> \n> +\tstd::vector streams(const Camera *camera) const;\n> +\n> \tbool match(DeviceEnumerator *enumerator);\n> \n> private:\n> @@ -36,6 +39,15 @@ PipeHandlerVimc::~PipeHandlerVimc()\n> \t\tmedia_->release();\n> }\n> \n> +std::vector PipeHandlerVimc::streams(const Camera *camera) const\n> +{\n> +\tstd::vector streams;\n> +\n> +\tstreams.push_back(Stream(0));\n> +\n> +\treturn streams;\n> +}\n> +\n> bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)\n> {\n> \tDeviceMatch dm(\"vimc\");\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 4d5344988cfe8aa2..a825e7bf882d23c2 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -84,6 +84,18 @@ PipelineHandler::~PipelineHandler()\n> \tcameraData_.clear();\n> };\n> \n> +/**\n> + * \\fn PipelineHandler::streams(const Camera *camera)\n> + * \\brief Retrive a array of all streams the camera provides\n\ns/Retrive a/Retrieve the/\n\n> + * \\param[in] camera The camera to retrieve streams from\n> + *\n> + * This function is the interface to extract information about streams from a\n> + * PipelineHandler.\n\nThat sentence doesn't seem to add extra information compared to the\n\\brief. I would drop it.\n\n> + * \\return A array of streams from the camera or a empty list if \\a camera\n> + * is not part of the PipelineHandler.\n\nNo need for custom indentation.\n\nAnd no need for this method if we create the streams when creating the\ncamera :-)\n\n> + */\n> +\n> /**\n> * \\fn PipelineHandler::match(DeviceEnumerator *enumerator)\n> * \\brief Match media devices and create camera instances","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 4B50160C65\n\tfor ;\n\tSat, 26 Jan 2019 10:20:14 +0100 (CET)","from pendragon.ideasonboard.com (unknown [62.119.166.9])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 821DE23D;\n\tSat, 26 Jan 2019 10:20:12 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548494413;\n\tbh=FPthD/aJbS+ORNR+j4O+zFMqu0SgST83lSkiq74YxLg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Kynt6LTt5NuOVzKQckalap36fGtfeNGUS5xFPK7/1advcOacStcl3cfyS3SeH8vkZ\n\tfRrYF89TxTn5hWjvVmgZGPs+Hd/VIwbgtiPypt/BYEgXaEE6ylUCAZpCqByr+smTsw\n\tEDMRmw5zf/Q6IiGThVp9zothEUtGEBLL3SqKmKJ4=","Date":"Sat, 26 Jan 2019 11:20:05 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190126092005.GC4596@pendragon.ideasonboard.com>","References":"<20190125153340.2744-1-niklas.soderlund@ragnatech.se>\n\t<20190125153340.2744-4-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":"<20190125153340.2744-4-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 3/7] libcamera: pipelines: add\n\tmethod to retrieve 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":"Sat, 26 Jan 2019 09:20:14 -0000"}}]