[libcamera-devel] [PATCH 4/5] test: v4l2_device: Reset media links and set a resolution

[Laurent Pinchart]

Hello,

On Thu, May 09, 2019 at 10:30:41AM +0100, Kieran Bingham wrote:
> On 08/05/2019 17:58, Niklas Söderlund wrote:
> > When initializing the test reset any media links and set a know
> > resolutions. This is needed to put the device under test into known
> > state and not have the v4l2 device tests depend on that no one have
> > touched the device before the test is executed.
> > 
> > The resolution is picked purely at random and could possibly be moved to
> > each test case if there is a need for different resolutions for a
> > specific one.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> >  test/v4l2_device/v4l2_device_test.cpp | 19 ++++++++++++++++++-
> >  1 file changed, 18 insertions(+), 1 deletion(-)
> > 
> > diff --git a/test/v4l2_device/v4l2_device_test.cpp b/test/v4l2_device/v4l2_device_test.cpp
> > index ee5a8e009bef2a5e..5bd80a6c68d796b6 100644
> > --- a/test/v4l2_device/v4l2_device_test.cpp
> > +++ b/test/v4l2_device/v4l2_device_test.cpp
> > @@ -54,7 +54,24 @@ int V4L2DeviceTest::init()
> >  	if (!capture_)
> >  		return TestFail;
> >  
> > -	return capture_->open();
> > +	media_->acquire();
> 
> Should you check the return code of acquire?
> 
> Does it block using lockf() internally?

It won't block, it will return an error, so I think it's a good idea to
check for that.

> > +	if (media_->disableLinks())
> 
> Where do we recreate / re-enable the links?
> 
> or does it reset to a sane default?

It won't enable links, so any test depending on this would have to
enable links manually.

> > +		return TestFail;
> 
> Is it an issue here that the media_ is not released? I now can't recall
> if the acquire/release is about locking globally using lockf() or if
> it's just a state machine state change.
> 
> If it must be released - perhaps we should do some sort of auto-lock
> release by using the destructor of a local object, so that it releases
> when it goes out of scope.

lockf() will not persist once the file descriptor is closed, so it
shouldn't be a problem, but proper error handling is still a good idea.

With those issues fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> > +	media_->release();
> > +
> > +	if (capture_->open())
> > +		return TestFail;
> > +
> > +	V4L2DeviceFormat format = {};
> > +	if (capture_->getFormat(&format))
> > +		return TestFail;
> > +
> > +	format.size.width = 640;
> > +	format.size.height = 480;
> > +	if (capture_->setFormat(&format))
> > +		return TestFail;
> > +
> > +	return TestPass;
> >  }
> >  
> >  void V4L2DeviceTest::cleanup()

-- 
Regards,

Laurent Pinchart