[libcamera-devel] [PATCH v5 8/9] test: v4l2_subdevice: Add ListFormat test

[Laurent Pinchart]

Hi Jacopo,

On Fri, Mar 01, 2019 at 10:27:03AM +0100, Jacopo Mondi wrote:
> On Fri, Mar 01, 2019 at 12:16:57AM +0200, Laurent Pinchart wrote:
> > On Thu, Feb 28, 2019 at 09:01:50PM +0100, Jacopo Mondi wrote:
> >> Add test to list formats on a v4l2 subdevice.
> >>
> >> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> >> ---
> >>  test/v4l2_subdevice/list_formats.cpp | 85 ++++++++++++++++++++++++++++
> >>  test/v4l2_subdevice/meson.build      |  1 +
> >>  2 files changed, 86 insertions(+)
> >>  create mode 100644 test/v4l2_subdevice/list_formats.cpp
> >>
> >> diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp
> >> new file mode 100644
> >> index 000000000000..b902a198b17e
> >> --- /dev/null
> >> +++ b/test/v4l2_subdevice/list_formats.cpp
> >> @@ -0,0 +1,85 @@
> >> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> >> +/*
> >> + * Copyright (C) 2019, Google Inc.
> >> + *
> >> + * libcamera V4L2 Subdevice format handling test
> >> + */
> >> +
> >> +#include <iostream>
> >> +#include <vector>
> >> +
> >> +#include "geometry.h"
> >> +#include "v4l2_subdevice.h"
> >> +#include "v4l2_subdevice_test.h"
> >> +
> >> +using namespace std;
> >> +using namespace libcamera;
> >> +
> >> +/* List image formats on the "Scaler" subdevice of vimc media device.  */
> >> +
> >> +class ListFormatsTest : public V4L2SubdeviceTest
> >> +{
> >> +protected:
> >> +	int run() override;
> >> +
> >> +private:
> >> +	void printFormats(unsigned int pad, unsigned code,
> >> +			  std::vector<SizeRange> &formats);
> >> +};
> >> +
> >> +void ListFormatsTest::printFormats(unsigned int pad,
> >> +				   unsigned int code,
> >> +				   std::vector<SizeRange> &sizes)
> >> +{
> >> +	cout << "Enumerate formats on pad " << pad << endl;
> >> +	for (SizeRange &size : sizes) {
> >> +		cout << "	Mbus code: 0x" << hex << code << endl;
> >
> > I'd write "media bus code" in full, or if you want to abbreviate, "mbus
> > code".
> >
> > Should you ensure that at least 4 characters get printed ?
> 
> Sorry, didn't get this..

I meant the equivalent of the 0x%04x printf specifier.

> >> +		cout << "	min Width: " << dec << size.minWidth << endl;
> >> +		cout << "	min Height: " << dec << size.minHeight << endl;
> >> +		cout << "	max Width: " << dec << size.maxWidth << endl;
> >> +		cout << "	max Height: " << dec << size.maxHeight << endl;
> >
> > s/Width/width/ and s/Height/height/ ?
> >
> >> +	}
> >> +}
> >> +
> >> +int ListFormatsTest::run()
> >> +{
> >> +	/* List all formats available on existing "Scaler" pads. */
> >> +	std::map<unsigned int, std::vector<SizeRange>> formats;
> >> +
> >> +	formats = scaler_->formats(0);
> >> +	if (formats.size() == 0) {
> >> +		cerr << "Failed to list formats on pad 0 of subdevice "
> >> +		     << scaler_->deviceName() << endl;
> >> +		return TestFail;
> >> +	}
> >> +	auto it = formats.begin();
> >> +	while (it != formats.end()) {
> >> +		printFormats(0, it->first, it->second);
> >> +		++it;
> >> +	}
> >
> > How about
> >
> > 	for (auto it = formats.begin(); it != formats.end(); ++it)
> > 		printFormats(0, it->first, it->second);
> >
> >> +
> >> +	formats = scaler_->formats(1);
> >> +	if (formats.size() == 0) {
> >> +		cerr << "Failed to list formats on pad 1 of subdevice "
> >> +		     << scaler_->deviceName() << endl;
> >> +		return TestFail;
> >> +	}
> >> +	it = formats.begin();
> >> +	while (it != formats.end()) {
> >> +		printFormats(1, it->first, it->second);
> >> +		++it;
> >> +	}
> >
> > Same here.
> >
> >> +
> >> +	/* List format on a non-existing pad, format vector shall be empty. */
> >> +	formats = scaler_->formats(2);
> >> +	if (formats.size() != 0) {
> >
> > Maybe
> >
> > 	if (!formats.empty()) {
> >
> 
> Ok, I'll update this.
> 
> >> +		cerr << "Listing formats on non-existing pad 2 of subdevice "
> >> +		     << scaler_->deviceName()
> >> +		     << " should return an empty format list" << endl;
> >> +		return TestFail;
> >> +	}
> >> +
> >> +	return TestPass;
> >> +}
> >> +
> >> +TEST_REGISTER(ListFormatsTest);
> >> diff --git a/test/v4l2_subdevice/meson.build b/test/v4l2_subdevice/meson.build
> >> index f45dca0d23d7..80cfbbbf9413 100644
> >> --- a/test/v4l2_subdevice/meson.build
> >> +++ b/test/v4l2_subdevice/meson.build
> >> @@ -1,4 +1,5 @@
> >>  v4l2_subdevice_tests = [
> >> +  [ 'list_formats',             'list_formats.cpp'],
> >>    [ 'test_formats',             'test_formats.cpp'],
> >>  ]
> >>

-- 
Regards,

Laurent Pinchart