[libcamera-devel,10/10] test: camera: buffer_import: Use DRM/KMS format
diff mbox series

Message ID 20191027234312.35284-11-jacopo@jmondi.org
State Superseded
Headers show
Series
  • libcamera: Use DRM_FORMAT_* fourcc codes
Related show

Commit Message

Jacopo Mondi Oct. 27, 2019, 11:43 p.m. UTC 
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 test/camera/buffer_import.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Laurent Pinchart Oct. 28, 2019, 12:13 a.m. UTC | #1 
Hi Jacopo,

Thank you for the patch.

On Mon, Oct 28, 2019 at 12:43:12AM +0100, Jacopo Mondi wrote:
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  test/camera/buffer_import.cpp | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp
> index 9cac19d8ce81..8411e69bbd09 100644
> --- a/test/camera/buffer_import.cpp
> +++ b/test/camera/buffer_import.cpp
> @@ -13,6 +13,8 @@
>  #include <random>
>  #include <vector>
>  
> +#include <linux/drm_fourcc.h>
> +
>  #include "device_enumerator.h"
>  #include "media_device.h"
>  #include "v4l2_videodevice.h"
> @@ -79,7 +81,7 @@ public:
>  
>  		format_.size.width = 1920;
>  		format_.size.height = 1080;
> -		format_.fourcc = V4L2_PIX_FMT_RGB24;
> +		format_.fourcc = DRM_FORMAT_BGR888;
>  		format_.planesCount = 1;
>  		format_.planes[0].size = 1920 * 1080 * 3;
>  		format_.planes[0].bpl = 1920 * 3;

This is actually not Reviewed-by as it breaks the test :-) format_ here
is a V4L2DeviceFormat so it should use V4L2 4CCs. You can just drop this
patch.
Laurent Pinchart Oct. 28, 2019, 12:27 a.m. UTC | #2 
Hi Jacopo,

On Mon, Oct 28, 2019 at 02:13:47AM +0200, Laurent Pinchart wrote:
> On Mon, Oct 28, 2019 at 12:43:12AM +0100, Jacopo Mondi wrote:
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  test/camera/buffer_import.cpp | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp
> > index 9cac19d8ce81..8411e69bbd09 100644
> > --- a/test/camera/buffer_import.cpp
> > +++ b/test/camera/buffer_import.cpp
> > @@ -13,6 +13,8 @@
> >  #include <random>
> >  #include <vector>
> >  
> > +#include <linux/drm_fourcc.h>
> > +
> >  #include "device_enumerator.h"
> >  #include "media_device.h"
> >  #include "v4l2_videodevice.h"
> > @@ -79,7 +81,7 @@ public:
> >  
> >  		format_.size.width = 1920;
> >  		format_.size.height = 1080;
> > -		format_.fourcc = V4L2_PIX_FMT_RGB24;
> > +		format_.fourcc = DRM_FORMAT_BGR888;
> >  		format_.planesCount = 1;
> >  		format_.planes[0].size = 1920 * 1080 * 3;
> >  		format_.planes[0].bpl = 1920 * 3;
> 
> This is actually not Reviewed-by as it breaks the test :-) format_ here
> is a V4L2DeviceFormat so it should use V4L2 4CCs. You can just drop this
> patch.

I spoke a bit too soon, dropping the patch isn't enough. It should be
replaced with the following patch.

>From 9611d40c66dfbfdd7688886ecc0c21584ad86dcb Mon Sep 17 00:00:00 2001
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: Fri, 25 Oct 2019 18:47:38 +0200
Subject: [PATCH] test: camera: buffer_import: Convert to PixelFormat

Convert the V4L2 4CC retrieved from the exporter video device to a
PixelFormat for the camera configuration.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 test/camera/buffer_import.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp
index 9cac19d8ce81..f62ccade066b 100644
--- a/test/camera/buffer_import.cpp
+++ b/test/camera/buffer_import.cpp
@@ -318,7 +318,7 @@ protected:
 
 		StreamConfiguration &cfg = config->at(0);
 		cfg.size = format.size;
-		cfg.pixelFormat = format.fourcc;
+		cfg.pixelFormat = video_->toPixelFormat(format_.fourcc);
 		cfg.bufferCount = CAMERA_BUFFER_COUNT;
 		cfg.memoryType = ExternalMemory;
 

