[PATCH v2 0/4] vicodec: a couple fixes towards spec compliancy

[PATCH v2 0/4] vicodec: a couple fixes towards spec compliancy

[Ezequiel Garcia]

Given the stateful codec specification is still a moving target,
it doesn't make any sense to try to comply fully with it.

On the other side, we can still comply with some basic userspace
expectations, with just a couple small changes.

This series implements proper resolution changes propagation,
and fixes the CMD_STOP so it actually works.

The intention of this series is to be able to test this driver
using already existing userspace, gstreamer in particular.
With this changes, it's possible to construct variations of
this pipeline:

  gst-launch-1.0 videotestsrc ! v4l2fwhtenc ! v4l2fwhtdec ! fakevideosink

Also, as discussed in v1 feedback [1,2], I'm including pixel format
helpers, as RFC for now. Hans, Tomasz: is this what you had in mind?

[1] https://www.spinics.net/lists/linux-media/msg141912.html
[2] https://www.spinics.net/lists/linux-media/msg142099.html

v2:
  * Add more info to commit logs
  * Propagate changes on both encoders and decoders
  * Add pixel format helpers

Ezequiel Garcia (4):
  media: Introduce helpers to fill pixel format structs
  vicodec: Use pixel format helpers
  vicodec: Propagate changes to the CAPTURE queue
  vicodec: Implement spec-compliant stop command

 .../media/platform/vicodec/codec-v4l2-fwht.c  |  42 ++--
 .../media/platform/vicodec/codec-v4l2-fwht.h  |   3 -
 drivers/media/platform/vicodec/vicodec-core.c | 197 +++++++++---------
 drivers/media/v4l2-core/Makefile              |   2 +-
 drivers/media/v4l2-core/v4l2-common.c         |  66 ++++++
 drivers/media/v4l2-core/v4l2-fourcc.c         |  93 +++++++++
 include/media/v4l2-common.h                   |   5 +
 include/media/v4l2-fourcc.h                   |  53 +++++
 8 files changed, 332 insertions(+), 129 deletions(-)
 create mode 100644 drivers/media/v4l2-core/v4l2-fourcc.c
 create mode 100644 include/media/v4l2-fourcc.h

-- 
2.19.1

[RFC v2 1/4] media: Introduce helpers to fill pixel format structs

[Ezequiel Garcia]

Add two new API helpers, v4l2_fill_pixfmt and v4l2_fill_pixfmt_mp,
to be used by drivers to calculate plane sizes and bytes per lines.

Note that driver-specific paddig and alignment are not yet
taken into account.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/media/v4l2-core/Makefile      |  2 +-
 drivers/media/v4l2-core/v4l2-common.c | 66 +++++++++++++++++++
 drivers/media/v4l2-core/v4l2-fourcc.c | 93 +++++++++++++++++++++++++++
 include/media/v4l2-common.h           |  5 ++
 include/media/v4l2-fourcc.h           | 53 +++++++++++++++
 5 files changed, 218 insertions(+), 1 deletion(-)
 create mode 100644 drivers/media/v4l2-core/v4l2-fourcc.c
 create mode 100644 include/media/v4l2-fourcc.h

diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index 9ee57e1efefe..bc23c3407c17 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -7,7 +7,7 @@ tuner-objs	:=	tuner-core.o
 
 videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
 			v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
-			v4l2-async.o
+			v4l2-async.o v4l2-fourcc.o
 ifeq ($(CONFIG_COMPAT),y)
   videodev-objs += v4l2-compat-ioctl32.o
 endif
diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 50763fb42a1b..97bb51d15188 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -61,6 +61,7 @@
 #include <media/v4l2-common.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-ctrls.h>
+#include <media/v4l2-fourcc.h>
 
 #include <linux/videodev2.h>
 
@@ -455,3 +456,68 @@ int v4l2_s_parm_cap(struct video_device *vdev,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(v4l2_s_parm_cap);
+
+void v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, int pixelformat, int width, int height)
+{
+	const struct v4l2_format_info *info;
+	struct v4l2_plane_pix_format *plane;
+	int i;
+
+	info = v4l2_format_info(pixelformat);
+	if (!info)
+		return;
+
+	pixfmt->width = width;
+	pixfmt->height = height;
+	pixfmt->pixelformat = pixelformat;
+
+	if (info->has_contiguous_planes) {
+		pixfmt->num_planes = 1;
+		plane = &pixfmt->plane_fmt[0];
+		plane->bytesperline = info->is_compressed ?
+					0 : width * info->cpp[0];
+		plane->sizeimage = info->header_size;
+		for (i = 0; i < info->num_planes; i++) {
+			unsigned int hsub = (i == 0) ? 1 : info->hsub;
+			unsigned int vsub = (i == 0) ? 1 : info->vsub;
+
+			plane->sizeimage += width * height * info->cpp[i] / (hsub * vsub);
+		}
+	} else {
+		pixfmt->num_planes = info->num_planes;
+		for (i = 0; i < info->num_planes; i++) {
+			unsigned int hsub = (i == 0) ? 1 : info->hsub;
+			unsigned int vsub = (i == 0) ? 1 : info->vsub;
+
+			plane = &pixfmt->plane_fmt[i];
+			plane->bytesperline = width * info->cpp[i] / hsub;
+			plane->sizeimage = width * height * info->cpp[i] / (hsub * vsub);
+		}
+	}
+}
+EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt_mp);
+
+void v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, int pixelformat, int width, int height)
+{
+	const struct v4l2_format_info *info;
+	char name[32];
+	int i;
+
+	pixfmt->width = width;
+	pixfmt->height = height;
+	pixfmt->pixelformat = pixelformat;
+
+	info = v4l2_format_info(pixelformat);
+	if (!info)
+		return;
+	pixfmt->bytesperline = info->is_compressed ? 0 : width * info->cpp[0];
+
+	pixfmt->sizeimage = info->header_size;
+	for (i = 0; i < info->num_planes; i++) {
+		unsigned int hsub = (i == 0) ? 1 : info->hsub;
+		unsigned int vsub = (i == 0) ? 1 : info->vsub;
+
+		pixfmt->sizeimage += width * height * info->cpp[i] / (hsub * vsub);
+	}
+}
+EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt);
diff --git a/drivers/media/v4l2-core/v4l2-fourcc.c b/drivers/media/v4l2-core/v4l2-fourcc.c
new file mode 100644
index 000000000000..4e8a15525b58
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-fourcc.c
@@ -0,0 +1,93 @@
+/*
+ * Copyright (c) 2018 Collabora, Ltd.
+ *
+ * Based on drm-fourcc:
+ * Copyright (c) 2016 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission.  The copyright holders make no representations
+ * about the suitability of this software for any purpose.  It is provided "as
+ * is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+
+#include <linux/ctype.h>
+#include <linux/videodev2.h>
+#include <media/v4l2-fourcc.h>
+#include "../platform/vicodec/codec-fwht.h"
+
+#if 0
+static char printable_char(int c)
+{
+	return isascii(c) && isprint(c) ? c : '?';
+}
+
+static const char *v4l2_get_format_name(uint32_t format, char *buf, size_t len)
+{
+	snprintf(buf, len,
+		 "%c%c%c%c %s-endian (0x%08x)",
+		 printable_char(format & 0xff),
+		 printable_char((format >> 8) & 0xff),
+		 printable_char((format >> 16) & 0xff),
+		 printable_char((format >> 24) & 0x7f),
+		 format & BIT(31) ? "big" : "little",
+		 format);
+
+	return buf;
+}
+#endif
+
+const struct v4l2_format_info *v4l2_format_info(u32 format)
+{
+	static const struct v4l2_format_info formats[] = {
+		/* RGB formats */
+		{ .format = V4L2_PIX_FMT_BGR24,		.num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = V4L2_PIX_FMT_RGB24,		.num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = V4L2_PIX_FMT_BGR32,		.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = V4L2_PIX_FMT_XBGR32,	.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = V4L2_PIX_FMT_RGB32,		.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = V4L2_PIX_FMT_XRGB32,	.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+
+		/* YUV formats */
+		{ .format = V4L2_PIX_FMT_YUV420,	.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 },
+		{ .format = V4L2_PIX_FMT_YVU420,	.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 },
+		{ .format = V4L2_PIX_FMT_YUV422P,	.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 },
+		{ .format = V4L2_PIX_FMT_NV12,		.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 },
+		{ .format = V4L2_PIX_FMT_NV21,		.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 },
+		{ .format = V4L2_PIX_FMT_NV16,		.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 },
+		{ .format = V4L2_PIX_FMT_NV61,		.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 },
+		{ .format = V4L2_PIX_FMT_NV24,		.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = V4L2_PIX_FMT_NV42,		.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = V4L2_PIX_FMT_YUYV,		.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
+		{ .format = V4L2_PIX_FMT_YVYU,		.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
+		{ .format = V4L2_PIX_FMT_UYVY,		.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
+		{ .format = V4L2_PIX_FMT_VYUY,		.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
+
+		/* Compressed formats */
+		{ .format = V4L2_PIX_FMT_FWHT,		.num_planes = 1, .cpp = { 3, 0, 0 }, .header_size = sizeof(struct fwht_cframe_hdr), .is_compressed = true },
+	};
+
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(formats); ++i) {
+		if (formats[i].format == format)
+			return &formats[i];
+	}
+
+	pr_warn("Unsupported V4L 4CC format (%08x)\n", format);
+	return NULL;
+}
+EXPORT_SYMBOL(v4l2_format_info);
+
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 82715645617b..45959a6e140e 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -396,4 +396,9 @@ int v4l2_g_parm_cap(struct video_device *vdev,
 int v4l2_s_parm_cap(struct video_device *vdev,
 		    struct v4l2_subdev *sd, struct v4l2_streamparm *a);
 
+void v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, int pixelformat,
+		      int width, int height);
+void v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, int pixelformat,
+			 int width, int height);
+
 #endif /* V4L2_COMMON_H_ */
diff --git a/include/media/v4l2-fourcc.h b/include/media/v4l2-fourcc.h
new file mode 100644
index 000000000000..9f6c1ba8907e
--- /dev/null
+++ b/include/media/v4l2-fourcc.h
@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) 2018 Collabora, Ltd.
+ *
+ * Based on drm-fourcc:
+ * Copyright (c) 2016 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission.  The copyright holders make no representations
+ * about the suitability of this software for any purpose.  It is provided "as
+ * is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+#ifndef __V4L2_FOURCC_H__
+#define __V4L2_FOURCC_H__
+
+#include <linux/types.h>
+
+/**
+ * struct v4l2_format_info - information about a V4L2 format
+ * @format: 4CC format identifier (V4L2_PIX_FMT_*)
+ * @header_size: Size of header, optional and used by compressed formats
+ * @num_planes: Number of planes (1 to 3)
+ * @cpp: Number of bytes per pixel (per plane)
+ * @hsub: Horizontal chroma subsampling factor
+ * @vsub: Vertical chroma subsampling factor
+ * @is_compressed: Is it a compressed format?
+ */
+struct v4l2_format_info {
+	u32 format;
+	u32 header_size;
+	u8 num_planes;
+	u8 cpp[3];
+	u8 hsub;
+	u8 vsub;
+	bool is_compressed;
+	bool has_contiguous_planes;
+};
+
+const struct v4l2_format_info *v4l2_format_info(u32 format);
+
+#endif
-- 
2.19.1

