[v3,3/5] media: imx-jpeg: Propagate the output frame size to the capture side

Message ID 473edd5666b24a659f651ca003add12ac9a0c2d2.1648023273.git.ming.qian@nxp.com (mailing list archive)
State Accepted
Delegated to: Hans Verkuil
Headers
Series imx-jpeg: Support dynamic resolution change |

Commit Message

Ming Qian March 23, 2022, 9:05 a.m. UTC
  The GStreamer v4l2videodec only ever calls S_FMT on the output side
and then expects G_FMT on the capture side to return a valid format.

Signed-off-by: Ming Qian <ming.qian@nxp.com>
---
 .../media/platform/nxp/imx-jpeg/mxc-jpeg.c    | 30 ++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)
  

Comments

Mirela Rabulea OSS April 11, 2022, 11:50 a.m. UTC | #1
Hi Ming,

On 23.03.2022 11:05, Ming Qian wrote:
> The GStreamer v4l2videodec only ever calls S_FMT on the output side
> and then expects G_FMT on the capture side to return a valid format.
> 
> Signed-off-by: Ming Qian <ming.qian@nxp.com>
> ---
>   .../media/platform/nxp/imx-jpeg/mxc-jpeg.c    | 30 ++++++++++++++++++-
>   1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> index a95305639dd9..5dfa6f87871e 100644
> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> @@ -1831,12 +1831,40 @@ static int mxc_jpeg_s_fmt_vid_out(struct file *file, void *priv,
>   				  struct v4l2_format *f)
>   {
>   	int ret;
> +	struct mxc_jpeg_ctx *ctx = mxc_jpeg_fh_to_ctx(priv);
> +	struct vb2_queue *dst_vq;
> +	struct mxc_jpeg_q_data *q_data_cap;
> +	enum v4l2_buf_type cap_type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> +	struct v4l2_format fc;
>   
>   	ret = mxc_jpeg_try_fmt_vid_out(file, priv, f);
>   	if (ret)
>   		return ret;
>   
> -	return mxc_jpeg_s_fmt(mxc_jpeg_fh_to_ctx(priv), f);
> +	ret = mxc_jpeg_s_fmt(mxc_jpeg_fh_to_ctx(priv), f);
> +	if (ret)
> +		return ret;
> +
> +	if (ctx->mxc_jpeg->mode != MXC_JPEG_DECODE)
> +		return 0;
> +
> +	dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, cap_type);
> +	if (!dst_vq)
> +		return -EINVAL;
> +
> +	if (vb2_is_busy(dst_vq))
> +		return 0;
> +
> +	q_data_cap = mxc_jpeg_get_q_data(ctx, cap_type);
> +	if (q_data_cap->w == f->fmt.pix_mp.width && q_data_cap->h == f->fmt.pix_mp.height)
> +		return 0;

Is this an optimization, to avoid propagating the format to the capture 
queue, if it is already set? If so, shouldn't fourcc also be part of the 
comparison?

Thanks,
Mirela

