[{"id":17115,"web_url":"https://patchwork.libcamera.org/comment/17115/","msgid":"<YKjY+laf2lP0JikN@oden.dyn.berto.se>","date":"2021-05-22T10:12:10","subject":"Re: [libcamera-devel] [PATCH v4 3/5] lc-compliance: Add Environment\n\tsingleton","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Nícolas,\n\nThanks for your patch.\n\nOn 2021-05-21 10:30:52 -0300, Nícolas F. R. A. Prado wrote:\n> Add a singleton Environment class in order to make the camera available\n> inside all tests. This is needed for the Googletest refactor, otherwise\n> the tests, which are statically declared, won't be able to access the\n> camera.\n> \n> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>\n> ---\n> Added in v4\n> \n>  src/lc-compliance/environment.cpp | 35 +++++++++++++++++++++++++++++++\n>  src/lc-compliance/environment.h   | 31 +++++++++++++++++++++++++++\n>  src/lc-compliance/meson.build     |  1 +\n>  3 files changed, 67 insertions(+)\n>  create mode 100644 src/lc-compliance/environment.cpp\n>  create mode 100644 src/lc-compliance/environment.h\n> \n> ronment::instance()diff --git a/src/lc-compliance/environment.cpp \n> b/src/lc-compliance/environment.cpp\n> new file mode 100644\n> index 000000000000..de7fa5bbadd1\n> --- /dev/null\n> +++ b/src/lc-compliance/environment.cpp\n> @@ -0,0 +1,35 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2021, Collabora Ltd.\n> + *\n> + * environment.cpp - Common environment for tests\n> + */\n> +\n> +#include \"environment.h\"\n> +\n> +Environment *Environment::instance_ = nullptr;\n> +std::shared_ptr<Camera> Environment::camera_ = nullptr;\n> +\n> +bool Environment::create(std::shared_ptr<Camera> camera)\n> +{\n> +\tif (instance_)\n> +\t\treturn false;\n> +\n> +\tcamera_ = camera;\n> +\tinstance_ = new Environment;\n> +\treturn true;\n> +}\n> +\n> +void Environment::destroy()\n> +{\n> +\tif (!camera_)\n> +\t\treturn;\n> +\n> +\tcamera_->release();\n> +\tcamera_.reset();\n> +}\n> +\n> +Environment *Environment::instance()\n> +{\n> +\treturn instance_;\n> +}\n\nWould it make sens to refactor this as Environment::get() while making \ninstance_ a shared pointer and keeping camera_ as a non-static member?\n\nstd::shared_ptr<Environment> Environment::instance_ = nullptr;\n\nEnvironment *Environment::get()\n{\n    if (!instance_)\n        instance_ = new Environment;\n\n    return instance_;\n}\n\nvoid Environment::setup(std::shared_ptr<Camera> camera)\n{\n        camera_ = camera;\n}\n\nvoid Environment::destroy()\n{\n\tif (!camera_)\n\t\treturn;\n\n\tcamera_->release();\n\tcamera_.reset();\n}\n\n> diff --git a/src/lc-compliance/environment.h b/src/lc-compliance/environment.h\n> new file mode 100644\n> index 000000000000..f4832d371b6c\n> --- /dev/null\n> +++ b/src/lc-compliance/environment.h\n> @@ -0,0 +1,31 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2021, Collabora Ltd.\n> + *\n> + * environment.h - Common environment for tests\n> + */\n> +#ifndef __LC_COMPLIANCE_ENVIRONMENT_H__\n> +#define __LC_COMPLIANCE_ENVIRONMENT_H__\n> +\n> +#include <libcamera/libcamera.h>\n> +\n> +using namespace libcamera;\n> +\n> +class Environment\n> +{\n> +public:\n> +\tstatic bool create(std::shared_ptr<Camera> camera);\n> +\tvoid destroy();\n> +\n> +\tstatic Environment *instance();\n> +\n> +\tstd::shared_ptr<Camera> camera() const { return camera_; }\n> +\n> +private:\n> +\tEnvironment() {};\n> +\n> +\tstatic Environment *instance_;\n> +\tstatic std::shared_ptr<Camera> camera_;\n> +};\n> +\n> +#endif /* __LC_COMPLIANCE_ENVIRONMENT_H__ */\n> diff --git a/src/lc-compliance/meson.build b/src/lc-compliance/meson.build\n> index a2bfcceb1259..c287017575bd 100644\n> --- a/src/lc-compliance/meson.build\n> +++ b/src/lc-compliance/meson.build\n> @@ -14,6 +14,7 @@ lc_compliance_sources = files([\n>      '../cam/options.cpp',\n>      'main.cpp',\n>      'results.cpp',\n> +    'environment.cpp',\n>      'simple_capture.cpp',\n>      'single_stream.cpp',\n>  ])\n> -- \n> 2.31.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 564B1C3201\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 22 May 2021 10:12:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B85A168918;\n\tSat, 22 May 2021 12:12:12 +0200 (CEST)","from mail-lj1-x231.google.com (mail-lj1-x231.google.com\n\t[IPv6:2a00:1450:4864:20::231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A805260510\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 22 May 2021 12:12:11 +0200 (CEST)","by mail-lj1-x231.google.com with SMTP id t17so10223759ljd.9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 22 May 2021 03:12:11 -0700 (PDT)","from localhost (h-155-4-209-203.A463.priv.bahnhof.se.\n\t[155.4.209.203]) by smtp.gmail.com with ESMTPSA id\n\tt1sm877084lfg.226.2021.05.22.03.12.10\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 22 May 2021 03:12:10 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"Qp5hpOZF\"; dkim-atps=neutral","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\tbh=tyo0HIMTAUf91cV6XTmzl56rh6VqCJX0O2ov/20l8x0=;\n\tb=Qp5hpOZFM7Kb0dhC62mqImqqH9t5BynApfr/jLnrObgTOzOO3RV1sjUSKxQVDs4LCC\n\t8yOG1L01Dl2n9rzZMvLiaJ7vK3MNAm58VoJX1NIzTB+4w/XJ5fWVwLKP/MRlbxDMNGgM\n\twqe9DyK4sWnhLtTtHZ5xiGmHsS4oXNnk0WVlH8/C0d9YhEaniOyAdv+qHnsn+MHV89rk\n\tmbZd16ercTcCENl4iIrmswG1bO4v4agAi62rX6KRdRBzpPBV81IFoD+F0YDfZoeVzKcx\n\tZxPu6EJLIOIoyvi3LMVtUvHpww8PGjsKh5Z+Sz33PGhPZ9lhMWZbm6FuwoiWEgXNGLti\n\tpgjg==","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;\n\tbh=tyo0HIMTAUf91cV6XTmzl56rh6VqCJX0O2ov/20l8x0=;\n\tb=ZxyEmfKDa3M2/xMhn8zVd19Mx21P52A23+Pe8gwL+Ag8q/rNCB4GA0Vm+fqvID8AwS\n\tcejxKLSKMVhc5pSdOdptc7bevbzzqRKo7skZs6Nrb1AeMsXZKTbKNY+dFlqoopv2P0sR\n\tfeV+dAazLwg/i/UwfgzYgSC8SUblNPfbU62enetrGwg5PWWi7N2E3eOMxjl/M/mTHvMU\n\tKqObp+i0jfQU6fKKWgOFY8TcmC2VtKEdqFo7raHx+IbwM5x+AFwww5TXMbrOyXgIUd4o\n\tWgStpQ8RaPq+dG+5pRJ5Pm67578poeohtIB6QG5bOaA8P98yQtXJU3TxIrZOJMI9usTw\n\tHOvA==","X-Gm-Message-State":"AOAM530omU9eJn0anRzuJnATAmXNHLwY+RSV0DSNaoud8/9hJkxKTdti\n\trnCuQLRCUito5+MQne0GqQRziAiekCRc0w==","X-Google-Smtp-Source":"ABdhPJxpo/YZAnjc51lEQa1WFKDcV3c6q5jj06cnVBfh9Xr0j/jqqdOFaquCt15WsZDR7S5dvUj5yw==","X-Received":"by 2002:a2e:a7d3:: with SMTP id\n\tx19mr9838022ljp.120.1621678331117; \n\tSat, 22 May 2021 03:12:11 -0700 (PDT)","Date":"Sat, 22 May 2021 12:12:10 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"=?iso-8859-1?q?N=EDcolas_F=2E_R=2E_A=2E?= Prado <nfraprado@collabora.com>","Message-ID":"<YKjY+laf2lP0JikN@oden.dyn.berto.se>","References":"<20210521133054.274502-1-nfraprado@collabora.com>\n\t<20210521133054.274502-4-nfraprado@collabora.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20210521133054.274502-4-nfraprado@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH v4 3/5] lc-compliance: Add Environment\n\tsingleton","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org, kernel@collabora.com,\n\t=?iso-8859-1?q?Andr=E9?= Almeida <andrealmeid@collabora.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":17144,"web_url":"https://patchwork.libcamera.org/comment/17144/","msgid":"<YKq/2UeL4ghwphYg@pendragon.ideasonboard.com>","date":"2021-05-23T20:49:29","subject":"Re: [libcamera-devel] [PATCH v4 3/5] lc-compliance: Add Environment\n\tsingleton","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Sat, May 22, 2021 at 12:12:10PM +0200, Niklas Söderlund wrote:\n> On 2021-05-21 10:30:52 -0300, Nícolas F. R. A. Prado wrote:\n> > Add a singleton Environment class in order to make the camera available\n> > inside all tests. This is needed for the Googletest refactor, otherwise\n> > the tests, which are statically declared, won't be able to access the\n> > camera.\n> > \n> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>\n> > ---\n> > Added in v4\n> > \n> >  src/lc-compliance/environment.cpp | 35 +++++++++++++++++++++++++++++++\n> >  src/lc-compliance/environment.h   | 31 +++++++++++++++++++++++++++\n> >  src/lc-compliance/meson.build     |  1 +\n> >  3 files changed, 67 insertions(+)\n> >  create mode 100644 src/lc-compliance/environment.cpp\n> >  create mode 100644 src/lc-compliance/environment.h\n> > \n> > ronment::instance()diff --git a/src/lc-compliance/environment.cpp \n> > b/src/lc-compliance/environment.cpp\n> > new file mode 100644\n> > index 000000000000..de7fa5bbadd1\n> > --- /dev/null\n> > +++ b/src/lc-compliance/environment.cpp\n> > @@ -0,0 +1,35 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2021, Collabora Ltd.\n> > + *\n> > + * environment.cpp - Common environment for tests\n> > + */\n> > +\n> > +#include \"environment.h\"\n> > +\n> > +Environment *Environment::instance_ = nullptr;\n> > +std::shared_ptr<Camera> Environment::camera_ = nullptr;\n> > +\n> > +bool Environment::create(std::shared_ptr<Camera> camera)\n> > +{\n> > +\tif (instance_)\n> > +\t\treturn false;\n> > +\n> > +\tcamera_ = camera;\n> > +\tinstance_ = new Environment;\n> > +\treturn true;\n> > +}\n> > +\n> > +void Environment::destroy()\n> > +{\n> > +\tif (!camera_)\n> > +\t\treturn;\n> > +\n> > +\tcamera_->release();\n> > +\tcamera_.reset();\n> > +}\n> > +\n> > +Environment *Environment::instance()\n> > +{\n> > +\treturn instance_;\n> > +}\n> \n> Would it make sens to refactor this as Environment::get() while making \n> instance_ a shared pointer and keeping camera_ as a non-static member?\n\nStoring the camera in the enviroment is better than storing it in a\nglobal variable, yes..\n\n> std::shared_ptr<Environment> Environment::instance_ = nullptr;\n\nI don't think the environment itself needs to be a shared_ptr.\n\n> Environment *Environment::get()\n> {\n>     if (!instance_)\n>         instance_ = new Environment;\n> \n>     return instance_;\n\nYou could also write\n\n\tstatic Environment instance;\n\treturn &instance;\n\nwhich will construct the environment the first time this function is\ncalled. It will get destroyed after main() returns (but in an\nunspecified order relatively to other global variables), so you could\ndrop the destroy() function.\n\n> }\n> \n> void Environment::setup(std::shared_ptr<Camera> camera)\n> {\n>         camera_ = camera;\n> }\n\nNot something that has to be addressed now, but would it be better to\nacquire the camera at the beginning of each test, to ensure that no\nstate is kept between tests ? Only the camera ID would then be stored in\nthe environment.\n\n> void Environment::destroy()\n> {\n> \tif (!camera_)\n> \t\treturn;\n> \n> \tcamera_->release();\n> \tcamera_.reset();\n> }\n> \n> > diff --git a/src/lc-compliance/environment.h b/src/lc-compliance/environment.h\n> > new file mode 100644\n> > index 000000000000..f4832d371b6c\n> > --- /dev/null\n> > +++ b/src/lc-compliance/environment.h\n> > @@ -0,0 +1,31 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2021, Collabora Ltd.\n> > + *\n> > + * environment.h - Common environment for tests\n> > + */\n> > +#ifndef __LC_COMPLIANCE_ENVIRONMENT_H__\n> > +#define __LC_COMPLIANCE_ENVIRONMENT_H__\n> > +\n> > +#include <libcamera/libcamera.h>\n> > +\n> > +using namespace libcamera;\n> > +\n> > +class Environment\n> > +{\n> > +public:\n> > +\tstatic bool create(std::shared_ptr<Camera> camera);\n> > +\tvoid destroy();\n> > +\n> > +\tstatic Environment *instance();\n> > +\n> > +\tstd::shared_ptr<Camera> camera() const { return camera_; }\n> > +\n> > +private:\n> > +\tEnvironment() {};\n> > +\n> > +\tstatic Environment *instance_;\n> > +\tstatic std::shared_ptr<Camera> camera_;\n> > +};\n> > +\n> > +#endif /* __LC_COMPLIANCE_ENVIRONMENT_H__ */\n> > diff --git a/src/lc-compliance/meson.build b/src/lc-compliance/meson.build\n> > index a2bfcceb1259..c287017575bd 100644\n> > --- a/src/lc-compliance/meson.build\n> > +++ b/src/lc-compliance/meson.build\n> > @@ -14,6 +14,7 @@ lc_compliance_sources = files([\n> >      '../cam/options.cpp',\n> >      'main.cpp',\n> >      'results.cpp',\n> > +    'environment.cpp',\n\nCould you please keep these alphatetically sorted ?\n\n> >      'simple_capture.cpp',\n> >      'single_stream.cpp',\n> >  ])","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id CDF76C3200\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 23 May 2021 20:49:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4BF35602B1;\n\tSun, 23 May 2021 22:49:34 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1FCAA601A9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 23 May 2021 22:49:33 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 988332A8;\n\tSun, 23 May 2021 22:49:32 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"bPWF16lE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621802972;\n\tbh=LGXcP5SAEyLKQMT5vjsnVS0Rxz7SG5xSWUgRbHxpIrc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bPWF16lEYOpgv9KON/R7zGH70uIDc8BGaIo9UXvvAw8mRJdbSTFEWwQ6JUUez3neU\n\tpyjDsgbz6Gg5oZOsydwuJMiELMIAmivFh2q89FlHKNWIK8BzVF++XoX3cOVw8TLOE9\n\tGyIPvETzBtiM499oqz5WILIcVQ55/7bR1TSWBw9o=","Date":"Sun, 23 May 2021 23:49:29 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<YKq/2UeL4ghwphYg@pendragon.ideasonboard.com>","References":"<20210521133054.274502-1-nfraprado@collabora.com>\n\t<20210521133054.274502-4-nfraprado@collabora.com>\n\t<YKjY+laf2lP0JikN@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<YKjY+laf2lP0JikN@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH v4 3/5] lc-compliance: Add Environment\n\tsingleton","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org, kernel@collabora.com, =?utf-8?q?A?=\n\t=?utf-8?b?bmRyw6k=?= Almeida <andrealmeid@collabora.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":17212,"web_url":"https://patchwork.libcamera.org/comment/17212/","msgid":"<CBLN6AUQGERO.2LWCZ1J7W1UDG@notapiano>","date":"2021-05-24T17:12:07","subject":"Re: [libcamera-devel] [PATCH v4 3/5] lc-compliance: Add Environment\n\tsingleton","submitter":{"id":84,"url":"https://patchwork.libcamera.org/api/people/84/","name":"Nícolas F. R. A. Prado","email":"nfraprado@collabora.com"},"content":"Hi,\n\nEm 2021-05-23 17:49, Laurent Pinchart escreveu:\n\n> Hello,\n>\n> On Sat, May 22, 2021 at 12:12:10PM +0200, Niklas Söderlund wrote:\n> > On 2021-05-21 10:30:52 -0300, Nícolas F. R. A. Prado wrote:\n> > > Add a singleton Environment class in order to make the camera available\n> > > inside all tests. This is needed for the Googletest refactor, otherwise\n> > > the tests, which are statically declared, won't be able to access the\n> > > camera.\n> > > \n> > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>\n> > > ---\n> > > Added in v4\n> > > \n> > >  src/lc-compliance/environment.cpp | 35 +++++++++++++++++++++++++++++++\n> > >  src/lc-compliance/environment.h   | 31 +++++++++++++++++++++++++++\n> > >  src/lc-compliance/meson.build     |  1 +\n> > >  3 files changed, 67 insertions(+)\n> > >  create mode 100644 src/lc-compliance/environment.cpp\n> > >  create mode 100644 src/lc-compliance/environment.h\n> > > \n> > > ronment::instance()diff --git a/src/lc-compliance/environment.cpp \n> > > b/src/lc-compliance/environment.cpp\n> > > new file mode 100644\n> > > index 000000000000..de7fa5bbadd1\n> > > --- /dev/null\n> > > +++ b/src/lc-compliance/environment.cpp\n> > > @@ -0,0 +1,35 @@\n> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > > +/*\n> > > + * Copyright (C) 2021, Collabora Ltd.\n> > > + *\n> > > + * environment.cpp - Common environment for tests\n> > > + */\n> > > +\n> > > +#include \"environment.h\"\n> > > +\n> > > +Environment *Environment::instance_ = nullptr;\n> > > +std::shared_ptr<Camera> Environment::camera_ = nullptr;\n> > > +\n> > > +bool Environment::create(std::shared_ptr<Camera> camera)\n> > > +{\n> > > +\tif (instance_)\n> > > +\t\treturn false;\n> > > +\n> > > +\tcamera_ = camera;\n> > > +\tinstance_ = new Environment;\n> > > +\treturn true;\n> > > +}\n> > > +\n> > > +void Environment::destroy()\n> > > +{\n> > > +\tif (!camera_)\n> > > +\t\treturn;\n> > > +\n> > > +\tcamera_->release();\n> > > +\tcamera_.reset();\n> > > +}\n> > > +\n> > > +Environment *Environment::instance()\n> > > +{\n> > > +\treturn instance_;\n> > > +}\n> > \n> > Would it make sens to refactor this as Environment::get() while making \n> > instance_ a shared pointer and keeping camera_ as a non-static member?\n>\n> Storing the camera in the enviroment is better than storing it in a\n> global variable, yes..\n>\n> > std::shared_ptr<Environment> Environment::instance_ = nullptr;\n>\n> I don't think the environment itself needs to be a shared_ptr.\n>\n> > Environment *Environment::get()\n> > {\n> >     if (!instance_)\n> >         instance_ = new Environment;\n> > \n> >     return instance_;\n>\n> You could also write\n>\n> static Environment instance;\n> return &instance;\n>\n> which will construct the environment the first time this function is\n> called. It will get destroyed after main() returns (but in an\n> unspecified order relatively to other global variables), so you could\n> drop the destroy() function.\n\nYep, that looks better. I'll change it for v5.\n\n>\n> > }\n> > \n> > void Environment::setup(std::shared_ptr<Camera> camera)\n> > {\n> >         camera_ = camera;\n> > }\n>\n> Not something that has to be addressed now, but would it be better to\n> acquire the camera at the beginning of each test, to ensure that no\n> state is kept between tests ? Only the camera ID would then be stored in\n> the environment.\n\nYeah, that does make sense. The tests should be as independent as possible. I've\nwent ahead and made that change for v5 as well. Was simple enough and it's a\ngood improvement.\n\n>\n> > void Environment::destroy()\n> > {\n> > \tif (!camera_)\n> > \t\treturn;\n> > \n> > \tcamera_->release();\n> > \tcamera_.reset();\n> > }\n> > \n> > > diff --git a/src/lc-compliance/environment.h b/src/lc-compliance/environment.h\n> > > new file mode 100644\n> > > index 000000000000..f4832d371b6c\n> > > --- /dev/null\n> > > +++ b/src/lc-compliance/environment.h\n> > > @@ -0,0 +1,31 @@\n> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > > +/*\n> > > + * Copyright (C) 2021, Collabora Ltd.\n> > > + *\n> > > + * environment.h - Common environment for tests\n> > > + */\n> > > +#ifndef __LC_COMPLIANCE_ENVIRONMENT_H__\n> > > +#define __LC_COMPLIANCE_ENVIRONMENT_H__\n> > > +\n> > > +#include <libcamera/libcamera.h>\n> > > +\n> > > +using namespace libcamera;\n> > > +\n> > > +class Environment\n> > > +{\n> > > +public:\n> > > +\tstatic bool create(std::shared_ptr<Camera> camera);\n> > > +\tvoid destroy();\n> > > +\n> > > +\tstatic Environment *instance();\n> > > +\n> > > +\tstd::shared_ptr<Camera> camera() const { return camera_; }\n> > > +\n> > > +private:\n> > > +\tEnvironment() {};\n> > > +\n> > > +\tstatic Environment *instance_;\n> > > +\tstatic std::shared_ptr<Camera> camera_;\n> > > +};\n> > > +\n> > > +#endif /* __LC_COMPLIANCE_ENVIRONMENT_H__ */\n> > > diff --git a/src/lc-compliance/meson.build b/src/lc-compliance/meson.build\n> > > index a2bfcceb1259..c287017575bd 100644\n> > > --- a/src/lc-compliance/meson.build\n> > > +++ b/src/lc-compliance/meson.build\n> > > @@ -14,6 +14,7 @@ lc_compliance_sources = files([\n> > >      '../cam/options.cpp',\n> > >      'main.cpp',\n> > >      'results.cpp',\n> > > +    'environment.cpp',\n>\n> Could you please keep these alphatetically sorted ?\n\nSure.\n\nThanks,\nNícolas\n\n>\n> > >      'simple_capture.cpp',\n> > >      'single_stream.cpp',\n> > >  ])\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n> --\n> To unsubscribe, send mail to kernel-unsubscribe@lists.collabora.co.uk.","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E5C3FC3200\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 May 2021 17:12:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6D19168919;\n\tMon, 24 May 2021 19:12:49 +0200 (CEST)","from bhuna.collabora.co.uk (bhuna.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 65960601AA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 May 2021 19:12:48 +0200 (CEST)","from [127.0.0.1] (localhost [127.0.0.1])\n\t(Authenticated sender: nfraprado) with ESMTPSA id A9F541F420EA"],"Mime-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","Content-Type":"text/plain; charset=UTF-8","Date":"Mon, 24 May 2021 14:12:07 -0300","Message-Id":"<CBLN6AUQGERO.2LWCZ1J7W1UDG@notapiano>","To":"\"Laurent Pinchart\" <laurent.pinchart@ideasonboard.com>, =?utf-8?q?Nikl?=\n\t=?utf-8?q?as_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","From":"=?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= <nfraprado@collabora.com>","References":"<20210521133054.274502-1-nfraprado@collabora.com>\n\t<20210521133054.274502-4-nfraprado@collabora.com>\n\t<YKjY+laf2lP0JikN@oden.dyn.berto.se>\n\t<YKq/2UeL4ghwphYg@pendragon.ideasonboard.com>","In-Reply-To":"<YKq/2UeL4ghwphYg@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 3/5] lc-compliance: Add Environment\n\tsingleton","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org, kernel@collabora.com, =?utf-8?q?A?=\n\t=?utf-8?q?ndr=C3=A9_Almeida?= <andrealmeid@collabora.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]