[{"id":3887,"web_url":"https://patchwork.libcamera.org/comment/3887/","msgid":"<20200229160950.GD18738@pendragon.ideasonboard.com>","date":"2020-02-29T16:09:50","subject":"Re: [libcamera-devel] [PATCH v2 2/4] test: Extract BufferSource\n\tclass out of camera tests to libtest","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 Mon, Feb 24, 2020 at 08:35:59PM +0100, Niklas Söderlund wrote:\n> The BufferSource class can be used by other tests other then the camera\n> buffer importer test, move it to libtest. The only changes to\n> BufferSource is for it to be allowed to be split in a header and source\n> file.\n> \n> This change makes it necessary for libtest to have access to internal\n> libcamera headers. As the internal headers already are accessible to all\n> test cases this do not increase the exposure of libcamera internals to\n\ns/do/does/\n\n> the test cases.\n\nI think we'll need to refactor the test classes, but this can be done\nlater.\n\n> \n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> ---\n>  test/camera/buffer_import.cpp  | 95 +-------------------------------\n>  test/libtest/buffer_source.cpp | 98 ++++++++++++++++++++++++++++++++++\n>  test/libtest/buffer_source.h   | 32 +++++++++++\n>  test/libtest/meson.build       | 11 ++--\n>  4 files changed, 138 insertions(+), 98 deletions(-)\n>  create mode 100644 test/libtest/buffer_source.cpp\n>  create mode 100644 test/libtest/buffer_source.h\n> \n> diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp\n> index 6997ea78c9f608c9..3f392cdc0732941f 100644\n> --- a/test/camera/buffer_import.cpp\n> +++ b/test/camera/buffer_import.cpp\n> @@ -16,6 +16,7 @@\n>  #include \"media_device.h\"\n>  #include \"v4l2_videodevice.h\"\n>  \n> +#include \"buffer_source.h\"\n>  #include \"camera_test.h\"\n>  #include \"test.h\"\n>  \n> @@ -23,100 +24,6 @@ using namespace libcamera;\n>  \n>  namespace {\n>  \n> -/* A provider of external buffers, suitable for import by a Camera. */\n> -class BufferSource\n> -{\n> -public:\n> -\tBufferSource()\n> -\t\t: video_(nullptr)\n> -\t{\n> -\t}\n> -\n> -\t~BufferSource()\n> -\t{\n> -\t\tif (video_) {\n> -\t\t\tvideo_->releaseBuffers();\n> -\t\t\tvideo_->close();\n> -\t\t}\n> -\n> -\t\tdelete video_;\n> -\t\tvideo_ = nullptr;\n> -\n> -\t\tif (media_)\n> -\t\t\tmedia_->release();\n> -\t}\n> -\n> -\tint allocate(const StreamConfiguration &config)\n> -\t{\n> -\t\t/* Locate and open the video device. */\n> -\t\tstd::string videoDeviceName = \"vivid-000-vid-out\";\n> -\n> -\t\tstd::unique_ptr<DeviceEnumerator> enumerator =\n> -\t\t\tDeviceEnumerator::create();\n> -\t\tif (!enumerator) {\n> -\t\t\tstd::cout << \"Failed to create device enumerator\" << std::endl;\n> -\t\t\treturn TestFail;\n> -\t\t}\n> -\n> -\t\tif (enumerator->enumerate()) {\n> -\t\t\tstd::cout << \"Failed to enumerate media devices\" << std::endl;\n> -\t\t\treturn TestFail;\n> -\t\t}\n> -\n> -\t\tDeviceMatch dm(\"vivid\");\n> -\t\tdm.add(videoDeviceName);\n> -\n> -\t\tmedia_ = enumerator->search(dm);\n> -\t\tif (!media_) {\n> -\t\t\tstd::cout << \"No vivid output device available\" << std::endl;\n> -\t\t\treturn TestSkip;\n> -\t\t}\n> -\n> -\t\tvideo_ = V4L2VideoDevice::fromEntityName(media_.get(), videoDeviceName);\n> -\t\tif (!video_) {\n> -\t\t\tstd::cout << \"Failed to get video device from entity \"\n> -\t\t\t\t  << videoDeviceName << std::endl;\n> -\t\t\treturn TestFail;\n> -\t\t}\n> -\n> -\t\tif (video_->open()) {\n> -\t\t\tstd::cout << \"Unable to open \" << videoDeviceName << std::endl;\n> -\t\t\treturn TestFail;\n> -\t\t}\n> -\n> -\t\t/* Configure the format. */\n> -\t\tV4L2DeviceFormat format;\n> -\t\tif (video_->getFormat(&format)) {\n> -\t\t\tstd::cout << \"Failed to get format on output device\" << std::endl;\n> -\t\t\treturn TestFail;\n> -\t\t}\n> -\n> -\t\tformat.size = config.size;\n> -\t\tformat.fourcc = V4L2VideoDevice::toV4L2Fourcc(config.pixelFormat, false);\n> -\t\tif (video_->setFormat(&format)) {\n> -\t\t\tstd::cout << \"Failed to set format on output device\" << std::endl;\n> -\t\t\treturn TestFail;\n> -\t\t}\n> -\n> -\t\tif (video_->exportBuffers(config.bufferCount, &buffers_) < 0) {\n> -\t\t\tstd::cout << \"Failed to export buffers\" << std::endl;\n> -\t\t\treturn TestFail;\n> -\t\t}\n> -\n> -\t\treturn TestPass;\n> -\t}\n> -\n> -\tconst std::vector<std::unique_ptr<FrameBuffer>> &buffers()\n> -\t{\n> -\t\treturn buffers_;\n> -\t}\n> -\n> -private:\n> -\tstd::shared_ptr<MediaDevice> media_;\n> -\tV4L2VideoDevice *video_;\n> -\tstd::vector<std::unique_ptr<FrameBuffer>> buffers_;\n> -};\n> -\n>  class BufferImportTest : public CameraTest, public Test\n>  {\n>  public:\n> diff --git a/test/libtest/buffer_source.cpp b/test/libtest/buffer_source.cpp\n> new file mode 100644\n> index 0000000000000000..066049d342a491f0\n> --- /dev/null\n> +++ b/test/libtest/buffer_source.cpp\n> @@ -0,0 +1,98 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * A provider of external buffers, suitable for use in tests.\n> + */\n> +\n> +#include \"buffer_source.h\"\n> +\n> +#include <iostream>\n> +\n> +#include \"device_enumerator.h\"\n> +\n> +#include \"test.h\"\n> +\n> +BufferSource::BufferSource()\n> +\t: video_(nullptr)\n> +{\n> +}\n> +\n> +BufferSource::~BufferSource()\n> +{\n> +\tif (video_) {\n> +\t\tvideo_->releaseBuffers();\n> +\t\tvideo_->close();\n> +\t}\n> +\n> +\tdelete video_;\n> +\tvideo_ = nullptr;\n> +\n> +\tif (media_)\n> +\t\tmedia_->release();\n> +}\n> +\n> +int BufferSource::allocate(const StreamConfiguration &config)\n> +{\n> +\t/* Locate and open the video device. */\n> +\tstd::string videoDeviceName = \"vivid-000-vid-out\";\n> +\n> +\tstd::unique_ptr<DeviceEnumerator> enumerator =\n> +\t\tDeviceEnumerator::create();\n> +\tif (!enumerator) {\n> +\t\tstd::cout << \"Failed to create device enumerator\" << std::endl;\n> +\t\treturn TestFail;\n> +\t}\n> +\n> +\tif (enumerator->enumerate()) {\n> +\t\tstd::cout << \"Failed to enumerate media devices\" << std::endl;\n> +\t\treturn TestFail;\n> +\t}\n> +\n> +\tDeviceMatch dm(\"vivid\");\n> +\tdm.add(videoDeviceName);\n> +\n> +\tmedia_ = enumerator->search(dm);\n> +\tif (!media_) {\n> +\t\tstd::cout << \"No vivid output device available\" << std::endl;\n> +\t\treturn TestSkip;\n> +\t}\n> +\n> +\tvideo_ = V4L2VideoDevice::fromEntityName(media_.get(), videoDeviceName);\n> +\tif (!video_) {\n> +\t\tstd::cout << \"Failed to get video device from entity \"\n> +\t\t\t  << videoDeviceName << std::endl;\n> +\t\treturn TestFail;\n> +\t}\n> +\n> +\tif (video_->open()) {\n> +\t\tstd::cout << \"Unable to open \" << videoDeviceName << std::endl;\n> +\t\treturn TestFail;\n> +\t}\n> +\n> +\t/* Configure the format. */\n> +\tV4L2DeviceFormat format;\n> +\tif (video_->getFormat(&format)) {\n> +\t\tstd::cout << \"Failed to get format on output device\" << std::endl;\n> +\t\treturn TestFail;\n> +\t}\n> +\n> +\tformat.size = config.size;\n> +\tformat.fourcc = V4L2VideoDevice::toV4L2Fourcc(config.pixelFormat, false);\n> +\tif (video_->setFormat(&format)) {\n> +\t\tstd::cout << \"Failed to set format on output device\" << std::endl;\n> +\t\treturn TestFail;\n> +\t}\n> +\n> +\tif (video_->exportBuffers(config.bufferCount, &buffers_) < 0) {\n> +\t\tstd::cout << \"Failed to export buffers\" << std::endl;\n> +\t\treturn TestFail;\n> +\t}\n> +\n> +\treturn TestPass;\n> +}\n> +\n> +const std::vector<std::unique_ptr<FrameBuffer>> &BufferSource::buffers()\n> +{\n> +\treturn buffers_;\n> +}\n> diff --git a/test/libtest/buffer_source.h b/test/libtest/buffer_source.h\n> new file mode 100644\n> index 0000000000000000..2d8fc5acf6d78771\n> --- /dev/null\n> +++ b/test/libtest/buffer_source.h\n> @@ -0,0 +1,32 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * buffer_source.h - libcamera camera test helper to create FrameBuffers\n> + */\n> +#ifndef __LIBCAMERA_BUFFER_SOURCE_TEST_H__\n> +#define __LIBCAMERA_BUFFER_SOURCE_TEST_H__\n> +\n> +#include <libcamera/libcamera.h>\n> +\n> +#include \"media_device.h\"\n> +#include \"v4l2_videodevice.h\"\n> +\n> +using namespace libcamera;\n> +\n> +class BufferSource\n> +{\n> +public:\n> +\tBufferSource();\n> +\t~BufferSource();\n> +\n> +\tint allocate(const StreamConfiguration &config);\n> +\tconst std::vector<std::unique_ptr<FrameBuffer>> &buffers();\n> +\n> +private:\n> +\tstd::shared_ptr<MediaDevice> media_;\n> +\tV4L2VideoDevice *video_;\n> +\tstd::vector<std::unique_ptr<FrameBuffer>> buffers_;\n> +};\n> +\n> +#endif /* __LIBCAMERA_BUFFER_SOURCE_TEST_H__ */\n> diff --git a/test/libtest/meson.build b/test/libtest/meson.build\n> index 3e798ef3810e9b0d..33565e0eb3b66d6a 100644\n> --- a/test/libtest/meson.build\n> +++ b/test/libtest/meson.build\n> @@ -1,14 +1,11 @@\n>  libtest_sources = files([\n> +    'buffer_source.cpp',\n>      'camera_test.cpp',\n>      'test.cpp',\n>  ])\n>  \n> -libtest = static_library('libtest', libtest_sources,\n> -                         dependencies : libcamera_dep)\n> -\n>  libtest_includes = include_directories('.')\n>  \n> -test_libraries = [libtest]\n>  \n>  test_includes_public = [\n>      libtest_includes,\n> @@ -18,3 +15,9 @@ test_includes_internal = [\n>      test_includes_public,\n>      libcamera_internal_includes,\n>  ]\n> +\n> +libtest = static_library('libtest', libtest_sources,\n> +                         dependencies : libcamera_dep,\n> +                         include_directories : test_includes_internal)\n> +\n> +test_libraries = [libtest]","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","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 27E9362689\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 29 Feb 2020 17:10:14 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 921B233E;\n\tSat, 29 Feb 2020 17:10:13 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1582992613;\n\tbh=Jk6/QVIn/fAESNXooeRboZ7KqmZHovTRQuNIdeXwUm0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=I+/VVrahQ+sDYeEVjb8OreGfuUU9BfGN4PcNUmnrAHF85oQqESaxfrcD6amok+4xl\n\tYrSpnmQQidPSjOnqs8vgeOhJHTWZWc17TNBPqiyffTlCcP7/L1zKKmNYuuPxiIUC7r\n\tLGgHi734Iy58u5kwkKWA/M06ZqY/p22FJVWfSLrc=","Date":"Sat, 29 Feb 2020 18:09:50 +0200","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":"<20200229160950.GD18738@pendragon.ideasonboard.com>","References":"<20200224193601.1040770-1-niklas.soderlund@ragnatech.se>\n\t<20200224193601.1040770-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":"<20200224193601.1040770-3-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 2/4] test: Extract BufferSource\n\tclass out of camera tests to libtest","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>","X-List-Received-Date":"Sat, 29 Feb 2020 16:10:14 -0000"}}]