Should this be squashed with "[PATCH 08/10] libcamera: pipeline: Use
PixelFormat for application formats" to avoid breaking bisection ? Same
for 09/10 actually.
Laurent Pinchart Oct. 28, 2019, 12:37 a.m. UTC | #3 
On Mon, Oct 28, 2019 at 02:27:07AM +0200, Laurent Pinchart wrote:
> On Mon, Oct 28, 2019 at 02:13:47AM +0200, Laurent Pinchart wrote:
> > On Mon, Oct 28, 2019 at 12:43:12AM +0100, Jacopo Mondi wrote:
> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > > ---
> > >  test/camera/buffer_import.cpp | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp
> > > index 9cac19d8ce81..8411e69bbd09 100644
> > > --- a/test/camera/buffer_import.cpp
> > > +++ b/test/camera/buffer_import.cpp
> > > @@ -13,6 +13,8 @@
> > >  #include <random>
> > >  #include <vector>
> > >  
> > > +#include <linux/drm_fourcc.h>
> > > +
> > >  #include "device_enumerator.h"
> > >  #include "media_device.h"
> > >  #include "v4l2_videodevice.h"
> > > @@ -79,7 +81,7 @@ public:
> > >  
> > >  		format_.size.width = 1920;
> > >  		format_.size.height = 1080;
> > > -		format_.fourcc = V4L2_PIX_FMT_RGB24;
> > > +		format_.fourcc = DRM_FORMAT_BGR888;
> > >  		format_.planesCount = 1;
> > >  		format_.planes[0].size = 1920 * 1080 * 3;
> > >  		format_.planes[0].bpl = 1920 * 3;
> > 
> > This is actually not Reviewed-by as it breaks the test :-) format_ here
> > is a V4L2DeviceFormat so it should use V4L2 4CCs. You can just drop this
> > patch.
> 
> I spoke a bit too soon, dropping the patch isn't enough. It should be
> replaced with the following patch.
> 
> From 9611d40c66dfbfdd7688886ecc0c21584ad86dcb Mon Sep 17 00:00:00 2001
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Date: Fri, 25 Oct 2019 18:47:38 +0200
> Subject: [PATCH] test: camera: buffer_import: Convert to PixelFormat
> 
> Convert the V4L2 4CC retrieved from the exporter video device to a
> PixelFormat for the camera configuration.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  test/camera/buffer_import.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp
> index 9cac19d8ce81..f62ccade066b 100644
> --- a/test/camera/buffer_import.cpp
> +++ b/test/camera/buffer_import.cpp
> @@ -318,7 +318,7 @@ protected:
>  
>  		StreamConfiguration &cfg = config->at(0);
>  		cfg.size = format.size;
> -		cfg.pixelFormat = format.fourcc;
> +		cfg.pixelFormat = video_->toPixelFormat(format_.fourcc);

It would help if I tested code before sending it...

>  		cfg.bufferCount = CAMERA_BUFFER_COUNT;
>  		cfg.memoryType = ExternalMemory;
>  
> 
> Should this be squashed with "[PATCH 08/10] libcamera: pipeline: Use
> PixelFormat for application formats" to avoid breaking bisection ? Same
> for 09/10 actually.

>From 4b1270ea094dfa24a0953d60adb19aa29182b4f8 Mon Sep 17 00:00:00 2001
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: Fri, 25 Oct 2019 18:47:38 +0200
Subject: [PATCH] test: camera: buffer_import: Convert to PixelFormat

Convert the V4L2 4CC retrieved from the exporter video device to a
PixelFormat for the camera configuration.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 test/camera/buffer_import.cpp | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp
index 9cac19d8ce81..bbc5a25c4019 100644
--- a/test/camera/buffer_import.cpp
+++ b/test/camera/buffer_import.cpp
@@ -155,7 +155,16 @@ public:
 	}
 
 	bool done() const { return done_; }
-	const V4L2DeviceFormat &format() const { return format_; }
+
+	PixelFormat format() const
+	{
+		return video_->toPixelFormat(format_.fourcc);
+	}
+
+	const Size &size() const
+	{
+		return format_.size;
+	}
 
 	Signal<uint64_t, int> requestReady;
 
@@ -314,11 +323,9 @@ protected:
 			return TestFail;
 		}
 
-		const V4L2DeviceFormat &format = sink_.format();
-
 		StreamConfiguration &cfg = config->at(0);
-		cfg.size = format.size;
-		cfg.pixelFormat = format.fourcc;
+		cfg.size = sink_.size();
+		cfg.pixelFormat = sink_.format();
 		cfg.bufferCount = CAMERA_BUFFER_COUNT;
 		cfg.memoryType = ExternalMemory;

Patch
diff mbox series 

diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp
index 9cac19d8ce81..8411e69bbd09 100644
--- a/test/camera/buffer_import.cpp
+++ b/test/camera/buffer_import.cpp
@@ -13,6 +13,8 @@ 
 #include <random>
 #include <vector>
 
+#include <linux/drm_fourcc.h>
+
 #include "device_enumerator.h"
 #include "media_device.h"
 #include "v4l2_videodevice.h"
@@ -79,7 +81,7 @@  public:
 
 		format_.size.width = 1920;
 		format_.size.height = 1080;
-		format_.fourcc = V4L2_PIX_FMT_RGB24;
+		format_.fourcc = DRM_FORMAT_BGR888;
 		format_.planesCount = 1;
 		format_.planes[0].size = 1920 * 1080 * 3;
 		format_.planes[0].bpl = 1920 * 3;