> +	memset(&fc, 0, sizeof(fc));
> +	fc.type = cap_type;
> +	fc.fmt.pix_mp.pixelformat = q_data_cap->fmt->fourcc;
> +	fc.fmt.pix_mp.width = f->fmt.pix_mp.width;
> +	fc.fmt.pix_mp.height = f->fmt.pix_mp.height;
> +
> +	return mxc_jpeg_s_fmt_vid_cap(file, priv, &fc);
>   }
>   
>   static int mxc_jpeg_g_fmt_vid(struct file *file, void *priv,
  
Ming Qian April 12, 2022, 2:11 a.m. UTC | #2
> From: Mirela Rabulea (OSS)
> Sent: Monday, April 11, 2022 7:50 PM
> To: Ming Qian <ming.qian@nxp.com>; mchehab@kernel.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de
> Cc: hverkuil-cisco@xs4all.nl; kernel@pengutronix.de; festevam@gmail.com;
> dl-linux-imx <linux-imx@nxp.com>; linux-media@vger.kernel.org;
> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH v3 3/5] media: imx-jpeg: Propagate the output frame size
> to the capture side
> 
> Hi Ming,
> 
> On 23.03.2022 11:05, Ming Qian wrote:
> > The GStreamer v4l2videodec only ever calls S_FMT on the output side
> > and then expects G_FMT on the capture side to return a valid format.
> >
> > Signed-off-by: Ming Qian <ming.qian@nxp.com>
> > ---
> >   .../media/platform/nxp/imx-jpeg/mxc-jpeg.c    | 30
> ++++++++++++++++++-
> >   1 file changed, 29 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> > b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> > index a95305639dd9..5dfa6f87871e 100644
> > --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> > +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> > @@ -1831,12 +1831,40 @@ static int mxc_jpeg_s_fmt_vid_out(struct file
> *file, void *priv,
> >   				  struct v4l2_format *f)
> >   {
> >   	int ret;
> > +	struct mxc_jpeg_ctx *ctx = mxc_jpeg_fh_to_ctx(priv);
> > +	struct vb2_queue *dst_vq;
> > +	struct mxc_jpeg_q_data *q_data_cap;
> > +	enum v4l2_buf_type cap_type =
> V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> > +	struct v4l2_format fc;
> >
> >   	ret = mxc_jpeg_try_fmt_vid_out(file, priv, f);
> >   	if (ret)
> >   		return ret;
> >
> > -	return mxc_jpeg_s_fmt(mxc_jpeg_fh_to_ctx(priv), f);
> > +	ret = mxc_jpeg_s_fmt(mxc_jpeg_fh_to_ctx(priv), f);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (ctx->mxc_jpeg->mode != MXC_JPEG_DECODE)
> > +		return 0;
> > +
> > +	dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, cap_type);
> > +	if (!dst_vq)
> > +		return -EINVAL;
> > +
> > +	if (vb2_is_busy(dst_vq))
> > +		return 0;
> > +
> > +	q_data_cap = mxc_jpeg_get_q_data(ctx, cap_type);
> > +	if (q_data_cap->w == f->fmt.pix_mp.width && q_data_cap->h ==
> f->fmt.pix_mp.height)
> > +		return 0;
> 
> Is this an optimization, to avoid propagating the format to the capture queue,
> if it is already set? If so, shouldn't fourcc also be part of the comparison?
> 
> Thanks,
> Mirela
> 

Hi Mirela,
    Yes, if the capture size is equal to the output size, we don't need to set it again. For decoder, the format of output is jpeg, and the format for capture is uncompressed format, such as nv12, so the fourcc are always different.

> > +	memset(&fc, 0, sizeof(fc));
> > +	fc.type = cap_type;
> > +	fc.fmt.pix_mp.pixelformat = q_data_cap->fmt->fourcc;
> > +	fc.fmt.pix_mp.width = f->fmt.pix_mp.width;
> > +	fc.fmt.pix_mp.height = f->fmt.pix_mp.height;
> > +
> > +	return mxc_jpeg_s_fmt_vid_cap(file, priv, &fc);
> >   }
> >
> >   static int mxc_jpeg_g_fmt_vid(struct file *file, void *priv,
  

Patch

diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
index a95305639dd9..5dfa6f87871e 100644
--- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
+++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
@@ -1831,12 +1831,40 @@  static int mxc_jpeg_s_fmt_vid_out(struct file *file, void *priv,
 				  struct v4l2_format *f)
 {
 	int ret;
+	struct mxc_jpeg_ctx *ctx = mxc_jpeg_fh_to_ctx(priv);
+	struct vb2_queue *dst_vq;
+	struct mxc_jpeg_q_data *q_data_cap;
+	enum v4l2_buf_type cap_type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
+	struct v4l2_format fc;
 
 	ret = mxc_jpeg_try_fmt_vid_out(file, priv, f);
 	if (ret)
 		return ret;
 
-	return mxc_jpeg_s_fmt(mxc_jpeg_fh_to_ctx(priv), f);
+	ret = mxc_jpeg_s_fmt(mxc_jpeg_fh_to_ctx(priv), f);
+	if (ret)
+		return ret;
+
+	if (ctx->mxc_jpeg->mode != MXC_JPEG_DECODE)
+		return 0;
+
+	dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, cap_type);
+	if (!dst_vq)
+		return -EINVAL;
+
+	if (vb2_is_busy(dst_vq))
+		return 0;
+
+	q_data_cap = mxc_jpeg_get_q_data(ctx, cap_type);
+	if (q_data_cap->w == f->fmt.pix_mp.width && q_data_cap->h == f->fmt.pix_mp.height)
+		return 0;
+	memset(&fc, 0, sizeof(fc));
+	fc.type = cap_type;
+	fc.fmt.pix_mp.pixelformat = q_data_cap->fmt->fourcc;
+	fc.fmt.pix_mp.width = f->fmt.pix_mp.width;
+	fc.fmt.pix_mp.height = f->fmt.pix_mp.height;
+
+	return mxc_jpeg_s_fmt_vid_cap(file, priv, &fc);
 }
 
 static int mxc_jpeg_g_fmt_vid(struct file *file, void *priv,