linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Helen Koike <helen.koike@collabora.com>
To: "André Almeida" <andrealmeid@collabora.com>, linux-media@vger.kernel.org
Cc: mchehab@kernel.org, hverkuil@xs4all.nl, lucmaga@gmail.com,
	linux-kernel@vger.kernel.org, kernel@collabora.com
Subject: Re: [PATCH 01/16] media: Move sp2mp functions to v4l2-common
Date: Fri, 15 Mar 2019 16:30:08 -0300	[thread overview]
Message-ID: <31ce0b97-4b1d-8f1e-0f0e-cbd2a4bcf6ce@collabora.com> (raw)
In-Reply-To: <20190315164359.626-2-andrealmeid@collabora.com>



On 3/15/19 1:43 PM, André Almeida wrote:
> Move sp2mp functions from vivid cote to v4l2-common as it will be reused
> by vimc driver for multiplanar support.

s/cote/code

> 
> Signed-off-by: André Almeida <andrealmeid@collabora.com>
> ---
>  drivers/media/platform/vivid/vivid-vid-cap.c  |  6 +-
>  .../media/platform/vivid/vivid-vid-common.c   | 59 ------------------
>  .../media/platform/vivid/vivid-vid-common.h   |  9 ---
>  drivers/media/platform/vivid/vivid-vid-out.c  |  6 +-
>  drivers/media/v4l2-core/v4l2-common.c         | 62 +++++++++++++++++++
>  include/media/v4l2-common.h                   | 31 ++++++++++
>  6 files changed, 99 insertions(+), 74 deletions(-)
> 
> diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c b/drivers/media/platform/vivid/vivid-vid-cap.c
> index 52eeda624d7e..b5ad71bbf7bf 100644
> --- a/drivers/media/platform/vivid/vivid-vid-cap.c
> +++ b/drivers/media/platform/vivid/vivid-vid-cap.c
> @@ -815,7 +815,7 @@ int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
>  
>  	if (dev->multiplanar)
>  		return -ENOTTY;
> -	return fmt_sp2mp_func(file, priv, f, vivid_g_fmt_vid_cap);
> +	return v4l2_fmt_sp2mp_func(file, priv, f, vivid_g_fmt_vid_cap);
>  }
>  
>  int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
> @@ -825,7 +825,7 @@ int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
>  
>  	if (dev->multiplanar)
>  		return -ENOTTY;
> -	return fmt_sp2mp_func(file, priv, f, vivid_try_fmt_vid_cap);
> +	return v4l2_fmt_sp2mp_func(file, priv, f, vivid_try_fmt_vid_cap);
>  }
>  
>  int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
> @@ -835,7 +835,7 @@ int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
>  
>  	if (dev->multiplanar)
>  		return -ENOTTY;
> -	return fmt_sp2mp_func(file, priv, f, vivid_s_fmt_vid_cap);
> +	return v4l2_fmt_sp2mp_func(file, priv, f, vivid_s_fmt_vid_cap);
>  }
>  
>  int vivid_vid_cap_g_selection(struct file *file, void *priv,
> diff --git a/drivers/media/platform/vivid/vivid-vid-common.c b/drivers/media/platform/vivid/vivid-vid-common.c
> index 74b83bcc6119..3dd3a05d2e67 100644
> --- a/drivers/media/platform/vivid/vivid-vid-common.c
> +++ b/drivers/media/platform/vivid/vivid-vid-common.c
> @@ -674,65 +674,6 @@ void vivid_send_source_change(struct vivid_dev *dev, unsigned type)
>  	}
>  }
>  
> -/*
> - * Conversion function that converts a single-planar format to a
> - * single-plane multiplanar format.
> - */
> -void fmt_sp2mp(const struct v4l2_format *sp_fmt, struct v4l2_format *mp_fmt)
> -{
> -	struct v4l2_pix_format_mplane *mp = &mp_fmt->fmt.pix_mp;
> -	struct v4l2_plane_pix_format *ppix = &mp->plane_fmt[0];
> -	const struct v4l2_pix_format *pix = &sp_fmt->fmt.pix;
> -	bool is_out = sp_fmt->type == V4L2_BUF_TYPE_VIDEO_OUTPUT;
> -
> -	memset(mp->reserved, 0, sizeof(mp->reserved));
> -	mp_fmt->type = is_out ? V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE :
> -			   V4L2_CAP_VIDEO_CAPTURE_MPLANE;
> -	mp->width = pix->width;
> -	mp->height = pix->height;
> -	mp->pixelformat = pix->pixelformat;
> -	mp->field = pix->field;
> -	mp->colorspace = pix->colorspace;
> -	mp->xfer_func = pix->xfer_func;
> -	/* Also copies hsv_enc */
> -	mp->ycbcr_enc = pix->ycbcr_enc;
> -	mp->quantization = pix->quantization;
> -	mp->num_planes = 1;
> -	mp->flags = pix->flags;
> -	ppix->sizeimage = pix->sizeimage;
> -	ppix->bytesperline = pix->bytesperline;
> -	memset(ppix->reserved, 0, sizeof(ppix->reserved));
> -}
> -
> -int fmt_sp2mp_func(struct file *file, void *priv,
> -		struct v4l2_format *f, fmtfunc func)
> -{
> -	struct v4l2_format fmt;
> -	struct v4l2_pix_format_mplane *mp = &fmt.fmt.pix_mp;
> -	struct v4l2_plane_pix_format *ppix = &mp->plane_fmt[0];
> -	struct v4l2_pix_format *pix = &f->fmt.pix;
> -	int ret;
> -
> -	/* Converts to a mplane format */
> -	fmt_sp2mp(f, &fmt);
> -	/* Passes it to the generic mplane format function */
> -	ret = func(file, priv, &fmt);
> -	/* Copies back the mplane data to the single plane format */
> -	pix->width = mp->width;
> -	pix->height = mp->height;
> -	pix->pixelformat = mp->pixelformat;
> -	pix->field = mp->field;
> -	pix->colorspace = mp->colorspace;
> -	pix->xfer_func = mp->xfer_func;
> -	/* Also copies hsv_enc */
> -	pix->ycbcr_enc = mp->ycbcr_enc;
> -	pix->quantization = mp->quantization;
> -	pix->sizeimage = ppix->sizeimage;
> -	pix->bytesperline = ppix->bytesperline;
> -	pix->flags = mp->flags;
> -	return ret;
> -}
> -
>  int vivid_vid_adjust_sel(unsigned flags, struct v4l2_rect *r)
>  {
>  	unsigned w = r->width;
> diff --git a/drivers/media/platform/vivid/vivid-vid-common.h b/drivers/media/platform/vivid/vivid-vid-common.h
> index 29b6c0b40a1b..13adea56baa0 100644
> --- a/drivers/media/platform/vivid/vivid-vid-common.h
> +++ b/drivers/media/platform/vivid/vivid-vid-common.h
> @@ -8,15 +8,6 @@
>  #ifndef _VIVID_VID_COMMON_H_
>  #define _VIVID_VID_COMMON_H_
>  
> -typedef int (*fmtfunc)(struct file *file, void *priv, struct v4l2_format *f);
> -
> -/*
> - * Conversion function that converts a single-planar format to a
> - * single-plane multiplanar format.
> - */
> -void fmt_sp2mp(const struct v4l2_format *sp_fmt, struct v4l2_format *mp_fmt);
> -int fmt_sp2mp_func(struct file *file, void *priv,
> -		struct v4l2_format *f, fmtfunc func);
>  
>  extern const struct v4l2_dv_timings_cap vivid_dv_timings_cap;
>  
> diff --git a/drivers/media/platform/vivid/vivid-vid-out.c b/drivers/media/platform/vivid/vivid-vid-out.c
> index e61b91b414f9..c42ba5ade6cf 100644
> --- a/drivers/media/platform/vivid/vivid-vid-out.c
> +++ b/drivers/media/platform/vivid/vivid-vid-out.c
> @@ -612,7 +612,7 @@ int vidioc_g_fmt_vid_out(struct file *file, void *priv,
>  
>  	if (dev->multiplanar)
>  		return -ENOTTY;
> -	return fmt_sp2mp_func(file, priv, f, vivid_g_fmt_vid_out);
> +	return v4l2_fmt_sp2mp_func(file, priv, f, vivid_g_fmt_vid_out);
>  }
>  
>  int vidioc_try_fmt_vid_out(struct file *file, void *priv,
> @@ -622,7 +622,7 @@ int vidioc_try_fmt_vid_out(struct file *file, void *priv,
>  
>  	if (dev->multiplanar)
>  		return -ENOTTY;
> -	return fmt_sp2mp_func(file, priv, f, vivid_try_fmt_vid_out);
> +	return v4l2_fmt_sp2mp_func(file, priv, f, vivid_try_fmt_vid_out);
>  }
>  
>  int vidioc_s_fmt_vid_out(struct file *file, void *priv,
> @@ -632,7 +632,7 @@ int vidioc_s_fmt_vid_out(struct file *file, void *priv,
>  
>  	if (dev->multiplanar)
>  		return -ENOTTY;
> -	return fmt_sp2mp_func(file, priv, f, vivid_s_fmt_vid_out);
> +	return v4l2_fmt_sp2mp_func(file, priv, f, vivid_s_fmt_vid_out);
>  }
>  
>  int vivid_vid_out_g_selection(struct file *file, void *priv,
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 779e44d6db43..d118f8f34d32 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -653,3 +653,65 @@ int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, int pixelformat, int width,
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt);
> +
> +/*
> + * Conversion functions that convert a single-planar format to a
> + * multi-planar format.
> + */
> +void v4l2_fmt_sp2mp(const struct v4l2_format *sp_fmt,
> +		struct v4l2_format *mp_fmt)
> +{
> +	struct v4l2_pix_format_mplane *mp = &mp_fmt->fmt.pix_mp;
> +	struct v4l2_plane_pix_format *ppix = &mp->plane_fmt[0];
> +	const struct v4l2_pix_format *pix = &sp_fmt->fmt.pix;
> +	bool is_out = sp_fmt->type == V4L2_BUF_TYPE_VIDEO_OUTPUT;
> +
> +	memset(mp->reserved, 0, sizeof(mp->reserved));
> +	mp_fmt->type = is_out ? V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE :
> +			   V4L2_CAP_VIDEO_CAPTURE_MPLANE;
> +	mp->width = pix->width;
> +	mp->height = pix->height;
> +	mp->pixelformat = pix->pixelformat;
> +	mp->field = pix->field;
> +	mp->colorspace = pix->colorspace;
> +	mp->xfer_func = pix->xfer_func;
> +	/* Also copies hsv_enc */
> +	mp->ycbcr_enc = pix->ycbcr_enc;
> +	mp->quantization = pix->quantization;
> +	mp->num_planes = 1;
> +	mp->flags = pix->flags;
> +	ppix->sizeimage = pix->sizeimage;
> +	ppix->bytesperline = pix->bytesperline;
> +	memset(ppix->reserved, 0, sizeof(ppix->reserved));
> +}
> +EXPORT_SYMBOL_GPL(v4l2_fmt_sp2mp);
> +
> +int v4l2_fmt_sp2mp_func(struct file *file, void *priv,
> +		struct v4l2_format *f, v4l2_fmtfunc func)
> +{
> +	struct v4l2_format fmt;
> +	struct v4l2_pix_format_mplane *mp = &fmt.fmt.pix_mp;
> +	struct v4l2_plane_pix_format *ppix = &mp->plane_fmt[0];
> +	struct v4l2_pix_format *pix = &f->fmt.pix;
> +	int ret;
> +
> +	/* Converts to a mplane format */
> +	v4l2_fmt_sp2mp(f, &fmt);
> +	/* Passes it to the generic mplane format function */
> +	ret = func(file, priv, &fmt);
> +	/* Copies back the mplane data to the single plane format */
> +	pix->width = mp->width;
> +	pix->height = mp->height;
> +	pix->pixelformat = mp->pixelformat;
> +	pix->field = mp->field;
> +	pix->colorspace = mp->colorspace;
> +	pix->xfer_func = mp->xfer_func;
> +	/* Also copies hsv_enc */
> +	pix->ycbcr_enc = mp->ycbcr_enc;
> +	pix->quantization = mp->quantization;
> +	pix->sizeimage = ppix->sizeimage;
> +	pix->bytesperline = ppix->bytesperline;
> +	pix->flags = mp->flags;
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_fmt_sp2mp_func);
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 937b74a946cd..d106f36ebaf4 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -424,4 +424,35 @@ int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, int pixelformat,
>  int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, int pixelformat,
>  			int width, int height);
>  
> +/**
> + * v4l2_fmtfunc - type to be used by v4l2_fmt_sp2mp_func to pass the generic
> + * mp function as argument

You can use a shorter title, e.g.
Function pointer type for v4l2_fmt_sp2mp()

And you can add a better description below,
e.g.https://git.linuxtv.org/media_tree.git/tree/include/media/v4l2-common.h#n287

this also applies to other docs.

> + * @file: device's descriptor file
> + * @priv: private data pointer
> + * @f: format that holds a mp pixel format
> + */
> +typedef int (*v4l2_fmtfunc)(struct file *file, void *priv,
> +		struct v4l2_format *f);

It would be nice if you could align the second line with the arguments
of the function in the first line.

> +
> +/**
> + * v4l2_fmt_sp2mp - transforms a single-planar format struct into a multi-planar
> + * struct
> + * @sp_fmt: pointer to the single-planar format struct (in)
> + * @mp_fmt: pointer to the multi-planar format struct (out)
> + */
> +void v4l2_fmt_sp2mp(const struct v4l2_format *sp_fmt,
> +		struct v4l2_format *mp_fmt);

same here regarding alignment.

> +
> +/**
> + * v4l2_fmt_sp2mp_func - handler to call a generic multi-planar format function
> + * using single-planar format. It converts the sp to a mp, calls the
> + * function and converts mp back to sp.
> + * @file: device's descriptor file
> + * @priv: private data pointer
> + * @f: format that holds a sp pixel format
> + * @func: generic mp function
> + */
> +int v4l2_fmt_sp2mp_func(struct file *file, void *priv,
> +		struct v4l2_format *f, v4l2_fmtfunc func);

same here regarding alignment.

Regards,
Helen

> +
>  #endif /* V4L2_COMMON_H_ */
> 

  reply	other threads:[~2019-03-15 19:30 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-15 16:43 [PATCH 00/16] media: vimc: Add support for multiplanar formats André Almeida
2019-03-15 16:43 ` [PATCH 01/16] media: Move sp2mp functions to v4l2-common André Almeida
2019-03-15 19:30   ` Helen Koike [this message]
2019-03-15 16:43 ` [PATCH 02/16] media: vimc: Remove unnecessary stream check André Almeida
2019-03-15 16:43 ` [PATCH 03/16] media: vimc: Check if the stream is on using ved.stream André Almeida
2019-03-15 19:30   ` Helen Koike
2019-03-15 16:43 ` [PATCH 04/16] media: vimc: cap: Change vimc_cap_device.format type André Almeida
2019-03-15 16:43 ` [PATCH 05/16] media: vimc: Create multiplanar parameter André Almeida
2019-03-15 19:30   ` Helen Koike
2019-03-15 16:43 ` [PATCH 06/16] media: vimc: cap: Dynamically define stream pixelformat André Almeida
2019-03-15 16:43 ` [PATCH 07/16] media: vimc: cap: Add handler for singleplanar fmt ioctls André Almeida
2019-03-15 19:31   ` Helen Koike
2019-03-15 16:43 ` [PATCH 08/16] media: vimc: cap: Add handler for multiplanar " André Almeida
2019-03-15 19:31   ` Helen Koike
2019-03-15 16:43 ` [PATCH 09/16] media: vimc: cap: Add multiplanar formats André Almeida
2019-03-15 19:31   ` Helen Koike
2019-03-15 16:43 ` [PATCH 10/16] media: vimc: cap: Add multiplanar default format André Almeida
2019-03-15 16:43 ` [PATCH 11/16] media: vimc: cap: Allocate and verify mplanar buffers André Almeida
2019-03-15 16:43 ` [PATCH 12/16] media: vimc: Add and use new struct vimc_frame André Almeida
2019-03-15 19:31   ` Helen Koike
2019-03-15 16:43 ` [PATCH 13/16] media: vimc: sen: Add support for multiplanar formats André Almeida
2019-03-15 19:32   ` Helen Koike
2019-03-15 16:43 ` [PATCH 14/16] media: vimc: sca: " André Almeida
2019-03-15 19:32   ` Helen Koike
2019-03-15 16:43 ` [PATCH 15/16] media: vimc: cap: " André Almeida
2019-03-15 16:43 ` [PATCH 16/16] media: vimc: cap: Dynamically define device caps André Almeida
2019-03-15 17:29 ` [PATCH 00/16] media: vimc: Add support for multiplanar formats André Almeida
2019-03-15 19:32   ` Helen Koike

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=31ce0b97-4b1d-8f1e-0f0e-cbd2a4bcf6ce@collabora.com \
    --to=helen.koike@collabora.com \
    --cc=andrealmeid@collabora.com \
    --cc=hverkuil@xs4all.nl \
    --cc=kernel@collabora.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=lucmaga@gmail.com \
    --cc=mchehab@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).