| Message ID | 201104090158.04827.jkrzyszt@tis.icnet.pl (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
Hi Janusz On Sat, 9 Apr 2011, Janusz Krzysztofik wrote: > Since commit 0e4c180d3e2cc11e248f29d4c604b6194739d05a, bytesperline and > sizeimage memebers of v4l2_pix_format structure have no longer been > calculated inside soc_camera_g_fmt_vid_cap(), but rather passed via > soc_camera_device structure from a host driver callback invoked by > soc_camera_set_fmt(). > > OMAP1 camera host driver has never been providing these parameters, so > it no longer works correctly. Fix it by adding suitable assignments to > omap1_cam_set_fmt(). Thanks for the patch, but now it looks like many soc-camera host drivers are re-implementing this very same calculation in different parts of their code - in try_fmt, set_fmt, get_fmt. Why don't we unify them all, implement this centrally in soc_camera.c and remove all those calculations? Could you cook up a patch or maybe several patches - for soc_camera.c and all drivers? Thanks Guennadi > > Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> > --- > drivers/media/video/omap1_camera.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > --- linux-2.6.39-rc2/drivers/media/video/omap1_camera.c.orig 2011-04-06 14:30:37.000000000 +0200 > +++ linux-2.6.39-rc2/drivers/media/video/omap1_camera.c 2011-04-09 00:16:36.000000000 +0200 > @@ -1292,6 +1292,12 @@ static int omap1_cam_set_fmt(struct soc_ > pix->colorspace = mf.colorspace; > icd->current_fmt = xlate; > > + pix->bytesperline = soc_mbus_bytes_per_line(pix->width, > + xlate->host_fmt); > + if (pix->bytesperline < 0) > + return pix->bytesperline; > + pix->sizeimage = pix->height * pix->bytesperline; > + > return 0; > } > > --- 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 niedziela 10 kwiecie? 2011 o 18:00:14 Guennadi Liakhovetski napisa?(a): > Hi Janusz > > On Sat, 9 Apr 2011, Janusz Krzysztofik wrote: > > Since commit 0e4c180d3e2cc11e248f29d4c604b6194739d05a, bytesperline > > and sizeimage memebers of v4l2_pix_format structure have no longer > > been calculated inside soc_camera_g_fmt_vid_cap(), but rather > > passed via soc_camera_device structure from a host driver callback > > invoked by soc_camera_set_fmt(). > > > > OMAP1 camera host driver has never been providing these parameters, > > so it no longer works correctly. Fix it by adding suitable > > assignments to omap1_cam_set_fmt(). > > Thanks for the patch, but now it looks like many soc-camera host > drivers are re-implementing this very same calculation in different > parts of their code - in try_fmt, set_fmt, get_fmt. Why don't we > unify them all, implement this centrally in soc_camera.c and remove > all those calculations? Wasn't it already unified before commit in question? > Could you cook up a patch or maybe several > patches - for soc_camera.c and all drivers? Perhaps I could, as soon as I found some spare time, but first I'd have to really understand why we need bytesperline or sizeimage handling being changed from how they worked before commit 0e4c180d3e2cc11e248f29d4c604b6194739d05a was introduced. I never had a need to customize bytesperline or sizeimage calculations in my driver. But even then, I think these new patches would rather qualify for next merge window, while the OMAP1 driver case is just a regression, caused by an alredy applied, unrelated change to the underlying framework, and requires a fix if that change is not going to be reverted. Maybe the author of the change, Sergio Aguirre form TI (CCing him), could rework his patch in a way which wouldn't impose, as a side effect, the new requirement of those structure members being passed from host drivers? 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
On Mon, 11 Apr 2011, Janusz Krzysztofik wrote: > Dnia niedziela 10 kwiecie? 2011 o 18:00:14 Guennadi Liakhovetski > napisa?(a): > > Hi Janusz > > > > On Sat, 9 Apr 2011, Janusz Krzysztofik wrote: > > > Since commit 0e4c180d3e2cc11e248f29d4c604b6194739d05a, bytesperline > > > and sizeimage memebers of v4l2_pix_format structure have no longer > > > been calculated inside soc_camera_g_fmt_vid_cap(), but rather > > > passed via soc_camera_device structure from a host driver callback > > > invoked by soc_camera_set_fmt(). > > > > > > OMAP1 camera host driver has never been providing these parameters, > > > so it no longer works correctly. Fix it by adding suitable > > > assignments to omap1_cam_set_fmt(). > > > > Thanks for the patch, but now it looks like many soc-camera host > > drivers are re-implementing this very same calculation in different > > parts of their code - in try_fmt, set_fmt, get_fmt. Why don't we > > unify them all, implement this centrally in soc_camera.c and remove > > all those calculations? > > Wasn't it already unified before commit in question? It was, but it was inconsistent. It was done centrally, and it was done in several other drivers additionally, creating a mess. > > Could you cook up a patch or maybe several > > patches - for soc_camera.c and all drivers? > > Perhaps I could, as soon as I found some spare time, but first I'd have > to really understand why we need bytesperline or sizeimage handling > being changed from how they worked before commit > 0e4c180d3e2cc11e248f29d4c604b6194739d05a was introduced. I never had a > need to customize bytesperline or sizeimage calculations in my driver. > > But even then, I think these new patches would rather qualify for next > merge window, while the OMAP1 driver case is just a regression, caused > by an alredy applied, unrelated change to the underlying framework, and > requires a fix if that change is not going to be reverted. Sure, we want this fixed for the current merge window. But I think it is possible with a relatively easy patch to soc_camera.c. I asked you, whether you'd be able to make a patch of that kind, if you don't have enough time ATM, I can try to make one and just ask you to test it on omap1. I'll have a look at it tomorrow. > Maybe the author of the change, Sergio Aguirre form TI (CCing him), > could rework his patch in a way which wouldn't impose, as a side effect, > the new requirement of those structure members being passed from host > drivers? I think we can just make an incremental patch to fill in those fields centrally, if the driver didn't do it for us. 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, 11 Apr 2011, Janusz Krzysztofik wrote: > Dnia niedziela 10 kwiecie? 2011 o 18:00:14 Guennadi Liakhovetski > napisa?(a): > > Hi Janusz > > > > On Sat, 9 Apr 2011, Janusz Krzysztofik wrote: > > > Since commit 0e4c180d3e2cc11e248f29d4c604b6194739d05a, bytesperline > > > and sizeimage memebers of v4l2_pix_format structure have no longer > > > been calculated inside soc_camera_g_fmt_vid_cap(), but rather > > > passed via soc_camera_device structure from a host driver callback > > > invoked by soc_camera_set_fmt(). > > > > > > OMAP1 camera host driver has never been providing these parameters, > > > so it no longer works correctly. Fix it by adding suitable > > > assignments to omap1_cam_set_fmt(). > > > > Thanks for the patch, but now it looks like many soc-camera host > > drivers are re-implementing this very same calculation in different > > parts of their code - in try_fmt, set_fmt, get_fmt. Why don't we > > unify them all, implement this centrally in soc_camera.c and remove > > all those calculations? > > Wasn't it already unified before commit in question? Please test the patch, I've sent a minute ago. Thanks Guennadi > > > Could you cook up a patch or maybe several > > patches - for soc_camera.c and all drivers? > > Perhaps I could, as soon as I found some spare time, but first I'd have > to really understand why we need bytesperline or sizeimage handling > being changed from how they worked before commit > 0e4c180d3e2cc11e248f29d4c604b6194739d05a was introduced. I never had a > need to customize bytesperline or sizeimage calculations in my driver. > > But even then, I think these new patches would rather qualify for next > merge window, while the OMAP1 driver case is just a regression, caused > by an alredy applied, unrelated change to the underlying framework, and > requires a fix if that change is not going to be reverted. > > Maybe the author of the change, Sergio Aguirre form TI (CCing him), > could rework his patch in a way which wouldn't impose, as a side effect, > the new requirement of those structure members being passed from host > drivers? > > Thanks, > Janusz > --- 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
hi, Guennadi:
a lot of sensors support JPEG output.
1) bytesperline is defined by sensor timing.
2) and sizeimage is unknow for jpeg.
how about for JPEG
1) host driver gets bytesperline from sensor driver.
2) sizeimage refilled by host driver after dma transfer done( a
frame is received)
thanks.
2011/4/11 Guennadi Liakhovetski <g.liakhovetski@gmx.de>:
> Hi Janusz
>
> On Sat, 9 Apr 2011, Janusz Krzysztofik wrote:
>
>> Since commit 0e4c180d3e2cc11e248f29d4c604b6194739d05a, bytesperline and
>> sizeimage memebers of v4l2_pix_format structure have no longer been
>> calculated inside soc_camera_g_fmt_vid_cap(), but rather passed via
>> soc_camera_device structure from a host driver callback invoked by
>> soc_camera_set_fmt().
>>
>> OMAP1 camera host driver has never been providing these parameters, so
>> it no longer works correctly. Fix it by adding suitable assignments to
>> omap1_cam_set_fmt().
>
> Thanks for the patch, but now it looks like many soc-camera host drivers
> are re-implementing this very same calculation in different parts of their
> code - in try_fmt, set_fmt, get_fmt. Why don't we unify them all,
> implement this centrally in soc_camera.c and remove all those
> calculations? Could you cook up a patch or maybe several patches - for
> soc_camera.c and all drivers?
>
> Thanks
> Guennadi
>
>>
>> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
>> ---
>> drivers/media/video/omap1_camera.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> --- linux-2.6.39-rc2/drivers/media/video/omap1_camera.c.orig 2011-04-06 14:30:37.000000000 +0200
>> +++ linux-2.6.39-rc2/drivers/media/video/omap1_camera.c 2011-04-09 00:16:36.000000000 +0200
>> @@ -1292,6 +1292,12 @@ static int omap1_cam_set_fmt(struct soc_
>> pix->colorspace = mf.colorspace;
>> icd->current_fmt = xlate;
>>
>> + pix->bytesperline = soc_mbus_bytes_per_line(pix->width,
>> + xlate->host_fmt);
>> + if (pix->bytesperline < 0)
>> + return pix->bytesperline;
>> + pix->sizeimage = pix->height * pix->bytesperline;
>> +
>> return 0;
>> }
>>
>>
>
> ---
> 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
>
--
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
Hi On Tue, 12 Apr 2011, Kassey Lee wrote: > hi, Guennadi: > a lot of sensors support JPEG output. > 1) bytesperline is defined by sensor timing. > 2) and sizeimage is unknow for jpeg. > > how about for JPEG > 1) host driver gets bytesperline from sensor driver. > 2) sizeimage refilled by host driver after dma transfer done( a > frame is received) > thanks. How is this done currently on other V4L2 drivers? To transfer a frame you usually first do at least one of S_FMT and G_FMT, at which time you already have to report sizeimage to the user - before any transfer has taken place. Currently with soc-camera it is already possible to override sizeimage and bytesperline from the host driver. Just set them to whatever you need in your try_fmt and they will be kept. Not sure how you want to do that, if you need to first read in a frame - do you want to perform some dummy frame transfer? You might not even have any buffers queued yet, so, it has to be a read without writing to RAM. Don't such compressed formats just put a value in sizeimage, that is a calculated maximum size? Thanks Guennadi > 2011/4/11 Guennadi Liakhovetski <g.liakhovetski@gmx.de>: > > Hi Janusz > > > > On Sat, 9 Apr 2011, Janusz Krzysztofik wrote: > > > >> Since commit 0e4c180d3e2cc11e248f29d4c604b6194739d05a, bytesperline and > >> sizeimage memebers of v4l2_pix_format structure have no longer been > >> calculated inside soc_camera_g_fmt_vid_cap(), but rather passed via > >> soc_camera_device structure from a host driver callback invoked by > >> soc_camera_set_fmt(). > >> > >> OMAP1 camera host driver has never been providing these parameters, so > >> it no longer works correctly. Fix it by adding suitable assignments to > >> omap1_cam_set_fmt(). > > > > Thanks for the patch, but now it looks like many soc-camera host drivers > > are re-implementing this very same calculation in different parts of their > > code - in try_fmt, set_fmt, get_fmt. Why don't we unify them all, > > implement this centrally in soc_camera.c and remove all those > > calculations? Could you cook up a patch or maybe several patches - for > > soc_camera.c and all drivers? > > > > Thanks > > Guennadi > > > >> > >> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> > >> --- > >> drivers/media/video/omap1_camera.c | 6 ++++++ > >> 1 file changed, 6 insertions(+) > >> > >> --- linux-2.6.39-rc2/drivers/media/video/omap1_camera.c.orig 2011-04-06 14:30:37.000000000 +0200 > >> +++ linux-2.6.39-rc2/drivers/media/video/omap1_camera.c 2011-04-09 00:16:36.000000000 +0200 > >> @@ -1292,6 +1292,12 @@ static int omap1_cam_set_fmt(struct soc_ > >> pix->colorspace = mf.colorspace; > >> icd->current_fmt = xlate; > >> > >> + pix->bytesperline = soc_mbus_bytes_per_line(pix->width, > >> + xlate->host_fmt); > >> + if (pix->bytesperline < 0) > >> + return pix->bytesperline; > >> + pix->sizeimage = pix->height * pix->bytesperline; > >> + > >> return 0; > >> } > >> > >> > > > > --- > > 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 > > > --- 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
Hi, On 04/12/2011 08:28 AM, Guennadi Liakhovetski wrote: > Hi > > On Tue, 12 Apr 2011, Kassey Lee wrote: > >> hi, Guennadi: >> a lot of sensors support JPEG output. >> 1) bytesperline is defined by sensor timing. Im not sure whether this is the case. Doesn't bytesperline refer only to the data layout in memory buffer written by the DMA? i.e. does padding really makes sens for JPEG files? >> 2) and sizeimage is unknow for jpeg. >> >> how about for JPEG >> 1) host driver gets bytesperline from sensor driver. >> 2) sizeimage refilled by host driver after dma transfer done( a >> frame is received) You might want to use v4l2_buffer::bytesused to inform user space about the actual size of the captured frame, which would be set before a buffer is dequeued from the driver. The size of JPEG file will depend on the content, so IMHO you could not use v4l2_pix_fmt::sizeimage in such way. >> thanks. > > How is this done currently on other V4L2 drivers? To transfer a frame you > usually first do at least one of S_FMT and G_FMT, at which time you > already have to report sizeimage to the user - before any transfer has > taken place. Currently with soc-camera it is already possible to override > sizeimage and bytesperline from the host driver. Just set them to whatever > you need in your try_fmt and they will be kept. Not sure how you want to > do that, if you need to first read in a frame - do you want to perform > some dummy frame transfer? You might not even have any buffers queued yet, > so, it has to be a read without writing to RAM. Don't such compressed > formats just put a value in sizeimage, that is a calculated maximum size? I the S5P FIMC driver I used to set sizeimage to some arbitrary value, (it's not yet in mainline kernel), e.g. sizeimage = width * height * C, where C = 1 bytesperline = width. However it would be useful to make the C coefficient dependent on JPEG compression quality, not to make the image buffer unnecessary large. I thought about creating a separate control class for JPEG but the quality control was so far everything I would need to put in this class. It's on my to do list to figure out what controls set would cover the standard. Regards,
2011/4/12 Kassey Lee <kassey1216@gmail.com>: > Hi, Guennadi; > for sizeimage , I agree with you. that we can overwrite it > after a frame is done. > > for byteperline: on Marvell soc. > it needs to know the bytesperline before receive frame from sensor. > what we did now is hardcode in host driver for bytesperline. > > since different sensors have different timing for JPEG, and > bytesperline is different. > while soc_mbus_bytes_per_line does not support JPEG. > > So, we want that host driver can get byteperline from sensor > driver (sub dev) before transfer a frame for JPEG format. > a way to do this: > soc_mbus_bytes_per_line return 0 for JPEG, and host driver will > try another API to get bytesperline for JPEG from sensor driver. > the effort is new API or reused other API. > > Is that reasonable ? > > > > 2011/4/12 Guennadi Liakhovetski <g.liakhovetski@gmx.de>: >> Hi >> >> On Tue, 12 Apr 2011, Kassey Lee wrote: >> >>> hi, Guennadi: >>> a lot of sensors support JPEG output. >>> 1) bytesperline is defined by sensor timing. >>> 2) and sizeimage is unknow for jpeg. >>> >>> how about for JPEG >>> 1) host driver gets bytesperline from sensor driver. >>> 2) sizeimage refilled by host driver after dma transfer done( a >>> frame is received) >>> thanks. >> >> How is this done currently on other V4L2 drivers? To transfer a frame you >> usually first do at least one of S_FMT and G_FMT, at which time you >> already have to report sizeimage to the user - before any transfer has >> taken place. Currently with soc-camera it is already possible to override >> sizeimage and bytesperline from the host driver. Just set them to whatever >> you need in your try_fmt and they will be kept. Not sure how you want to >> do that, if you need to first read in a frame - do you want to perform >> some dummy frame transfer? You might not even have any buffers queued yet, >> so, it has to be a read without writing to RAM. Don't such compressed >> formats just put a value in sizeimage, that is a calculated maximum size? >> >> Thanks >> Guennadi >> >>> 2011/4/11 Guennadi Liakhovetski <g.liakhovetski@gmx.de>: >>> > Hi Janusz >>> > >>> > On Sat, 9 Apr 2011, Janusz Krzysztofik wrote: >>> > >>> >> Since commit 0e4c180d3e2cc11e248f29d4c604b6194739d05a, bytesperline and >>> >> sizeimage memebers of v4l2_pix_format structure have no longer been >>> >> calculated inside soc_camera_g_fmt_vid_cap(), but rather passed via >>> >> soc_camera_device structure from a host driver callback invoked by >>> >> soc_camera_set_fmt(). >>> >> >>> >> OMAP1 camera host driver has never been providing these parameters, so >>> >> it no longer works correctly. Fix it by adding suitable assignments to >>> >> omap1_cam_set_fmt(). >>> > >>> > Thanks for the patch, but now it looks like many soc-camera host drivers >>> > are re-implementing this very same calculation in different parts of their >>> > code - in try_fmt, set_fmt, get_fmt. Why don't we unify them all, >>> > implement this centrally in soc_camera.c and remove all those >>> > calculations? Could you cook up a patch or maybe several patches - for >>> > soc_camera.c and all drivers? >>> > >>> > Thanks >>> > Guennadi >>> > >>> >> >>> >> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> >>> >> --- >>> >> drivers/media/video/omap1_camera.c | 6 ++++++ >>> >> 1 file changed, 6 insertions(+) >>> >> >>> >> --- linux-2.6.39-rc2/drivers/media/video/omap1_camera.c.orig 2011-04-06 14:30:37.000000000 +0200 >>> >> +++ linux-2.6.39-rc2/drivers/media/video/omap1_camera.c 2011-04-09 00:16:36.000000000 +0200 >>> >> @@ -1292,6 +1292,12 @@ static int omap1_cam_set_fmt(struct soc_ >>> >> pix->colorspace = mf.colorspace; >>> >> icd->current_fmt = xlate; >>> >> >>> >> + pix->bytesperline = soc_mbus_bytes_per_line(pix->width, >>> >> + xlate->host_fmt); >>> >> + if (pix->bytesperline < 0) >>> >> + return pix->bytesperline; >>> >> + pix->sizeimage = pix->height * pix->bytesperline; >>> >> + >>> >> return 0; >>> >> } >>> >> >>> >> >>> > >>> > --- >>> > 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 >>> > >>> >> >> --- >> 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 >> > -- 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 Tue, 12 Apr 2011, Kassey Lee wrote: > 2011/4/12 Kassey Lee <kassey1216@gmail.com>: > > Hi, Guennadi; > > for sizeimage , I agree with you. that we can overwrite it > > after a frame is done. > > > > for byteperline: on Marvell soc. > > it needs to know the bytesperline before receive frame from sensor. > > what we did now is hardcode in host driver for bytesperline. > > > > since different sensors have different timing for JPEG, and > > bytesperline is different. > > while soc_mbus_bytes_per_line does not support JPEG. > > > > So, we want that host driver can get byteperline from sensor > > driver (sub dev) before transfer a frame for JPEG format. > > a way to do this: > > soc_mbus_bytes_per_line return 0 for JPEG, and host driver will > > try another API to get bytesperline for JPEG from sensor driver. > > the effort is new API or reused other API. > > > > Is that reasonable ? If you mean this your patch http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/31323/match= then I've queued it for 2.6.40. Thanks Guennadi > > > > > > > > 2011/4/12 Guennadi Liakhovetski <g.liakhovetski@gmx.de>: > >> Hi > >> > >> On Tue, 12 Apr 2011, Kassey Lee wrote: > >> > >>> hi, Guennadi: > >>> a lot of sensors support JPEG output. > >>> 1) bytesperline is defined by sensor timing. > >>> 2) and sizeimage is unknow for jpeg. > >>> > >>> how about for JPEG > >>> 1) host driver gets bytesperline from sensor driver. > >>> 2) sizeimage refilled by host driver after dma transfer done( a > >>> frame is received) > >>> thanks. > >> > >> How is this done currently on other V4L2 drivers? To transfer a frame you > >> usually first do at least one of S_FMT and G_FMT, at which time you > >> already have to report sizeimage to the user - before any transfer has > >> taken place. Currently with soc-camera it is already possible to override > >> sizeimage and bytesperline from the host driver. Just set them to whatever > >> you need in your try_fmt and they will be kept. Not sure how you want to > >> do that, if you need to first read in a frame - do you want to perform > >> some dummy frame transfer? You might not even have any buffers queued yet, > >> so, it has to be a read without writing to RAM. Don't such compressed > >> formats just put a value in sizeimage, that is a calculated maximum size? > >> > >> Thanks > >> Guennadi > >> > >>> 2011/4/11 Guennadi Liakhovetski <g.liakhovetski@gmx.de>: > >>> > Hi Janusz > >>> > > >>> > On Sat, 9 Apr 2011, Janusz Krzysztofik wrote: > >>> > > >>> >> Since commit 0e4c180d3e2cc11e248f29d4c604b6194739d05a, bytesperline and > >>> >> sizeimage memebers of v4l2_pix_format structure have no longer been > >>> >> calculated inside soc_camera_g_fmt_vid_cap(), but rather passed via > >>> >> soc_camera_device structure from a host driver callback invoked by > >>> >> soc_camera_set_fmt(). > >>> >> > >>> >> OMAP1 camera host driver has never been providing these parameters, so > >>> >> it no longer works correctly. Fix it by adding suitable assignments to > >>> >> omap1_cam_set_fmt(). > >>> > > >>> > Thanks for the patch, but now it looks like many soc-camera host drivers > >>> > are re-implementing this very same calculation in different parts of their > >>> > code - in try_fmt, set_fmt, get_fmt. Why don't we unify them all, > >>> > implement this centrally in soc_camera.c and remove all those > >>> > calculations? Could you cook up a patch or maybe several patches - for > >>> > soc_camera.c and all drivers? > >>> > > >>> > Thanks > >>> > Guennadi > >>> > > >>> >> > >>> >> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> > >>> >> --- > >>> >> drivers/media/video/omap1_camera.c | 6 ++++++ > >>> >> 1 file changed, 6 insertions(+) > >>> >> > >>> >> --- linux-2.6.39-rc2/drivers/media/video/omap1_camera.c.orig 2011-04-06 14:30:37.000000000 +0200 > >>> >> +++ linux-2.6.39-rc2/drivers/media/video/omap1_camera.c 2011-04-09 00:16:36.000000000 +0200 > >>> >> @@ -1292,6 +1292,12 @@ static int omap1_cam_set_fmt(struct soc_ > >>> >> pix->colorspace = mf.colorspace; > >>> >> icd->current_fmt = xlate; > >>> >> > >>> >> + pix->bytesperline = soc_mbus_bytes_per_line(pix->width, > >>> >> + xlate->host_fmt); > >>> >> + if (pix->bytesperline < 0) > >>> >> + return pix->bytesperline; > >>> >> + pix->sizeimage = pix->height * pix->bytesperline; > >>> >> + > >>> >> return 0; > >>> >> } > >>> >> > >>> >> > >>> > > >>> > --- > >>> > 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 > >>> > > >>> > >> > >> --- > >> 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 > >> > > > --- 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
Yes, thank you very much! 2011/4/12 Guennadi Liakhovetski <g.liakhovetski@gmx.de>: > On Tue, 12 Apr 2011, Kassey Lee wrote: > >> 2011/4/12 Kassey Lee <kassey1216@gmail.com>: >> > Hi, Guennadi; >> > for sizeimage , I agree with you. that we can overwrite it >> > after a frame is done. >> > >> > for byteperline: on Marvell soc. >> > it needs to know the bytesperline before receive frame from sensor. >> > what we did now is hardcode in host driver for bytesperline. >> > >> > since different sensors have different timing for JPEG, and >> > bytesperline is different. >> > while soc_mbus_bytes_per_line does not support JPEG. >> > >> > So, we want that host driver can get byteperline from sensor >> > driver (sub dev) before transfer a frame for JPEG format. >> > a way to do this: >> > soc_mbus_bytes_per_line return 0 for JPEG, and host driver will >> > try another API to get bytesperline for JPEG from sensor driver. >> > the effort is new API or reused other API. >> > >> > Is that reasonable ? > > If you mean this your patch > > http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/31323/match= > > then I've queued it for 2.6.40. > > Thanks > Guennadi > >> > >> > >> > >> > 2011/4/12 Guennadi Liakhovetski <g.liakhovetski@gmx.de>: >> >> Hi >> >> >> >> On Tue, 12 Apr 2011, Kassey Lee wrote: >> >> >> >>> hi, Guennadi: >> >>> a lot of sensors support JPEG output. >> >>> 1) bytesperline is defined by sensor timing. >> >>> 2) and sizeimage is unknow for jpeg. >> >>> >> >>> how about for JPEG >> >>> 1) host driver gets bytesperline from sensor driver. >> >>> 2) sizeimage refilled by host driver after dma transfer done( a >> >>> frame is received) >> >>> thanks. >> >> >> >> How is this done currently on other V4L2 drivers? To transfer a frame you >> >> usually first do at least one of S_FMT and G_FMT, at which time you >> >> already have to report sizeimage to the user - before any transfer has >> >> taken place. Currently with soc-camera it is already possible to override >> >> sizeimage and bytesperline from the host driver. Just set them to whatever >> >> you need in your try_fmt and they will be kept. Not sure how you want to >> >> do that, if you need to first read in a frame - do you want to perform >> >> some dummy frame transfer? You might not even have any buffers queued yet, >> >> so, it has to be a read without writing to RAM. Don't such compressed >> >> formats just put a value in sizeimage, that is a calculated maximum size? >> >> >> >> Thanks >> >> Guennadi >> >> >> >>> 2011/4/11 Guennadi Liakhovetski <g.liakhovetski@gmx.de>: >> >>> > Hi Janusz >> >>> > >> >>> > On Sat, 9 Apr 2011, Janusz Krzysztofik wrote: >> >>> > >> >>> >> Since commit 0e4c180d3e2cc11e248f29d4c604b6194739d05a, bytesperline and >> >>> >> sizeimage memebers of v4l2_pix_format structure have no longer been >> >>> >> calculated inside soc_camera_g_fmt_vid_cap(), but rather passed via >> >>> >> soc_camera_device structure from a host driver callback invoked by >> >>> >> soc_camera_set_fmt(). >> >>> >> >> >>> >> OMAP1 camera host driver has never been providing these parameters, so >> >>> >> it no longer works correctly. Fix it by adding suitable assignments to >> >>> >> omap1_cam_set_fmt(). >> >>> > >> >>> > Thanks for the patch, but now it looks like many soc-camera host drivers >> >>> > are re-implementing this very same calculation in different parts of their >> >>> > code - in try_fmt, set_fmt, get_fmt. Why don't we unify them all, >> >>> > implement this centrally in soc_camera.c and remove all those >> >>> > calculations? Could you cook up a patch or maybe several patches - for >> >>> > soc_camera.c and all drivers? >> >>> > >> >>> > Thanks >> >>> > Guennadi >> >>> > >> >>> >> >> >>> >> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> >> >>> >> --- >> >>> >> drivers/media/video/omap1_camera.c | 6 ++++++ >> >>> >> 1 file changed, 6 insertions(+) >> >>> >> >> >>> >> --- linux-2.6.39-rc2/drivers/media/video/omap1_camera.c.orig 2011-04-06 14:30:37.000000000 +0200 >> >>> >> +++ linux-2.6.39-rc2/drivers/media/video/omap1_camera.c 2011-04-09 00:16:36.000000000 +0200 >> >>> >> @@ -1292,6 +1292,12 @@ static int omap1_cam_set_fmt(struct soc_ >> >>> >> pix->colorspace = mf.colorspace; >> >>> >> icd->current_fmt = xlate; >> >>> >> >> >>> >> + pix->bytesperline = soc_mbus_bytes_per_line(pix->width, >> >>> >> + xlate->host_fmt); >> >>> >> + if (pix->bytesperline < 0) >> >>> >> + return pix->bytesperline; >> >>> >> + pix->sizeimage = pix->height * pix->bytesperline; >> >>> >> + >> >>> >> return 0; >> >>> >> } >> >>> >> >> >>> >> >> >>> > >> >>> > --- >> >>> > 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 >> >>> > >> >>> >> >> >> >> --- >> >> 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 >> >> >> > >> > > --- > 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
--- linux-2.6.39-rc2/drivers/media/video/omap1_camera.c.orig 2011-04-06 14:30:37.000000000 +0200 +++ linux-2.6.39-rc2/drivers/media/video/omap1_camera.c 2011-04-09 00:16:36.000000000 +0200 @@ -1292,6 +1292,12 @@ static int omap1_cam_set_fmt(struct soc_ pix->colorspace = mf.colorspace; icd->current_fmt = xlate; + pix->bytesperline = soc_mbus_bytes_per_line(pix->width, + xlate->host_fmt); + if (pix->bytesperline < 0) + return pix->bytesperline; + pix->sizeimage = pix->height * pix->bytesperline; + return 0; }
Since commit 0e4c180d3e2cc11e248f29d4c604b6194739d05a, bytesperline and sizeimage memebers of v4l2_pix_format structure have no longer been calculated inside soc_camera_g_fmt_vid_cap(), but rather passed via soc_camera_device structure from a host driver callback invoked by soc_camera_set_fmt(). OMAP1 camera host driver has never been providing these parameters, so it no longer works correctly. Fix it by adding suitable assignments to omap1_cam_set_fmt(). Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> --- drivers/media/video/omap1_camera.c | 6 ++++++ 1 file changed, 6 insertions(+) -- 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