Re: [RFC v2 1/4] media: Introduce helpers to fill pixel format structs

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 12292 bytes --]

Le vendredi 02 novembre 2018 à 12:52 -0300, Ezequiel Garcia a écrit :
> Add two new API helpers, v4l2_fill_pixfmt and v4l2_fill_pixfmt_mp,
> to be used by drivers to calculate plane sizes and bytes per lines.
> 
> Note that driver-specific paddig and alignment are not yet
> taken into account.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  drivers/media/v4l2-core/Makefile      |  2 +-
>  drivers/media/v4l2-core/v4l2-common.c | 66 +++++++++++++++++++
>  drivers/media/v4l2-core/v4l2-fourcc.c | 93 +++++++++++++++++++++++++++
>  include/media/v4l2-common.h           |  5 ++
>  include/media/v4l2-fourcc.h           | 53 +++++++++++++++
>  5 files changed, 218 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/media/v4l2-core/v4l2-fourcc.c
>  create mode 100644 include/media/v4l2-fourcc.h
> 
> diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
> index 9ee57e1efefe..bc23c3407c17 100644
> --- a/drivers/media/v4l2-core/Makefile
> +++ b/drivers/media/v4l2-core/Makefile
> @@ -7,7 +7,7 @@ tuner-objs	:=	tuner-core.o
>  
>  videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
>  			v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
> -			v4l2-async.o
> +			v4l2-async.o v4l2-fourcc.o
>  ifeq ($(CONFIG_COMPAT),y)
>    videodev-objs += v4l2-compat-ioctl32.o
>  endif
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 50763fb42a1b..97bb51d15188 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -61,6 +61,7 @@
>  #include <media/v4l2-common.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-ctrls.h>
> +#include <media/v4l2-fourcc.h>
>  
>  #include <linux/videodev2.h>
>  
> @@ -455,3 +456,68 @@ int v4l2_s_parm_cap(struct video_device *vdev,
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(v4l2_s_parm_cap);
> +
> +void v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, int pixelformat, int width, int height)

My first impression is that this code expects width/height to be
aligned to the sub-sampling. Hence, I don't think it works for odd
width/height. It would be nice to document the constraint, specially
that this will lead to under-allocation due to the lack of round up.

