Hi, Guennadi: Do you mean this ? Signed-off-by: Qing Xu <qingx@xxxxxxxxxxx> Signed-off-by: Kassey Lee <ygli@xxxxxxxxxxx> --- drivers/media/video/soc_mediabus.c | 9 +++++++++ include/media/soc_mediabus.h | 1 + include/media/v4l2-mediabus.h | 1 + 3 files changed, 11 insertions(+), 0 deletions(-) diff --git a/drivers/media/video/soc_mediabus.c b/drivers/media/video/soc_mediabus.c index 9139121..a33f9ec 100644 --- a/drivers/media/video/soc_mediabus.c +++ b/drivers/media/video/soc_mediabus.c @@ -130,6 +130,13 @@ static const struct soc_mbus_pixelfmt mbus_fmt[] = { .packing = SOC_MBUS_PACKING_2X8_PADLO, .order = SOC_MBUS_ORDER_BE, }, + [MBUS_IDX(JPEG_1X8)] = { + .fourcc = V4L2_PIX_FMT_JPEG, + .name = "JPEG", + .bits_per_sample = 8, + .packing = SOC_MBUS_PACKING_VARIABLE, + .order = SOC_MBUS_ORDER_LE, + }, }; s32 soc_mbus_bytes_per_line(u32 width, const struct soc_mbus_pixelfmt *mf) @@ -141,6 +148,8 @@ s32 soc_mbus_bytes_per_line(u32 width, const struct soc_mbus_pixelfmt *mf) case SOC_MBUS_PACKING_2X8_PADLO: case SOC_MBUS_PACKING_EXTEND16: return width * 2; + case SOC_MBUS_PACKING_VARIABLE: + return 0; } return -EINVAL; } diff --git a/include/media/soc_mediabus.h b/include/media/soc_mediabus.h index 037cd7b..d351408 100644 --- a/include/media/soc_mediabus.h +++ b/include/media/soc_mediabus.h @@ -29,6 +29,7 @@ enum soc_mbus_packing { SOC_MBUS_PACKING_2X8_PADHI, SOC_MBUS_PACKING_2X8_PADLO, SOC_MBUS_PACKING_EXTEND16, + SOC_MBUS_PACKING_VARIABLE, }; /** diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h index 8e65598..f0d9146 100644 --- a/include/media/v4l2-mediabus.h +++ b/include/media/v4l2-mediabus.h @@ -54,6 +54,7 @@ enum v4l2_mbus_pixelcode { V4L2_MBUS_FMT_YVYU8_1_5X8, V4L2_MBUS_FMT_UYVY8_1_5X8, V4L2_MBUS_FMT_VYUY8_1_5X8, + V4L2_MBUS_FMT_JPEG_1X8, }; Best regards Kassey Lee Email: ygli@xxxxxxxxxxx Application Processor Systems Engineering, Marvell Technology Group Ltd. Shanghai, China. -----Original Message----- From: linux-media-owner@xxxxxxxxxxxxxxx [mailto:linux-media-owner@xxxxxxxxxxxxxxx] On Behalf Of Guennadi Liakhovetski Sent: 2011å1æ20æ 0:00 To: Qing Xu Cc: Laurent Pinchart; Linux Media Mailing List Subject: RE: soc-camera jpeg support? On Tue, 18 Jan 2011, Qing Xu wrote: > > Sorry, which solution you would like me to implement? > (1) is to add SOC_MBUS_PACKING_VARIABLE define and add error code in > soc_mbus_bytes_per_line(), > and (2) is to implement int (*try_fmt)(struct v4l2_subdev *sd, struct > v4l2_format *fmt); in subdev, directly get V4L2_PIX_FMT_JPEG info from > host driver. Number (1), please. Sensor drivers should not use v4l2_format, only mediabus formats. Your host driver should proceed as usual: if the user is requesting the JPEG fourcc format from it, it should look in the format translation table, find there the respective JPEG mediabus code, and configure the sensor with it. Only when calculating buffer sizes the host driver will have to treat JPEG specially. This way you have to add 3 things to the generic code: definitions for JPEG mediabus code and a variable-line length packing, and an entry to the fourcc-mediabus translation table. Thanks Guennadi > > -Qing > > -----Original Message----- > From: Guennadi Liakhovetski [mailto:g.liakhovetski@xxxxxx] > Sent: 2011å1æ19æ 1:31 > To: Qing Xu > Cc: Laurent Pinchart; Linux Media Mailing List > Subject: RE: soc-camera jpeg support? > > Thanks for the code! With it at hand it is going to be easier to > understand and evaluate changes, that you propose to the generic modules. > > On Mon, 17 Jan 2011, Qing Xu wrote: > > > Hi, Guennadi, > > > > Oh, yes, I agree with you, you are right, it is really not that simple, > > JPEG is always a headache,:(, as it is quite different from original > > yuv/rgb format, it has neither fixed bits-per-sample, nor fixed > > packing/bytes-per-line/per-frame, so when coding, I just hack value of > > .bits_per_sample and .packing, in fact, you will see in our host driver, > > if we find it is JPEG, will ignore bytes-per-line value, for example, in > > pxa955_videobuf_prepare(), for jpeg, we always allocate fixed buffer > > size for it (or, a better method is to allocate buffer size = > > width*height/jpeg-compress-ratio). > > > > I have 2 ideas: > > 1) Do you think it is reasonable to add a not-fixed define into soc_mbus_packing: > > enum soc_mbus_packing { > > SOC_MBUS_PACKING_NOT_FIXED, > > ... > > }; > > And then, .bits_per_sample = 0, /* indicate that sample bits is not-fixed*/ > > And, in function: > > s32 soc_mbus_bytes_per_line(u32 width, const struct soc_mbus_pixelfmt *mf) > > { > > switch (mf->packing) { > > case SOC_MBUS_PACKING_NOT_FIXED: > > return 0; > > case SOC_MBUS_PACKING_NONE: > > return width * mf->bits_per_sample / 8; > > ... > > } > > return -EINVAL; > > } > > > > 2) Or, an workaround in sensor(ov5642.c), is to implement: > > int (*try_fmt)(struct v4l2_subdev *sd, struct v4l2_format *fmt); > > use the member (fmt->pix->pixelformat == V4L2_PIX_FMT_JPEG) to find out > > if it is jpeg. And in host driver, it calls try_fmt() into sensor. In > > this way, no need to change soc-camera common code, but I feel that it > > goes against with your xxx_mbus_xxx design purpose, right? > > I actually prefer this one, but with an addition of V4L2_MBUS_FMT_JPEG_1X8 > as per your additional Ãatch, but, please, also add a new packing, e.g., > SOC_MBUS_PACKING_COMPRESSED (or SOC_MBUS_PACKING_VARIABLE?). So, that we > can cleanly translate between V4L2_MBUS_FMT_JPEG_1X8 and > V4L2_PIX_FMT_JPEG, but host drivers, that want to support this, will have > to know to calculate frame sizes in some special way, not just aborting, > if soc_mbus_bytes_per_line() returned an error. We might also consider > returning a different error code for this case, e.g., we could change the > general error case to return -ENOENT, and use -EINVAL for cases like JPEG, > where data is valid, but no valid calculation is possible. > > Thanks > Guennadi > > > What do you think? > > > > Please check the attachment, it is our host camera controller driver and sensor. > > > > Thanks a lot! > > -Qing > > > > -----Original Message----- > > From: Guennadi Liakhovetski [mailto:g.liakhovetski@xxxxxx] > > Sent: 2011ÃÃ1ÃÃ18Ãà 1:39 > > To: Qing Xu > > Cc: Laurent Pinchart; Linux Media Mailing List > > Subject: Re: soc-camera jpeg support? > > > > On Mon, 17 Jan 2011, Qing Xu wrote: > > > > > Hi, > > > > > > Many of our sensors support directly outputting JPEG data to camera > > > controller, do you feel it's reasonable to add jpeg support into > > > soc-camera? As it seems that there is no define in v4l2-mediabus.h which > > > is suitable for our case. > > > > In principle I have nothing against this, but, I'm afraid, it is not quite > > that simple. I haven't worked with such sensors myself, but, AFAIU, the > > JPEG image format doesn't have fixed bytes-per-line and bytes-per-frame > > values. If you add it like you propose, this would mean, that it just > > delivers "normal" 8 bits per pixel images. OTOH, soc_mbus_bytes_per_line() > > would just return -EINVAL for your JPEG format, so, unsupporting drivers > > would just error out, which is not all that bad. When the first driver > > decides to support JPEG, they will have to handle that error. But maybe > > we'll want to return a special error code for it. > > > > But, in fact, that is in a way my problem with your patches: you propose > > extensions to generic code without showing how this is going to be used in > > specific drivers. Could you show us your host driver, please? I don't > > think this is still the pxa27x driver, is it? > > > > Thanks > > Guennadi > > > > > > > > Such as: > > > --- a/drivers/media/video/soc_mediabus.c > > > +++ b/drivers/media/video/soc_mediabus.c > > > @@ -130,6 +130,13 @@ static const struct soc_mbus_pixelfmt mbus_fmt[] = { > > > .packing = SOC_MBUS_PACKING_2X8_PADLO, > > > .order = SOC_MBUS_ORDER_BE, > > > }, > > > + [MBUS_IDX(JPEG_1X8)] = { > > > + .fourcc = V4L2_PIX_FMT_JPEG, > > > + .name = "JPEG", > > > + .bits_per_sample = 8, > > > + .packing = SOC_MBUS_PACKING_NONE, > > > + .order = SOC_MBUS_ORDER_LE, > > > + }, > > > }; > > > > > > --- a/include/media/v4l2-mediabus.h > > > +++ b/include/media/v4l2-mediabus.h > > > @@ -41,6 +41,7 @@ enum v4l2_mbus_pixelcode { > > > V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE, > > > V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE, > > > V4L2_MBUS_FMT_SGRBG8_1X8, > > > + V4L2_MBUS_FMT_JPEG_1X8, > > > }; > > > > > > Any ideas will be appreciated! > > > Thanks! > > > Qing Xu > > > > > > Email: qingx@xxxxxxxxxxx > > > Application Processor Systems Engineering, > > > Marvell Technology Group Ltd. > > > > > > > --- > > Guennadi Liakhovetski, Ph.D. > > Freelance Open-Source Software Developer > > http://www.open-technology.de/ > > > > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ > --- 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html ÿô.nÇ·®+%˱é¥wÿº{.nÇ·¥{±þg¯â^nr¡öë¨è&£ûz¹Þúzf£¢·h§~Ûÿÿïÿê_èæ+v¨þ)ßø