Hi Guennadi,

On 03/11/2011 10:36 AM, Guennadi Liakhovetski wrote:
On Tue, 8 Mar 2011, Sergio Aguirre wrote:

Hi Guennadi,

On 03/08/2011 01:19 AM, Guennadi Liakhovetski wrote:
On Mon, 7 Mar 2011, Sergio Aguirre wrote:

This fixes the problem in which a host driver
sets a personalized sizeimage or bytesperline field,
and gets ignored when doing G_FMT.

Can you tell what that personalised value is? Is it not covered by
soc_mbus_bytes_per_line()? Maybe something like a JPEG format?

In my case, my omap4_camera driver requires to have a bytesperline which is a
multiple of 32, and sometimes (depending on the internal HW blocks used) a
page aligned byte offset between lines.

For example, I want to use such configuration that, for an NV12 buffer, I
require a 4K offset between lines, so the vaues are:

pix->bytesperline = PAGE_SIZE;
pix->sizeimage = pix->bytesperline * height * 3 / 2;

Which I filled in TRY_FMT/S_FMT ioctl calls.

Ok, I think, I agree with this. Until now we didn't have drivers, that
wanted to pad data. Even the pxa270 driver adjusts the frame format (see
pxa_camera.c::pxa_camera_try_fmt() and the call to v4l_bound_align_image()
there in the YUV422P case) to avoid padding, even though that's a
different kind of padding - between planes. So, if line padding - as in
your case - is indeed needed, I agree, that the correct way to support
that is to implement driver-specific bytesperline and sizeimage
calculations and, logically, those values should be used in
soc_camera_g_fmt_vid_cap().

I'll just change your patch a bit - I'll use "u32" types instead of
"__u32" - this is a kernel internal struct and we don't need user-space
exported types.

Ok, thanks.

You're right about u32 type... my bad.

So, I'll change that, rebase to:

git://linuxtv.org/media_tree.git        staging/for_v2.6.39

And resubmit for review. No problem.

Regards,
Sergio


Thanks
Guennadi

So, next time a driver tries a G_FMT, it currently gets recalculated by
a prefixed table (which comes from soc_mbus_bytes_per_line), which won't give
me what i had set before. And it will also recalculate a size image based on
this wrong bytesperline * height, which is also wrong, (lacks the * 3 / 2 for
NV12).

Regards,
Sergio


Thanks
Guennadi


Signed-off-by: Sergio Aguirre<saagui...@ti.com>
---
   drivers/media/video/soc_camera.c |    9 ++++-----
   include/media/soc_camera.h       |    2 ++
   2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/media/video/soc_camera.c
b/drivers/media/video/soc_camera.c
index a66811b..59dc71d 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -363,6 +363,8 @@ static int soc_camera_set_fmt(struct soc_camera_device
*icd,
        icd->user_width              = pix->width;
        icd->user_height     = pix->height;
        icd->colorspace              = pix->colorspace;
+       icd->bytesperline    = pix->bytesperline;
+       icd->sizeimage               = pix->sizeimage;
        icd->vb_vidq.field   =
                icd->field   = pix->field;

@@ -608,12 +610,9 @@ static int soc_camera_g_fmt_vid_cap(struct file
*file, void *priv,
        pix->height          = icd->user_height;
        pix->field           = icd->vb_vidq.field;
        pix->pixelformat     = icd->current_fmt->host_fmt->fourcc;
-       pix->bytesperline    = soc_mbus_bytes_per_line(pix->width,
-                                               icd->current_fmt->host_fmt);
+       pix->bytesperline    = icd->bytesperline;
        pix->colorspace              = icd->colorspace;
-       if (pix->bytesperline<   0)
-               return pix->bytesperline;
-       pix->sizeimage               = pix->height * pix->bytesperline;
+       pix->sizeimage               = icd->sizeimage;
        dev_dbg(&icd->dev, "current_fmt->fourcc: 0x%08x\n",
                icd->current_fmt->host_fmt->fourcc);
        return 0;
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index 9386db8..de81370 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -30,6 +30,8 @@ struct soc_camera_device {
        s32 user_width;
        s32 user_height;
        enum v4l2_colorspace colorspace;
+       __u32 bytesperline;     /* for padding, zero if unused */
+       __u32 sizeimage;
        unsigned char iface;            /* Host number */
        unsigned char devnum;           /* Device number per host */
        struct soc_camera_sense *sense; /* See comment in struct definition */
--
1.7.1

---
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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to