> +{
> +	const struct v4l2_format_info *info;
> +	struct v4l2_plane_pix_format *plane;
> +	int i;
> +
> +	info = v4l2_format_info(pixelformat);
> +	if (!info)
> +		return;
> +
> +	pixfmt->width = width;
> +	pixfmt->height = height;
> +	pixfmt->pixelformat = pixelformat;
> +
> +	if (info->has_contiguous_planes) {
> +		pixfmt->num_planes = 1;
> +		plane = &pixfmt->plane_fmt[0];
> +		plane->bytesperline = info->is_compressed ?
> +					0 : width * info->cpp[0];
> +		plane->sizeimage = info->header_size;
> +		for (i = 0; i < info->num_planes; i++) {
> +			unsigned int hsub = (i == 0) ? 1 : info->hsub;
> +			unsigned int vsub = (i == 0) ? 1 : info->vsub;
> +
> +			plane->sizeimage += width * height * info->cpp[i] / (hsub * vsub);
> +		}
> +	} else {
> +		pixfmt->num_planes = info->num_planes;
> +		for (i = 0; i < info->num_planes; i++) {
> +			unsigned int hsub = (i == 0) ? 1 : info->hsub;
> +			unsigned int vsub = (i == 0) ? 1 : info->vsub;
> +
> +			plane = &pixfmt->plane_fmt[i];
> +			plane->bytesperline = width * info->cpp[i] / hsub;
> +			plane->sizeimage = width * height * info->cpp[i] / (hsub * vsub);
> +		}
> +	}
> +}
> +EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt_mp);
> +
> +void v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, int pixelformat, int width, int height)
> +{
> +	const struct v4l2_format_info *info;
> +	char name[32];
> +	int i;
> +
> +	pixfmt->width = width;
> +	pixfmt->height = height;
> +	pixfmt->pixelformat = pixelformat;
> +
> +	info = v4l2_format_info(pixelformat);
> +	if (!info)
> +		return;
> +	pixfmt->bytesperline = info->is_compressed ? 0 : width * info->cpp[0];
> +
> +	pixfmt->sizeimage = info->header_size;
> +	for (i = 0; i < info->num_planes; i++) {
> +		unsigned int hsub = (i == 0) ? 1 : info->hsub;
> +		unsigned int vsub = (i == 0) ? 1 : info->vsub;
> +
> +		pixfmt->sizeimage += width * height * info->cpp[i] / (hsub * vsub);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt);
> diff --git a/drivers/media/v4l2-core/v4l2-fourcc.c b/drivers/media/v4l2-core/v4l2-fourcc.c
> new file mode 100644
> index 000000000000..4e8a15525b58
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-fourcc.c
> @@ -0,0 +1,93 @@
> +/*
> + * Copyright (c) 2018 Collabora, Ltd.
> + *
> + * Based on drm-fourcc:
> + * Copyright (c) 2016 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> + *
> + * Permission to use, copy, modify, distribute, and sell this software and its
> + * documentation for any purpose is hereby granted without fee, provided that
> + * the above copyright notice appear in all copies and that both that copyright
> + * notice and this permission notice appear in supporting documentation, and
> + * that the name of the copyright holders not be used in advertising or
> + * publicity pertaining to distribution of the software without specific,
> + * written prior permission.  The copyright holders make no representations
> + * about the suitability of this software for any purpose.  It is provided "as
> + * is" without express or implied warranty.
> + *
> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
> + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
> + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
> + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
> + * OF THIS SOFTWARE.
> + */
> +
> +#include <linux/ctype.h>
> +#include <linux/videodev2.h>
> +#include <media/v4l2-fourcc.h>
> +#include "../platform/vicodec/codec-fwht.h"
> +
> +#if 0
> +static char printable_char(int c)
> +{
> +	return isascii(c) && isprint(c) ? c : '?';
> +}
> +
> +static const char *v4l2_get_format_name(uint32_t format, char *buf, size_t len)
> +{
> +	snprintf(buf, len,
> +		 "%c%c%c%c %s-endian (0x%08x)",
> +		 printable_char(format & 0xff),
> +		 printable_char((format >> 8) & 0xff),
> +		 printable_char((format >> 16) & 0xff),
> +		 printable_char((format >> 24) & 0x7f),
> +		 format & BIT(31) ? "big" : "little",
> +		 format);
> +
> +	return buf;
> +}
> +#endif
> +
> +const struct v4l2_format_info *v4l2_format_info(u32 format)
> +{
> +	static const struct v4l2_format_info formats[] = {
> +		/* RGB formats */
> +		{ .format = V4L2_PIX_FMT_BGR24,		.num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_RGB24,		.num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_BGR32,		.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_XBGR32,	.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_RGB32,		.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_XRGB32,	.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
> +
> +		/* YUV formats */
> +		{ .format = V4L2_PIX_FMT_YUV420,	.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 },
> +		{ .format = V4L2_PIX_FMT_YVU420,	.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 },
> +		{ .format = V4L2_PIX_FMT_YUV422P,	.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_NV12,		.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 },
> +		{ .format = V4L2_PIX_FMT_NV21,		.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 },
> +		{ .format = V4L2_PIX_FMT_NV16,		.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_NV61,		.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_NV24,		.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_NV42,		.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_YUYV,		.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_YVYU,		.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_UYVY,		.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_VYUY,		.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> +
> +		/* Compressed formats */
> +		{ .format = V4L2_PIX_FMT_FWHT,		.num_planes = 1, .cpp = { 3, 0, 0 }, .header_size = sizeof(struct fwht_cframe_hdr), .is_compressed = true },
> +	};
> +
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(formats); ++i) {
> +		if (formats[i].format == format)
> +			return &formats[i];
> +	}
> +
> +	pr_warn("Unsupported V4L 4CC format (%08x)\n", format);
> +	return NULL;
> +}
> +EXPORT_SYMBOL(v4l2_format_info);
> +
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 82715645617b..45959a6e140e 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -396,4 +396,9 @@ int v4l2_g_parm_cap(struct video_device *vdev,
>  int v4l2_s_parm_cap(struct video_device *vdev,
>  		    struct v4l2_subdev *sd, struct v4l2_streamparm *a);
>  
> +void v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, int pixelformat,
> +		      int width, int height);
> +void v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, int pixelformat,
> +			 int width, int height);
> +
>  #endif /* V4L2_COMMON_H_ */
> diff --git a/include/media/v4l2-fourcc.h b/include/media/v4l2-fourcc.h
> new file mode 100644
> index 000000000000..9f6c1ba8907e
> --- /dev/null
> +++ b/include/media/v4l2-fourcc.h
> @@ -0,0 +1,53 @@
> +/*
> + * Copyright (c) 2018 Collabora, Ltd.
> + *
> + * Based on drm-fourcc:
> + * Copyright (c) 2016 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> + *
> + * Permission to use, copy, modify, distribute, and sell this software and its
> + * documentation for any purpose is hereby granted without fee, provided that
> + * the above copyright notice appear in all copies and that both that copyright
> + * notice and this permission notice appear in supporting documentation, and
> + * that the name of the copyright holders not be used in advertising or
> + * publicity pertaining to distribution of the software without specific,
> + * written prior permission.  The copyright holders make no representations
> + * about the suitability of this software for any purpose.  It is provided "as
> + * is" without express or implied warranty.
> + *
> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
> + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
> + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
> + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
> + * OF THIS SOFTWARE.
> + */
> +#ifndef __V4L2_FOURCC_H__
> +#define __V4L2_FOURCC_H__
> +
> +#include <linux/types.h>
> +
> +/**
> + * struct v4l2_format_info - information about a V4L2 format
> + * @format: 4CC format identifier (V4L2_PIX_FMT_*)
> + * @header_size: Size of header, optional and used by compressed formats
> + * @num_planes: Number of planes (1 to 3)
> + * @cpp: Number of bytes per pixel (per plane)
> + * @hsub: Horizontal chroma subsampling factor
> + * @vsub: Vertical chroma subsampling factor
> + * @is_compressed: Is it a compressed format?
> + */
> +struct v4l2_format_info {
> +	u32 format;
> +	u32 header_size;
> +	u8 num_planes;
> +	u8 cpp[3];
> +	u8 hsub;
> +	u8 vsub;
> +	bool is_compressed;
> +	bool has_contiguous_planes;
> +};
> +
> +const struct v4l2_format_info *v4l2_format_info(u32 format);
> +
> +#endif

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [RFC v2 1/4] media: Introduce helpers to fill pixel format structs

[Ezequiel Garcia]

On Fri, 2018-11-02 at 12:52 -0300, Ezequiel Garcia wrote:
> Add two new API helpers, v4l2_fill_pixfmt and v4l2_fill_pixfmt_mp,
> to be used by drivers to calculate plane sizes and bytes per lines.
> 
> Note that driver-specific paddig and alignment are not yet
> taken into account.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---

Heads-up: I plan to submit a new version of this patch, as part of
the VPU JPEG patchset.

Thanks,
Ezequiel

>  drivers/media/v4l2-core/Makefile      |  2 +-
>  drivers/media/v4l2-core/v4l2-common.c | 66 +++++++++++++++++++
>  drivers/media/v4l2-core/v4l2-fourcc.c | 93 +++++++++++++++++++++++++++
>  include/media/v4l2-common.h           |  5 ++
>  include/media/v4l2-fourcc.h           | 53 +++++++++++++++
>  5 files changed, 218 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/media/v4l2-core/v4l2-fourcc.c
>  create mode 100644 include/media/v4l2-fourcc.h
> 
> diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
> index 9ee57e1efefe..bc23c3407c17 100644
> --- a/drivers/media/v4l2-core/Makefile
> +++ b/drivers/media/v4l2-core/Makefile
> @@ -7,7 +7,7 @@ tuner-objs	:=	tuner-core.o
>  
>  videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
>  			v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
> -			v4l2-async.o
> +			v4l2-async.o v4l2-fourcc.o
>  ifeq ($(CONFIG_COMPAT),y)
>    videodev-objs += v4l2-compat-ioctl32.o
>  endif
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 50763fb42a1b..97bb51d15188 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -61,6 +61,7 @@
>  #include <media/v4l2-common.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-ctrls.h>
> +#include <media/v4l2-fourcc.h>
>  
>  #include <linux/videodev2.h>
>  
> @@ -455,3 +456,68 @@ int v4l2_s_parm_cap(struct video_device *vdev,
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(v4l2_s_parm_cap);
> +
> +void v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, int pixelformat, int width, int height)
> +{
> +	const struct v4l2_format_info *info;
> +	struct v4l2_plane_pix_format *plane;
> +	int i;
> +
> +	info = v4l2_format_info(pixelformat);
> +	if (!info)
> +		return;
> +
> +	pixfmt->width = width;
> +	pixfmt->height = height;
> +	pixfmt->pixelformat = pixelformat;
> +
> +	if (info->has_contiguous_planes) {
> +		pixfmt->num_planes = 1;
> +		plane = &pixfmt->plane_fmt[0];
> +		plane->bytesperline = info->is_compressed ?
> +					0 : width * info->cpp[0];
> +		plane->sizeimage = info->header_size;
> +		for (i = 0; i < info->num_planes; i++) {
> +			unsigned int hsub = (i == 0) ? 1 : info->hsub;
> +			unsigned int vsub = (i == 0) ? 1 : info->vsub;
> +
> +			plane->sizeimage += width * height * info->cpp[i] / (hsub * vsub);
> +		}
> +	} else {
> +		pixfmt->num_planes = info->num_planes;
> +		for (i = 0; i < info->num_planes; i++) {
> +			unsigned int hsub = (i == 0) ? 1 : info->hsub;
> +			unsigned int vsub = (i == 0) ? 1 : info->vsub;
> +
> +			plane = &pixfmt->plane_fmt[i];
> +			plane->bytesperline = width * info->cpp[i] / hsub;
> +			plane->sizeimage = width * height * info->cpp[i] / (hsub * vsub);
> +		}
> +	}
> +}
> +EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt_mp);
> +
> +void v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, int pixelformat, int width, int height)
> +{
> +	const struct v4l2_format_info *info;
> +	char name[32];
> +	int i;
> +
> +	pixfmt->width = width;
> +	pixfmt->height = height;
> +	pixfmt->pixelformat = pixelformat;
> +
> +	info = v4l2_format_info(pixelformat);
> +	if (!info)
> +		return;
> +	pixfmt->bytesperline = info->is_compressed ? 0 : width * info->cpp[0];
> +
> +	pixfmt->sizeimage = info->header_size;
> +	for (i = 0; i < info->num_planes; i++) {
> +		unsigned int hsub = (i == 0) ? 1 : info->hsub;
> +		unsigned int vsub = (i == 0) ? 1 : info->vsub;
> +
> +		pixfmt->sizeimage += width * height * info->cpp[i] / (hsub * vsub);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt);
> diff --git a/drivers/media/v4l2-core/v4l2-fourcc.c b/drivers/media/v4l2-core/v4l2-fourcc.c
> new file mode 100644
> index 000000000000..4e8a15525b58
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-fourcc.c
> @@ -0,0 +1,93 @@
> +/*
> + * Copyright (c) 2018 Collabora, Ltd.
> + *
> + * Based on drm-fourcc:
> + * Copyright (c) 2016 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> + *
> + * Permission to use, copy, modify, distribute, and sell this software and its
> + * documentation for any purpose is hereby granted without fee, provided that
> + * the above copyright notice appear in all copies and that both that copyright
> + * notice and this permission notice appear in supporting documentation, and
> + * that the name of the copyright holders not be used in advertising or
> + * publicity pertaining to distribution of the software without specific,
> + * written prior permission.  The copyright holders make no representations
> + * about the suitability of this software for any purpose.  It is provided "as
> + * is" without express or implied warranty.
> + *
> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
> + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
> + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
> + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
> + * OF THIS SOFTWARE.
> + */
> +
> +#include <linux/ctype.h>
> +#include <linux/videodev2.h>
> +#include <media/v4l2-fourcc.h>
> +#include "../platform/vicodec/codec-fwht.h"
> +
> +#if 0
> +static char printable_char(int c)
> +{
> +	return isascii(c) && isprint(c) ? c : '?';
> +}
> +
> +static const char *v4l2_get_format_name(uint32_t format, char *buf, size_t len)
> +{
> +	snprintf(buf, len,
> +		 "%c%c%c%c %s-endian (0x%08x)",
> +		 printable_char(format & 0xff),
> +		 printable_char((format >> 8) & 0xff),
> +		 printable_char((format >> 16) & 0xff),
> +		 printable_char((format >> 24) & 0x7f),
> +		 format & BIT(31) ? "big" : "little",
> +		 format);
> +
> +	return buf;
> +}
> +#endif
> +
> +const struct v4l2_format_info *v4l2_format_info(u32 format)
> +{
> +	static const struct v4l2_format_info formats[] = {
> +		/* RGB formats */
> +		{ .format = V4L2_PIX_FMT_BGR24,		.num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_RGB24,		.num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_BGR32,		.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_XBGR32,	.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_RGB32,		.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_XRGB32,	.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
> +
> +		/* YUV formats */
> +		{ .format = V4L2_PIX_FMT_YUV420,	.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 },
> +		{ .format = V4L2_PIX_FMT_YVU420,	.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 },
> +		{ .format = V4L2_PIX_FMT_YUV422P,	.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_NV12,		.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 },
> +		{ .format = V4L2_PIX_FMT_NV21,		.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 },
> +		{ .format = V4L2_PIX_FMT_NV16,		.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_NV61,		.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_NV24,		.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_NV42,		.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_YUYV,		.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_YVYU,		.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_UYVY,		.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> +		{ .format = V4L2_PIX_FMT_VYUY,		.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> +
> +		/* Compressed formats */
> +		{ .format = V4L2_PIX_FMT_FWHT,		.num_planes = 1, .cpp = { 3, 0, 0 }, .header_size = sizeof(struct fwht_cframe_hdr),
> .is_compressed = true },
> +	};
> +
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(formats); ++i) {
> +		if (formats[i].format == format)
> +			return &formats[i];
> +	}
> +
> +	pr_warn("Unsupported V4L 4CC format (%08x)\n", format);
> +	return NULL;
> +}
> +EXPORT_SYMBOL(v4l2_format_info);
> +
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 82715645617b..45959a6e140e 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -396,4 +396,9 @@ int v4l2_g_parm_cap(struct video_device *vdev,
>  int v4l2_s_parm_cap(struct video_device *vdev,
>  		    struct v4l2_subdev *sd, struct v4l2_streamparm *a);
>  
> +void v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, int pixelformat,
> +		      int width, int height);
> +void v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, int pixelformat,
> +			 int width, int height);
> +
>  #endif /* V4L2_COMMON_H_ */
> diff --git a/include/media/v4l2-fourcc.h b/include/media/v4l2-fourcc.h
> new file mode 100644
> index 000000000000..9f6c1ba8907e
> --- /dev/null
> +++ b/include/media/v4l2-fourcc.h
> @@ -0,0 +1,53 @@
> +/*
> + * Copyright (c) 2018 Collabora, Ltd.
> + *
> + * Based on drm-fourcc:
> + * Copyright (c) 2016 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> + *
> + * Permission to use, copy, modify, distribute, and sell this software and its
> + * documentation for any purpose is hereby granted without fee, provided that
> + * the above copyright notice appear in all copies and that both that copyright
> + * notice and this permission notice appear in supporting documentation, and
> + * that the name of the copyright holders not be used in advertising or
> + * publicity pertaining to distribution of the software without specific,
> + * written prior permission.  The copyright holders make no representations
> + * about the suitability of this software for any purpose.  It is provided "as
> + * is" without express or implied warranty.
> + *
> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
> + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
> + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
> + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
> + * OF THIS SOFTWARE.
> + */
> +#ifndef __V4L2_FOURCC_H__
> +#define __V4L2_FOURCC_H__
> +
> +#include <linux/types.h>
> +
> +/**
> + * struct v4l2_format_info - information about a V4L2 format
> + * @format: 4CC format identifier (V4L2_PIX_FMT_*)
> + * @header_size: Size of header, optional and used by compressed formats
> + * @num_planes: Number of planes (1 to 3)
> + * @cpp: Number of bytes per pixel (per plane)
> + * @hsub: Horizontal chroma subsampling factor
> + * @vsub: Vertical chroma subsampling factor
> + * @is_compressed: Is it a compressed format?
> + */
> +struct v4l2_format_info {
> +	u32 format;
> +	u32 header_size;
> +	u8 num_planes;
> +	u8 cpp[3];
> +	u8 hsub;
> +	u8 vsub;
> +	bool is_compressed;
> +	bool has_contiguous_planes;
> +};
> +
> +const struct v4l2_format_info *v4l2_format_info(u32 format);
> +
> +#endif

[PATCH v2 2/4] vicodec: Use pixel format helpers

[Ezequiel Garcia]

Now that we've introduced the pixel format helpers, use them
in vicodec driver, and get rid of driver specific pixel
format specifiers.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 .../media/platform/vicodec/codec-v4l2-fwht.c  | 42 ++++----
 .../media/platform/vicodec/codec-v4l2-fwht.h  |  3 -
 drivers/media/platform/vicodec/vicodec-core.c | 95 ++++++-------------
 3 files changed, 48 insertions(+), 92 deletions(-)

diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.c b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
index e5b68fb38aac..42cf64dccaba 100644
--- a/drivers/media/platform/vicodec/codec-v4l2-fwht.c
+++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
@@ -11,27 +11,27 @@
 #include "codec-v4l2-fwht.h"
 
 static const struct v4l2_fwht_pixfmt_info v4l2_fwht_pixfmts[] = {
-	{ V4L2_PIX_FMT_YUV420,  1, 3, 2, 1, 1, 2, 2 },
-	{ V4L2_PIX_FMT_YVU420,  1, 3, 2, 1, 1, 2, 2 },
-	{ V4L2_PIX_FMT_YUV422P, 1, 2, 1, 1, 1, 2, 1 },
-	{ V4L2_PIX_FMT_NV12,    1, 3, 2, 1, 2, 2, 2 },
-	{ V4L2_PIX_FMT_NV21,    1, 3, 2, 1, 2, 2, 2 },
-	{ V4L2_PIX_FMT_NV16,    1, 2, 1, 1, 2, 2, 1 },
-	{ V4L2_PIX_FMT_NV61,    1, 2, 1, 1, 2, 2, 1 },
-	{ V4L2_PIX_FMT_NV24,    1, 3, 1, 1, 2, 1, 1 },
-	{ V4L2_PIX_FMT_NV42,    1, 3, 1, 1, 2, 1, 1 },
-	{ V4L2_PIX_FMT_YUYV,    2, 2, 1, 2, 4, 2, 1 },
-	{ V4L2_PIX_FMT_YVYU,    2, 2, 1, 2, 4, 2, 1 },
-	{ V4L2_PIX_FMT_UYVY,    2, 2, 1, 2, 4, 2, 1 },
-	{ V4L2_PIX_FMT_VYUY,    2, 2, 1, 2, 4, 2, 1 },
-	{ V4L2_PIX_FMT_BGR24,   3, 3, 1, 3, 3, 1, 1 },
-	{ V4L2_PIX_FMT_RGB24,   3, 3, 1, 3, 3, 1, 1 },
-	{ V4L2_PIX_FMT_HSV24,   3, 3, 1, 3, 3, 1, 1 },
-	{ V4L2_PIX_FMT_BGR32,   4, 4, 1, 4, 4, 1, 1 },
-	{ V4L2_PIX_FMT_XBGR32,  4, 4, 1, 4, 4, 1, 1 },
-	{ V4L2_PIX_FMT_RGB32,   4, 4, 1, 4, 4, 1, 1 },
-	{ V4L2_PIX_FMT_XRGB32,  4, 4, 1, 4, 4, 1, 1 },
-	{ V4L2_PIX_FMT_HSV32,   4, 4, 1, 4, 4, 1, 1 },
+	{ V4L2_PIX_FMT_YUV420,  1, 1, 2, 2 },
+	{ V4L2_PIX_FMT_YVU420,  1, 1, 2, 2 },
+	{ V4L2_PIX_FMT_YUV422P, 1, 1, 2, 1 },
+	{ V4L2_PIX_FMT_NV12,    1, 2, 2, 2 },
+	{ V4L2_PIX_FMT_NV21,    1, 2, 2, 2 },
+	{ V4L2_PIX_FMT_NV16,    1, 2, 2, 1 },
+	{ V4L2_PIX_FMT_NV61,    1, 2, 2, 1 },
+	{ V4L2_PIX_FMT_NV24,    1, 2, 1, 1 },
+	{ V4L2_PIX_FMT_NV42,    1, 2, 1, 1 },
+	{ V4L2_PIX_FMT_YUYV,    2, 4, 2, 1 },
+	{ V4L2_PIX_FMT_YVYU,    2, 4, 2, 1 },
+	{ V4L2_PIX_FMT_UYVY,    2, 4, 2, 1 },
+	{ V4L2_PIX_FMT_VYUY,    2, 4, 2, 1 },
+	{ V4L2_PIX_FMT_BGR24,   3, 3, 1, 1 },
+	{ V4L2_PIX_FMT_RGB24,   3, 3, 1, 1 },
+	{ V4L2_PIX_FMT_HSV24,   3, 3, 1, 1 },
+	{ V4L2_PIX_FMT_BGR32,   4, 4, 1, 1 },
+	{ V4L2_PIX_FMT_XBGR32,  4, 4, 1, 1 },
+	{ V4L2_PIX_FMT_RGB32,   4, 4, 1, 1 },
+	{ V4L2_PIX_FMT_XRGB32,  4, 4, 1, 1 },
+	{ V4L2_PIX_FMT_HSV32,   4, 4, 1, 1 },
 };
 
 const struct v4l2_fwht_pixfmt_info *v4l2_fwht_find_pixfmt(u32 pixelformat)
diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.h b/drivers/media/platform/vicodec/codec-v4l2-fwht.h
index 162465b78067..298a13732406 100644
--- a/drivers/media/platform/vicodec/codec-v4l2-fwht.h
+++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.h
@@ -10,9 +10,6 @@
 
 struct v4l2_fwht_pixfmt_info {
 	u32 id;
-	unsigned int bytesperline_mult;
-	unsigned int sizeimage_mult;
-	unsigned int sizeimage_div;
 	unsigned int luma_step;
 	unsigned int chroma_step;
 	/* Chroma plane subsampling */
diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 1eb9132bfc85..dbc8b68894e7 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -43,25 +43,14 @@ MODULE_PARM_DESC(debug, " activates debug info");
 #define MIN_WIDTH		640U
 #define MAX_HEIGHT		2160U
 #define MIN_HEIGHT		480U
+#define DEF_WIDTH		1280U
+#define DEF_HEIGHT		720U
 
 #define dprintk(dev, fmt, arg...) \
 	v4l2_dbg(1, debug, &dev->v4l2_dev, "%s: " fmt, __func__, ## arg)
 
-
-struct pixfmt_info {
-	u32 id;
-	unsigned int bytesperline_mult;
-	unsigned int sizeimage_mult;
-	unsigned int sizeimage_div;
-	unsigned int luma_step;
-	unsigned int chroma_step;
-	/* Chroma plane subsampling */
-	unsigned int width_div;
-	unsigned int height_div;
-};
-
 static const struct v4l2_fwht_pixfmt_info pixfmt_fwht = {
-	V4L2_PIX_FMT_FWHT, 0, 3, 1, 1, 1, 1, 1
+	V4L2_PIX_FMT_FWHT, 1, 1, 1, 1
 };
 
 static void vicodec_dev_release(struct device *dev)
@@ -466,12 +455,8 @@ static int vidioc_g_fmt(struct vicodec_ctx *ctx, struct v4l2_format *f)
 		if (multiplanar)
 			return -EINVAL;
 		pix = &f->fmt.pix;
-		pix->width = q_data->width;
-		pix->height = q_data->height;
+		v4l2_fill_pixfmt(pix, info->id, q_data->width, q_data->height);
 		pix->field = V4L2_FIELD_NONE;
-		pix->pixelformat = info->id;
-		pix->bytesperline = q_data->width * info->bytesperline_mult;
-		pix->sizeimage = q_data->sizeimage;
 		pix->colorspace = ctx->state.colorspace;
 		pix->xfer_func = ctx->state.xfer_func;
 		pix->ycbcr_enc = ctx->state.ycbcr_enc;
@@ -483,14 +468,9 @@ static int vidioc_g_fmt(struct vicodec_ctx *ctx, struct v4l2_format *f)
 		if (!multiplanar)
 			return -EINVAL;
 		pix_mp = &f->fmt.pix_mp;
-		pix_mp->width = q_data->width;
-		pix_mp->height = q_data->height;
+
+		v4l2_fill_pixfmt_mp(pix_mp, info->id, q_data->width, q_data->height);
 		pix_mp->field = V4L2_FIELD_NONE;
-		pix_mp->pixelformat = info->id;
-		pix_mp->num_planes = 1;
-		pix_mp->plane_fmt[0].bytesperline =
-				q_data->width * info->bytesperline_mult;
-		pix_mp->plane_fmt[0].sizeimage = q_data->sizeimage;
 		pix_mp->colorspace = ctx->state.colorspace;
 		pix_mp->xfer_func = ctx->state.xfer_func;
 		pix_mp->ycbcr_enc = ctx->state.ycbcr_enc;
@@ -520,43 +500,26 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
 static int vidioc_try_fmt(struct vicodec_ctx *ctx, struct v4l2_format *f)
 {
 	struct v4l2_pix_format_mplane *pix_mp;
-	struct v4l2_pix_format *pix;
 	struct v4l2_plane_pix_format *plane;
-	const struct v4l2_fwht_pixfmt_info *info = &pixfmt_fwht;
+	struct v4l2_pix_format *pix;
+	unsigned int width, height;
 
 	switch (f->type) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
 		pix = &f->fmt.pix;
-		if (pix->pixelformat != V4L2_PIX_FMT_FWHT)
-			info = find_fmt(pix->pixelformat);
-		pix->width = clamp(pix->width, MIN_WIDTH, MAX_WIDTH) & ~7;
-		pix->height = clamp(pix->height, MIN_HEIGHT, MAX_HEIGHT) & ~7;
+		width = clamp(pix->width, MIN_WIDTH, MAX_WIDTH) & ~7;
+		height = clamp(pix->height, MIN_HEIGHT, MAX_HEIGHT) & ~7;
+		v4l2_fill_pixfmt(pix, pix->pixelformat, width, height);
 		pix->field = V4L2_FIELD_NONE;
-		pix->bytesperline =
-			pix->width * info->bytesperline_mult;
-		pix->sizeimage = pix->width * pix->height *
-			info->sizeimage_mult / info->sizeimage_div;
-		if (pix->pixelformat == V4L2_PIX_FMT_FWHT)
-			pix->sizeimage += sizeof(struct fwht_cframe_hdr);
 		break;
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
 		pix_mp = &f->fmt.pix_mp;
-		plane = pix_mp->plane_fmt;
-		if (pix_mp->pixelformat != V4L2_PIX_FMT_FWHT)
-			info = find_fmt(pix_mp->pixelformat);
-		pix_mp->num_planes = 1;
-		pix_mp->width = clamp(pix_mp->width, MIN_WIDTH, MAX_WIDTH) & ~7;
-		pix_mp->height =
-			clamp(pix_mp->height, MIN_HEIGHT, MAX_HEIGHT) & ~7;
+		width = clamp(pix_mp->width, MIN_WIDTH, MAX_WIDTH) & ~7;
+		height = clamp(pix_mp->height, MIN_HEIGHT, MAX_HEIGHT) & ~7;
+		v4l2_fill_pixfmt_mp(pix_mp, pix_mp->pixelformat, width, height);
 		pix_mp->field = V4L2_FIELD_NONE;
-		plane->bytesperline =
-			pix_mp->width * info->bytesperline_mult;
-		plane->sizeimage = pix_mp->width * pix_mp->height *
-			info->sizeimage_mult / info->sizeimage_div;
-		if (pix_mp->pixelformat == V4L2_PIX_FMT_FWHT)
-			plane->sizeimage += sizeof(struct fwht_cframe_hdr);
 		memset(pix_mp->reserved, 0, sizeof(pix_mp->reserved));
 		memset(plane->reserved, 0, sizeof(plane->reserved));
 		break;
@@ -1143,7 +1106,7 @@ static int vicodec_open(struct file *file)
 	struct vicodec_dev *dev = video_drvdata(file);
 	struct vicodec_ctx *ctx = NULL;
 	struct v4l2_ctrl_handler *hdl;
-	unsigned int size;
+	struct v4l2_pix_format pixfmt;
 	int rc = 0;
 
 	if (mutex_lock_interruptible(vfd->lock))
@@ -1177,25 +1140,21 @@ static int vicodec_open(struct file *file)
 
 	ctx->q_data[V4L2_M2M_SRC].info =
 		ctx->is_enc ? v4l2_fwht_get_pixfmt(0) : &pixfmt_fwht;
-	ctx->q_data[V4L2_M2M_SRC].width = 1280;
-	ctx->q_data[V4L2_M2M_SRC].height = 720;
-	size = 1280 * 720 * ctx->q_data[V4L2_M2M_SRC].info->sizeimage_mult /
-		ctx->q_data[V4L2_M2M_SRC].info->sizeimage_div;
-	if (ctx->is_enc)
-		ctx->q_data[V4L2_M2M_SRC].sizeimage = size;
-	else
-		ctx->q_data[V4L2_M2M_SRC].sizeimage =
-			size + sizeof(struct fwht_cframe_hdr);
+	v4l2_fill_pixfmt(&pixfmt, ctx->q_data[V4L2_M2M_SRC].info->id,
+			DEF_WIDTH, DEF_HEIGHT);
+	ctx->q_data[V4L2_M2M_SRC].width = DEF_WIDTH;
+	ctx->q_data[V4L2_M2M_SRC].height = DEF_HEIGHT;
+	ctx->q_data[V4L2_M2M_SRC].sizeimage = pixfmt.sizeimage;
+
 	ctx->q_data[V4L2_M2M_DST] = ctx->q_data[V4L2_M2M_SRC];
 	ctx->q_data[V4L2_M2M_DST].info =
 		ctx->is_enc ? &pixfmt_fwht : v4l2_fwht_get_pixfmt(0);
-	size = 1280 * 720 * ctx->q_data[V4L2_M2M_DST].info->sizeimage_mult /
-		ctx->q_data[V4L2_M2M_DST].info->sizeimage_div;
-	if (ctx->is_enc)
-		ctx->q_data[V4L2_M2M_DST].sizeimage =
-			size + sizeof(struct fwht_cframe_hdr);
-	else
-		ctx->q_data[V4L2_M2M_DST].sizeimage = size;
+	v4l2_fill_pixfmt(&pixfmt, ctx->q_data[V4L2_M2M_DST].info->id,
+			DEF_WIDTH, DEF_HEIGHT);
+	ctx->q_data[V4L2_M2M_SRC].width = DEF_WIDTH;
+	ctx->q_data[V4L2_M2M_SRC].height = DEF_HEIGHT;
+	ctx->q_data[V4L2_M2M_SRC].sizeimage = pixfmt.sizeimage;
+
 	ctx->state.colorspace = V4L2_COLORSPACE_REC709;
 
 	if (ctx->is_enc) {
-- 
2.19.1

[PATCH v2 3/4] vicodec: Propagate changes to the CAPTURE queue

[Ezequiel Garcia]

For stateful codecs, width and height values set
on the OUTPUT queue, must be propagated to the CAPTURE queue.

This is not enough to fully comply with the specification,
which would require to properly support stream resolution
changes detection and notification.

However, it's a relatively small change, which fixes behavior
required by some applications such as gstreamer.

With this change, it's possible to run a simple T(T⁻¹) pipeline:

gst-launch-1.0 videotestsrc ! v4l2fwhtenc ! v4l2fwhtdec ! fakevideosink

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/media/platform/vicodec/vicodec-core.c | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index dbc8b68894e7..cffd41c3fc17 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -636,6 +636,17 @@ static int vidioc_s_fmt(struct vicodec_ctx *ctx, struct v4l2_format *f)
 		q_data->width = pix->width;
 		q_data->height = pix->height;
 		q_data->sizeimage = pix->sizeimage;
+
+		/* Propagate changes to CAPTURE queue */
+		if (V4L2_TYPE_IS_OUTPUT(f->type)) {
+			struct v4l2_pix_format dst_pix;
+
+			v4l2_fill_pixfmt(&dst_pix, ctx->q_data[V4L2_M2M_DST].info->id,
+					pix->width, pix->height);
+			ctx->q_data[V4L2_M2M_DST].width = pix->width;
+			ctx->q_data[V4L2_M2M_DST].height = pix->height;
+			ctx->q_data[V4L2_M2M_DST].sizeimage = dst_pix.sizeimage;
+		}
 		break;
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
@@ -656,6 +667,17 @@ static int vidioc_s_fmt(struct vicodec_ctx *ctx, struct v4l2_format *f)
 		q_data->width = pix_mp->width;
 		q_data->height = pix_mp->height;
 		q_data->sizeimage = pix_mp->plane_fmt[0].sizeimage;
+
+		/* Propagate changes to CAPTURE queue */
+		if (V4L2_TYPE_IS_OUTPUT(f->type)) {
+			struct v4l2_pix_format_mplane dst_pix;
+
+			v4l2_fill_pixfmt_mp(&dst_pix, ctx->q_data[V4L2_M2M_DST].info->id,
+					pix_mp->width, pix_mp->height);
+			ctx->q_data[V4L2_M2M_DST].width = pix_mp->width;
+			ctx->q_data[V4L2_M2M_DST].height = pix_mp->height;
+			ctx->q_data[V4L2_M2M_DST].sizeimage = dst_pix.plane_fmt[0].sizeimage;
+		}
 		break;
 	default:
 		return -EINVAL;
-- 
2.19.1

[PATCH v2 4/4] vicodec: Implement spec-compliant stop command

[Ezequiel Garcia]

Currently on a V4L2_ENC_CMD_STOP command, the driver sets
V4L2_BUF_FLAG_LAST to the destination buffer, but only if
there's no source buffer.

This alone has no effects, because .device_run never
gets to run (there is no source buffer), therefore destination
buffer is never dequeued.

Fix this by setting up a statically-allocated, dummy buffer to
be used as flush buffer, used to signal a encoding (or decoding) stop.

This works by queueing the flush buffer to the OUTPUT queue,
so the driver will send an V4L2_EVENT_EOS event, and
mark the CAPTURE buffer with V4L2_BUF_FLAG_LAST.

Once the buffer is marked as V4L2_BUF_FLAG_LAST, the kernel
returns -EPIPE on a VIDIOC_DQBUF. Applications can use
this error to detect the stop condition.

With this change, it's possible to run a pipeline to completion:

gst-launch-1.0 videotestsrc num-buffers=10 ! v4l2fwhtenc ! v4l2fwhtdec ! fakevideosink

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/media/platform/vicodec/vicodec-core.c | 80 ++++++++++---------
 1 file changed, 44 insertions(+), 36 deletions(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index cffd41c3fc17..b973833e21f5 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -102,7 +102,7 @@ struct vicodec_ctx {
 	struct v4l2_ctrl_handler hdl;
 
 	struct vb2_v4l2_buffer *last_src_buf;
-	struct vb2_v4l2_buffer *last_dst_buf;
+	struct vb2_v4l2_buffer  flush_buf;
 
 	/* Source and destination queue data */
 	struct vicodec_q_data   q_data[2];
@@ -209,6 +209,7 @@ static void device_run(void *priv)
 	struct vicodec_dev *dev = ctx->dev;
 	struct vb2_v4l2_buffer *src_buf, *dst_buf;
 	struct vicodec_q_data *q_out;
+	bool flushing;
 	u32 state;
 
 	src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
@@ -216,26 +217,36 @@ static void device_run(void *priv)
 	q_out = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
 
 	state = VB2_BUF_STATE_DONE;
-	if (device_process(ctx, src_buf, dst_buf))
+
+	flushing = (src_buf == &ctx->flush_buf);
+	if (!flushing && device_process(ctx, src_buf, dst_buf))
 		state = VB2_BUF_STATE_ERROR;
-	ctx->last_dst_buf = dst_buf;
 
 	spin_lock(ctx->lock);
-	if (!ctx->comp_has_next_frame && src_buf == ctx->last_src_buf) {
+	if (!flushing) {
+		if (!ctx->comp_has_next_frame && src_buf == ctx->last_src_buf) {
+			dst_buf->flags |= V4L2_BUF_FLAG_LAST;
+			v4l2_event_queue_fh(&ctx->fh, &eos_event);
+		}
+
+		if (ctx->is_enc) {
+			src_buf->sequence = q_out->sequence++;
+			src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
+			v4l2_m2m_buf_done(src_buf, state);
+		} else if (vb2_get_plane_payload(&src_buf->vb2_buf, 0)
+				== ctx->cur_buf_offset) {
+			src_buf->sequence = q_out->sequence++;
+			src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
+			v4l2_m2m_buf_done(src_buf, state);
+			ctx->cur_buf_offset = 0;
+			ctx->comp_has_next_frame = false;
+		}
+	} else {
+		src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
+		vb2_set_plane_payload(&dst_buf->vb2_buf, 0, 0);
 		dst_buf->flags |= V4L2_BUF_FLAG_LAST;
 		v4l2_event_queue_fh(&ctx->fh, &eos_event);
 	}
-	if (ctx->is_enc) {
-		src_buf->sequence = q_out->sequence++;
-		src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
-		v4l2_m2m_buf_done(src_buf, state);
-	} else if (vb2_get_plane_payload(&src_buf->vb2_buf, 0) == ctx->cur_buf_offset) {
-		src_buf->sequence = q_out->sequence++;
-		src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
-		v4l2_m2m_buf_done(src_buf, state);
-		ctx->cur_buf_offset = 0;
-		ctx->comp_has_next_frame = false;
-	}
 	v4l2_m2m_buf_done(dst_buf, state);
 	ctx->comp_size = 0;
 	ctx->comp_magic_cnt = 0;
@@ -282,6 +293,8 @@ static int job_ready(void *priv)
 	src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
 	if (!src_buf)
 		return 0;
+	if (src_buf == &ctx->flush_buf)
+		return 1;
 	p_out = vb2_plane_vaddr(&src_buf->vb2_buf, 0);
 	sz = vb2_get_plane_payload(&src_buf->vb2_buf, 0);
 	p = p_out + ctx->cur_buf_offset;
@@ -740,21 +753,6 @@ static int vidioc_s_fmt_vid_out(struct file *file, void *priv,
 	return ret;
 }
 
-static void vicodec_mark_last_buf(struct vicodec_ctx *ctx)
-{
-	static const struct v4l2_event eos_event = {
-		.type = V4L2_EVENT_EOS
-	};
-
-	spin_lock(ctx->lock);
-	ctx->last_src_buf = v4l2_m2m_last_src_buf(ctx->fh.m2m_ctx);
-	if (!ctx->last_src_buf && ctx->last_dst_buf) {
-		ctx->last_dst_buf->flags |= V4L2_BUF_FLAG_LAST;
-		v4l2_event_queue_fh(&ctx->fh, &eos_event);
-	}
-	spin_unlock(ctx->lock);
-}
-
 static int vicodec_try_encoder_cmd(struct file *file, void *fh,
 				struct v4l2_encoder_cmd *ec)
 {
@@ -776,8 +774,8 @@ static int vicodec_encoder_cmd(struct file *file, void *fh,
 	ret = vicodec_try_encoder_cmd(file, fh, ec);
 	if (ret < 0)
 		return ret;
-
-	vicodec_mark_last_buf(ctx);
+	v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, &ctx->flush_buf);
+	v4l2_m2m_try_schedule(ctx->fh.m2m_ctx);
 	return 0;
 }
 
@@ -805,8 +803,8 @@ static int vicodec_decoder_cmd(struct file *file, void *fh,
 	ret = vicodec_try_decoder_cmd(file, fh, dc);
 	if (ret < 0)
 		return ret;
-
-	vicodec_mark_last_buf(ctx);
+	v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, &ctx->flush_buf);
+	v4l2_m2m_try_schedule(ctx->fh.m2m_ctx);
 	return 0;
 }
 
@@ -961,7 +959,7 @@ static void vicodec_return_bufs(struct vb2_queue *q, u32 state)
 			vbuf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
 		else
 			vbuf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
-		if (vbuf == NULL)
+		if (!vbuf || vbuf == &ctx->flush_buf)
 			return;
 		spin_lock(ctx->lock);
 		v4l2_m2m_buf_done(vbuf, state);
@@ -1001,7 +999,6 @@ static int vicodec_start_streaming(struct vb2_queue *q,
 	state->ref_frame.cb = state->ref_frame.luma + size;
 	state->ref_frame.cr = state->ref_frame.cb + size / chroma_div;
 	ctx->last_src_buf = NULL;
-	ctx->last_dst_buf = NULL;
 	state->gop_cnt = 0;
 	ctx->cur_buf_offset = 0;
 	ctx->comp_size = 0;
@@ -1129,6 +1126,7 @@ static int vicodec_open(struct file *file)
 	struct vicodec_ctx *ctx = NULL;
 	struct v4l2_ctrl_handler *hdl;
 	struct v4l2_pix_format pixfmt;
+	struct vb2_queue *vq;
 	int rc = 0;
 
 	if (mutex_lock_interruptible(vfd->lock))
@@ -1200,6 +1198,16 @@ static int vicodec_open(struct file *file)
 
 	v4l2_fh_add(&ctx->fh);
 
+	/* Setup a dummy flush buffer, used to signal
+	 * encoding/decoding stop operation. When this buffer
+	 * is queued to the OUTPUT queue, the driver will send
+	 * V4L2_EVENT_EOS and send the last buffer to userspace.
+	 */
+	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, multiplanar ?
+			     V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE :
+			     V4L2_BUF_TYPE_VIDEO_OUTPUT);
+	ctx->flush_buf.vb2_buf.vb2_queue = vq;
+
 open_unlock:
 	mutex_unlock(vfd->lock);
 	return rc;
-- 
2.19.1

Re: [PATCH v2 0/4] vicodec: a couple fixes towards spec compliancy

[Hans Verkuil]

Hi Ezequiel,

On 11/2/18 4:52 PM, Ezequiel Garcia wrote:
> Given the stateful codec specification is still a moving target,
> it doesn't make any sense to try to comply fully with it.
> 
> On the other side, we can still comply with some basic userspace
> expectations, with just a couple small changes.
> 
> This series implements proper resolution changes propagation,
> and fixes the CMD_STOP so it actually works.
> 
> The intention of this series is to be able to test this driver
> using already existing userspace, gstreamer in particular.
> With this changes, it's possible to construct variations of
> this pipeline:
> 
>   gst-launch-1.0 videotestsrc ! v4l2fwhtenc ! v4l2fwhtdec ! fakevideosink
> 
> Also, as discussed in v1 feedback [1,2], I'm including pixel format
> helpers, as RFC for now. Hans, Tomasz: is this what you had in mind?
> 
> [1] https://www.spinics.net/lists/linux-media/msg141912.html
> [2] https://www.spinics.net/lists/linux-media/msg142099.html
> 
> v2:
>   * Add more info to commit logs
>   * Propagate changes on both encoders and decoders
>   * Add pixel format helpers
> 
> Ezequiel Garcia (4):
>   media: Introduce helpers to fill pixel format structs

This one has been applied (or probably a later version) already.

>   vicodec: Use pixel format helpers

This doesn't apply anymore, but it would be useful to have.

If you can rebase it, then that would be nice.

>   vicodec: Propagate changes to the CAPTURE queue
>   vicodec: Implement spec-compliant stop command

This two are obsolete as far as I know. The vicodec driver should handle
this correctly now. If you think otherwise, please let me know.

Regards,

	Hans

> 
>  .../media/platform/vicodec/codec-v4l2-fwht.c  |  42 ++--
>  .../media/platform/vicodec/codec-v4l2-fwht.h  |   3 -
>  drivers/media/platform/vicodec/vicodec-core.c | 197 +++++++++---------
>  drivers/media/v4l2-core/Makefile              |   2 +-
>  drivers/media/v4l2-core/v4l2-common.c         |  66 ++++++
>  drivers/media/v4l2-core/v4l2-fourcc.c         |  93 +++++++++
>  include/media/v4l2-common.h                   |   5 +
>  include/media/v4l2-fourcc.h                   |  53 +++++
>  8 files changed, 332 insertions(+), 129 deletions(-)
>  create mode 100644 drivers/media/v4l2-core/v4l2-fourcc.c
>  create mode 100644 include/media/v4l2-fourcc.h
> 

Re: [PATCH v2 0/4] vicodec: a couple fixes towards spec compliancy

[Ezequiel Garcia]

On Mon, 2019-06-17 at 14:43 +0200, Hans Verkuil wrote:
> Hi Ezequiel,
> 
> On 11/2/18 4:52 PM, Ezequiel Garcia wrote:
> > Given the stateful codec specification is still a moving target,
> > it doesn't make any sense to try to comply fully with it.
> > 
> > On the other side, we can still comply with some basic userspace
> > expectations, with just a couple small changes.
> > 
> > This series implements proper resolution changes propagation,
> > and fixes the CMD_STOP so it actually works.
> > 
> > The intention of this series is to be able to test this driver
> > using already existing userspace, gstreamer in particular.
> > With this changes, it's possible to construct variations of
> > this pipeline:
> > 
> >   gst-launch-1.0 videotestsrc ! v4l2fwhtenc ! v4l2fwhtdec ! fakevideosink
> > 
> > Also, as discussed in v1 feedback [1,2], I'm including pixel format
> > helpers, as RFC for now. Hans, Tomasz: is this what you had in mind?
> > 
> > [1] https://www.spinics.net/lists/linux-media/msg141912.html
> > [2] https://www.spinics.net/lists/linux-media/msg142099.html
> > 
> > v2:
> >   * Add more info to commit logs
> >   * Propagate changes on both encoders and decoders
> >   * Add pixel format helpers
> > 
> > Ezequiel Garcia (4):
> >   media: Introduce helpers to fill pixel format structs
> 
> This one has been applied (or probably a later version) already.
> 
> >   vicodec: Use pixel format helpers
> 
> This doesn't apply anymore, but it would be useful to have.
> 
> If you can rebase it, then that would be nice.
> 

Sure.

> >   vicodec: Propagate changes to the CAPTURE queue
> >   vicodec: Implement spec-compliant stop command
> 
> This two are obsolete as far as I know. The vicodec driver should handle
> this correctly now. If you think otherwise, please let me know.
> 

Sure, let me re-test it.

Thanks,