Em Fri, 10 Mar 2017 15:57:09 +0000
Russell King - ARM Linux [off-list ref] escreveu:

On Fri, Mar 10, 2017 at 12:26:34PM -0300, Mauro Carvalho Chehab wrote:

quoted

Hi Russell,

Em Fri, 10 Mar 2017 13:07:33 +0000
Russell King - ARM Linux [off-list ref] escreveu:
  

quoted

The idea that the v4l libraries should intercept the format negotiation
between the application and kernel is a particularly painful one - the
default gstreamer build detects the v4l libraries, and links against it.
That much is fine.

However, the problem comes when you're trying to use bayer formats. The
v4l libraries "helpfully" (or rather unhelpfully) intercept the format
negotiation, and decide that they'll invoke v4lconvert to convert the
bayer to RGB for you, whether you want them to do that or not.

v4lconvert may not be the most efficient way to convert, or even what
is desired (eg, you may want to receive the raw bayer image.)  However,
since the v4l libraries/v4lconvert gives you no option but to have its
conversion forced into the pipeline, other options (such as using the
gstreamer neon accelerated de-bayer plugin) isn't an option   

That's not true. There is an special flag, used only by libv4l2
emulated formats, that indicates when a video format is handled
via v4lconvert:  

I'm afraid that my statement comes from trying to use gstreamer with
libv4l2 and _not_ being able to use the 8-bit bayer formats there at
all - they are simply not passed across to the application through
libv4l2/v4lconvert.

Instead, the formats that are passed across are the emulated formats.
As I said above, that forces applications to use only the v4lconvert
formats, the raw formats are not available.

So, the presence or absence of the V4L2_FMT_FLAG_EMULATED is quite
meaningless if you can't even enumerate the non-converted formats.

The problem comes from the "always needs conversion" stuff in
v4lconvert coupled with the way this subdev stuff works - since it
requires manual configuration of all the pads within the kernel
media pipeline, the kernel ends up only advertising _one_ format
to userspace - in my case, that's RGGB8.

When v4lconvert_create_with_dev_ops() enumerates the formats from
the kernel, it gets only RGGB8.  That causes always_needs_conversion
in there to remain true, so the special v4l control which enables/
disables conversion gets created with a default value of "true".
The RGGB8 bit is also set in data->supported_src_formats.

This causes v4lconvert_supported_dst_fmt_only() to return true.

What this all means is that v4lconvert_enum_fmt() will _not_ return
any of the kernel formats, only the faked formats.

Ergo, the RGGB8 format from the kernel is completely hidden from the
application, and only the emulated format is made available.  As I
said above, this forces v4lconvert's debayering on the application,
whether you want it or not.

In the gstreamer case, it knows nothing about this special control,
which means that trying to use this gstreamer pipeline:

$ gst-launch-1.0 v4l2src device=/dev/video6 ! bayer2rgbneon ! xvimagesink

is completely impossible without first rebuilding gstreamer _without_
libv4l support.  Build gstreamer without libv4l support, and the above
works.

Enabling debug output in gstreamer's v4l2src plugin confirms that
the kernel's bayer format are totally hidden from gstreamer when
linked with libv4l2, but are present when it isn't linked with
libv4l2.

Argh! that is indeed a bug at libv4l (and maybe at gstreamer).

I guess that the always_needs_conversion logic was meant to be used to
really odd proprietary formats, e. g:
	
/*  Vendor-specific formats   */
#define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */
#define V4L2_PIX_FMT_WNVA     v4l2_fourcc('W', 'N', 'V', 'A') /* Winnov hw compress */
#define V4L2_PIX_FMT_SN9C10X  v4l2_fourcc('S', '9', '1', '0') /* SN9C10x compression */
...

I suspect that nobody uses libv4l2 with MC-based V4L2 devices. That's
likely why nobody reported this bug before (that I know of).

In any case, for non-proprietary formats, the default should be to
always offer both the emulated format and the original one.

I suspect that the enclosed patch should fix the issue with bayer formats.

Thanks,
Mauro

[PATCH RFC] libv4lconvert: by default, offer the original format to the client

Applications should have the right to decide between using a
libv4lconvert emulated format or to implement the decoding themselves,
as this may have significative performance impact.

So, change the default to always show both formats.

Change also the default for Bayer encoded formats, as userspace
likely will want to handle it directly.

Signed-off-by: Mauro Carvalho Chehab <redacted>

diff --git a/lib/libv4lconvert/libv4lconvert.c b/lib/libv4lconvert/libv4lconvert.c
index da718918b030..2e5458fa420d 100644
--- a/lib/libv4lconvert/libv4lconvert.c
+++ b/lib/libv4lconvert/libv4lconvert.c

@@ -118,10 +118,10 @@ static const struct v4lconvert_pixfmt supported_src_pixfmts[] = {
 	{ V4L2_PIX_FMT_OV511,		 0,	 7,	 7,	1 },
 	{ V4L2_PIX_FMT_OV518,		 0,	 7,	 7,	1 },
 	/* uncompressed bayer */
-	{ V4L2_PIX_FMT_SBGGR8,		 8,	 8,	 8,	1 },
-	{ V4L2_PIX_FMT_SGBRG8,		 8,	 8,	 8,	1 },
-	{ V4L2_PIX_FMT_SGRBG8,		 8,	 8,	 8,	1 },
-	{ V4L2_PIX_FMT_SRGGB8,		 8,	 8,	 8,	1 },
+	{ V4L2_PIX_FMT_SBGGR8,		 8,	 8,	 8,	0 },
+	{ V4L2_PIX_FMT_SGBRG8,		 8,	 8,	 8,	0 },
+	{ V4L2_PIX_FMT_SGRBG8,		 8,	 8,	 8,	0 },
+	{ V4L2_PIX_FMT_SRGGB8,		 8,	 8,	 8,	0 },
 	{ V4L2_PIX_FMT_STV0680,		 8,	 8,	 8,	1 },
 	/* compressed bayer */
 	{ V4L2_PIX_FMT_SPCA561,		 0,	 9,	 9,	1 },

@@ -178,7 +178,7 @@ struct v4lconvert_data *v4lconvert_create_with_dev_ops(int fd, void *dev_ops_pri
 	/* This keeps tracks of devices which have only formats for which apps
 	   most likely will need conversion and we can thus safely add software
 	   processing controls without a performance impact. */
-	int always_needs_conversion = 1;
+	int always_needs_conversion = 0;
 
 	if (!data) {
 		fprintf(stderr, "libv4lconvert: error: out of memory!\n");

@@ -208,8 +208,8 @@ struct v4lconvert_data *v4lconvert_create_with_dev_ops(int fd, void *dev_ops_pri
 		if (j < ARRAY_SIZE(supported_src_pixfmts)) {
 			data->supported_src_formats |= 1ULL << j;
 			v4lconvert_get_framesizes(data, fmt.pixelformat, j);
-			if (!supported_src_pixfmts[j].needs_conversion)
-				always_needs_conversion = 0;
+			if (supported_src_pixfmts[j].needs_conversion)
+				always_needs_conversion = 1;
 		} else
 			always_needs_conversion = 0;
 	}