| Message ID | BANLkTikQSaUKtNZCexhKeNEPM+id+J_2gw@mail.gmail.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
On Mon, 11 Apr 2011, Aguirre, Sergio wrote: > Hi Guennadi, > > On Mon, Apr 11, 2011 at 8:23 AM, Guennadi Liakhovetski > <g.liakhovetski@gmx.de> wrote: > > > > On Mon, 11 Apr 2011, Aguirre, Sergio wrote: > > > > > Hi Guennadi, > > > > > > On Mon, Apr 11, 2011 at 3:58 AM, Guennadi Liakhovetski < > > > g.liakhovetski@gmx.de> 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@gmx.de> > > > > --- > > > > 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? Please, post patches inline. Yes, I think, ot would work too, only the call to soc_camera_xlate_by_fourcc() in the S_FMT case is superfluous, after ici->ops->set_fmt() we already have it in icd->current_fmt->host_fmt. Otherwise - yes, we could do it this way too. Janusz, could you test, please? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 11, 2011 at 11:58 AM, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > On Mon, 11 Apr 2011, Aguirre, Sergio wrote: > >> Hi Guennadi, >> >> On Mon, Apr 11, 2011 at 8:23 AM, Guennadi Liakhovetski >> <g.liakhovetski@gmx.de> wrote: >> > >> > On Mon, 11 Apr 2011, Aguirre, Sergio wrote: >> > >> > > Hi Guennadi, >> > > >> > > On Mon, Apr 11, 2011 at 3:58 AM, Guennadi Liakhovetski < >> > > g.liakhovetski@gmx.de> 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@gmx.de> >> > > > --- >> > > > 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? > > Please, post patches inline. Ok, sorry. > > Yes, I think, ot would work too, only the call to > soc_camera_xlate_by_fourcc() in the S_FMT case is superfluous, after > ici->ops->set_fmt() we already have it in icd->current_fmt->host_fmt. Well, the difference is that, after my patch, this is called after ici->ops->try_fmt, so icd->current_fmt->host_fmt doesn't match the intended format to try, as it's not stored as a current_fmt. Unless i'm getting confused with something... Regards, Sergio > Otherwise - yes, we could do it this way too. Janusz, could you test, > please? > > Thanks > Guennadi > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dnia poniedzia?ek 11 kwiecie? 2011 o 18:58:51 Guennadi Liakhovetski napisa?(a): > On Mon, 11 Apr 2011, Aguirre, Sergio wrote: > > > > Ok. And how about the attached patch? Would that work? > > Yes, I think, ot would work too, only the call to > soc_camera_xlate_by_fourcc() in the S_FMT case is superfluous, after > ici->ops->set_fmt() we already have it in icd->current_fmt->host_fmt. > Otherwise - yes, we could do it this way too. Janusz, could you test, > please? Looks like not based on the current mainline (-rc2) tree: CHECK drivers/media/video/soc_camera.c drivers/media/video/soc_camera.c:146:9: error: undefined identifier 'pixfmtstr' CC drivers/media/video/soc_camera.o drivers/media/video/soc_camera.c: In function 'soc_camera_try_fmt': drivers/media/video/soc_camera.c:146: error: implicit declaration of function 'pixfmtstr' drivers/media/video/soc_camera.c:146: warning: too few arguments for format drivers/media/video/soc_camera.c: In function 'soc_camera_try_fmt_vid_cap': drivers/media/video/soc_camera.c:180: warning: unused variable 'ici' Thanks, Janusz -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From 5442074c7760429c06f62917503c1d8a2f79cd21 Mon Sep 17 00:00:00 2001 From: Sergio Aguirre <saaguirre@ti.com> 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@ti.com> --- 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