[libcamera-devel,v2,3/4] tests: v4l2_videodevice: Set media bus and pixel formats for vimc

Message ID 20190807204915.23942-4-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • libcamera: Fix issues with vimc and Linux v5.2
Related show

Commit Message

Niklas Söderlund Aug. 7, 2019, 8:49 p.m. UTC
Most of the video device tests are based on vimc and Linux commit
85ab1aa1fac17bcd ("media: vimc: deb: fix default sink bayer format")
changes the default media bus format for the debayer subdevices. This
leads to a -EPIPE error when trying to use the raw capture video device
nodes.

Fix this by explicitly setting media bus and pixel formats to known good
values which works before and after the upstream change.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 .../v4l2_videodevice_test.cpp                 | 26 +++++++++++++++++++
 test/v4l2_videodevice/v4l2_videodevice_test.h |  7 ++++-
 2 files changed, 32 insertions(+), 1 deletion(-)

Comments

Laurent Pinchart Aug. 7, 2019, 9:20 p.m. UTC | #1
Hi Niklas,

Thank you for the patch.

On Wed, Aug 07, 2019 at 10:49:14PM +0200, Niklas Söderlund wrote:
> Most of the video device tests are based on vimc and Linux commit
> 85ab1aa1fac17bcd ("media: vimc: deb: fix default sink bayer format")
> changes the default media bus format for the debayer subdevices. This
> leads to a -EPIPE error when trying to use the raw capture video device
> nodes.
> 
> Fix this by explicitly setting media bus and pixel formats to known good
> values which works before and after the upstream change.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  .../v4l2_videodevice_test.cpp                 | 26 +++++++++++++++++++
>  test/v4l2_videodevice/v4l2_videodevice_test.h |  7 ++++-
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.cpp b/test/v4l2_videodevice/v4l2_videodevice_test.cpp
> index b26d06ad45197c8f..a0d269fef7f43895 100644
> --- a/test/v4l2_videodevice/v4l2_videodevice_test.cpp
> +++ b/test/v4l2_videodevice/v4l2_videodevice_test.cpp
> @@ -8,6 +8,8 @@
>  #include <iostream>
>  #include <sys/stat.h>
>  
> +#include <linux/media-bus-format.h>
> +
>  #include "v4l2_videodevice_test.h"
>  
>  #include "device_enumerator.h"
> @@ -69,6 +71,28 @@ int V4L2VideoDeviceTest::init()
>  	if (capture_->getFormat(&format))
>  		return TestFail;
>  
> +	if (driver_ == "vimc") {
> +		sensor_ = new CameraSensor(media_->getEntityByName("Sensor A"));
> +		if (sensor_->init())
> +			return TestFail;
> +
> +		debayer_ = new V4L2Subdevice(media_->getEntityByName("Debayer A"));
> +		if (debayer_->open())
> +			return TestFail;

Could sensor_ and debayer_ be local variables ? Apart from that,

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

> +
> +		format.fourcc = V4L2_PIX_FMT_SBGGR8;
> +
> +		V4L2SubdeviceFormat subformat = {};
> +		subformat.mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8;
> +		subformat.size = format.size;
> +
> +		if (sensor_->setFormat(&subformat))
> +			return TestFail;
> +
> +		if (debayer_->setFormat(0, &subformat))
> +			return TestFail;
> +	}
> +
>  	format.size.width = 640;
>  	format.size.height = 480;
>  	if (capture_->setFormat(&format))
> @@ -83,5 +107,7 @@ void V4L2VideoDeviceTest::cleanup()
>  	capture_->releaseBuffers();
>  	capture_->close();
>  
> +	delete debayer_;
> +	delete sensor_;
>  	delete capture_;
>  };
> diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.h b/test/v4l2_videodevice/v4l2_videodevice_test.h
> index 3321b5a4f98fdb1d..34dd231c6d9d108d 100644
> --- a/test/v4l2_videodevice/v4l2_videodevice_test.h
> +++ b/test/v4l2_videodevice/v4l2_videodevice_test.h
> @@ -13,8 +13,10 @@
>  
>  #include "test.h"
>  
> +#include "camera_sensor.h"
>  #include "device_enumerator.h"
>  #include "media_device.h"
> +#include "v4l2_subdevice.h"
>  #include "v4l2_videodevice.h"
>  
>  using namespace libcamera;
> @@ -23,7 +25,8 @@ class V4L2VideoDeviceTest : public Test
>  {
>  public:
>  	V4L2VideoDeviceTest(const char *driver, const char *entity)
> -		: driver_(driver), entity_(entity), capture_(nullptr)
> +		: driver_(driver), entity_(entity), sensor_(nullptr),
> +		  debayer_(nullptr), capture_(nullptr)
>  	{
>  	}
>  
> @@ -35,6 +38,8 @@ protected:
>  	std::string entity_;
>  	std::unique_ptr<DeviceEnumerator> enumerator_;
>  	std::shared_ptr<MediaDevice> media_;
> +	CameraSensor *sensor_;
> +	V4L2Subdevice *debayer_;
>  	V4L2VideoDevice *capture_;
>  	BufferPool pool_;
>  };

Patch

diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.cpp b/test/v4l2_videodevice/v4l2_videodevice_test.cpp
index b26d06ad45197c8f..a0d269fef7f43895 100644
--- a/test/v4l2_videodevice/v4l2_videodevice_test.cpp
+++ b/test/v4l2_videodevice/v4l2_videodevice_test.cpp
@@ -8,6 +8,8 @@ 
 #include <iostream>
 #include <sys/stat.h>
 
+#include <linux/media-bus-format.h>
+
 #include "v4l2_videodevice_test.h"
 
 #include "device_enumerator.h"
@@ -69,6 +71,28 @@  int V4L2VideoDeviceTest::init()
 	if (capture_->getFormat(&format))
 		return TestFail;
 
+	if (driver_ == "vimc") {
+		sensor_ = new CameraSensor(media_->getEntityByName("Sensor A"));
+		if (sensor_->init())
+			return TestFail;
+
+		debayer_ = new V4L2Subdevice(media_->getEntityByName("Debayer A"));
+		if (debayer_->open())
+			return TestFail;
+
+		format.fourcc = V4L2_PIX_FMT_SBGGR8;
+
+		V4L2SubdeviceFormat subformat = {};
+		subformat.mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8;
+		subformat.size = format.size;
+
+		if (sensor_->setFormat(&subformat))
+			return TestFail;
+
+		if (debayer_->setFormat(0, &subformat))
+			return TestFail;
+	}
+
 	format.size.width = 640;
 	format.size.height = 480;
 	if (capture_->setFormat(&format))
@@ -83,5 +107,7 @@  void V4L2VideoDeviceTest::cleanup()
 	capture_->releaseBuffers();
 	capture_->close();
 
+	delete debayer_;
+	delete sensor_;
 	delete capture_;
 };
diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.h b/test/v4l2_videodevice/v4l2_videodevice_test.h
index 3321b5a4f98fdb1d..34dd231c6d9d108d 100644
--- a/test/v4l2_videodevice/v4l2_videodevice_test.h
+++ b/test/v4l2_videodevice/v4l2_videodevice_test.h
@@ -13,8 +13,10 @@ 
 
 #include "test.h"
 
+#include "camera_sensor.h"
 #include "device_enumerator.h"
 #include "media_device.h"
+#include "v4l2_subdevice.h"
 #include "v4l2_videodevice.h"
 
 using namespace libcamera;
@@ -23,7 +25,8 @@  class V4L2VideoDeviceTest : public Test
 {
 public:
 	V4L2VideoDeviceTest(const char *driver, const char *entity)
-		: driver_(driver), entity_(entity), capture_(nullptr)
+		: driver_(driver), entity_(entity), sensor_(nullptr),
+		  debayer_(nullptr), capture_(nullptr)
 	{
 	}
 
@@ -35,6 +38,8 @@  protected:
 	std::string entity_;
 	std::unique_ptr<DeviceEnumerator> enumerator_;
 	std::shared_ptr<MediaDevice> media_;
+	CameraSensor *sensor_;
+	V4L2Subdevice *debayer_;
 	V4L2VideoDevice *capture_;
 	BufferPool pool_;
 };