[{"id":774,"web_url":"https://patchwork.libcamera.org/comment/774/","msgid":"<20190210130000.GI32622@bigcity.dyn.berto.se>","date":"2019-02-10T13:00:00","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline_handler: Reorder\n\tmember declaration order","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-09 01:01:38 +0200, Laurent Pinchart wrote:\n> Reorder the member declaration order in the PipelineHandler class to\n> match the control flow order, and to declare variables after methods\n> according to the coding style. Update the documentation accordingly,\n> preserving the order within the public, protected and private sections,\n> but grouping related methods together between the sections.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> src/libcamera/include/pipeline_handler.h | 10 +--\n> src/libcamera/pipeline_handler.cpp | 101 +++++++++++------------\n> 2 files changed, 55 insertions(+), 56 deletions(-)\n> \n> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> index 7f2ec2975db0..4363dcd8ed8e 100644\n> --- a/src/libcamera/include/pipeline_handler.h\n> +++ b/src/libcamera/include/pipeline_handler.h\n> @@ -42,6 +42,8 @@ public:\n> \tPipelineHandler(CameraManager *manager);\n> \tvirtual ~PipelineHandler();\n> \n> +\tvirtual bool match(DeviceEnumerator *enumerator) = 0;\n> +\n> \tvirtual std::map\n> \tstreamConfiguration(Camera *camera, std::vector &streams) = 0;\n> \tvirtual int configureStreams(Camera *camera,\n> @@ -55,20 +57,18 @@ public:\n> \n> \tvirtual int queueRequest(const Camera *camera, Request *request) = 0;\n> \n> -\tvirtual bool match(DeviceEnumerator *enumerator) = 0;\n> -\n> protected:\n> -\tCameraManager *manager_;\n> -\n> \tvoid registerCamera(std::shared_ptr camera);\n> \tvoid hotplugMediaDevice(MediaDevice *media);\n> \n> \tCameraData *cameraData(const Camera *camera);\n> \tvoid setCameraData(const Camera *camera, std::unique_ptr data);\n> \n> +\tCameraManager *manager_;\n> +\n> private:\n> -\tvirtual void disconnect();\n> \tvoid mediaDeviceDisconnected(MediaDevice *media);\n> +\tvirtual void disconnect();\n> \n> \tstd::vector> cameras_;\n> \tstd::map> cameraData_;\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index cc2d4643ec2f..4e111d6d2f55 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -75,6 +75,36 @@ PipelineHandler::~PipelineHandler()\n> {\n> };\n> \n> +/**\n> + * \\fn PipelineHandler::match(DeviceEnumerator *enumerator)\n> + * \\brief Match media devices and create camera instances\n> + * \\param enumerator The enumerator providing all media devices found in the\n> + * system\n> + *\n> + * This function is the main entry point of the pipeline handler. It is called\n> + * by the camera manager with the \\a enumerator passed as an argument. It shall\n> + * acquire from the \\a enumerator all the media devices it needs for a single\n> + * pipeline, create one or multiple Camera instances and register them with the\n> + * camera manager.\n> + *\n> + * If all media devices needed by the pipeline handler are found, they must all\n> + * be acquired by a call to MediaDevice::acquire(). This function shall then\n> + * create the corresponding Camera instances, store them internally, and return\n> + * true. Otherwise it shall not acquire any media device (or shall release all\n> + * the media devices is has acquired by calling MediaDevice::release()) and\n> + * return false.\n> + *\n> + * If multiple instances of a pipeline are available in the system, the\n> + * PipelineHandler class will be instanciated once per instance, and its match()\n> + * function called for every instance. Each call shall acquire media devices for\n> + * one pipeline instance, until all compatible media devices are exhausted.\n> + *\n> + * If this function returns true, a new instance of the pipeline handler will\n> + * be created and its match() function called.\n> + *\n> + * \\return true if media devices have been acquired and camera instances\n> + * created, or false otherwise\n> + */\n> \n> /**\n> * \\fn PipelineHandler::streamConfiguration()\n> @@ -176,46 +206,6 @@ PipelineHandler::~PipelineHandler()\n> * \\return 0 on success or a negative error code on error\n> */\n> \n> -/**\n> - * \\fn PipelineHandler::match(DeviceEnumerator *enumerator)\n> - * \\brief Match media devices and create camera instances\n> - * \\param enumerator The enumerator providing all media devices found in the\n> - * system\n> - *\n> - * This function is the main entry point of the pipeline handler. It is called\n> - * by the camera manager with the \\a enumerator passed as an argument. It shall\n> - * acquire from the \\a enumerator all the media devices it needs for a single\n> - * pipeline, create one or multiple Camera instances and register them with the\n> - * camera manager.\n> - *\n> - * If all media devices needed by the pipeline handler are found, they must all\n> - * be acquired by a call to MediaDevice::acquire(). This function shall then\n> - * create the corresponding Camera instances, store them internally, and return\n> - * true. Otherwise it shall not acquire any media device (or shall release all\n> - * the media devices is has acquired by calling MediaDevice::release()) and\n> - * return false.\n> - *\n> - * If multiple instances of a pipeline are available in the system, the\n> - * PipelineHandler class will be instanciated once per instance, and its match()\n> - * function called for every instance. Each call shall acquire media devices for\n> - * one pipeline instance, until all compatible media devices are exhausted.\n> - *\n> - * If this function returns true, a new instance of the pipeline handler will\n> - * be created and its match() function called.\n> - *\n> - * \\return true if media devices have been acquired and camera instances\n> - * created, or false otherwise\n> - */\n> -\n> -/**\n> - * \\var PipelineHandler::manager_\n> - * \\brief The Camera manager associated with the pipeline handler\n> - *\n> - * The camera manager pointer is stored in the pipeline handler for the\n> - * convenience of pipeline handler implementations. It remains valid and\n> - * constant for the whole lifetime of the pipeline handler.\n> - */\n> -\n> /**\n> * \\brief Register a camera to the camera manager and pipeline handler\n> * \\param[in] camera The camera to be added\n> @@ -246,6 +236,17 @@ void PipelineHandler::hotplugMediaDevice(MediaDevice *media)\n> \tmedia->disconnected.connect(this, &PipelineHandler::mediaDeviceDisconnected);\n> }\n> \n> +/**\n> + * \\brief Slot for the MediaDevice disconnected signal\n> + */\n> +void PipelineHandler::mediaDeviceDisconnected(MediaDevice *media)\n> +{\n> +\tif (cameras_.empty())\n> +\t\treturn;\n> +\n> +\tdisconnect();\n> +}\n> +\n> /**\n> * \\brief Device disconnection handler\n> *\n> @@ -272,17 +273,6 @@ void PipelineHandler::disconnect()\n> \tcameras_.clear();\n> }\n> \n> -/**\n> - * \\brief Slot for the MediaDevice disconnected signal\n> - */\n> -void PipelineHandler::mediaDeviceDisconnected(MediaDevice *media)\n> -{\n> -\tif (cameras_.empty())\n> -\t\treturn;\n> -\n> -\tdisconnect();\n> -}\n> -\n> /**\n> * \\brief Retrieve the pipeline-specific data associated with a Camera\n> * \\param camera The camera whose data to retrieve\n> @@ -331,6 +321,15 @@ void PipelineHandler::setCameraData(const Camera *camera,\n> \tcameraData_[camera] = std::move(data);\n> }\n> \n> +/**\n> + * \\var PipelineHandler::manager_\n> + * \\brief The Camera manager associated with the pipeline handler\n> + *\n> + * The camera manager pointer is stored in the pipeline handler for the\n> + * convenience of pipeline handler implementations. It remains valid and\n> + * constant for the whole lifetime of the pipeline handler.\n> + */\n> +\n> /**\n> * \\class PipelineHandlerFactory\n> * \\brief Registration of PipelineHandler classes and creation of instances\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-x234.google.com (mail-lj1-x234.google.com\n\t[IPv6:2a00:1450:4864:20::234])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B2DB9610B1\n\tfor ;\n\tSun, 10 Feb 2019 14:00:03 +0100 (CET)","by mail-lj1-x234.google.com with SMTP id v12-v6so1881200ljc.6\n\tfor ;\n\tSun, 10 Feb 2019 05:00:03 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tf25-v6sm991193ljg.19.2019.02.10.05.00.01\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSun, 10 Feb 2019 05:00:01 -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=6ZZrBcWz/oqE+HIoREGAgtFuJFYr1qjepibp9iDZyvA=;\n\tb=L0N12Xv4ODklmgYc9RWKMWERicYe3JB9xRqkEn2DRqMrBfL8Sx8tNvQTshKclJxl5Z\n\tD/ndzKLvkV0gvdH6txVFGIaGOEtOLTTBWJGidp/KHaOMCw62QNksGUU0c08AJmck789l\n\ti5XU/cRIZuB4C8ohesaocdlTu/kw7pT1lzmEDhCmk6DgCWm1o1B1bbsCuYuESR+hNRv4\n\toqpgl4HqTaGTU5soFP1/ZW4I7fP6YxngUPQyHZgAovIHoYohvcBrMvVFNvE24rcKbplu\n\t5/vd8I15rom1kW4SAqsuaPUyODpjacT4N0M9PX9FD8GKy5ORNPd7z8it8DGqPfHhln+E\n\tmJmg==","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=6ZZrBcWz/oqE+HIoREGAgtFuJFYr1qjepibp9iDZyvA=;\n\tb=L06d1krYVjB0Y8XS6I7bkF07MzngVqTcCjYobggMpiMgQ4Gd9UvT/Lquyjwt9HWvDl\n\tq09amw9LWmsdeAfYyDkA5R1wr17ZeADgZUF098JzQXLQdDyfV7ORpIpMB0zbbMs9Wy3h\n\tQSRDcsnnpjCldJIAMYW1/OJDwH7WARy68CYKg8U+hIwSHeEIB2XkmH1iaNganmlpn28q\n\tiJwFEwEFUgtSh4cPArlNhJcN3q0pQYXkbt4nwg0zx/QCmDzKRLSzBmK6+gBwomK2HGjs\n\t4rs9kuvcd/2bJxW4rwOKraxjdBuF6N5uDVPE3WZVFECkmgR1/WVVmJ6HdTf2qMJBKbAT\n\tu8tA==","X-Gm-Message-State":"AHQUAuYzkn53YndIJTkt3kGnYQVVdd1nDjEvxBY2WynI45rAfhEDqi9D\n\tOERIstwIcWhGturksbWBfOZWyQ==","X-Google-Smtp-Source":"AHgI3IZgdPPLh/HP+CvQ+vOEzQ0cXfTQMkLpxdOKZ1Ura2yfNRTq9NAxoXf8aJNjZDmgwaQYO4tn6A==","X-Received":"by 2002:a2e:c41:: with SMTP id\n\to1-v6mr9426962ljd.152.1549803602669; \n\tSun, 10 Feb 2019 05:00:02 -0800 (PST)","Date":"Sun, 10 Feb 2019 14:00:00 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190210130000.GI32622@bigcity.dyn.berto.se>","References":"<20190208230138.8182-1-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":"<20190208230138.8182-1-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline_handler: Reorder\n\tmember declaration order","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, 10 Feb 2019 13:00:04 -0000"}}]