On 07/20/2018 08:59 AM, Keiichi Watanabe wrote:
> Support multi-planar APIs in the virtual codec driver.
> Multi-planar APIs are enabled by the module parameter 'multiplanar'.
>
> Signed-off-by: Keiichi Watanabe <keiic...@chromium.org>
Thank you for contributing this code! I've folded it into patch series
(I just posted v2) and added a Co-Developed-by tag.
BTW, for future reference: always run v4l2-compliance after making driver
changes. It caught a bunch of missing checks in this code. I've fixed those,
but you should always check V4L2 driver changes with v4l2-compliance.
Regards,
Hans
> ---
> drivers/media/platform/vicodec/vicodec-core.c | 219 ++++++++++++++----
> 1 file changed, 171 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/media/platform/vicodec/vicodec-core.c
> b/drivers/media/platform/vicodec/vicodec-core.c
> index 12c12cb0c1c0..1717f44e1743 100644
> --- a/drivers/media/platform/vicodec/vicodec-core.c
> +++ b/drivers/media/platform/vicodec/vicodec-core.c
> @@ -29,6 +29,11 @@ MODULE_DESCRIPTION("Virtual codec device");
> MODULE_AUTHOR("Hans Verkuil <hans.verk...@cisco.com>");
> MODULE_LICENSE("GPL v2");
>
> +static bool multiplanar;
> +module_param(multiplanar, bool, 0444);
> +MODULE_PARM_DESC(multiplanar,
> + " use multi-planar API instead of single-planar API");
> +
> static unsigned int debug;
> module_param(debug, uint, 0644);
> MODULE_PARM_DESC(debug, "activates debug info");
> @@ -135,8 +140,10 @@ static struct vicodec_q_data *get_q_data(struct
> vicodec_ctx *ctx,
> {
> switch (type) {
> case V4L2_BUF_TYPE_VIDEO_OUTPUT:
> + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> return &ctx->q_data[V4L2_M2M_SRC];
> case V4L2_BUF_TYPE_VIDEO_CAPTURE:
> + case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> return &ctx->q_data[V4L2_M2M_DST];
> default:
> WARN_ON(1);
> @@ -530,7 +537,10 @@ static int vidioc_querycap(struct file *file, void *priv,
> strncpy(cap->card, VICODEC_NAME, sizeof(cap->card) - 1);
> snprintf(cap->bus_info, sizeof(cap->bus_info),
> "platform:%s", VICODEC_NAME);
> - cap->device_caps = V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING;
> + cap->device_caps = V4L2_CAP_STREAMING |
> + (multiplanar ?
> + V4L2_CAP_VIDEO_M2M_MPLANE :
> + V4L2_CAP_VIDEO_M2M);
> cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
> return 0;
> }
> @@ -576,20 +586,44 @@ static int vidioc_g_fmt(struct vicodec_ctx *ctx, struct
> v4l2_format *f)
>
> q_data = get_q_data(ctx, f->type);
>
> - f->fmt.pix.width = q_data->width;
> - f->fmt.pix.height = q_data->height;
> - f->fmt.pix.field = V4L2_FIELD_NONE;
> - f->fmt.pix.pixelformat = q_data->fourcc;
> - if (q_data->fourcc == V4L2_PIX_FMT_FWHT)
> - f->fmt.pix.bytesperline = 0;
> - else
> - f->fmt.pix.bytesperline = q_data->width;
> - f->fmt.pix.sizeimage = q_data->sizeimage;
> - f->fmt.pix.colorspace = ctx->colorspace;
> - f->fmt.pix.xfer_func = ctx->xfer_func;
> - f->fmt.pix.ycbcr_enc = ctx->ycbcr_enc;
> - f->fmt.pix.quantization = ctx->quantization;
> + switch (f->type) {
> + case V4L2_BUF_TYPE_VIDEO_CAPTURE:
> + case V4L2_BUF_TYPE_VIDEO_OUTPUT:
> + f->fmt.pix.width = q_data->width;
> + f->fmt.pix.height = q_data->height;
> + f->fmt.pix.field = V4L2_FIELD_NONE;
> + f->fmt.pix.pixelformat = q_data->fourcc;
> + if (q_data->fourcc == V4L2_PIX_FMT_FWHT)
> + f->fmt.pix.bytesperline = 0;
> + else
> + f->fmt.pix.bytesperline = q_data->width;
> + f->fmt.pix.sizeimage = q_data->sizeimage;
> + f->fmt.pix.colorspace = ctx->colorspace;
> + f->fmt.pix.xfer_func = ctx->xfer_func;
> + f->fmt.pix.ycbcr_enc = ctx->ycbcr_enc;
> + f->fmt.pix.quantization = ctx->quantization;
> + break;
>
> + case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> + f->fmt.pix_mp.width = q_data->width;
> + f->fmt.pix_mp.height = q_data->height;
> + f->fmt.pix_mp.field = V4L2_FIELD_NONE;
> + f->fmt.pix_mp.pixelformat = q_data->fourcc;
> + f->fmt.pix_mp.num_planes = 1;
> + if (q_data->fourcc == V4L2_PIX_FMT_FWHT)
> + f->fmt.pix_mp.plane_fmt[0].bytesperline = 0;
> + else
> + f->fmt.pix_mp.plane_fmt[0].bytesperline = q_data->width;
> + f->fmt.pix_mp.plane_fmt[0].sizeimage = q_data->sizeimage;
> + f->fmt.pix_mp.colorspace = ctx->colorspace;
> + f->fmt.pix_mp.xfer_func = ctx->xfer_func;
> + f->fmt.pix_mp.ycbcr_enc = ctx->ycbcr_enc;
> + f->fmt.pix_mp.quantization = ctx->quantization;
> + break;
> + default:
> + return -EINVAL;
> + }
> return 0;
> }
>
> @@ -607,16 +641,41 @@ 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 *pix = &f->fmt.pix;
> -
> - pix->width = clamp(pix->width, MIN_WIDTH, MAX_WIDTH) & ~7;
> - pix->height = clamp(pix->height, MIN_HEIGHT, MAX_HEIGHT) & ~7;
> - pix->bytesperline = pix->width;
> - pix->sizeimage = pix->width * pix->height * 3 / 2;
> - pix->field = V4L2_FIELD_NONE;
> - if (pix->pixelformat == V4L2_PIX_FMT_FWHT) {
> - pix->bytesperline = 0;
> - pix->sizeimage += sizeof(struct cframe_hdr);
> + struct v4l2_pix_format *pix;
> + struct v4l2_pix_format_mplane *pix_mp;
> +
> + switch (f->type) {
> + case V4L2_BUF_TYPE_VIDEO_CAPTURE:
> + case V4L2_BUF_TYPE_VIDEO_OUTPUT:
> + pix = &f->fmt.pix;
> + pix->width = clamp(pix->width, MIN_WIDTH, MAX_WIDTH) & ~7;
> + pix->height = clamp(pix->height, MIN_HEIGHT, MAX_HEIGHT) & ~7;
> + pix->bytesperline = pix->width;
> + pix->sizeimage = pix->width * pix->height * 3 / 2;
> + pix->field = V4L2_FIELD_NONE;
> + if (pix->pixelformat == V4L2_PIX_FMT_FWHT) {
> + pix->bytesperline = 0;
> + pix->sizeimage += sizeof(struct cframe_hdr);
> + }
> + break;
> + case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> + pix_mp = &f->fmt.pix_mp;
> + pix_mp->width = clamp(pix_mp->width, MIN_WIDTH, MAX_WIDTH) & ~7;
> + pix_mp->height =
> + clamp(pix_mp->height, MIN_HEIGHT, MAX_HEIGHT) & ~7;
> + pix_mp->plane_fmt[0].bytesperline = pix_mp->width;
> + pix_mp->plane_fmt[0].sizeimage =
> + pix_mp->width * pix_mp->height * 3 / 2;
> + pix_mp->field = V4L2_FIELD_NONE;
> + if (pix_mp->pixelformat == V4L2_PIX_FMT_FWHT) {
> + pix_mp->plane_fmt[0].bytesperline = 0;
> + pix_mp->plane_fmt[0].sizeimage +=
> + sizeof(struct cframe_hdr);
> + }
> + break;
> + default:
> + return -EINVAL;
> }
>
> return 0;
> @@ -627,12 +686,26 @@ static int vidioc_try_fmt_vid_cap(struct file *file,
> void *priv,
> {
> struct vicodec_ctx *ctx = file2ctx(file);
>
> - f->fmt.pix.pixelformat = ctx->is_enc ? V4L2_PIX_FMT_FWHT :
> - find_fmt(f->fmt.pix.pixelformat);
> - f->fmt.pix.colorspace = ctx->colorspace;
> - f->fmt.pix.xfer_func = ctx->xfer_func;
> - f->fmt.pix.ycbcr_enc = ctx->ycbcr_enc;
> - f->fmt.pix.quantization = ctx->quantization;
> + switch (f->type) {
> + case V4L2_BUF_TYPE_VIDEO_CAPTURE:
> + f->fmt.pix.pixelformat = ctx->is_enc ? V4L2_PIX_FMT_FWHT :
> + find_fmt(f->fmt.pix.pixelformat);
> + f->fmt.pix.colorspace = ctx->colorspace;
> + f->fmt.pix.xfer_func = ctx->xfer_func;
> + f->fmt.pix.ycbcr_enc = ctx->ycbcr_enc;
> + f->fmt.pix.quantization = ctx->quantization;
> + break;
> + case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> + f->fmt.pix_mp.pixelformat = ctx->is_enc ? V4L2_PIX_FMT_FWHT :
> + find_fmt(f->fmt.pix_mp.pixelformat);
> + f->fmt.pix_mp.colorspace = ctx->colorspace;
> + f->fmt.pix_mp.xfer_func = ctx->xfer_func;
> + f->fmt.pix_mp.ycbcr_enc = ctx->ycbcr_enc;
> + f->fmt.pix_mp.quantization = ctx->quantization;
> + break;
> + default:
> + return -EINVAL;
> + }
>
> return vidioc_try_fmt(ctx, f);
> }
> @@ -642,10 +715,22 @@ static int vidioc_try_fmt_vid_out(struct file *file,
> void *priv,
> {
> struct vicodec_ctx *ctx = file2ctx(file);
>
> - f->fmt.pix.pixelformat = !ctx->is_enc ? V4L2_PIX_FMT_FWHT :
> - find_fmt(f->fmt.pix.pixelformat);
> - if (!f->fmt.pix.colorspace)
> - f->fmt.pix.colorspace = V4L2_COLORSPACE_REC709;
> + switch (f->type) {
> + case V4L2_BUF_TYPE_VIDEO_OUTPUT:
> + f->fmt.pix.pixelformat = !ctx->is_enc ? V4L2_PIX_FMT_FWHT :
> + find_fmt(f->fmt.pix.pixelformat);
> + if (!f->fmt.pix.colorspace)
> + f->fmt.pix.colorspace = V4L2_COLORSPACE_REC709;
> + break;
> + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> + f->fmt.pix_mp.pixelformat = !ctx->is_enc ? V4L2_PIX_FMT_FWHT :
> + find_fmt(f->fmt.pix_mp.pixelformat);
> + if (!f->fmt.pix_mp.colorspace)
> + f->fmt.pix_mp.colorspace = V4L2_COLORSPACE_REC709;
> + break;
> + default:
> + return -EINVAL;
> + }
>
> return vidioc_try_fmt(ctx, f);
> }
> @@ -664,18 +749,42 @@ static int vidioc_s_fmt(struct vicodec_ctx *ctx, struct
> v4l2_format *f)
> if (!q_data)
> return -EINVAL;
>
> - if (ctx->is_enc && V4L2_TYPE_IS_OUTPUT(f->type))
> - fmt_changed = q_data->fourcc != f->fmt.pix.pixelformat ||
> - q_data->width != f->fmt.pix.width ||
> - q_data->height != f->fmt.pix.height;
> -
> - if (vb2_is_busy(vq) && fmt_changed)
> - return -EBUSY;
> -
> - q_data->fourcc = f->fmt.pix.pixelformat;
> - q_data->width = f->fmt.pix.width;
> - q_data->height = f->fmt.pix.height;
> - q_data->sizeimage = f->fmt.pix.sizeimage;
> + switch (f->type) {
> + case V4L2_BUF_TYPE_VIDEO_CAPTURE:
> + case V4L2_BUF_TYPE_VIDEO_OUTPUT:
> + if (ctx->is_enc && V4L2_TYPE_IS_OUTPUT(f->type))
> + fmt_changed =
> + q_data->fourcc != f->fmt.pix.pixelformat ||
> + q_data->width != f->fmt.pix.width ||
> + q_data->height != f->fmt.pix.height;
> +
> + if (vb2_is_busy(vq) && fmt_changed)
> + return -EBUSY;
> +
> + q_data->fourcc = f->fmt.pix.pixelformat;
> + q_data->width = f->fmt.pix.width;
> + q_data->height = f->fmt.pix.height;
> + q_data->sizeimage = f->fmt.pix.sizeimage;
> + break;
> + case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> + if (ctx->is_enc && V4L2_TYPE_IS_OUTPUT(f->type))
> + fmt_changed =
> + q_data->fourcc != f->fmt.pix_mp.pixelformat ||
> + q_data->width != f->fmt.pix_mp.width ||
> + q_data->height != f->fmt.pix_mp.height;
> +
> + if (vb2_is_busy(vq) && fmt_changed)
> + return -EBUSY;
> +
> + q_data->fourcc = f->fmt.pix_mp.pixelformat;
> + q_data->width = f->fmt.pix_mp.width;
> + q_data->height = f->fmt.pix_mp.height;
> + q_data->sizeimage = f->fmt.pix_mp.plane_fmt[0].sizeimage;
> + break;
> + default:
> + return -EINVAL;
> + }
>
> dprintk(ctx->dev,
> "Setting format for type %d, wxh: %dx%d, fourcc: %08x\n",
> @@ -832,11 +941,21 @@ static const struct v4l2_ioctl_ops vicodec_ioctl_ops = {
> .vidioc_try_fmt_vid_cap = vidioc_try_fmt_vid_cap,
> .vidioc_s_fmt_vid_cap = vidioc_s_fmt_vid_cap,
>
> + .vidioc_enum_fmt_vid_cap_mplane = vidioc_enum_fmt_vid_cap,
> + .vidioc_g_fmt_vid_cap_mplane = vidioc_g_fmt_vid_cap,
> + .vidioc_try_fmt_vid_cap_mplane = vidioc_try_fmt_vid_cap,
> + .vidioc_s_fmt_vid_cap_mplane = vidioc_s_fmt_vid_cap,
> +
> .vidioc_enum_fmt_vid_out = vidioc_enum_fmt_vid_out,
> .vidioc_g_fmt_vid_out = vidioc_g_fmt_vid_out,
> .vidioc_try_fmt_vid_out = vidioc_try_fmt_vid_out,
> .vidioc_s_fmt_vid_out = vidioc_s_fmt_vid_out,
>
> + .vidioc_enum_fmt_vid_out_mplane = vidioc_enum_fmt_vid_out,
> + .vidioc_g_fmt_vid_out_mplane = vidioc_g_fmt_vid_out,
> + .vidioc_try_fmt_vid_out_mplane = vidioc_try_fmt_vid_out,
> + .vidioc_s_fmt_vid_out_mplane = vidioc_s_fmt_vid_out,
> +
> .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,
> .vidioc_querybuf = v4l2_m2m_ioctl_querybuf,
> .vidioc_qbuf = v4l2_m2m_ioctl_qbuf,
> @@ -1002,7 +1121,9 @@ static int queue_init(void *priv, struct vb2_queue
> *src_vq,
> struct vicodec_ctx *ctx = priv;
> int ret;
>
> - src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
> + src_vq->type = (multiplanar ?
> + V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE :
> + V4L2_BUF_TYPE_VIDEO_OUTPUT);
> src_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> src_vq->drv_priv = ctx;
> src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> @@ -1016,7 +1137,9 @@ static int queue_init(void *priv, struct vb2_queue
> *src_vq,
> if (ret)
> return ret;
>
> - dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> + dst_vq->type = (multiplanar ?
> + V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE :
> + V4L2_BUF_TYPE_VIDEO_CAPTURE);
> dst_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> dst_vq->drv_priv = ctx;
> dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> --
> 2.18.0.233.g985f88cf7e-goog
>
> This is an additional patch to Hans's patch series of the new vicodec driver.
> This patch adds multi-planar API support. I confirmed that v4l2-ctl uses
> multi-planar APIs to decode a FWHT format video when vicodec module is loaded
> with module parameter 'multiplanar'.
>
