[{"id":1582,"web_url":"https://patchwork.libcamera.org/comment/1582/","msgid":"<20190511124621.GA4946@pendragon.ideasonboard.com>","date":"2019-05-11T12:46:21","subject":"Re: [libcamera-devel] [PATCH v2 05/11] test: media_device: Create a\n\tcommon MediaDeviceTest base class","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, May 08, 2019 at 05:18:25PM +0200, Niklas Söderlund wrote:\n> Before adding more tests which will act on the vimc pipeline break out a\n> common base from media_device_link_test.cpp which already acts on vimc.\n> The new common base class will help reduce code duplication.\n> \n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  test/media_device/media_device_link_test.cpp | 73 +++++---------------\n>  test/media_device/media_device_test.cpp      | 41 +++++++++++\n>  test/media_device/media_device_test.h        | 35 ++++++++++\n>  test/media_device/meson.build                |  2 +-\n>  4 files changed, 96 insertions(+), 55 deletions(-)\n>  create mode 100644 test/media_device/media_device_test.cpp\n>  create mode 100644 test/media_device/media_device_test.h\n> \n> diff --git a/test/media_device/media_device_link_test.cpp b/test/media_device/media_device_link_test.cpp\n> index 334bb44a6fc14371..26aedf0ea23ec709 100644\n> --- a/test/media_device/media_device_link_test.cpp\n> +++ b/test/media_device/media_device_link_test.cpp\n> @@ -4,13 +4,10 @@\n>   *\n>   * media_device_link_test.cpp - Tests link handling on VIMC media device\n>   */\n> +\n>  #include <iostream>\n> -#include <memory>\n>  \n> -#include \"device_enumerator.h\"\n> -#include \"media_device.h\"\n> -\n> -#include \"test.h\"\n> +#include \"media_device_test.h\"\n>  \n>  using namespace libcamera;\n>  using namespace std;\n> @@ -29,44 +26,21 @@ using namespace std;\n>   * loaded) the test is skipped.\n>   */\n>  \n> -class MediaDeviceLinkTest : public Test\n> +class MediaDeviceLinkTest : public MediaDeviceTest\n>  {\n> -\tint init()\n> -\t{\n> -\t\tenumerator = unique_ptr<DeviceEnumerator>(DeviceEnumerator::create());\n> -\t\tif (!enumerator) {\n> -\t\t\tcerr << \"Failed to create device enumerator\" << endl;\n> -\t\t\treturn TestFail;\n> -\t\t}\n> -\n> -\t\tif (enumerator->enumerate()) {\n> -\t\t\tcerr << \"Failed to enumerate media devices\" << endl;\n> -\t\t\treturn TestFail;\n> -\t\t}\n> -\n> -\t\tDeviceMatch dm(\"vimc\");\n> -\t\tdev_ = enumerator->search(dm);\n> -\t\tif (!dev_) {\n> -\t\t\tcerr << \"No VIMC media device found: skip test\" << endl;\n> -\t\t\treturn TestSkip;\n> -\t\t}\n> -\n> -\t\tif (!dev_->acquire()) {\n> -\t\t\tcerr << \"Unable to acquire media device \"\n> -\t\t\t     << dev_->deviceNode() << endl;\n> -\t\t\treturn TestSkip;\n> -\t\t}\n> -\n> -\t\treturn 0;\n> -\t}\n> -\n>  \tint run()\n>  \t{\n> +\t\tif (!media_->acquire()) {\n> +\t\t\tcerr << \"Unable to acquire media device \"\n> +\t\t\t     << media_->deviceNode() << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n\nShouldn't this go to he init() method of the base class, given that the\nrelease() call is in cleanup() ?\n\n> +\n>  \t\t/*\n>  \t\t * First of all disable all links in the media graph to\n>  \t\t * ensure we start from a known state.\n>  \t\t */\n> -\t\tif (dev_->disableLinks()) {\n> +\t\tif (media_->disableLinks()) {\n>  \t\t\tcerr << \"Failed to disable all links in the media graph\";\n>  \t\t\treturn TestFail;\n>  \t\t}\n> @@ -76,26 +50,26 @@ class MediaDeviceLinkTest : public Test\n>  \t\t * different methods the media device offers.\n>  \t\t */\n>  \t\tstring linkName(\"'Debayer A':[1] -> 'Scaler':[0]'\");\n> -\t\tMediaLink *link = dev_->link(\"Debayer A\", 1, \"Scaler\", 0);\n> +\t\tMediaLink *link = media_->link(\"Debayer A\", 1, \"Scaler\", 0);\n>  \t\tif (!link) {\n>  \t\t\tcerr << \"Unable to find link: \" << linkName\n>  \t\t\t     << \" using lookup by name\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n>  \n> -\t\tMediaEntity *source = dev_->getEntityByName(\"Debayer A\");\n> +\t\tMediaEntity *source = media_->getEntityByName(\"Debayer A\");\n>  \t\tif (!source) {\n>  \t\t\tcerr << \"Unable to find entity: 'Debayer A'\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n>  \n> -\t\tMediaEntity *sink = dev_->getEntityByName(\"Scaler\");\n> +\t\tMediaEntity *sink = media_->getEntityByName(\"Scaler\");\n>  \t\tif (!sink) {\n>  \t\t\tcerr << \"Unable to find entity: 'Scaler'\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n>  \n> -\t\tMediaLink *link2 = dev_->link(source, 1, sink, 0);\n> +\t\tMediaLink *link2 = media_->link(source, 1, sink, 0);\n>  \t\tif (!link2) {\n>  \t\t\tcerr << \"Unable to find link: \" << linkName\n>  \t\t\t     << \" using lookup by entity\" << endl;\n> @@ -108,7 +82,7 @@ class MediaDeviceLinkTest : public Test\n>  \t\t\treturn TestFail;\n>  \t\t}\n>  \n> -\t\tlink2 = dev_->link(source->getPadByIndex(1),\n> +\t\tlink2 = media_->link(source->getPadByIndex(1),\n>  \t\t\t\t   sink->getPadByIndex(0));\n>  \t\tif (!link2) {\n>  \t\t\tcerr << \"Unable to find link: \" << linkName\n> @@ -160,7 +134,7 @@ class MediaDeviceLinkTest : public Test\n>  \n>  \t\t/* Try to get a non existing link. */\n>  \t\tlinkName = \"'Sensor A':[1] -> 'Scaler':[0]\";\n> -\t\tlink = dev_->link(\"Sensor A\", 1, \"Scaler\", 0);\n> +\t\tlink = media_->link(\"Sensor A\", 1, \"Scaler\", 0);\n>  \t\tif (link) {\n>  \t\t\tcerr << \"Link lookup for \" << linkName\n>  \t\t\t     << \" succeeded but link does not exist\"\n> @@ -170,7 +144,7 @@ class MediaDeviceLinkTest : public Test\n>  \n>  \t\t/* Now get an immutable link and try to disable it. */\n>  \t\tlinkName = \"'Sensor A':[0] -> 'Raw Capture 0':[0]\";\n> -\t\tlink = dev_->link(\"Sensor A\", 0, \"Raw Capture 0\", 0);\n> +\t\tlink = media_->link(\"Sensor A\", 0, \"Raw Capture 0\", 0);\n>  \t\tif (!link) {\n>  \t\t\tcerr << \"Unable to find link: \" << linkName\n>  \t\t\t     << \" using lookup by name\" << endl;\n> @@ -195,7 +169,7 @@ class MediaDeviceLinkTest : public Test\n>  \t\t * after disabling all links in the media graph.\n>  \t\t */\n>  \t\tlinkName = \"'Debayer B':[1] -> 'Scaler':[0]'\";\n> -\t\tlink = dev_->link(\"Debayer B\", 1, \"Scaler\", 0);\n> +\t\tlink = media_->link(\"Debayer B\", 1, \"Scaler\", 0);\n>  \t\tif (!link) {\n>  \t\t\tcerr << \"Unable to find link: \" << linkName\n>  \t\t\t     << \" using lookup by name\" << endl;\n> @@ -215,7 +189,7 @@ class MediaDeviceLinkTest : public Test\n>  \t\t\treturn TestFail;\n>  \t\t}\n>  \n> -\t\tif (dev_->disableLinks()) {\n> +\t\tif (media_->disableLinks()) {\n>  \t\t\tcerr << \"Failed to disable all links in the media graph\";\n>  \t\t\treturn TestFail;\n>  \t\t}\n> @@ -229,15 +203,6 @@ class MediaDeviceLinkTest : public Test\n>  \n>  \t\treturn 0;\n>  \t}\n> -\n> -\tvoid cleanup()\n> -\t{\n> -\t\tdev_->release();\n> -\t}\n> -\n> -private:\n> -\tunique_ptr<DeviceEnumerator> enumerator;\n> -\tshared_ptr<MediaDevice> dev_;\n>  };\n>  \n>  TEST_REGISTER(MediaDeviceLinkTest);\n> diff --git a/test/media_device/media_device_test.cpp b/test/media_device/media_device_test.cpp\n> new file mode 100644\n> index 0000000000000000..710613eeb80b6de1\n> --- /dev/null\n> +++ b/test/media_device/media_device_test.cpp\n> @@ -0,0 +1,41 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * libcamera MediaDevice API tests\n> + */\n> +\n> +#include <iostream>\n> +\n> +#include \"media_device_test.h\"\n> +\n> +using namespace libcamera;\n> +using namespace std;\n> +\n> +int MediaDeviceTest::init()\n> +{\n> +\tenumerator_ = unique_ptr<DeviceEnumerator>(DeviceEnumerator::create());\n> +\tif (!enumerator_) {\n> +\t\tcerr << \"Failed to create device enumerator\" << endl;\n> +\t\treturn TestFail;\n> +\t}\n> +\n> +\tif (enumerator_->enumerate()) {\n> +\t\tcerr << \"Failed to enumerate media devices\" << endl;\n> +\t\treturn TestFail;\n> +\t}\n> +\n> +\tDeviceMatch dm(\"vimc\");\n> +\tmedia_ = enumerator_->search(dm);\n> +\tif (!media_) {\n> +\t\tcerr << \"No VIMC media device found: skip test\" << endl;\n> +\t\treturn TestSkip;\n> +\t}\n> +\n> +\treturn TestPass;\n> +}\n> +\n> +void MediaDeviceTest::cleanup()\n> +{\n> +\tmedia_->release();\n> +}\n> diff --git a/test/media_device/media_device_test.h b/test/media_device/media_device_test.h\n> new file mode 100644\n> index 0000000000000000..af5c89f4fff2f63e\n> --- /dev/null\n> +++ b/test/media_device/media_device_test.h\n> @@ -0,0 +1,35 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * media_device_test.h - libcamera media devie base class\n\ns/devie/device test/\n\n> + */\n> +#ifndef __LIBCAMERA_MEDIADEVICE_TEST_H__\n> +#define __LIBCAMERA_MEDIADEVICE_TEST_H__\n> +\n> +#include <memory>\n> +\n> +#include \"device_enumerator.h\"\n> +#include \"media_device.h\"\n> +\n> +#include \"test.h\"\n> +\n> +using namespace libcamera;\n> +\n> +class MediaDeviceTest : public Test\n> +{\n> +public:\n> +\tMediaDeviceTest()\n> +\t\t: media_(nullptr), enumerator_(nullptr) {}\n> +\n> +protected:\n> +\tint init();\n> +\tvoid cleanup();\n> +\n> +\tstd::shared_ptr<MediaDevice> media_;\n> +\n> +private:\n> +\tstd::unique_ptr<DeviceEnumerator> enumerator_;\n> +};\n> +\n> +#endif /* __LIBCAMERA_MEDIADEVICE_TEST_H__ */\n> diff --git a/test/media_device/meson.build b/test/media_device/meson.build\n> index d91a022fab190abf..1005685409d99aa7 100644\n> --- a/test/media_device/meson.build\n> +++ b/test/media_device/meson.build\n> @@ -4,7 +4,7 @@ media_device_tests = [\n>  ]\n>  \n>  foreach t : media_device_tests\n> -    exe = executable(t[0], t[1],\n> +    exe = executable(t[0], [t[1], 'media_device_test.cpp'],\n\nThis is a bit of a hack. How about creating a static library for this\nfile, the same way we do with libtest ? It can live in this directory,\nthere's no need for a subdirectory or a separate meson.build file.\n\nAlternatively you could include media_device_test.cpp in libtest, but\nthat seems to be a bit of a hack too.\n\n>                       link_with : test_libraries,\n>                       include_directories : test_includes_internal)\n>","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7A8D760E4F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 11 May 2019 14:46:38 +0200 (CEST)","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 07915D5;\n\tSat, 11 May 2019 14:46:36 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1557578797;\n\tbh=4AVV+/LSPwRtRh3W54JJPoTIRTFuj7f2lOR+ZvJGXB4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=b+rEpKZ7+1dx7ose5qBsOZyxSkJR8vcjGhTXjkJgBf4dx1QZZ69N1dcxwlYXWPbZy\n\taWnNOJbfdjtmcR48/5O5VL2qDdlfnAtFvZAxIWlDnnIEq797gPloVCnbqM3BNbkLr6\n\tjFh8HX+C0o3QFyinflLWKA3FqN2Y+QJomvo6G33k=","Date":"Sat, 11 May 2019 15:46:21 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190511124621.GA4946@pendragon.ideasonboard.com>","References":"<20190508151831.12274-1-niklas.soderlund@ragnatech.se>\n\t<20190508151831.12274-6-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":"<20190508151831.12274-6-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 05/11] test: media_device: Create a\n\tcommon MediaDeviceTest base class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Sat, 11 May 2019 12:46:38 -0000"}},{"id":1604,"web_url":"https://patchwork.libcamera.org/comment/1604/","msgid":"<20190516235152.GA22849@bigcity.dyn.berto.se>","date":"2019-05-16T23:51:52","subject":"Re: [libcamera-devel] [PATCH v2 05/11] test: media_device: Create a\n\tcommon MediaDeviceTest base class","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-05-11 15:46:21 +0300, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Wed, May 08, 2019 at 05:18:25PM +0200, Niklas Söderlund wrote:\n> > Before adding more tests which will act on the vimc pipeline break out a\n> > common base from media_device_link_test.cpp which already acts on vimc.\n> > The new common base class will help reduce code duplication.\n> > \n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  test/media_device/media_device_link_test.cpp | 73 +++++---------------\n> >  test/media_device/media_device_test.cpp      | 41 +++++++++++\n> >  test/media_device/media_device_test.h        | 35 ++++++++++\n> >  test/media_device/meson.build                |  2 +-\n> >  4 files changed, 96 insertions(+), 55 deletions(-)\n> >  create mode 100644 test/media_device/media_device_test.cpp\n> >  create mode 100644 test/media_device/media_device_test.h\n> > \n> > diff --git a/test/media_device/media_device_link_test.cpp b/test/media_device/media_device_link_test.cpp\n> > index 334bb44a6fc14371..26aedf0ea23ec709 100644\n> > --- a/test/media_device/media_device_link_test.cpp\n> > +++ b/test/media_device/media_device_link_test.cpp\n> > @@ -4,13 +4,10 @@\n> >   *\n> >   * media_device_link_test.cpp - Tests link handling on VIMC media device\n> >   */\n> > +\n> >  #include <iostream>\n> > -#include <memory>\n> >  \n> > -#include \"device_enumerator.h\"\n> > -#include \"media_device.h\"\n> > -\n> > -#include \"test.h\"\n> > +#include \"media_device_test.h\"\n> >  \n> >  using namespace libcamera;\n> >  using namespace std;\n> > @@ -29,44 +26,21 @@ using namespace std;\n> >   * loaded) the test is skipped.\n> >   */\n> >  \n> > -class MediaDeviceLinkTest : public Test\n> > +class MediaDeviceLinkTest : public MediaDeviceTest\n> >  {\n> > -\tint init()\n> > -\t{\n> > -\t\tenumerator = unique_ptr<DeviceEnumerator>(DeviceEnumerator::create());\n> > -\t\tif (!enumerator) {\n> > -\t\t\tcerr << \"Failed to create device enumerator\" << endl;\n> > -\t\t\treturn TestFail;\n> > -\t\t}\n> > -\n> > -\t\tif (enumerator->enumerate()) {\n> > -\t\t\tcerr << \"Failed to enumerate media devices\" << endl;\n> > -\t\t\treturn TestFail;\n> > -\t\t}\n> > -\n> > -\t\tDeviceMatch dm(\"vimc\");\n> > -\t\tdev_ = enumerator->search(dm);\n> > -\t\tif (!dev_) {\n> > -\t\t\tcerr << \"No VIMC media device found: skip test\" << endl;\n> > -\t\t\treturn TestSkip;\n> > -\t\t}\n> > -\n> > -\t\tif (!dev_->acquire()) {\n> > -\t\t\tcerr << \"Unable to acquire media device \"\n> > -\t\t\t     << dev_->deviceNode() << endl;\n> > -\t\t\treturn TestSkip;\n> > -\t\t}\n> > -\n> > -\t\treturn 0;\n> > -\t}\n> > -\n> >  \tint run()\n> >  \t{\n> > +\t\tif (!media_->acquire()) {\n> > +\t\t\tcerr << \"Unable to acquire media device \"\n> > +\t\t\t     << media_->deviceNode() << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> \n> Shouldn't this go to he init() method of the base class, given that the\n> release() call is in cleanup() ?\n\nWops, this is indeed wrong. However the release() should be moved to the \nMediaDeviceLinkTest cleanup() function, and not the base class \nMediaDeviceTest. Reason for this is that there are tests which we wish \nto run without the media device acquired, so IMHO it's best for those \ntests which needs to acquire it to do so themself.\n\nWill fix for next version.\n\n> \n> > +\n> >  \t\t/*\n> >  \t\t * First of all disable all links in the media graph to\n> >  \t\t * ensure we start from a known state.\n> >  \t\t */\n> > -\t\tif (dev_->disableLinks()) {\n> > +\t\tif (media_->disableLinks()) {\n> >  \t\t\tcerr << \"Failed to disable all links in the media graph\";\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> > @@ -76,26 +50,26 @@ class MediaDeviceLinkTest : public Test\n> >  \t\t * different methods the media device offers.\n> >  \t\t */\n> >  \t\tstring linkName(\"'Debayer A':[1] -> 'Scaler':[0]'\");\n> > -\t\tMediaLink *link = dev_->link(\"Debayer A\", 1, \"Scaler\", 0);\n> > +\t\tMediaLink *link = media_->link(\"Debayer A\", 1, \"Scaler\", 0);\n> >  \t\tif (!link) {\n> >  \t\t\tcerr << \"Unable to find link: \" << linkName\n> >  \t\t\t     << \" using lookup by name\" << endl;\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> >  \n> > -\t\tMediaEntity *source = dev_->getEntityByName(\"Debayer A\");\n> > +\t\tMediaEntity *source = media_->getEntityByName(\"Debayer A\");\n> >  \t\tif (!source) {\n> >  \t\t\tcerr << \"Unable to find entity: 'Debayer A'\" << endl;\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> >  \n> > -\t\tMediaEntity *sink = dev_->getEntityByName(\"Scaler\");\n> > +\t\tMediaEntity *sink = media_->getEntityByName(\"Scaler\");\n> >  \t\tif (!sink) {\n> >  \t\t\tcerr << \"Unable to find entity: 'Scaler'\" << endl;\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> >  \n> > -\t\tMediaLink *link2 = dev_->link(source, 1, sink, 0);\n> > +\t\tMediaLink *link2 = media_->link(source, 1, sink, 0);\n> >  \t\tif (!link2) {\n> >  \t\t\tcerr << \"Unable to find link: \" << linkName\n> >  \t\t\t     << \" using lookup by entity\" << endl;\n> > @@ -108,7 +82,7 @@ class MediaDeviceLinkTest : public Test\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> >  \n> > -\t\tlink2 = dev_->link(source->getPadByIndex(1),\n> > +\t\tlink2 = media_->link(source->getPadByIndex(1),\n> >  \t\t\t\t   sink->getPadByIndex(0));\n> >  \t\tif (!link2) {\n> >  \t\t\tcerr << \"Unable to find link: \" << linkName\n> > @@ -160,7 +134,7 @@ class MediaDeviceLinkTest : public Test\n> >  \n> >  \t\t/* Try to get a non existing link. */\n> >  \t\tlinkName = \"'Sensor A':[1] -> 'Scaler':[0]\";\n> > -\t\tlink = dev_->link(\"Sensor A\", 1, \"Scaler\", 0);\n> > +\t\tlink = media_->link(\"Sensor A\", 1, \"Scaler\", 0);\n> >  \t\tif (link) {\n> >  \t\t\tcerr << \"Link lookup for \" << linkName\n> >  \t\t\t     << \" succeeded but link does not exist\"\n> > @@ -170,7 +144,7 @@ class MediaDeviceLinkTest : public Test\n> >  \n> >  \t\t/* Now get an immutable link and try to disable it. */\n> >  \t\tlinkName = \"'Sensor A':[0] -> 'Raw Capture 0':[0]\";\n> > -\t\tlink = dev_->link(\"Sensor A\", 0, \"Raw Capture 0\", 0);\n> > +\t\tlink = media_->link(\"Sensor A\", 0, \"Raw Capture 0\", 0);\n> >  \t\tif (!link) {\n> >  \t\t\tcerr << \"Unable to find link: \" << linkName\n> >  \t\t\t     << \" using lookup by name\" << endl;\n> > @@ -195,7 +169,7 @@ class MediaDeviceLinkTest : public Test\n> >  \t\t * after disabling all links in the media graph.\n> >  \t\t */\n> >  \t\tlinkName = \"'Debayer B':[1] -> 'Scaler':[0]'\";\n> > -\t\tlink = dev_->link(\"Debayer B\", 1, \"Scaler\", 0);\n> > +\t\tlink = media_->link(\"Debayer B\", 1, \"Scaler\", 0);\n> >  \t\tif (!link) {\n> >  \t\t\tcerr << \"Unable to find link: \" << linkName\n> >  \t\t\t     << \" using lookup by name\" << endl;\n> > @@ -215,7 +189,7 @@ class MediaDeviceLinkTest : public Test\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> >  \n> > -\t\tif (dev_->disableLinks()) {\n> > +\t\tif (media_->disableLinks()) {\n> >  \t\t\tcerr << \"Failed to disable all links in the media graph\";\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> > @@ -229,15 +203,6 @@ class MediaDeviceLinkTest : public Test\n> >  \n> >  \t\treturn 0;\n> >  \t}\n> > -\n> > -\tvoid cleanup()\n> > -\t{\n> > -\t\tdev_->release();\n> > -\t}\n> > -\n> > -private:\n> > -\tunique_ptr<DeviceEnumerator> enumerator;\n> > -\tshared_ptr<MediaDevice> dev_;\n> >  };\n> >  \n> >  TEST_REGISTER(MediaDeviceLinkTest);\n> > diff --git a/test/media_device/media_device_test.cpp b/test/media_device/media_device_test.cpp\n> > new file mode 100644\n> > index 0000000000000000..710613eeb80b6de1\n> > --- /dev/null\n> > +++ b/test/media_device/media_device_test.cpp\n> > @@ -0,0 +1,41 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * libcamera MediaDevice API tests\n> > + */\n> > +\n> > +#include <iostream>\n> > +\n> > +#include \"media_device_test.h\"\n> > +\n> > +using namespace libcamera;\n> > +using namespace std;\n> > +\n> > +int MediaDeviceTest::init()\n> > +{\n> > +\tenumerator_ = unique_ptr<DeviceEnumerator>(DeviceEnumerator::create());\n> > +\tif (!enumerator_) {\n> > +\t\tcerr << \"Failed to create device enumerator\" << endl;\n> > +\t\treturn TestFail;\n> > +\t}\n> > +\n> > +\tif (enumerator_->enumerate()) {\n> > +\t\tcerr << \"Failed to enumerate media devices\" << endl;\n> > +\t\treturn TestFail;\n> > +\t}\n> > +\n> > +\tDeviceMatch dm(\"vimc\");\n> > +\tmedia_ = enumerator_->search(dm);\n> > +\tif (!media_) {\n> > +\t\tcerr << \"No VIMC media device found: skip test\" << endl;\n> > +\t\treturn TestSkip;\n> > +\t}\n> > +\n> > +\treturn TestPass;\n> > +}\n> > +\n> > +void MediaDeviceTest::cleanup()\n> > +{\n> > +\tmedia_->release();\n> > +}\n> > diff --git a/test/media_device/media_device_test.h b/test/media_device/media_device_test.h\n> > new file mode 100644\n> > index 0000000000000000..af5c89f4fff2f63e\n> > --- /dev/null\n> > +++ b/test/media_device/media_device_test.h\n> > @@ -0,0 +1,35 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * media_device_test.h - libcamera media devie base class\n> \n> s/devie/device test/\n> \n> > + */\n> > +#ifndef __LIBCAMERA_MEDIADEVICE_TEST_H__\n> > +#define __LIBCAMERA_MEDIADEVICE_TEST_H__\n> > +\n> > +#include <memory>\n> > +\n> > +#include \"device_enumerator.h\"\n> > +#include \"media_device.h\"\n> > +\n> > +#include \"test.h\"\n> > +\n> > +using namespace libcamera;\n> > +\n> > +class MediaDeviceTest : public Test\n> > +{\n> > +public:\n> > +\tMediaDeviceTest()\n> > +\t\t: media_(nullptr), enumerator_(nullptr) {}\n> > +\n> > +protected:\n> > +\tint init();\n> > +\tvoid cleanup();\n> > +\n> > +\tstd::shared_ptr<MediaDevice> media_;\n> > +\n> > +private:\n> > +\tstd::unique_ptr<DeviceEnumerator> enumerator_;\n> > +};\n> > +\n> > +#endif /* __LIBCAMERA_MEDIADEVICE_TEST_H__ */\n> > diff --git a/test/media_device/meson.build b/test/media_device/meson.build\n> > index d91a022fab190abf..1005685409d99aa7 100644\n> > --- a/test/media_device/meson.build\n> > +++ b/test/media_device/meson.build\n> > @@ -4,7 +4,7 @@ media_device_tests = [\n> >  ]\n> >  \n> >  foreach t : media_device_tests\n> > -    exe = executable(t[0], t[1],\n> > +    exe = executable(t[0], [t[1], 'media_device_test.cpp'],\n> \n> This is a bit of a hack. How about creating a static library for this\n> file, the same way we do with libtest ? It can live in this directory,\n> there's no need for a subdirectory or a separate meson.build file.\n> \n> Alternatively you could include media_device_test.cpp in libtest, but\n> that seems to be a bit of a hack too.\n\nGood point, will fix for next version.\n\n> \n> >                       link_with : test_libraries,\n> >                       include_directories : test_includes_internal)\n> >  \n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lf1-x141.google.com (mail-lf1-x141.google.com\n\t[IPv6:2a00:1450:4864:20::141])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0F7B860103\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 17 May 2019 01:51:55 +0200 (CEST)","by mail-lf1-x141.google.com with SMTP id x132so3984915lfd.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 16 May 2019 16:51:54 -0700 (PDT)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tw17sm1124955ljj.31.2019.05.16.16.51.53\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tThu, 16 May 2019 16:51:53 -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=1Mt+ZdayKQNvRpPew5hl2WtKaJnbvh4mPGzKs1UUOpw=;\n\tb=te7Ztf5kuU/pyWh5EN2lMoftu+wulxDcVc+yzAmqZffVaihwk1suW8VRCy9rg2DYli\n\thdfF329GhV1SwQlCm/A9PV8BubahmOnb7VIvgvu4POVJYDhpXwrAepvXDCnW6tpjy+9Y\n\t2N1tqfGSx/OEP93ZqNZLuCFaxyqesPRUC9E81JNpFiC3iu2HdNkISoJQBFVFYOyO5fTj\n\tW2rQtpZq8+dC8S+i8ZVMwKA5deWvI6lsjWR/AXWZ+PJAqqqYizZvlrcj8EDwvd3xPNLo\n\td4SskwdmJDvYwesWEMXxxbfpWnCJh9iB39YmiXi4htSm9MD7A4BQ9jcbsAwor0oBTbtm\n\tdhrQ==","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=1Mt+ZdayKQNvRpPew5hl2WtKaJnbvh4mPGzKs1UUOpw=;\n\tb=A5BU+VV0wDNwlmWfPPylGSiYbN8teFaxjAaqaTzElGoCQKenBPzsagreCaqyRkFnoz\n\tZlwRPbwX0WKxKsg8ERnpZZoem/Mi+C8x4cuxQAG2d8LOKWp0NzMdwXZOgm0BmxWRaGyL\n\t+KZn+kahWLadfRPJp9FZi+WVvSlzL0d4OQviE12yGAmqlQJyd3t6ficVoxh4IcxLe4ZW\n\trf91g0SOHduUhT+C6w3uwtGlqvJrBnOT7Y2KXwRg29eiKVg35hOAIqRnmM7F0FjlmA6C\n\twdjUH5qNKlI4+gZ9wHve0xv5pXbs0UcD7iMfYe39hBUmd1qn5opx7ZP4hfFbj/m28Gl8\n\tASpQ==","X-Gm-Message-State":"APjAAAVEM5jIFecKpAtp71azvO6q+tGuYm1kMJEkIG3+AYrZh6dRPDta\n\t0fXh1K4ZiH4lRQru60fYSkQPeQ==","X-Google-Smtp-Source":"APXvYqzKt2Wno41QbVTcMN33oziDST0/Ig/r/BKFBj+OqwU/qne3C0BPotKCA4s7h4QtUAw8GIFHqQ==","X-Received":"by 2002:ac2:4315:: with SMTP id\n\tl21mr4919380lfh.143.1558050714200; \n\tThu, 16 May 2019 16:51:54 -0700 (PDT)","Date":"Fri, 17 May 2019 01:51:52 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190516235152.GA22849@bigcity.dyn.berto.se>","References":"<20190508151831.12274-1-niklas.soderlund@ragnatech.se>\n\t<20190508151831.12274-6-niklas.soderlund@ragnatech.se>\n\t<20190511124621.GA4946@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":"<20190511124621.GA4946@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.11.3 (2019-02-01)","Subject":"Re: [libcamera-devel] [PATCH v2 05/11] test: media_device: Create a\n\tcommon MediaDeviceTest base class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Thu, 16 May 2019 23:51:55 -0000"}}]