* [PATCH 3/3] soc-camera: let camera host drivers decide upon pixel format
@ 2008-11-08 18:48 Guennadi Liakhovetski
2008-11-09 0:47 ` Robert Jarzmik
0 siblings, 1 reply; 6+ messages in thread
From: Guennadi Liakhovetski @ 2008-11-08 18:48 UTC (permalink / raw)
To: video4linux-list
Pixel format requested by the user is not necessarily the same, as what
a sensor driver provides. There are situations, when a camera host driver
provides the required format, but requires a different format from the
sensor. Further, the list of formats, supported by sensors is pretty static
and can be pretty good described with a constant list of structures. Whereas
decisions, made by camera host drivers to support requested formats can be
quite complex, therefore it is better to let the host driver do the work.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
Robert, this patch shall actually let us cleanly implement format
conversions in camera host-drivers. The previous two patches I've cc'ed to
you for completeness, so you can test this one. Now the camera
host-driver, e.g., pxa_camera.c gets the s_fmt_vid_cap request practically
directly and can then decide which format to request from the sensor
driver. Have a look if you can use this. I'll try to think about it a bit
more too.
drivers/media/video/pxa_camera.c | 32 +++++++++++++++-
drivers/media/video/sh_mobile_ceu_camera.c | 32 +++++++++++++++-
drivers/media/video/soc_camera.c | 58 ++++++++++-----------------
include/media/soc_camera.h | 3 +
4 files changed, 87 insertions(+), 38 deletions(-)
diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
index 2a811f8..a375872 100644
--- a/drivers/media/video/pxa_camera.c
+++ b/drivers/media/video/pxa_camera.c
@@ -907,17 +907,43 @@ static int pxa_camera_try_bus_param(struct soc_camera_device *icd, __u32 pixfmt)
static int pxa_camera_set_fmt_cap(struct soc_camera_device *icd,
__u32 pixfmt, struct v4l2_rect *rect)
{
- return icd->ops->set_fmt_cap(icd, pixfmt, rect);
+ const struct soc_camera_data_format *cam_fmt;
+ int ret;
+
+ /*
+ * TODO: find a suitable supported by the SoC output format, check
+ * whether the sensor supports one of acceptable input formats.
+ */
+ if (pixfmt) {
+ cam_fmt = soc_camera_format_by_fourcc(icd, pixfmt);
+ if (!cam_fmt)
+ return -EINVAL;
+ }
+
+ ret = icd->ops->set_fmt_cap(icd, pixfmt, rect);
+ if (pixfmt && !ret)
+ icd->current_fmt = cam_fmt;
+
+ return ret;
}
static int pxa_camera_try_fmt_cap(struct soc_camera_device *icd,
struct v4l2_format *f)
{
+ const struct soc_camera_data_format *cam_fmt;
int ret = pxa_camera_try_bus_param(icd, f->fmt.pix.pixelformat);
if (ret < 0)
return ret;
+ /*
+ * TODO: find a suitable supported by the SoC output format, check
+ * whether the sensor supports one of acceptable input formats.
+ */
+ cam_fmt = soc_camera_format_by_fourcc(icd, f->fmt.pix.pixelformat);
+ if (!cam_fmt)
+ return -EINVAL;
+
/* limit to pxa hardware capabilities */
if (f->fmt.pix.height < 32)
f->fmt.pix.height = 32;
@@ -929,6 +955,10 @@ static int pxa_camera_try_fmt_cap(struct soc_camera_device *icd,
f->fmt.pix.width = 2048;
f->fmt.pix.width &= ~0x01;
+ f->fmt.pix.bytesperline = f->fmt.pix.width *
+ DIV_ROUND_UP(cam_fmt->depth, 8);
+ f->fmt.pix.sizeimage = f->fmt.pix.height * f->fmt.pix.bytesperline;
+
/* limit to sensor capabilities */
return icd->ops->try_fmt_cap(icd, f);
}
diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
index a6b29a4..1bacfc7 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -447,17 +447,43 @@ static int sh_mobile_ceu_try_bus_param(struct soc_camera_device *icd,
static int sh_mobile_ceu_set_fmt_cap(struct soc_camera_device *icd,
__u32 pixfmt, struct v4l2_rect *rect)
{
- return icd->ops->set_fmt_cap(icd, pixfmt, rect);
+ const struct soc_camera_data_format *cam_fmt;
+ int ret;
+
+ /*
+ * TODO: find a suitable supported by the SoC output format, check
+ * whether the sensor supports one of acceptable input formats.
+ */
+ if (pixfmt) {
+ cam_fmt = soc_camera_format_by_fourcc(icd, pixfmt);
+ if (!cam_fmt)
+ return -EINVAL;
+ }
+
+ ret = icd->ops->set_fmt_cap(icd, pixfmt, rect);
+ if (pixfmt && !ret)
+ icd->current_fmt = cam_fmt;
+
+ return ret;
}
static int sh_mobile_ceu_try_fmt_cap(struct soc_camera_device *icd,
struct v4l2_format *f)
{
+ const struct soc_camera_data_format *cam_fmt;
int ret = sh_mobile_ceu_try_bus_param(icd, f->fmt.pix.pixelformat);
if (ret < 0)
return ret;
+ /*
+ * TODO: find a suitable supported by the SoC output format, check
+ * whether the sensor supports one of acceptable input formats.
+ */
+ cam_fmt = soc_camera_format_by_fourcc(icd, f->fmt.pix.pixelformat);
+ if (!cam_fmt)
+ return -EINVAL;
+
/* FIXME: calculate using depth and bus width */
if (f->fmt.pix.height < 4)
@@ -471,6 +497,10 @@ static int sh_mobile_ceu_try_fmt_cap(struct soc_camera_device *icd,
f->fmt.pix.width &= ~0x01;
f->fmt.pix.height &= ~0x03;
+ f->fmt.pix.bytesperline = f->fmt.pix.width *
+ DIV_ROUND_UP(cam_fmt->depth, 8);
+ f->fmt.pix.sizeimage = f->fmt.pix.height * f->fmt.pix.bytesperline;
+
/* limit to sensor capabilities */
return icd->ops->try_fmt_cap(icd, f);
}
diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index 2d1f474..afadb33 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -35,7 +35,7 @@ static LIST_HEAD(devices);
static DEFINE_MUTEX(list_lock);
static DEFINE_MUTEX(video_lock);
-const static struct soc_camera_data_format *format_by_fourcc(
+const struct soc_camera_data_format *soc_camera_format_by_fourcc(
struct soc_camera_device *icd, unsigned int fourcc)
{
unsigned int i;
@@ -45,6 +45,7 @@ const static struct soc_camera_data_format *format_by_fourcc(
return icd->formats + i;
return NULL;
}
+EXPORT_SYMBOL(soc_camera_format_by_fourcc);
static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv,
struct v4l2_format *f)
@@ -54,25 +55,19 @@ static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv,
struct soc_camera_host *ici =
to_soc_camera_host(icd->dev.parent);
enum v4l2_field field;
- const struct soc_camera_data_format *fmt;
int ret;
WARN_ON(priv != file->private_data);
- fmt = format_by_fourcc(icd, f->fmt.pix.pixelformat);
- if (!fmt) {
- dev_dbg(&icd->dev, "invalid format 0x%08x\n",
- f->fmt.pix.pixelformat);
- return -EINVAL;
- }
-
- dev_dbg(&icd->dev, "fmt: 0x%08x\n", fmt->fourcc);
-
+ /*
+ * TODO: this might also have to migrate to host-drivers, if anyone
+ * wishes to support other fields
+ */
field = f->fmt.pix.field;
if (field == V4L2_FIELD_ANY) {
- field = V4L2_FIELD_NONE;
- } else if (V4L2_FIELD_NONE != field) {
+ f->fmt.pix.field = V4L2_FIELD_NONE;
+ } else if (field != V4L2_FIELD_NONE) {
dev_err(&icd->dev, "Field type invalid.\n");
return -EINVAL;
}
@@ -80,13 +75,6 @@ static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv,
/* limit format to hardware capabilities */
ret = ici->ops->try_fmt_cap(icd, f);
- /* calculate missing fields */
- f->fmt.pix.field = field;
- f->fmt.pix.bytesperline =
- (f->fmt.pix.width * fmt->depth) >> 3;
- f->fmt.pix.sizeimage =
- f->fmt.pix.height * f->fmt.pix.bytesperline;
-
return ret;
}
@@ -325,18 +313,10 @@ static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
to_soc_camera_host(icd->dev.parent);
int ret;
struct v4l2_rect rect;
- const static struct soc_camera_data_format *data_fmt;
WARN_ON(priv != file->private_data);
- data_fmt = format_by_fourcc(icd, f->fmt.pix.pixelformat);
- if (!data_fmt)
- return -EINVAL;
-
- /* buswidth may be further adjusted by the ici */
- icd->buswidth = data_fmt->depth;
-
- ret = soc_camera_try_fmt_vid_cap(file, icf, f);
+ ret = soc_camera_try_fmt_vid_cap(file, priv, f);
if (ret < 0)
return ret;
@@ -345,14 +325,21 @@ static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
rect.width = f->fmt.pix.width;
rect.height = f->fmt.pix.height;
ret = ici->ops->set_fmt_cap(icd, f->fmt.pix.pixelformat, &rect);
- if (ret < 0)
+ if (ret < 0) {
return ret;
+ } else if (!icd->current_fmt ||
+ icd->current_fmt->fourcc != f->fmt.pix.pixelformat) {
+ dev_err(&ici->dev, "Host driver hasn't set up current "
+ "format correctly!\n");
+ return -EINVAL;
+ }
- icd->current_fmt = data_fmt;
+ /* buswidth may be further adjusted by the ici */
+ icd->buswidth = icd->current_fmt->depth;
icd->width = rect.width;
icd->height = rect.height;
icf->vb_vidq.field = f->fmt.pix.field;
- if (V4L2_BUF_TYPE_VIDEO_CAPTURE != f->type)
+ if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
dev_warn(&icd->dev, "Attention! Wrong buf-type %d\n",
f->type);
@@ -394,10 +381,9 @@ static int soc_camera_g_fmt_vid_cap(struct file *file, void *priv,
f->fmt.pix.height = icd->height;
f->fmt.pix.field = icf->vb_vidq.field;
f->fmt.pix.pixelformat = icd->current_fmt->fourcc;
- f->fmt.pix.bytesperline =
- (f->fmt.pix.width * icd->current_fmt->depth) >> 3;
- f->fmt.pix.sizeimage =
- f->fmt.pix.height * f->fmt.pix.bytesperline;
+ f->fmt.pix.bytesperline = f->fmt.pix.width *
+ DIV_ROUND_UP(icd->current_fmt->depth, 8);
+ f->fmt.pix.sizeimage = f->fmt.pix.height * f->fmt.pix.bytesperline;
dev_dbg(&icd->dev, "current_fmt->fourcc: 0x%08x\n",
icd->current_fmt->fourcc);
return 0;
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index 5eb9540..c9b2b7f 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -105,6 +105,9 @@ extern void soc_camera_device_unregister(struct soc_camera_device *icd);
extern int soc_camera_video_start(struct soc_camera_device *icd);
extern void soc_camera_video_stop(struct soc_camera_device *icd);
+extern const struct soc_camera_data_format *soc_camera_format_by_fourcc(
+ struct soc_camera_device *icd, unsigned int fourcc);
+
struct soc_camera_data_format {
char *name;
unsigned int depth;
--
1.5.4
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] soc-camera: let camera host drivers decide upon pixel format
2008-11-08 18:48 [PATCH 3/3] soc-camera: let camera host drivers decide upon pixel format Guennadi Liakhovetski
@ 2008-11-09 0:47 ` Robert Jarzmik
2008-11-09 10:32 ` Guennadi Liakhovetski
2008-11-09 11:13 ` Robert Jarzmik
0 siblings, 2 replies; 6+ messages in thread
From: Robert Jarzmik @ 2008-11-09 0:47 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: video4linux-list
Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:
> diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
> index 2a811f8..a375872 100644
> --- a/drivers/media/video/pxa_camera.c
> +++ b/drivers/media/video/pxa_camera.c
> @@ -907,17 +907,43 @@ static int pxa_camera_try_bus_param(struct soc_camera_device *icd, __u32 pixfmt)
> static int pxa_camera_set_fmt_cap(struct soc_camera_device *icd,
> __u32 pixfmt, struct v4l2_rect *rect)
> {
> - return icd->ops->set_fmt_cap(icd, pixfmt, rect);
> + const struct soc_camera_data_format *cam_fmt;
> + int ret;
> +
> + /*
> + * TODO: find a suitable supported by the SoC output format, check
> + * whether the sensor supports one of acceptable input formats.
> + */
> + if (pixfmt) {
> + cam_fmt = soc_camera_format_by_fourcc(icd, pixfmt);
> + if (!cam_fmt)
> + return -EINVAL;
> + }
All right, here is something I don't understand.
Let's take an example : the pxa_camera was asked a YUV422P pixel format. It can
deserve it by asking the sensor a UYVY format. So the logical step would be to
do something like :
if (pixfmt == V4L2_PIX_FMT_YUV422P)
pixfmt = V4L2_PIX_FMT_UYVY;
at the beginning of pxa_camera_set_fmt_cap().
> +
> + ret = icd->ops->set_fmt_cap(icd, pixfmt, rect);
> + if (pixfmt && !ret)
> + icd->current_fmt = cam_fmt;
So here, icd->current_fmt = V4L2_PIX_FMT_UYVY, and not V4L2_PIX_FMT_YUV422P;
> @@ -345,14 +325,21 @@ static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
> rect.width = f->fmt.pix.width;
> rect.height = f->fmt.pix.height;
> ret = ici->ops->set_fmt_cap(icd, f->fmt.pix.pixelformat, &rect);
> - if (ret < 0)
> + if (ret < 0) {
> return ret;
> + } else if (!icd->current_fmt ||
> + icd->current_fmt->fourcc != f->fmt.pix.pixelformat) {
> + dev_err(&ici->dev, "Host driver hasn't set up current "
> + "format correctly!\n");
> + return -EINVAL;
> + }
And here, we fall into the error case, because icd->current_fmt is
V4L2_PIX_FMT_UYVY, and f->fmt.pix.pixelformat = V4L2_PIX_FMT_YUV422P.
So there is still something to improve, or have I missed something ?
--
Robert
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] soc-camera: let camera host drivers decide upon pixel format
2008-11-09 0:47 ` Robert Jarzmik
@ 2008-11-09 10:32 ` Guennadi Liakhovetski
2008-11-09 11:13 ` Robert Jarzmik
1 sibling, 0 replies; 6+ messages in thread
From: Guennadi Liakhovetski @ 2008-11-09 10:32 UTC (permalink / raw)
To: Robert Jarzmik; +Cc: video4linux-list
On Sun, 9 Nov 2008, Robert Jarzmik wrote:
> Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:
>
> > diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
> > index 2a811f8..a375872 100644
> > --- a/drivers/media/video/pxa_camera.c
> > +++ b/drivers/media/video/pxa_camera.c
> > @@ -907,17 +907,43 @@ static int pxa_camera_try_bus_param(struct soc_camera_device *icd, __u32 pixfmt)
> > static int pxa_camera_set_fmt_cap(struct soc_camera_device *icd,
> > __u32 pixfmt, struct v4l2_rect *rect)
> > {
> > - return icd->ops->set_fmt_cap(icd, pixfmt, rect);
> > + const struct soc_camera_data_format *cam_fmt;
> > + int ret;
> > +
> > + /*
> > + * TODO: find a suitable supported by the SoC output format, check
> > + * whether the sensor supports one of acceptable input formats.
> > + */
> > + if (pixfmt) {
> > + cam_fmt = soc_camera_format_by_fourcc(icd, pixfmt);
> > + if (!cam_fmt)
> > + return -EINVAL;
> > + }
> All right, here is something I don't understand.
>
> Let's take an example : the pxa_camera was asked a YUV422P pixel format. It can
> deserve it by asking the sensor a UYVY format. So the logical step would be to
> do something like :
>
> if (pixfmt == V4L2_PIX_FMT_YUV422P)
> pixfmt = V4L2_PIX_FMT_UYVY;
>
> at the beginning of pxa_camera_set_fmt_cap().
Right.
> > +
> > + ret = icd->ops->set_fmt_cap(icd, pixfmt, rect);
> > + if (pixfmt && !ret)
> > + icd->current_fmt = cam_fmt;
> So here, icd->current_fmt = V4L2_PIX_FMT_UYVY, and not V4L2_PIX_FMT_YUV422P;
>
> > @@ -345,14 +325,21 @@ static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
> > rect.width = f->fmt.pix.width;
> > rect.height = f->fmt.pix.height;
> > ret = ici->ops->set_fmt_cap(icd, f->fmt.pix.pixelformat, &rect);
> > - if (ret < 0)
> > + if (ret < 0) {
> > return ret;
> > + } else if (!icd->current_fmt ||
> > + icd->current_fmt->fourcc != f->fmt.pix.pixelformat) {
> > + dev_err(&ici->dev, "Host driver hasn't set up current "
> > + "format correctly!\n");
> > + return -EINVAL;
> > + }
> And here, we fall into the error case, because icd->current_fmt is
> V4L2_PIX_FMT_UYVY, and f->fmt.pix.pixelformat = V4L2_PIX_FMT_YUV422P.
>
> So there is still something to improve, or have I missed something ?
Yes, there is something to improve. With this patch I modified central
soc_camera.c algorithms to make such format-conversions possible, but I so
far just preserved the functionality in camera host drivers. Now we have
to change that as needed too.
You correctly identified the first place to modify. Now this is the second
one:
+ icd->current_fmt = cam_fmt;
You now have to substitute another struct soc_camera_data_format to
current_fmt. Ok, I wasn't sure what's the best way to do this. Preserving
current_fmt as a pointer is easier, because it requires no code
modifications, and in most cases, where you don't perform any format
substitution, you can still use a struct from sensor's list. Now if you do
make such a substitution, I would say, you have to allocate such a struct
and use it there. Now, how do we avoid leaking this structs. I think, the
best would be to add a
void *host_priv;
to struct soc_camera_device for per-sensor host-private data. It can be
generally useful in the future too. Then add some
struct camera_data {
struct soc_camera_data_format data_fmt;
};
to pxa_camera.c, allocate it in pxa_camera_add_device() and assign to
->host_priv. Then fill it in as required in pxa_camera_set_fmt_cap() iff
such a format conversion is requested and use its data_fmt member for
current_fmt. Of course, remembering to free it in
pxa_camera_remove_device().
Another possibility would be to make current_fmt to hold the struct
instead of being just a pointer, thus saving above allocations, but that
would require all drivers do a memcpy on each format change instead of
just a pointer assignment...
How does this sound?
Thanks
Guennadi
---
Guennadi Liakhovetski
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] soc-camera: let camera host drivers decide upon pixel format
2008-11-09 0:47 ` Robert Jarzmik
2008-11-09 10:32 ` Guennadi Liakhovetski
@ 2008-11-09 11:13 ` Robert Jarzmik
2008-11-09 12:36 ` Guennadi Liakhovetski
1 sibling, 1 reply; 6+ messages in thread
From: Robert Jarzmik @ 2008-11-09 11:13 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: video4linux-list
Guennadi,
I thought a bit about pixel format negociation.
The thing that came to my mind is that it is not the sensor that can tell all
the pixel formats available, it's the camera host. That means that icd->formats
should be filled in by the host, not the sensor.
What I would see is a generic call in soc_camera (or a structure), called by
each sensor to declare the pixel formats it handles. This call (or structure)
will be used by camera host driver to fill in icd->formats, deduced from what
the sensors offers, and the host possible translations.
Tell me you opinion about it please.
--
Robert
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] soc-camera: let camera host drivers decide upon pixel format
2008-11-09 11:13 ` Robert Jarzmik
@ 2008-11-09 12:36 ` Guennadi Liakhovetski
2008-11-09 13:31 ` Robert Jarzmik
0 siblings, 1 reply; 6+ messages in thread
From: Guennadi Liakhovetski @ 2008-11-09 12:36 UTC (permalink / raw)
To: Robert Jarzmik; +Cc: video4linux-list
Hi Robert,
On Sun, 9 Nov 2008, Robert Jarzmik wrote:
> I thought a bit about pixel format negociation.
>
> The thing that came to my mind is that it is not the sensor that can tell all
> the pixel formats available, it's the camera host. That means that icd->formats
> should be filled in by the host, not the sensor.
>
> What I would see is a generic call in soc_camera (or a structure), called by
> each sensor to declare the pixel formats it handles. This call (or structure)
> will be used by camera host driver to fill in icd->formats, deduced from what
> the sensors offers, and the host possible translations.
>
> Tell me you opinion about it please.
Well, actually, I consider pixel format negotiation implemented with my
latest patch. Well, almost, I forgot about (at least) one part, which I
can implement with an incremental patch.
As I wrote in the comment to the patch, I think, lists of pixel formats,
supported by sensors are rather static, therefore they can be easily
represented by a list of structures, that's what our ->formats are about.
Now, the latest patch changes the logic in a way, that this list is now
what a sensor offers, and not what the user gets, requests to set a format
are now handled by camera hosts, so they decide how to implement the
requested format. Now, we are almost that far. What I've forgotten about
and why, probably, you decided we still don't do that, is that the
->formats array is still used for format enumeration. It shall not be. So,
I'm going to write another patch, that would move format enumeration into
host drivers. To do that, we will probably have to create such a list
_dynamically_ in .add() method based on the ->formats list _and_ host's
capabilities. We might use the ->host_priv link, I suggested in my
previous email, to hold that list. It would be even better to not have to
create such a list and just enumerate formats dynamically in the host
driver, but I am not sure how to handle the index... I'll have to think
about it a bit more.
Does this answer your question?
Thanks
Guennadi
---
Guennadi Liakhovetski
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] soc-camera: let camera host drivers decide upon pixel format
2008-11-09 12:36 ` Guennadi Liakhovetski
@ 2008-11-09 13:31 ` Robert Jarzmik
0 siblings, 0 replies; 6+ messages in thread
From: Robert Jarzmik @ 2008-11-09 13:31 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: video4linux-list
Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:
> Hi Robert,
> As I wrote in the comment to the patch, I think, lists of pixel formats,
> supported by sensors are rather static, therefore they can be easily
> represented by a list of structures, that's what our ->formats are about.
> Now, the latest patch changes the logic in a way, that this list is now
> what a sensor offers, and not what the user gets, requests to set a format
> are now handled by camera hosts, so they decide how to implement the
> requested format. Now, we are almost that far. What I've forgotten about
> and why, probably, you decided we still don't do that, is that the
> ->formats array is still used for format enumeration. It shall not be. So,
> I'm going to write another patch, that would move format enumeration into
> host drivers. To do that, we will probably have to create such a list
> _dynamically_ in .add() method based on the ->formats list _and_ host's
> capabilities. We might use the ->host_priv link, I suggested in my
> previous email, to hold that list. It would be even better to not have to
> create such a list and just enumerate formats dynamically in the host
> driver, but I am not sure how to handle the index... I'll have to think
> about it a bit more.
>
> Does this answer your question?
Yes, absolutely. That's the right direction. I'm looking forward to see the
incremental patch, as you may guess ;) My YUV work is over, I'm just waiting for
the soc_camera patchset to stabilize to fire my own serie.
I'll try to think about the fully dynamic formats list, even if I prefer the
computed list at sensor attachment.
--
Robert
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-11-09 13:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-11-08 18:48 [PATCH 3/3] soc-camera: let camera host drivers decide upon pixel format Guennadi Liakhovetski
2008-11-09 0:47 ` Robert Jarzmik
2008-11-09 10:32 ` Guennadi Liakhovetski
2008-11-09 11:13 ` Robert Jarzmik
2008-11-09 12:36 ` Guennadi Liakhovetski
2008-11-09 13:31 ` Robert Jarzmik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox