Re: [PATCH/RFC] v4l: Add subdev sensor g_skip_frames operation
Hi Eino-Ville, On Tuesday 23 November 2010 00:18:18 Eino-Ville Talvala wrote: On 11/19/2010 6:22 AM, Laurent Pinchart wrote: On Friday 19 November 2010 15:15:11 Guennadi Liakhovetski wrote: On Fri, 19 Nov 2010, Laurent Pinchart wrote: On Friday 19 November 2010 14:42:31 Hans Verkuil wrote: On Friday 19 November 2010 14:26:42 Laurent Pinchart wrote: Some buggy sensors generate corrupt frames when the stream is started. This new operation returns the number of corrupt frames to skip when starting the stream. Looks OK, but perhaps the two should be combined to one function? I'm fine with both. Guennadi, any opinion ? Same as before;) I think, there can be many more such micro parameters, that we'll want to collect from the sensor. So, if we had a good idea - what those parameters are like, we could implement just one API call to get them all, or even just pass one object with this information - if it is constant. If we don't have a good idea yet, what to expect there, it might be best to wait and first collect a more complete understanding of this kind of information. A few days late on this, but I wanted to add my two cents. When using V4L2 for still photography applications (like what's done on the N900, or on our custom Frankencamera), it becomes a lot more important to know what data is good or bad, when typically you capture only one or a few frames at a high resolution, before dropping back down to a viewfinding mode. One doesn't want to throw any frames on the floor if it isn't absolutely neccessary ( 100 ms extra shutter lag for every frame you have to drop, on the N900). So having some reasonable way to know when the data becomes good is very helpful. I agree with this. To make things worse, as Guennadi noted, the world isn't black and white and we have lots of shades of grey between good and bad. Depending on the use case the same image could be considered good or bad. That's why we will need to pass metadata to userspace at some point (providing the kernel can receive the metadata from the hardware of course). This patch aims at solving a subset of the good-bad problem, namely sensors that always produce a fixed number of completely corrupted frames when the stream starts. Sensor data sheets are also often rather vague, and state things like changing regions of interest, digital zoom, etc, 'may cause a bad frame'. It'd be great if once somebody figures out what 'may' translates to, not everyone has to figure it out again for every application. I totally agree. We would need better cooperation from sensor vendors. -- Regards, Laurent Pinchart -- 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
Re: [PATCH/RFC] v4l: Add subdev sensor g_skip_frames operation
On 11/19/2010 6:22 AM, Laurent Pinchart wrote: Hi Guennadi, On Friday 19 November 2010 15:15:11 Guennadi Liakhovetski wrote: On Fri, 19 Nov 2010, Laurent Pinchart wrote: On Friday 19 November 2010 14:42:31 Hans Verkuil wrote: On Friday 19 November 2010 14:26:42 Laurent Pinchart wrote: Some buggy sensors generate corrupt frames when the stream is started. This new operation returns the number of corrupt frames to skip when starting the stream. Looks OK, but perhaps the two should be combined to one function? I'm fine with both. Guennadi, any opinion ? Same as before;) I think, there can be many more such micro parameters, that we'll want to collect from the sensor. So, if we had a good idea - what those parameters are like, we could implement just one API call to get them all, or even just pass one object with this information - if it is constant. If we don't have a good idea yet, what to expect there, it might be best to wait and first collect a more complete understanding of this kind of information. A few days late on this, but I wanted to add my two cents. When using V4L2 for still photography applications (like what's done on the N900, or on our custom Frankencamera), it becomes a lot more important to know what data is good or bad, when typically you capture only one or a few frames at a high resolution, before dropping back down to a viewfinding mode. One doesn't want to throw any frames on the floor if it isn't absolutely neccessary ( 100 ms extra shutter lag for every frame you have to drop, on the N900). So having some reasonable way to know when the data becomes good is very helpful. Sensor data sheets are also often rather vague, and state things like changing regions of interest, digital zoom, etc, 'may cause a bad frame'. It'd be great if once somebody figures out what 'may' translates to, not everyone has to figure it out again for every application. Eino-Ville Talvala Stanford University -- 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
[PATCH/RFC] v4l: Add subdev sensor g_skip_frames operation
Some buggy sensors generate corrupt frames when the stream is started.
This new operation returns the number of corrupt frames to skip when
starting the stream.
Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
include/media/v4l2-subdev.h |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 04878e1..b196966 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -291,9 +291,13 @@ struct v4l2_subdev_pad_ops {
* This is needed for some sensors, which always corrupt
* several top lines of the output image, or which send their
* metadata in them.
+ * @g_skip_frames: number of frames to skip at stream start. This is needed for
+ *buggy sensors that generate faulty frames when they are
+ *turned on.
*/
struct v4l2_subdev_sensor_ops {
int (*g_skip_top_lines)(struct v4l2_subdev *sd, u32 *lines);
+ int (*g_skip_frames)(struct v4l2_subdev *sd, u32 *frames);
};
struct v4l2_subdev_ops {
--
1.7.2.2
--
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
Re: [PATCH/RFC] v4l: Add subdev sensor g_skip_frames operation
On Friday 19 November 2010 14:26:42 Laurent Pinchart wrote:
Some buggy sensors generate corrupt frames when the stream is started.
This new operation returns the number of corrupt frames to skip when
starting the stream.
Looks OK, but perhaps the two should be combined to one function?
I also have my doubts about the sensor_ops in general. I expected originally
to see a lot of ops here, but apparently there is little or no need for it.
Do we expect to see this grow, or would it make more sense to move the ops
to video_ops? I'd be interested to hear what sensor specialists think.
Regards.
Hans
Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
include/media/v4l2-subdev.h |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 04878e1..b196966 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -291,9 +291,13 @@ struct v4l2_subdev_pad_ops {
* This is needed for some sensors, which always corrupt
* several top lines of the output image, or which send their
* metadata in them.
+ * @g_skip_frames: number of frames to skip at stream start. This is needed
for
+ * buggy sensors that generate faulty frames when they are
+ * turned on.
*/
struct v4l2_subdev_sensor_ops {
int (*g_skip_top_lines)(struct v4l2_subdev *sd, u32 *lines);
+ int (*g_skip_frames)(struct v4l2_subdev *sd, u32 *frames);
};
struct v4l2_subdev_ops {
--
Hans Verkuil - video4linux developer - sponsored by Cisco
--
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
Re: [PATCH/RFC] v4l: Add subdev sensor g_skip_frames operation
On Fri, 19 Nov 2010, Laurent Pinchart wrote:
Some buggy sensors generate corrupt frames when the stream is started.
This new operation returns the number of corrupt frames to skip when
starting the stream.
Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
include/media/v4l2-subdev.h |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 04878e1..b196966 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -291,9 +291,13 @@ struct v4l2_subdev_pad_ops {
* This is needed for some sensors, which always corrupt
* several top lines of the output image, or which send their
* metadata in them.
+ * @g_skip_frames: number of frames to skip at stream start. This is needed
for
+ * buggy sensors that generate faulty frames when they are
+ * turned on.
*/
struct v4l2_subdev_sensor_ops {
int (*g_skip_top_lines)(struct v4l2_subdev *sd, u32 *lines);
+ int (*g_skip_frames)(struct v4l2_subdev *sd, u32 *frames);
Well, yes, this would be useful, but, I think, it is a part of a larger
problem - general video quality from sensors. Like, I think, there are
other situations, when the sensor driver knows, that the next few frames
will be of poor quality. E.g., when changing some parameters, which will
make the sensor re-adjust auto-exposure, auto-gain or something similar.
So, we can either just handle this one specific case, or try to design a
more generic approach, or leave frame quality analysis completely to the
user-space. Like - for a normal generic mplayer, just streaming video to
an output device - you don't really care in most cases. If recording video
- you edit it afterwords, if building an industrial quality
purpose-designed application - it will, probably, take care of these
things itself. And yes, there is also out-of-band data, that can carry
such quality-related information. So, this is just one bit of a bigger
problem, no idea though, wheather it's worth trying to tackle all those
issues at once or better just fix this one small specific problem.
};
struct v4l2_subdev_ops {
--
1.7.2.2
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] v4l: Add subdev sensor g_skip_frames operation
Hi Hans, On Friday 19 November 2010 14:42:31 Hans Verkuil wrote: On Friday 19 November 2010 14:26:42 Laurent Pinchart wrote: Some buggy sensors generate corrupt frames when the stream is started. This new operation returns the number of corrupt frames to skip when starting the stream. Looks OK, but perhaps the two should be combined to one function? I'm fine with both. Guennadi, any opinion ? I also have my doubts about the sensor_ops in general. I expected originally to see a lot of ops here, but apparently there is little or no need for it. Do we expect to see this grow, or would it make more sense to move the ops to video_ops? I'd be interested to hear what sensor specialists think. Good question. It won't remove the need for the g_skip_frames operation, but it's certainly worth asking. Standards are emerging for sensors in specific markets (SMIA comes to mind - I'm not sure if the spec is public, but some information can be found online) and there will probably be a need to provide more sensor information to both bridge drivers and userspace applications in the future. -- Regards, Laurent Pinchart -- 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
Re: [PATCH/RFC] v4l: Add subdev sensor g_skip_frames operation
On Fri, 19 Nov 2010, Hans Verkuil wrote: On Friday 19 November 2010 14:26:42 Laurent Pinchart wrote: Some buggy sensors generate corrupt frames when the stream is started. This new operation returns the number of corrupt frames to skip when starting the stream. Looks OK, but perhaps the two should be combined to one function? I also have my doubts about the sensor_ops in general. I expected originally to see a lot of ops here, but apparently there is little or no need for it. Do we expect to see this grow, or would it make more sense to move the ops to video_ops? I'd be interested to hear what sensor specialists think. I would keep the struct. 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] v4l: Add subdev sensor g_skip_frames operation
Hi Guennadi,
On Friday 19 November 2010 14:49:44 Guennadi Liakhovetski wrote:
On Fri, 19 Nov 2010, Laurent Pinchart wrote:
Some buggy sensors generate corrupt frames when the stream is started.
This new operation returns the number of corrupt frames to skip when
starting the stream.
Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
include/media/v4l2-subdev.h |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 04878e1..b196966 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -291,9 +291,13 @@ struct v4l2_subdev_pad_ops {
* This is needed for some sensors, which always corrupt
* several top lines of the output image, or which send their
* metadata in them.
+ * @g_skip_frames: number of frames to skip at stream start. This is
needed for + * buggy sensors that generate faulty frames
when they
are + *turned on.
*/
struct v4l2_subdev_sensor_ops {
int (*g_skip_top_lines)(struct v4l2_subdev *sd, u32 *lines);
+ int (*g_skip_frames)(struct v4l2_subdev *sd, u32 *frames);
Well, yes, this would be useful, but, I think, it is a part of a larger
problem - general video quality from sensors. Like, I think, there are
other situations, when the sensor driver knows, that the next few frames
will be of poor quality. E.g., when changing some parameters, which will
make the sensor re-adjust auto-exposure, auto-gain or something similar.
So, we can either just handle this one specific case, or try to design a
more generic approach, or leave frame quality analysis completely to the
user-space. Like - for a normal generic mplayer, just streaming video to
an output device - you don't really care in most cases. If recording video
- you edit it afterwords, if building an industrial quality
purpose-designed application - it will, probably, take care of these
things itself. And yes, there is also out-of-band data, that can carry
such quality-related information. So, this is just one bit of a bigger
problem, no idea though, wheather it's worth trying to tackle all those
issues at once or better just fix this one small specific problem.
Lots of different issues there. Quality handling should be implemented in
userspace, but drivers need to provide enough information to
applications/libraries. Metadata could be bundled with frame using the recent
multiplane formats support infrastructure.
The purpose of this operation is to skip frames that are know to be real bad.
Think of a buggy sensor that outputs a frame of complete garbage when you
start the stream. Not just bad quality data, but real garbage (either random
or a flat color).
--
Regards,
Laurent Pinchart
--
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
Re: [PATCH/RFC] v4l: Add subdev sensor g_skip_frames operation
On Fri, 19 Nov 2010, Laurent Pinchart wrote: Hi Hans, On Friday 19 November 2010 14:42:31 Hans Verkuil wrote: On Friday 19 November 2010 14:26:42 Laurent Pinchart wrote: Some buggy sensors generate corrupt frames when the stream is started. This new operation returns the number of corrupt frames to skip when starting the stream. Looks OK, but perhaps the two should be combined to one function? I'm fine with both. Guennadi, any opinion ? Same as before;) I think, there can be many more such micro parameters, that we'll want to collect from the sensor. So, if we had a good idea - what those parameters are like, we could implement just one API call to get them all, or even just pass one object with this information - if it is constant. If we don't have a good idea yet, what to expect there, it might be best to wait and first collect a more complete understanding of this kind of information. In any case I wouldn't convert these two calls to one like int (*get_bad_things)(struct v4l2_subdev *sd, u32 *lines, u32 *frames) ;) 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] v4l: Add subdev sensor g_skip_frames operation
On Friday 19 November 2010 15:15:11 Guennadi Liakhovetski wrote: On Fri, 19 Nov 2010, Laurent Pinchart wrote: Hi Hans, On Friday 19 November 2010 14:42:31 Hans Verkuil wrote: On Friday 19 November 2010 14:26:42 Laurent Pinchart wrote: Some buggy sensors generate corrupt frames when the stream is started. This new operation returns the number of corrupt frames to skip when starting the stream. Looks OK, but perhaps the two should be combined to one function? I'm fine with both. Guennadi, any opinion ? Same as before;) I think, there can be many more such micro parameters, that we'll want to collect from the sensor. So, if we had a good idea - what those parameters are like, we could implement just one API call to get them all, or even just pass one object with this information - if it is constant. If we don't have a good idea yet, what to expect there, it might be best to wait and first collect a more complete understanding of this kind of information. In any case I wouldn't convert these two calls to one like int (*get_bad_things)(struct v4l2_subdev *sd, u32 *lines, u32 *frames) ;) OK, let's go with Laurent's proposal. But I do think this should be reviewed at some point in time. Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by Cisco -- 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
Re: [PATCH/RFC] v4l: Add subdev sensor g_skip_frames operation
Hi Guennadi, On Friday 19 November 2010 15:15:11 Guennadi Liakhovetski wrote: On Fri, 19 Nov 2010, Laurent Pinchart wrote: On Friday 19 November 2010 14:42:31 Hans Verkuil wrote: On Friday 19 November 2010 14:26:42 Laurent Pinchart wrote: Some buggy sensors generate corrupt frames when the stream is started. This new operation returns the number of corrupt frames to skip when starting the stream. Looks OK, but perhaps the two should be combined to one function? I'm fine with both. Guennadi, any opinion ? Same as before;) I think, there can be many more such micro parameters, that we'll want to collect from the sensor. So, if we had a good idea - what those parameters are like, we could implement just one API call to get them all, or even just pass one object with this information - if it is constant. If we don't have a good idea yet, what to expect there, it might be best to wait and first collect a more complete understanding of this kind of information. I agree. In any case I wouldn't convert these two calls to one like int (*get_bad_things)(struct v4l2_subdev *sd, u32 *lines, u32 *frames) ;) -- Regards, Laurent Pinchart -- 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
