Re: [PATCH] V4L: soc-camera: regression fix: calculate .sizeimage in soc_camera.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Guennadi,

On Mon, Apr 11, 2011 at 8:23 AM, Guennadi Liakhovetski
<g.liakhovetski@xxxxxx> wrote:
>
> On Mon, 11 Apr 2011, Aguirre, Sergio wrote:
>
> > Hi Guennadi,
> >
> > On Mon, Apr 11, 2011 at 3:58 AM, Guennadi Liakhovetski <
> > g.liakhovetski@xxxxxx> wrote:
> >
> > > A recent patch has given individual soc-camera host drivers a possibility
> > > to calculate .sizeimage and .bytesperline pixel format fields internally,
> > > however, some drivers relied on the core calculating these values for
> > > them, following a default algorithm. This patch restores the default
> > > calculation for such drivers.
> > >
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
> > > ---
> > > diff --git a/drivers/media/video/soc_camera.c
> > > b/drivers/media/video/soc_camera.c
> > > index 4628448..0918c48 100644
> > > --- a/drivers/media/video/soc_camera.c
> > > +++ b/drivers/media/video/soc_camera.c
> > > @@ -376,6 +376,9 @@ static int soc_camera_set_fmt(struct soc_camera_device
> > > *icd,
> > >        dev_dbg(&icd->dev, "S_FMT(%c%c%c%c, %ux%u)\n",
> > >                pixfmtstr(pix->pixelformat), pix->width, pix->height);
> > >
> > > +       pix->bytesperline = 0;
> > > +       pix->sizeimage = 0;
> > > +
> > >        /* We always call try_fmt() before set_fmt() or set_crop() */
> > >        ret = ici->ops->try_fmt(icd, f);
> > >        if (ret < 0)
> > > @@ -391,6 +394,17 @@ static int soc_camera_set_fmt(struct soc_camera_device
> > > *icd,
> > >                return -EINVAL;
> > >        }
> > >
> > > +       if (!pix->sizeimage) {
> > > +               if (!pix->bytesperline) {
> > > +                       ret = soc_mbus_bytes_per_line(pix->width,
> > > +
> > > icd->current_fmt->host_fmt);
> > > +                       if (ret > 0)
> > > +                               pix->bytesperline = ret;
> > > +               }
> > > +               if (pix->bytesperline)
> > > +                       pix->sizeimage = pix->bytesperline * pix->height;
> > > +       }
> > > +
> > >
> >
> > Shouldn't all this be better done in try_fmt?
>
> Not _better_. We might choose to additionally do it for try_fmt too. But
>
> 1. we didn't do it before, applications don't seem to care.
> 2. you cannot store / reuse those .sizeimage & .bytesperline values - this
> is just a "try"
> 3. it would be a bit difficult to realise - we need a struct
> soc_mbus_pixelfmt to call soc_mbus_bytes_per_line(), which we don't have,
> so, we'd need to obtain it using soc_camera_xlate_by_fourcc().
>
> This all would work, but in any case it would be an enhancement, but not a
> regression fix.

Ok. And how about the attached patch? Would that work?

Regards,
Sergio

>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
From 5442074c7760429c06f62917503c1d8a2f79cd21 Mon Sep 17 00:00:00 2001
From: Sergio Aguirre <saaguirre@xxxxxx>
Date: Mon, 11 Apr 2011 11:14:33 -0500
Subject: [PATCH] V4L: soc-camera: regression fix: calculate .sizeimage in soc_camera.c

A recent patch has given individual soc-camera host drivers a possibility
to calculate .sizeimage and .bytesperline pixel format fields internally,
however, some drivers relied on the core calculating these values for
them, following a default algorithm. This patch restores the default
calculation for such drivers.

Based on initial patch by Guennadi Liakhovetski, found here:

http://www.spinics.net/lists/linux-media/msg31282.html

Except that this covers try_fmt aswell.

Signed-off-by: Sergio Aguirre <saaguirre@xxxxxx>
---
 drivers/media/video/soc_camera.c |   41 ++++++++++++++++++++++++++++++++++++-
 1 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index 4628448..875842a 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -136,6 +136,43 @@ unsigned long soc_camera_apply_sensor_flags(struct soc_camera_link *icl,
 }
 EXPORT_SYMBOL(soc_camera_apply_sensor_flags);
 
+static int soc_camera_try_fmt(struct soc_camera_device *icd,
+			      struct v4l2_format *f)
+{
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
+	struct v4l2_pix_format *pix = &f->fmt.pix;
+	int ret;
+
+	dev_dbg(&icd->dev, "TRY_FMT(%c%c%c%c, %ux%u)\n",
+		pixfmtstr(pix->pixelformat), pix->width, pix->height);
+
+	pix->bytesperline = 0;
+	pix->sizeimage = 0;
+
+	ret = ici->ops->try_fmt(icd, f);
+	if (ret < 0)
+		return ret;
+
+	if (!pix->sizeimage) {
+		if (!pix->bytesperline) {
+			const struct soc_camera_format_xlate *xlate;
+
+			xlate = soc_camera_xlate_by_fourcc(icd, pix->pixelformat);
+			if (!xlate)
+				return -EINVAL;
+
+			ret = soc_mbus_bytes_per_line(pix->width,
+						      xlate->host_fmt);
+			if (ret > 0)
+				pix->bytesperline = ret;
+		}
+		if (pix->bytesperline)
+			pix->sizeimage = pix->bytesperline * pix->height;
+	}
+
+	return 0;
+}
+
 static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv,
 				      struct v4l2_format *f)
 {
@@ -149,7 +186,7 @@ static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv,
 		return -EINVAL;
 
 	/* limit format to hardware capabilities */
-	return ici->ops->try_fmt(icd, f);
+	return soc_camera_try_fmt(icd, f);
 }
 
 static int soc_camera_enum_input(struct file *file, void *priv,
@@ -377,7 +414,7 @@ static int soc_camera_set_fmt(struct soc_camera_device *icd,
 		pixfmtstr(pix->pixelformat), pix->width, pix->height);
 
 	/* We always call try_fmt() before set_fmt() or set_crop() */
-	ret = ici->ops->try_fmt(icd, f);
+	ret = soc_camera_try_fmt(icd, f);
 	if (ret < 0)
 		return ret;
 
-- 
1.7.0.4


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux