[{"id":959,"web_url":"https://patchwork.libcamera.org/comment/959/","msgid":"<20190228172805.GH899@bigcity.dyn.berto.se>","date":"2019-02-28T17:28:05","subject":"Re: [libcamera-devel] [PATCH 05/10] libcamera: pipeline_handler:\n\tStore pipe and camera in CameraData","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2019-02-28 18:29:08 +0200, Laurent Pinchart wrote:\n> Extend the CameraData class with two member variables pipe_ and camera_\n> that store pointers to the pipeline handler and camera that the\n> CameraData instance is related to. This will be used by pipeline\n> handlers to access the camera and the pipeline in member methods of\n> their CameraData derived classes.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> src/libcamera/include/pipeline_handler.h | 9 ++++-\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +++--\n> src/libcamera/pipeline/uvcvideo.cpp | 5 ++-\n> src/libcamera/pipeline/vimc.cpp | 5 ++-\n> src/libcamera/pipeline_handler.cpp | 49 +++++++++++++++++++++---\n> 5 files changed, 63 insertions(+), 14 deletions(-)\n> \n> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> index 45e1f07ffca9..bc4da5820ac4 100644\n> --- a/src/libcamera/include/pipeline_handler.h\n> +++ b/src/libcamera/include/pipeline_handler.h\n> @@ -19,6 +19,7 @@ class Camera;\n> class CameraManager;\n> class DeviceEnumerator;\n> class MediaDevice;\n> +class PipelineHandler;\n> class Request;\n> class Stream;\n> class StreamConfiguration;\n> @@ -26,10 +27,14 @@ class StreamConfiguration;\n> class CameraData\n> {\n> public:\n> +\texplicit CameraData(PipelineHandler *pipe)\n> +\t\t: pipe_(pipe)\n> +\t{\n> +\t}\n> \tvirtual ~CameraData() {}\n> \n> -protected:\n> -\tCameraData() {}\n> +\tCamera *camera_;\n> +\tPipelineHandler *pipe_;\n> \n> private:\n> \tCameraData(const CameraData &) = delete;\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 248e921117f4..347ee657fddf 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -50,8 +50,11 @@ private:\n> \tclass IPU3CameraData : public CameraData\n> \t{\n> \tpublic:\n> -\t\tIPU3CameraData()\n> -\t\t\t: cio2_(nullptr), csi2_(nullptr), sensor_(nullptr) {}\n> +\t\tIPU3CameraData(PipelineHandler *pipe)\n> +\t\t\t: CameraData(pipe), cio2_(nullptr), csi2_(nullptr),\n> +\t\t\t sensor_(nullptr)\n> +\t\t{\n> +\t\t}\n> \n> \t\t~IPU3CameraData()\n> \t\t{\n> @@ -365,7 +368,7 @@ void PipelineHandlerIPU3::registerCameras()\n> \t\tif (link->setEnabled(true))\n> \t\t\tcontinue;\n> \n> -\t\tstd::unique_ptr data = utils::make_unique();\n> +\t\tstd::unique_ptr data = utils::make_unique(this);\n> \n> \t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> \t\tstd::vector streams{ &data->stream_ };\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index cc513513cb34..9af4838891f4 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -46,7 +46,8 @@ private:\n> \tclass UVCCameraData : public CameraData\n> \t{\n> \tpublic:\n> -\t\tUVCCameraData()\n> +\t\tUVCCameraData(PipelineHandler *pipe)\n> +\t\t\t: CameraData(pipe)\n> \t\t{\n> \t\t}\n> \n> @@ -180,7 +181,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> \n> \tmedia_->acquire();\n> \n> -\tstd::unique_ptr data = utils::make_unique();\n> +\tstd::unique_ptr data = utils::make_unique(this);\n> \n> \t/* Locate and open the default video node. */\n> \tfor (MediaEntity *entity : media_->entities()) {\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index 1d01fa80f8d5..c4c1eb0dc19f 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -46,7 +46,8 @@ private:\n> \tclass VimcCameraData : public CameraData\n> \t{\n> \tpublic:\n> -\t\tVimcCameraData()\n> +\t\tVimcCameraData(PipelineHandler *pipe)\n> +\t\t\t: CameraData(pipe)\n> \t\t{\n> \t\t}\n> \n> @@ -190,7 +191,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> \n> \tmedia_->acquire();\n> \n> -\tstd::unique_ptr data = utils::make_unique();\n> +\tstd::unique_ptr data = utils::make_unique(this);\n> \n> \t/* Locate and open the capture video node. */\n> \tMediaEntity *entity = media_->getEntityByName(\"Raw Capture 1\");\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index c440aa8382ff..ac1acea5a318 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -11,6 +11,7 @@\n> #include \"log.h\"\n> #include \"media_device.h\"\n> #include \"pipeline_handler.h\"\n> +#include \"utils.h\"\n> \n> /**\n> * \\file pipeline_handler.h\n> @@ -44,6 +45,35 @@ LOG_DEFINE_CATEGORY(Pipeline)\n> * with cameraData().\n> */\n> \n> +/**\n> + * \\fn CameraData::CameraData(PipelineHandler *pipe)\n> + * \\brief Construct a CameraData instance for the given pipeline handler\n> + * \\param[in] pipe The pipeline handler\n> + *\n> + * The reference to the pipeline handler is stored internally, the caller shall\n> + * guarantee that the pointer remains valid as long as the CameraData instance\n> + * exists.\n> + */\n> +\n> +/**\n> + * \\var CameraData::camera_\n> + * \\brief The camera related to this CameraData instance\n> + *\n> + * The camera_ pointer provides access to the Camera object that this instance\n> + * is related to. It is set when the Camera is registered with\n> + * PipelineHandler::registerCamera() and remains valid until the CameraData\n> + * instance is destroyed.\n> + */\n> +\n> +/**\n> + * \\var CameraData::pipe_\n> + * \\brief The pipeline handler related to this CameraData instance\n> + *\n> + * The pipe_ pointer provides access to the PipelineHandler object that this\n> + * instance is related to. It is set when the CameraData instance is created\n> + * and remains valid until the instance is destroyed.\n> + */\n> +\n> /**\n> * \\class PipelineHandler\n> * \\brief Create and manage cameras based on a set of media devices\n> @@ -218,11 +248,19 @@ PipelineHandler::~PipelineHandler()\n> * \\brief Register a camera to the camera manager and pipeline handler\n> * \\param[in] camera The camera to be added\n> *\n> - * This function is called by pipeline handlers to register the cameras they\n> - * handle with the camera manager.\n> + * This method is called by pipeline handlers to register the cameras they\n> + * handle with the camera manager. If no CameraData has been associated with\n> + * the camera with setCameraData() by the pipeline handler, the method creates\n> + * a default CameraData instance for the \\a camera.\n> */\n> void PipelineHandler::registerCamera(std::shared_ptr camera)\n> {\n> +\tif (!cameraData_.count(camera.get())) {\n> +\t\tstd::unique_ptr data = utils::make_unique(this);\n> +\t\tsetCameraData(camera.get(), std::move(data));\n> +\t}\n> +\n> +\tcameraData(camera.get())->camera_ = camera.get();\n> \tcameras_.push_back(camera);\n> \tmanager_->addCamera(std::move(camera));\n> }\n> @@ -313,9 +351,10 @@ CameraData *PipelineHandler::cameraData(const Camera *camera)\n> * information with \\a camera. Ownership of \\a data is transferred to\n> * the PipelineHandler.\n> *\n> - * Pipeline-specific data can only be set once. Any attempt to call\n> - * this method after the first one with the same camera won't change\n> - * the pipeline-specific data.\n> + * Pipeline-specific data can only be set once, and shall be set before\n> + * registering the camera with registerCamera(). Any attempt to call this\n> + * method more than once for the same camera, or to call it after registering\n> + * the camera, will not change the pipeline-specific data.\n> *\n> * The data can be retrieved by pipeline handlers using the cameraData() method.\n> */\n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lj1-x22c.google.com (mail-lj1-x22c.google.com\n\t[IPv6:2a00:1450:4864:20::22c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5B2B7610B6\n\tfor ;\n\tThu, 28 Feb 2019 18:28:07 +0100 (CET)","by mail-lj1-x22c.google.com with SMTP id l5so17892526lje.1\n\tfor ;\n\tThu, 28 Feb 2019 09:28:07 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\ta23sm2543401ljd.20.2019.02.28.09.28.05\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tThu, 28 Feb 2019 09:28:06 -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=dwbJCTwZgiVChxox3GHtNljujP1w3ShbKRpvMdYZQLI=;\n\tb=xiffmVowYRmbpPYfzrI2k9B8rbf794xirlB6rMYnmABGDMlAnRQtXsDC1upTCdgIlU\n\tSUN3FEyM+2wtwNdAu8tJ9lNmUJN1mLnC8ELK9aal86E1YDwMqGek1C/uy/Qax5wJWFt5\n\tVBoVFDlhj/1HZ1RmAD4ChQnI6KrnTmCflyM0yeh4v0xAz4Z2HIsRiBRUvBCAyyDEKAQW\n\tjVhZiVJDNzYi2pjBL4OXAXaAqXXm+Fs33pQ3dxm+FpT7nkXqejzzWH681V50dCLmZ2q4\n\tduvUmHJ+UDHzljFI4SSm7jEszS5pQl8mrXpibAp47JgaotWgaL2KpEEda4kKyIT+3jbN\n\tKKqg==","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=dwbJCTwZgiVChxox3GHtNljujP1w3ShbKRpvMdYZQLI=;\n\tb=heRKl+IN/AkYsAEQXQEKDkNzlUuV0mMzZlf3/rh31wNYBn2gw4cmdxVFNjKXsLfo1X\n\twww2emefEtNQ8WSZvo3KZbh+t2QBtcsj/ioavkwpZ+9I2eEMubpjWh8LANgcQSrZCcC/\n\tbTOwbVtV4xjWk0/l+N+dehTa2EO9eHRi/oM+dXObZVVFa6TW2XTOVPdiVExpTmgPtWwh\n\twAyaNyUG8ciG5pmrns7uNl436HR43GLlMQes+n1bdBYEUFaEIwnv/KuAqxSJ/FecbGxb\n\t76xWX0jtDCs7UtHrtO9slScfAG/tbr8lyKWrM/31fz0oEnzVomz5GRluS31aCMZqg/Go\n\tTXgw==","X-Gm-Message-State":"APjAAAVD6b9row5F9QX+IXksPlk1LFPapNMU/k0NPTGaYMiM92N5kj3t\n\tkYvx6ufNhqF3tB7V258WRKfbqgEj0jE=","X-Google-Smtp-Source":"APXvYqwZrQ53wAPYyji7m4htcQS1uocvoIERFxx0Zi1+95SNGrxT1a73aoVLSvx9yYfVqWoteTKgBQ==","X-Received":"by 2002:a2e:87d4:: with SMTP id v20mr29597ljj.47.1551374886599; \n\tThu, 28 Feb 2019 09:28:06 -0800 (PST)","Date":"Thu, 28 Feb 2019 18:28:05 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190228172805.GH899@bigcity.dyn.berto.se>","References":"<20190228162913.6508-1-laurent.pinchart@ideasonboard.com>\n\t<20190228162913.6508-6-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190228162913.6508-6-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 05/10] libcamera: pipeline_handler:\n\tStore pipe and camera in CameraData","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":"Thu, 28 Feb 2019 17:28:07 -0000"}}]