[{"id":1030,"web_url":"https://patchwork.libcamera.org/comment/1030/","msgid":"<20190310132632.GF4814@pendragon.ideasonboard.com>","date":"2019-03-10T13:26:32","subject":"Re: [libcamera-devel] [PATCH 2/5] test: camera: Add read default\n\tformat test","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 Wed, Mar 06, 2019 at 03:47:52AM +0100, Niklas Söderlund wrote:\n> Add a test to verify reading the default format from a camera works.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> test/camera/camera_test.cpp | 47 ++++++++++++++++++++++\n> test/camera/camera_test.h | 32 +++++++++++++++\n> test/camera/format_default.cpp | 71 ++++++++++++++++++++++++++++++++++\n> test/camera/meson.build | 12 ++++++\n> test/meson.build | 1 +\n> 5 files changed, 163 insertions(+)\n> create mode 100644 test/camera/camera_test.cpp\n> create mode 100644 test/camera/camera_test.h\n> create mode 100644 test/camera/format_default.cpp\n> create mode 100644 test/camera/meson.build\n> \n> diff --git a/test/camera/camera_test.cpp b/test/camera/camera_test.cpp\n> new file mode 100644\n> index 0000000000000000..d39a0ed066665946\n> --- /dev/null\n> +++ b/test/camera/camera_test.cpp\n> @@ -0,0 +1,47 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * libcamera Camera API tests\n> + */\n> +\n> +#include \n> +\n> +#include \"camera_test.h\"\n> +\n> +using namespace std;\n> +using namespace libcamera;\n\nAlphabetically sorted please.\n\n> +\n> +int CameraTest::init()\n> +{\n> +\tcm_ = CameraManager::instance();\n> +\n> +\tif (cm_->start()) {\n> +\t\tcout << \"Failed to start camera manager\" << endl;\n> +\t\treturn TestFail;\n> +\t}\n> +\n> +\tcamera_ = cm_->get(\"VIMC Sensor B\");\n> +\tif (!camera_) {\n> +\t\tcout << \"Can not find VIMC camera, skip.\" << endl;\n> +\t\treturn TestSkip;\n> +\t}\n> +\n> +\t/* Sanity check that camera have streams. */\n\ns/camera have/the camera has/\n\n> +\tif (camera_->streams().size() == 0) {\n\nempty() instead of size() == 0 ?\n\n> +\t\tcout << \"Camera have no streams, fail.\" << endl;\n\ns/have no streams/has no stream/\n\nDo we need the \"fail\" suffix ? Same for the skip suffix in the previous\nmessage.\n\n> +\t\treturn TestFail;\n> +\t}\n> +\n> +\treturn TestPass;\n> +}\n> +\n> +void CameraTest::cleanup()\n> +{\n> +\tif (camera_) {\n> +\t\tcamera_->release();\n> +\t\tcamera_.reset();\n\nIs the reset needed ?\n\n> +\t}\n> +\n> +\tcm_->stop();\n> +};\n> diff --git a/test/camera/camera_test.h b/test/camera/camera_test.h\n> new file mode 100644\n> index 0000000000000000..4aadd027675a5dbd\n> --- /dev/null\n> +++ b/test/camera/camera_test.h\n> @@ -0,0 +1,32 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * camera_test.h - libcamera camera test base class\n> + */\n> +#ifndef __LIBCAMERA_CAMERA_TEST_H_\n> +#define __LIBCAMERA_CAMERA_TEST_H_\n\nDouble underscore at the end ? Same for the #endif down below.\n\n> +\n> +#include \n> +\n> +#include \"test.h\"\n> +\n> +using namespace libcamera;\n> +\n> +class CameraTest : public Test\n> +{\n> +public:\n> +\tCameraTest()\n> +\t\t: cm_(nullptr){};\n\nSpace before {} and no need for ;\n\n> +\n> +protected:\n> +\tint init();\n> +\tvoid cleanup();\n> +\n> +\tstd::shared_ptr camera_;\n> +\n> +private:\n> +\tCameraManager *cm_;\n> +};\n> +\n> +#endif /* __LIBCAMERA_CAMERA_TEST_H_ */\n> diff --git a/test/camera/format_default.cpp b/test/camera/format_default.cpp\n> new file mode 100644\n> index 0000000000000000..11b947f44c39b40a\n> --- /dev/null\n> +++ b/test/camera/format_default.cpp\n> @@ -0,0 +1,71 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * libcamera Camera API tests\n> + */\n> +\n> +#include \n> +\n> +#include \"camera_test.h\"\n> +\n> +using namespace std;\n> +\n> +namespace {\n> +\n> +class FormatDefault : public CameraTest\n> +{\n> +protected:\n> +\tint run()\n> +\t{\n> +\t\tstd::map conf;\n> +\t\tstd::set streams = { *camera_->streams().begin() };\n> +\n> +\t\t/*\n> +\t\t * Test that asking for default format for a valid array of\n> +\t\t * streams returns formats and that the formats are somewhat\n> +\t\t * sane.\n\nIt's not just a default format, but a default configuration. Could you\nrename that through the whole patch ?\n\n> +\t\t */\n> +\t\tconf = camera_->streamConfiguration(streams);\n> +\t\tif (!conf.size()) {\n\n.empty() ?\n\n> +\t\t\tcout << \"Default format for valid streams test failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tStreamConfiguration *sconf = &conf.begin()->second;\n> +\t\tif (sconf->width == 0 || sconf->height == 0 ||\n> +\t\t sconf->pixelFormat == 0 || sconf->bufferCount == 0) {\n> +\t\t\tcout << \"Default format is set test failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n\nShould you also test that conf contains a single element, with the same\nstream as the one passed in streams ? You could make that generic for\nany number of streams, and extract the code to a separate method moved\nto the CameraTest class to make it reusable by multi-streams tests.\n\n> +\n> +\t\t/*\n> +\t\t * Test that asking for format for an empty array of streams\n> +\t\t * returns an empty list of configurations.\n> +\t\t */\n> +\t\tstd::set streams_empty = {};\n\nYou can reuse the streams variable declared above.\n\n> +\t\tconf = camera_->streamConfiguration(streams_empty);\n> +\t\tif (conf.size()) {\n\n!.empty()\n\n> +\t\t\tcout << \"Default format for empty streams test failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/*\n> +\t\t * Test that asking for format for an array of streams bad streams\n> +\t\t * returns an empty list of configurations.\n> +\t\t */\n> +\t\tStream *stream_bad = *streams.end();\n\nIdeally you should pass a stream pointer for another camera instance.\nOtherwise you can just pass a random value. *streams.end() isn't a good\nidea, as according to the std::set documentation \"This element acts as a\nplaceholder; attempting to access it results in undefined behavior\".\n\n> +\t\tstd::set streams_bad = { stream_bad };\n> +\t\tconf = camera_->streamConfiguration(streams_bad);\n> +\t\tif (conf.size()) {\n\n!.empty()\n\n> +\t\t\tcout << \"Default format for bad streams test failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +};\n> +\n> +} /* namespace */\n> +\n> +TEST_REGISTER(FormatDefault);\n> diff --git a/test/camera/meson.build b/test/camera/meson.build\n> new file mode 100644\n> index 0000000000000000..4f2ed244a9240512\n> --- /dev/null\n> +++ b/test/camera/meson.build\n> @@ -0,0 +1,12 @@\n> +# Tests are listed in order of complexity.\n> +# They are not alphabetically sorted.\n> +camera_tests = [\n> + [ 'format_default', 'format_default.cpp' ],\n> +]\n> +\n> +foreach t : camera_tests\n> + exe = executable(t[0], [t[1], 'camera_test.cpp'],\n> +\t\t link_with : test_libraries,\n> +\t\t include_directories : test_includes_internal)\n> + test(t[0], exe, suite: 'camera', is_parallel: false)\n> +endforeach\n> diff --git a/test/meson.build b/test/meson.build\n> index 5fb16fa6afb62f8d..71a96921697c0e9e 100644\n> --- a/test/meson.build\n> +++ b/test/meson.build\n> @@ -1,5 +1,6 @@\n> subdir('libtest')\n> \n> +subdir('camera')\n> subdir('media_device')\n> subdir('pipeline')\n> subdir('v4l2_device')","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 271DA600CB\n\tfor ;\n\tSun, 10 Mar 2019 14:26:39 +0100 (CET)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 98406255;\n\tSun, 10 Mar 2019 14:26:38 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1552224398;\n\tbh=lp056GBjaByAKlP3m17yR4anFRDBdAzBa50+k8muUjQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kS8QXhBep8T/Q7zBbj3x3Cn8/lz5CHOo/y+WuCNTDiYd1bybiP+exK3A6lnTNJCGA\n\tkvIpPMAwKa7cSW/t7nCo6dPM1HtCV9jocXCrLNygFy2GZYDxcpFy3ABV2/JCTxir04\n\ttd8TshrAOBzrXSq5JGwebQnXw3MNNo7Zaz0/C8Lo=","Date":"Sun, 10 Mar 2019 15:26:32 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190310132632.GF4814@pendragon.ideasonboard.com>","References":"<20190306024755.28726-1-niklas.soderlund@ragnatech.se>\n\t<20190306024755.28726-3-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":"<20190306024755.28726-3-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 2/5] test: camera: Add read default\n\tformat test","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 Mar 2019 13:26:39 -0000"}},{"id":1035,"web_url":"https://patchwork.libcamera.org/comment/1035/","msgid":"<20190311004323.GE5281@bigcity.dyn.berto.se>","date":"2019-03-11T00:43:24","subject":"Re: [libcamera-devel] [PATCH 2/5] test: camera: Add read default\n\tformat test","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-03-10 15:26:32 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Wed, Mar 06, 2019 at 03:47:52AM +0100, Niklas Söderlund wrote:\n> > Add a test to verify reading the default format from a camera works.\n> > \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > test/camera/camera_test.cpp | 47 ++++++++++++++++++++++\n> > test/camera/camera_test.h | 32 +++++++++++++++\n> > test/camera/format_default.cpp | 71 ++++++++++++++++++++++++++++++++++\n> > test/camera/meson.build | 12 ++++++\n> > test/meson.build | 1 +\n> > 5 files changed, 163 insertions(+)\n> > create mode 100644 test/camera/camera_test.cpp\n> > create mode 100644 test/camera/camera_test.h\n> > create mode 100644 test/camera/format_default.cpp\n> > create mode 100644 test/camera/meson.build\n> > \n> > diff --git a/test/camera/camera_test.cpp b/test/camera/camera_test.cpp\n> > new file mode 100644\n> > index 0000000000000000..d39a0ed066665946\n> > --- /dev/null\n> > +++ b/test/camera/camera_test.cpp\n> > @@ -0,0 +1,47 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * libcamera Camera API tests\n> > + */\n> > +\n> > +#include \n> > +\n> > +#include \"camera_test.h\"\n> > +\n> > +using namespace std;\n> > +using namespace libcamera;\n> \n> Alphabetically sorted please.\n\nThanks for spotting this.\n\n> \n> > +\n> > +int CameraTest::init()\n> > +{\n> > +\tcm_ = CameraManager::instance();\n> > +\n> > +\tif (cm_->start()) {\n> > +\t\tcout << \"Failed to start camera manager\" << endl;\n> > +\t\treturn TestFail;\n> > +\t}\n> > +\n> > +\tcamera_ = cm_->get(\"VIMC Sensor B\");\n> > +\tif (!camera_) {\n> > +\t\tcout << \"Can not find VIMC camera, skip.\" << endl;\n> > +\t\treturn TestSkip;\n> > +\t}\n> > +\n> > +\t/* Sanity check that camera have streams. */\n> \n> s/camera have/the camera has/\n> \n> > +\tif (camera_->streams().size() == 0) {\n> \n> empty() instead of size() == 0 ?\n\nYes that is easier to read.\n\n> \n> > +\t\tcout << \"Camera have no streams, fail.\" << endl;\n> \n> s/have no streams/has no stream/\n> \n> Do we need the \"fail\" suffix ? Same for the skip suffix in the previous\n> message.\n\nGood point, I will drop it thru out this series.\n\n> \n> > +\t\treturn TestFail;\n> > +\t}\n> > +\n> > +\treturn TestPass;\n> > +}\n> > +\n> > +void CameraTest::cleanup()\n> > +{\n> > +\tif (camera_) {\n> > +\t\tcamera_->release();\n> > +\t\tcamera_.reset();\n> \n> Is the reset needed ?\n\nNo it's not, will remove for v2.\n\n> \n> > +\t}\n> > +\n> > +\tcm_->stop();\n> > +};\n> > diff --git a/test/camera/camera_test.h b/test/camera/camera_test.h\n> > new file mode 100644\n> > index 0000000000000000..4aadd027675a5dbd\n> > --- /dev/null\n> > +++ b/test/camera/camera_test.h\n> > @@ -0,0 +1,32 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * camera_test.h - libcamera camera test base class\n> > + */\n> > +#ifndef __LIBCAMERA_CAMERA_TEST_H_\n> > +#define __LIBCAMERA_CAMERA_TEST_H_\n> \n> Double underscore at the end ? Same for the #endif down below.\n\nGood catch, thanks for noticing.\n\n> \n> > +\n> > +#include \n> > +\n> > +#include \"test.h\"\n> > +\n> > +using namespace libcamera;\n> > +\n> > +class CameraTest : public Test\n> > +{\n> > +public:\n> > +\tCameraTest()\n> > +\t\t: cm_(nullptr){};\n> \n> Space before {} and no need for ;\n\nOdd that checkstyle.py did not complain about this, thanks for setting \nme straight.\n\n> \n> > +\n> > +protected:\n> > +\tint init();\n> > +\tvoid cleanup();\n> > +\n> > +\tstd::shared_ptr camera_;\n> > +\n> > +private:\n> > +\tCameraManager *cm_;\n> > +};\n> > +\n> > +#endif /* __LIBCAMERA_CAMERA_TEST_H_ */\n> > diff --git a/test/camera/format_default.cpp b/test/camera/format_default.cpp\n> > new file mode 100644\n> > index 0000000000000000..11b947f44c39b40a\n> > --- /dev/null\n> > +++ b/test/camera/format_default.cpp\n> > @@ -0,0 +1,71 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * libcamera Camera API tests\n> > + */\n> > +\n> > +#include \n> > +\n> > +#include \"camera_test.h\"\n> > +\n> > +using namespace std;\n> > +\n> > +namespace {\n> > +\n> > +class FormatDefault : public CameraTest\n> > +{\n> > +protected:\n> > +\tint run()\n> > +\t{\n> > +\t\tstd::map conf;\n> > +\t\tstd::set streams = { *camera_->streams().begin() };\n> > +\n> > +\t\t/*\n> > +\t\t * Test that asking for default format for a valid array of\n> > +\t\t * streams returns formats and that the formats are somewhat\n> > +\t\t * sane.\n> \n> It's not just a default format, but a default configuration. Could you\n> rename that through the whole patch ?\n\nDone.\n\n> \n> > +\t\t */\n> > +\t\tconf = camera_->streamConfiguration(streams);\n> > +\t\tif (!conf.size()) {\n> \n> .empty() ?\n\nBetter.\n\n> \n> > +\t\t\tcout << \"Default format for valid streams test failed\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tStreamConfiguration *sconf = &conf.begin()->second;\n> > +\t\tif (sconf->width == 0 || sconf->height == 0 ||\n> > +\t\t sconf->pixelFormat == 0 || sconf->bufferCount == 0) {\n> > +\t\t\tcout << \"Default format is set test failed\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> \n> Should you also test that conf contains a single element, with the same\n> stream as the one passed in streams ? You could make that generic for\n> any number of streams, and extract the code to a separate method moved\n> to the CameraTest class to make it reusable by multi-streams tests.\n\nGood idea, I will give it a short in v2.\n\n> \n> > +\n> > +\t\t/*\n> > +\t\t * Test that asking for format for an empty array of streams\n> > +\t\t * returns an empty list of configurations.\n> > +\t\t */\n> > +\t\tstd::set streams_empty = {};\n> \n> You can reuse the streams variable declared above.\n\nI could, but I would prefer to keep one named streams_empty for clarity. \nIf you feel strongly about it I'm willing to budge.\n\n> \n> > +\t\tconf = camera_->streamConfiguration(streams_empty);\n> > +\t\tif (conf.size()) {\n> \n> !.empty()\n\nDone.\n\n> \n> > +\t\t\tcout << \"Default format for empty streams test failed\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/*\n> > +\t\t * Test that asking for format for an array of streams bad streams\n> > +\t\t * returns an empty list of configurations.\n> > +\t\t */\n> > +\t\tStream *stream_bad = *streams.end();\n> \n> Ideally you should pass a stream pointer for another camera instance.\n> Otherwise you can just pass a random value. *streams.end() isn't a good\n> idea, as according to the std::set documentation \"This element acts as a\n> placeholder; attempting to access it results in undefined behavior\".\n\nI agree a stream from another camera would be nice, but I fear the extra \nrequirement of finding another camera in this test case would be to much \ncode for to little gain. I'm will go for 0xdeadbeef in v2.\n\n> \n> > +\t\tstd::set streams_bad = { stream_bad };\n> > +\t\tconf = camera_->streamConfiguration(streams_bad);\n> > +\t\tif (conf.size()) {\n> \n> !.empty()\n\nDone.\n\n> \n> > +\t\t\tcout << \"Default format for bad streams test failed\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +};\n> > +\n> > +} /* namespace */\n> > +\n> > +TEST_REGISTER(FormatDefault);\n> > diff --git a/test/camera/meson.build b/test/camera/meson.build\n> > new file mode 100644\n> > index 0000000000000000..4f2ed244a9240512\n> > --- /dev/null\n> > +++ b/test/camera/meson.build\n> > @@ -0,0 +1,12 @@\n> > +# Tests are listed in order of complexity.\n> > +# They are not alphabetically sorted.\n> > +camera_tests = [\n> > + [ 'format_default', 'format_default.cpp' ],\n> > +]\n> > +\n> > +foreach t : camera_tests\n> > + exe = executable(t[0], [t[1], 'camera_test.cpp'],\n> > +\t\t link_with : test_libraries,\n> > +\t\t include_directories : test_includes_internal)\n> > + test(t[0], exe, suite: 'camera', is_parallel: false)\n> > +endforeach\n> > diff --git a/test/meson.build b/test/meson.build\n> > index 5fb16fa6afb62f8d..71a96921697c0e9e 100644\n> > --- a/test/meson.build\n> > +++ b/test/meson.build\n> > @@ -1,5 +1,6 @@\n> > subdir('libtest')\n> > \n> > +subdir('camera')\n> > subdir('media_device')\n> > subdir('pipeline')\n> > subdir('v4l2_device')\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["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 EDA18600F9\n\tfor ;\n\tMon, 11 Mar 2019 01:43:25 +0100 (CET)","by mail-lj1-x231.google.com with SMTP id v10so2506783lji.3\n\tfor ;\n\tSun, 10 Mar 2019 17:43:25 -0700 (PDT)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tb23sm862836lff.76.2019.03.10.17.43.24\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSun, 10 Mar 2019 17:43:24 -0700 (PDT)"],"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=2+peNeBjqm214I1HHgQeLI0qiigmlwauvjfdXZhZT7c=;\n\tb=12zhZ2JrqBLCNmrJl9rkhYefxDjpGnarQEZYBo3XLR0Ek9yiyNjtDaphRpZBv50pp0\n\tsLOMmCvtAjHGhaQTNtiwd27ykZWYxWYGULamiV1A8SW5Ht7HJGNBjP2mhKtPm0HdRkgu\n\tfHim3j6upGaj65VwdvrZUsQSxStnm/KEjYAH786ztjEr/XnSJL7sCP5AEQ7P+w7Hb0CX\n\tUvAb15W6/ESjn2HzOSch65vGwKXSXvoDz4+hf1fBLSB2HOdTNi84q6XTrXJOg1iNnyUx\n\tWJgn33wevt55o/FryMjvC9b4YdLB0kegv3k3tHsTK9/VU1SPDoRYTHQOEhDaq8xRAsGW\n\tPiQA==","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=2+peNeBjqm214I1HHgQeLI0qiigmlwauvjfdXZhZT7c=;\n\tb=Q9Zn1UDxGu3yfPeC6ztghEJ+MGC5KVT2aPVAskgl9hxT2JPIRGj4m3T5kfTP69k8lw\n\taNfJJB0Mx96pc3PjoWw3wUjt78UCW6RzanjMODtaXbLGkWS1vN2cmDfp9+G+tLOaC3n0\n\tXNcvWOzIPL6++RPaX9jMc9YnW2as4kHJ5yvwZ5HzzC9b3VdO0GmZvDG+l1LVdvxtek8T\n\tkgIGFMOI00XcjxDK4Ych0F7t4fwLKzqFu0qMDu5N7H5odK3fJFGr2CJLY3pCc6pBxtv/\n\tUkhjj6d+Sx8RehzHnWxi5ZaWzRSdTmgbF1A3CrD64cP8FeGBhp2OjdXdr9rV9mBofGzx\n\tis8g==","X-Gm-Message-State":"APjAAAXAZPHVbj4eFUcVIXXZmV0cl7vwZXuG+e4apui8/CAuB/T29gel\n\t1nhauuRNpqaHMTtkyZcAfS4QgXbwalM=","X-Google-Smtp-Source":"APXvYqyZCq6ZC0A2Op+/Oxaq4GY/rgZc6XXiap0j2YtmJlF1aVtOOuHzBojafhGPJ1n1q/THFV7VNQ==","X-Received":"by 2002:a2e:5b11:: with SMTP id\n\tp17mr14438598ljb.37.1552265005089; \n\tSun, 10 Mar 2019 17:43:25 -0700 (PDT)","Date":"Mon, 11 Mar 2019 01:43:24 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190311004323.GE5281@bigcity.dyn.berto.se>","References":"<20190306024755.28726-1-niklas.soderlund@ragnatech.se>\n\t<20190306024755.28726-3-niklas.soderlund@ragnatech.se>\n\t<20190310132632.GF4814@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":"<20190310132632.GF4814@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 2/5] test: camera: Add read default\n\tformat test","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, 11 Mar 2019 00:43:26 -0000"}},{"id":1037,"web_url":"https://patchwork.libcamera.org/comment/1037/","msgid":"<20190311015256.GA19350@bigcity.dyn.berto.se>","date":"2019-03-11T01:52:56","subject":"Re: [libcamera-devel] [PATCH 2/5] test: camera: Add read default\n\tformat test","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi again,\n\nOn 2019-03-11 01:43:23 +0100, Niklas Söderlund wrote:\n[snip]\n> > > +void CameraTest::cleanup()\n> > > +{\n> > > +\tif (camera_) {\n> > > +\t\tcamera_->release();\n> > > +\t\tcamera_.reset();\n> > \n> > Is the reset needed ?\n> \n> No it's not, will remove for v2.\n\nI take this back, it is needed.\n\nWithout this running a test results in an error being printed\n\n ERR DeviceEnumerator device_enumerator.cpp:173 Removing media device while still in use\n\nThis is because the shard_ptr design as we call CameraManager::stop() \nwhile we still hold a reference to a camera. Maybe we could improve this \ndesign somehow? For now I will keep this in v2 as it works with our \ncurrent design.","headers":{"Return-Path":"","Received":["from mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 57C4F600F9\n\tfor ;\n\tMon, 11 Mar 2019 02:52:59 +0100 (CET)","by mail-lj1-x243.google.com with SMTP id d14so2573428ljl.9\n\tfor ;\n\tSun, 10 Mar 2019 18:52:59 -0700 (PDT)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\ty20sm878208lfg.44.2019.03.10.18.52.57\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSun, 10 Mar 2019 18:52:57 -0700 (PDT)"],"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=Zv88q5SajTSX1Evy9ovqcO0w0MR04Jud+pBlxa76HYQ=;\n\tb=LIK2gQH8ZbVbqY1PdyXXJPKgfjJbngMwQUcUv9ohOd8u4eTMYypbztARjO6xa20FM2\n\toBjuWR8Dppy3o6yOczBbg7zJ/iSUyDPvIPohI/ewNhct40gVQz9XS+UlDfao4g0v9whE\n\tgPaAgwPrj3zCDCm74H/ueO0DLKCMdAowxR4TJaG142VEqB+E15yuchqrE6zs7h8zOdFv\n\tCleI8xZclfYWt1SCjE6lG9vvG69CphwDHulyE2aJ7ppRsXXCkE0ctZxhRlymtyTjG9vX\n\tv9Sp9NfToXk3NO3BYqPwqR2AcjpIniCJ6mZljn331ssoZS9cYbbn06zZG+km14Wsq3IP\n\t8D8g==","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=Zv88q5SajTSX1Evy9ovqcO0w0MR04Jud+pBlxa76HYQ=;\n\tb=P1SACsngD6pq1SgsgcaUcm3moU1gsSDnuX5tfIAix3vRPVXyK50rpo3u/0O1x6/ifj\n\ta6ES9lTl7LydxPnREGoyhHDT3/ENJrRbmt/fGwIpYOqWpzTp9BA0I8tAN/ehuMpfrlxu\n\tHqCOofuN1MosQU6aienVhtqzPRZqNDxp39OPBfZVM0oJF+mosD680nagPs7dt+NdnxDd\n\tyQ4BargzFNZY4K5a8A+EgQdN5StjRKwx6JYbmat4HWBzRP5LkExghYZZwiPilMT2Who3\n\tql0pZvPswbqqJqv9Gbn/tfWQw7REJPotG0yH59a2BG4TNGBY8jTCnpjp6LpvTA6GJZBh\n\ty53A==","X-Gm-Message-State":"APjAAAWwafKKOiJ1OJOSopogcWeAJ2LIJWYONZ0KAuE7uJ2e+F4Ou3+e\n\tXJ7DchkrhHsJpBD5E1jsAjeVRw==","X-Google-Smtp-Source":"APXvYqx3VUURz0rqEabO+wuIQORlXV+LmlfYKUr17O7H//kg7BshsT11uOFlHvXApOBcohqefjZfcA==","X-Received":"by 2002:a2e:3009:: with SMTP id\n\tw9mr15175891ljw.144.1552269178340; \n\tSun, 10 Mar 2019 18:52:58 -0700 (PDT)","Date":"Mon, 11 Mar 2019 02:52:56 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190311015256.GA19350@bigcity.dyn.berto.se>","References":"<20190306024755.28726-1-niklas.soderlund@ragnatech.se>\n\t<20190306024755.28726-3-niklas.soderlund@ragnatech.se>\n\t<20190310132632.GF4814@pendragon.ideasonboard.com>\n\t<20190311004323.GE5281@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":"<20190311004323.GE5281@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 2/5] test: camera: Add read default\n\tformat test","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, 11 Mar 2019 01:52:59 -0000"}},{"id":1040,"web_url":"https://patchwork.libcamera.org/comment/1040/","msgid":"<20190311082128.GA4775@pendragon.ideasonboard.com>","date":"2019-03-11T08:21:28","subject":"Re: [libcamera-devel] [PATCH 2/5] test: camera: Add read default\n\tformat test","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Mon, Mar 11, 2019 at 02:52:56AM +0100, Niklas Söderlund wrote:\n> On 2019-03-11 01:43:23 +0100, Niklas Söderlund wrote:\n> [snip]\n> >>> +void CameraTest::cleanup()\n> >>> +{\n> >>> +\tif (camera_) {\n> >>> +\t\tcamera_->release();\n> >>> +\t\tcamera_.reset();\n> >> \n> >> Is the reset needed ?\n> > \n> > No it's not, will remove for v2.\n> \n> I take this back, it is needed.\n> \n> Without this running a test results in an error being printed\n> \n> ERR DeviceEnumerator device_enumerator.cpp:173 Removing media device while still in use\n> \n> This is because the shard_ptr design as we call CameraManager::stop() \n> while we still hold a reference to a camera. Maybe we could improve this \n> design somehow? For now I will keep this in v2 as it works with our \n> current design.\n\nI suspected that. Let's keep it then, and see if we can improve the\nsituation when creating the camera facade object.","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 178C2600FC\n\tfor ;\n\tMon, 11 Mar 2019 09:21:35 +0100 (CET)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 83E0D255;\n\tMon, 11 Mar 2019 09:21:34 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1552292494;\n\tbh=fdxMOOGtTMXByqrmoaN4az+y6iwwkLUQ/l9EvfwMfQ8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=AmJF8RS8ENn/OLIl1ZVG92tScunjXB132ll9NFhgw/qgc7Ar33gTfTBTMBeSiCQpG\n\tMk60Qg+y3rGRjjExR/HI3/ch66IUVE4k7qdczgW035vlA8eT53lPFAHdMn2ZtFKgG8\n\tDMsQDi9oYFY71F42+uxs13Sr8dOBLSpdVeiu0sYk=","Date":"Mon, 11 Mar 2019 10:21:28 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190311082128.GA4775@pendragon.ideasonboard.com>","References":"<20190306024755.28726-1-niklas.soderlund@ragnatech.se>\n\t<20190306024755.28726-3-niklas.soderlund@ragnatech.se>\n\t<20190310132632.GF4814@pendragon.ideasonboard.com>\n\t<20190311004323.GE5281@bigcity.dyn.berto.se>\n\t<20190311015256.GA19350@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":"<20190311015256.GA19350@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 2/5] test: camera: Add read default\n\tformat test","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, 11 Mar 2019 08:21:35 -0000"}}]