[{"id":1043,"web_url":"https://patchwork.libcamera.org/comment/1043/","msgid":"<20190311084507.GE4775@pendragon.ideasonboard.com>","date":"2019-03-11T08:45:07","subject":"Re: [libcamera-devel] [PATCH v2 3/4] test: camera: Add capture 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 Mon, Mar 11, 2019 at 03:22:31AM +0100, Niklas Söderlund wrote:\n> Correctly configure the camera using the default configuration and run a\n> capture session for 100 milliseconds, which is plenty of time, in tests\n> over 600 requests completed using the vimc pipeline.\n> \n> The test passes if at least the number of buffers used in the capture\n> times two number of requests completes to prove we cycle through all\n> buffers.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> test/camera/capture.cpp | 136 ++++++++++++++++++++++++++++++++++++++++\n> test/camera/meson.build | 1 +\n> 2 files changed, 137 insertions(+)\n> create mode 100644 test/camera/capture.cpp\n> \n> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp\n> new file mode 100644\n> index 0000000000000000..44adcf3639393d3f\n> --- /dev/null\n> +++ b/test/camera/capture.cpp\n> @@ -0,0 +1,136 @@\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 Capture : public CameraTest\n> +{\n> +protected:\n> +\tunsigned int completeBuffersCount_;\n> +\tunsigned int completeRequestsCount_;\n> +\n> +\tvoid bufferComplete(Request *request, Buffer *buffer)\n> +\t{\n> +\t\tif (buffer->status() != Buffer::BufferSuccess)\n> +\t\t\treturn;\n> +\n> +\t\tcompleteBuffersCount_++;\n> +\t}\n> +\n> +\tvoid requestComplete(Request *request, const std::map &buffers)\n> +\t{\n> +\t\tif (request->status() != Request::RequestComplete)\n> +\t\t\treturn;\n> +\n> +\t\tcompleteRequestsCount_++;\n> +\n> +\t\t/* Reuse the buffers for a new request. */\n> +\t\trequest = camera_->createRequest();\n> +\t\trequest->setBuffers(buffers);\n> +\t\tcamera_->queueRequest(request);\n> +\t}\n> +\n> +\tint run()\n> +\t{\n> +\t\tStream *stream = *camera_->streams().begin();\n> +\t\tstd::set streams = { stream };\n> +\t\tstd::map conf =\n> +\t\t\tcamera_->streamConfiguration(streams);\n> +\t\tStreamConfiguration *sconf = &conf.begin()->second;\n> +\n> +\t\tif (!configurationValid(streams, conf)) {\n> +\t\t\tcout << \"Reading default configuration\" << endl;\n\n\"Failed to read\"\n\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (camera_->acquire()) {\n> +\t\t\tcout << \"Acquiring the camera\" << endl;\n\nSame here. And everywhere below.\n\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (camera_->configureStreams(conf)) {\n> +\t\t\tcout << \"Setting default configuration\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (camera_->allocateBuffers()) {\n> +\t\t\tcout << \"Allocating buffers\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tBufferPool &pool = stream->bufferPool();\n> +\t\tstd::vector requests;\n> +\t\tfor (Buffer &buffer : pool.buffers()) {\n> +\t\t\tRequest *request = camera_->createRequest();\n> +\t\t\tif (!request) {\n> +\t\t\t\tcout << \"Creating requests\" << endl;\n> +\t\t\t\treturn TestFail;\n> +\t\t\t}\n> +\n> +\t\t\tstd::map map = { { stream, &buffer } };\n> +\t\t\tif (request->setBuffers(map)) {\n> +\t\t\t\tcout << \"Associating buffers with requests\" << endl;\n> +\t\t\t\treturn TestFail;\n> +\t\t\t}\n> +\n> +\t\t\trequests.push_back(request);\n> +\t\t}\n> +\n> +\t\tcompleteRequestsCount_ = 0;\n> +\t\tcompleteBuffersCount_ = 0;\n> +\n> +\t\tcamera_->bufferCompleted.connect(this, &Capture::bufferComplete);\n> +\t\tcamera_->requestCompleted.connect(this, &Capture::requestComplete);\n> +\n> +\t\tif (camera_->start()) {\n> +\t\t\tcout << \"Starting camera\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tfor (Request *request : requests) {\n> +\t\t\tif (camera_->queueRequest(request)) {\n> +\t\t\t\tcout << \"Queueing requests\" << endl;\n> +\t\t\t\treturn TestFail;\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\tEventDispatcher *dispatcher = CameraManager::instance()->eventDispatcher();\n> +\n> +\t\tTimer timer;\n> +\t\ttimer.start(100);\n> +\t\twhile (timer.isRunning())\n> +\t\t\tdispatcher->processEvents();\n> +\n> +\t\tif (completeRequestsCount_ <= sconf->bufferCount * 2 ||\n> +\t\t completeRequestsCount_ != completeBuffersCount_) {\n> +\t\t\tcout << \"Capture failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n\nI would split those two error cases and detail the failure messages.\nMaybe \"Failed to capture enough frames (got %u, expected at least %u)\"\nand \"Number of completed buffers and requests differ\" ?\n\nWith these small issues fixed,\n\nReviewed-by: Laurent Pinchart \n\n> +\n> +\t\tif (camera_->stop()) {\n> +\t\t\tcout << \"Stopping camera\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (camera_->freeBuffers()) {\n> +\t\t\tcout << \"Freeing buffers\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +};\n> +\n> +} /* namespace */\n> +\n> +TEST_REGISTER(Capture);\n> diff --git a/test/camera/meson.build b/test/camera/meson.build\n> index ed4837190ff6c52c..a1fd20326e97538b 100644\n> --- a/test/camera/meson.build\n> +++ b/test/camera/meson.build\n> @@ -3,6 +3,7 @@\n> camera_tests = [\n> [ 'configuration_default', 'configuration_default.cpp' ],\n> [ 'configuration_set', 'configuration_set.cpp' ],\n> + [ 'capture', 'capture.cpp' ],\n> ]\n> \n> foreach t : camera_tests","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 D8DF5600FC\n\tfor ;\n\tMon, 11 Mar 2019 09:45:13 +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 5A762255;\n\tMon, 11 Mar 2019 09:45:13 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1552293913;\n\tbh=5QDeY/u5g8C2idCrkDVLXtr5ivwy2Wme/0VLJ0HAv1A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Y5nJS04u0kN+sYDKN9rb2WU9f3RInbCj2MsFh99K4XqHyNQKjSg9hLkRoQuxiQfeD\n\tPZaBXASyWVcQUccADA21uwDnQASqj6UhzTmM1XaHKrRwydFEv42jMaVqtvmH83OLTZ\n\tCfMBdR2FPBWHcL7GN6og/ztV1Mm3XYvaQXi/j7IE=","Date":"Mon, 11 Mar 2019 10:45:07 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190311084507.GE4775@pendragon.ideasonboard.com>","References":"<20190311022232.4759-1-niklas.soderlund@ragnatech.se>\n\t<20190311022232.4759-4-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":"<20190311022232.4759-4-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 3/4] test: camera: Add capture 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:45:14 -0000"}}]