[v4l-utils] Add options to v4l2-ctrl to save/load settings to/from a file
Hi, AFAIK every time a new v4l device is initialized (e.g. USB webcam plugged in) the driver sets the controls to the default value decided by the author in the source code. It is then the responsibility of each v4l2 applications to save and restore any variation to the controls values made by the user if this is required. I was looking for a more generic way to set some controls to non-default values in a more persistent way, my main use case is to avoid *manually* setting "Power Line Frequency" to 50Hz every time I plug in the webcam. Something like what alsactrl[0] does with mixer settings. Maybe pipewire will do that? In the mean time, inspired by [1] I cleaned up the concept and published it as v4l2-persistent-settings[2], the idea is that the user can save the current state of a device and it would be restored automatically via a udev rule the next time the device is initialized. For that, the current device state has to be stored into a file. For now I am massaging the output of "v4l2-ctl -l", saving that to a file, and then parsing the file to generate something I can pass to "v4l2-ctl --set-ctrl"; however it would be handier if v4l2-ctl had a native mechanism to export and import settings. v4l2ctrl from v4l2ucp[3] has options to save settings to a file and reload them from a file, but I would like to use v4l2-ctl instead which is actively maintained. What about adding such options to v4l2-ctl? Thank you, Antonio [0] http://git.alsa-project.org/?p=alsa-utils.git;a=tree;f=alsactl;hb=HEAD [1] https://superuser.com/questions/471597/linux-v4l-webcam-make-settings-stick [2] https://git.ao2.it/v4l2-persistent-settings.git/ [3] https://sourceforge.net/projects/v4l2ucp/ -- Antonio Ospite https://ao2.it https://twitter.com/ao2it A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing?
[RFC PATCH 5/5] v4l2-ctl: add an option to list controls in a machine-readable format
Add a new option --list-ctrls-values to list the values of controls in
a format which can be passed again to --set-ctrl.
This can be useful to save and restore device settings:
$ v4l2-ctl --list-ctrls-values >settings.txt 2>/dev/null
$ v4l2-ctl --set-ctrl "$(cat settings.txt)"
The new option has been tested with the vivid driver and it works well
enough to be useful with a real driver as well.
String controls are not supported for now, as they may not be parsed
correctly by --set-ctrl if they contain a comma or a single quote.
Signed-off-by: Antonio Ospite
---
utils/v4l2-ctl/v4l2-ctl-common.cpp | 72 ++
utils/v4l2-ctl/v4l2-ctl.1.in | 4 ++
utils/v4l2-ctl/v4l2-ctl.cpp| 1 +
utils/v4l2-ctl/v4l2-ctl.h | 1 +
4 files changed, 69 insertions(+), 9 deletions(-)
diff --git a/utils/v4l2-ctl/v4l2-ctl-common.cpp
b/utils/v4l2-ctl/v4l2-ctl-common.cpp
index b45c..b4124608 100644
--- a/utils/v4l2-ctl/v4l2-ctl-common.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl-common.cpp
@@ -93,6 +93,9 @@ void common_usage(void)
" -l, --list-ctrls display all controls and their values
[VIDIOC_QUERYCTRL]\n"
" -L, --list-ctrls-menus\n"
" display all controls and their menus
[VIDIOC_QUERYMENU]\n"
+ " -m, --list-ctrls-values\n"
+ " display all controls and their values in a
format compatible with\n"
+ " --set-ctrls (the 'm' stands for \"machine
readable output\")\n"
" -r, --subset [,,]+\n"
" the subset of the N-dimensional array to
get/set for control ,\n"
" for every dimension an (, )
tuple is given.\n"
@@ -409,6 +412,46 @@ static void print_qctrl(int fd, struct v4l2_query_ext_ctrl
*queryctrl,
}
}
+static void print_qctrl_values(int fd, struct v4l2_query_ext_ctrl *queryctrl,
+ struct v4l2_ext_control *ctrl, int show_menus)
+{
+ std::string s = name2var(queryctrl->name);
+
+ if (queryctrl->nr_of_dims == 0) {
+ switch (queryctrl->type) {
+ case V4L2_CTRL_TYPE_INTEGER:
+ case V4L2_CTRL_TYPE_BOOLEAN:
+ case V4L2_CTRL_TYPE_MENU:
+ case V4L2_CTRL_TYPE_INTEGER_MENU:
+ printf("%s=%d,", s.c_str(), ctrl->value);
+ break;
+ case V4L2_CTRL_TYPE_BITMASK:
+ printf("%s=0x%08x,", s.c_str(), ctrl->value);
+ break;
+ case V4L2_CTRL_TYPE_INTEGER64:
+ printf("%s=%lld,", s.c_str(), ctrl->value64);
+ break;
+ case V4L2_CTRL_TYPE_STRING:
+ fprintf(stderr, "%s: string controls unsupported for
now\n", queryctrl->name);
+ break;
+ default:
+ fprintf(stderr, "%s: unsupported payload type\n",
queryctrl->name);
+ break;
+ }
+ }
+
+ if (queryctrl->nr_of_dims)
+ fprintf(stderr, "%s: unsupported payload type
(multi-dimensional)\n", queryctrl->name);
+
+ if (queryctrl->flags)
+ fprintf(stderr, "%s: ignoring flags\n", queryctrl->name);
+
+ if ((queryctrl->type == V4L2_CTRL_TYPE_MENU ||
+queryctrl->type == V4L2_CTRL_TYPE_INTEGER_MENU) && show_menus) {
+ fprintf(stderr, "%s: ignoring menus\n", queryctrl->name);
+ }
+}
+
static void print_class_name(const char *name)
{
printf("\n%s\n\n", name);
@@ -426,7 +469,8 @@ static int print_control(int fd, struct v4l2_query_ext_ctrl
&qctrl, struct print
if (qctrl.flags & V4L2_CTRL_FLAG_DISABLED)
return 1;
if (qctrl.type == V4L2_CTRL_TYPE_CTRL_CLASS) {
- format->print_class_name(qctrl.name);
+ if (format->print_class_name)
+ format->print_class_name(qctrl.name);
return 1;
}
ext_ctrl.id = qctrl.id;
@@ -1102,13 +1146,23 @@ void common_get(cv4l_fd &_fd)
void common_list(cv4l_fd &fd)
{
- if (options[OptListCtrls] || options[OptListCtrlsMenus]) {
- struct print_format classic_format = {
- .print_class_name = print_class_name,
- .print_qctrl = print_qctrl,
- .show_menus = options[OptListCtrlsMenus],
- };
-
- list_controls(fd.g_fd(), &classic_format);
+ if (options[OptListCtrls] || options[OptListCtrlsMenus] ||
options[OptListCtrl
[RFC PATCH 2/5] v4l2-ctl: list once when both OptListCtrls and OptListCtrlsMenus are there
When both --list-ctrls and --list-ctrls-menus are passed, controls are
listed twice which is accurate but can be confusing.
Treat --list-ctrls-menus as an option modifier when also --list-ctrls is
passed, in order to have the controls listed only once.
Signed-off-by: Antonio Ospite
---
utils/v4l2-ctl/v4l2-ctl-common.cpp | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/utils/v4l2-ctl/v4l2-ctl-common.cpp
b/utils/v4l2-ctl/v4l2-ctl-common.cpp
index 8256cbd9..e2710335 100644
--- a/utils/v4l2-ctl/v4l2-ctl-common.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl-common.cpp
@@ -1091,11 +1091,7 @@ void common_get(cv4l_fd &_fd)
void common_list(cv4l_fd &fd)
{
- if (options[OptListCtrlsMenus]) {
- list_controls(fd.g_fd(), 1);
- }
-
- if (options[OptListCtrls]) {
- list_controls(fd.g_fd(), 0);
+ if (options[OptListCtrls] || options[OptListCtrlsMenus]) {
+ list_controls(fd.g_fd(), options[OptListCtrlsMenus]);
}
}
--
2.20.1
[RFC PATCH 4/5] v4l2-ctl: abstract the mechanism used to print the list of controls
Sometimes it may be useful to list the controls using a different output
format than the current one used by --list-ctrls, for instance a new
printing format could output a string which can be later fed to
--set-ctrl.
Add an abstraction mechanism to make it possible to add new output
formats for controls.
Signed-off-by: Antonio Ospite
---
utils/v4l2-ctl/v4l2-ctl-common.cpp | 32 --
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/utils/v4l2-ctl/v4l2-ctl-common.cpp
b/utils/v4l2-ctl/v4l2-ctl-common.cpp
index 5d41d720..b45c 100644
--- a/utils/v4l2-ctl/v4l2-ctl-common.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl-common.cpp
@@ -30,6 +30,12 @@ struct ctrl_subset {
unsigned size[V4L2_CTRL_MAX_DIMS];
};
+struct print_format {
+ void (*print_class_name)(const char *);
+ void (*print_qctrl)(int, struct v4l2_query_ext_ctrl *, struct
v4l2_ext_control *, int);
+ int show_menus;
+};
+
typedef std::map >
class2ctrls_map;
typedef std::map ctrl_qmap;
@@ -408,7 +414,7 @@ static void print_class_name(const char *name)
printf("\n%s\n\n", name);
}
-static int print_control(int fd, struct v4l2_query_ext_ctrl &qctrl, int
show_menus)
+static int print_control(int fd, struct v4l2_query_ext_ctrl &qctrl, struct
print_format *format)
{
struct v4l2_control ctrl;
struct v4l2_ext_control ext_ctrl;
@@ -420,17 +426,17 @@ static int print_control(int fd, struct
v4l2_query_ext_ctrl &qctrl, int show_men
if (qctrl.flags & V4L2_CTRL_FLAG_DISABLED)
return 1;
if (qctrl.type == V4L2_CTRL_TYPE_CTRL_CLASS) {
- print_class_name(qctrl.name);
+ format->print_class_name(qctrl.name);
return 1;
}
ext_ctrl.id = qctrl.id;
if ((qctrl.flags & V4L2_CTRL_FLAG_WRITE_ONLY) ||
qctrl.type == V4L2_CTRL_TYPE_BUTTON) {
- print_qctrl(fd, &qctrl, &ext_ctrl, show_menus);
+ format->print_qctrl(fd, &qctrl, &ext_ctrl, format->show_menus);
return 1;
}
if (qctrl.type >= V4L2_CTRL_COMPOUND_TYPES) {
- print_qctrl(fd, &qctrl, NULL, show_menus);
+ format->print_qctrl(fd, &qctrl, NULL, format->show_menus);
return 1;
}
ctrls.which = V4L2_CTRL_ID2WHICH(qctrl.id);
@@ -460,7 +466,7 @@ static int print_control(int fd, struct v4l2_query_ext_ctrl
&qctrl, int show_men
}
ext_ctrl.value = ctrl.value;
}
- print_qctrl(fd, &qctrl, &ext_ctrl, show_menus);
+ format->print_qctrl(fd, &qctrl, &ext_ctrl, format->show_menus);
if (qctrl.type == V4L2_CTRL_TYPE_STRING)
free(ext_ctrl.string);
return 1;
@@ -512,7 +518,7 @@ static int query_ext_ctrl_ioctl(int fd, struct
v4l2_query_ext_ctrl &qctrl)
return rc;
}
-static void list_controls(int fd, int show_menus)
+static void list_controls(int fd, struct print_format *format)
{
const unsigned next_fl = V4L2_CTRL_FLAG_NEXT_CTRL |
V4L2_CTRL_FLAG_NEXT_COMPOUND;
struct v4l2_query_ext_ctrl qctrl;
@@ -521,7 +527,7 @@ static void list_controls(int fd, int show_menus)
memset(&qctrl, 0, sizeof(qctrl));
qctrl.id = next_fl;
while (query_ext_ctrl_ioctl(fd, qctrl) == 0) {
- print_control(fd, qctrl, show_menus);
+ print_control(fd, qctrl, format);
qctrl.id |= next_fl;
}
if (qctrl.id != next_fl)
@@ -529,11 +535,11 @@ static void list_controls(int fd, int show_menus)
for (id = V4L2_CID_USER_BASE; id < V4L2_CID_LASTP1; id++) {
qctrl.id = id;
if (query_ext_ctrl_ioctl(fd, qctrl) == 0)
- print_control(fd, qctrl, show_menus);
+ print_control(fd, qctrl, format);
}
for (qctrl.id = V4L2_CID_PRIVATE_BASE;
query_ext_ctrl_ioctl(fd, qctrl) == 0; qctrl.id++) {
- print_control(fd, qctrl, show_menus);
+ print_control(fd, qctrl, format);
}
}
@@ -1097,6 +1103,12 @@ void common_get(cv4l_fd &_fd)
void common_list(cv4l_fd &fd)
{
if (options[OptListCtrls] || options[OptListCtrlsMenus]) {
- list_controls(fd.g_fd(), options[OptListCtrlsMenus]);
+ struct print_format classic_format = {
+ .print_class_name = print_class_name,
+ .print_qctrl = print_qctrl,
+ .show_menus = options[OptListCtrlsMenus],
+ };
+
+ list_controls(fd.g_fd(), &classic_format);
}
}
--
2.20.1
[RFC PATCH 0/5] v4l2-ctl: list controls values in a machine-readable format
Hi, here is an experiment about listing controls values with v4l2-ctl in a way that makes it more easy to reload them, I would use something like that for https://git.ao2.it/v4l2-persistent-settings.git/ Patches 1 and 2 are just warm-up patches to get me familiar again with the v4l2-ctrl codebase, patch 2 is a small preparatory cleanup, and patches 4 and 5 showcase the idea. Thanks, Antonio Antonio Ospite (5): v4l2-ctl: list controls with menus when OptAll is specified v4l2-ctl: list once when both OptListCtrls and OptListCtrlsMenus are there v4l2-ctl: use a dedicated function to print the control class name v4l2-ctl: abstract the mechanism used to print the list of controls v4l2-ctl: add an option to list controls in a machine-readable format utils/v4l2-ctl/v4l2-ctl-common.cpp | 95 +- utils/v4l2-ctl/v4l2-ctl.1.in | 4 ++ utils/v4l2-ctl/v4l2-ctl.cpp| 3 +- utils/v4l2-ctl/v4l2-ctl.h | 1 + 4 files changed, 88 insertions(+), 15 deletions(-) -- Antonio Ospite https://ao2.it https://twitter.com/ao2it A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing?
[RFC PATCH 1/5] v4l2-ctl: list controls with menus when OptAll is specified
When calling "v4l2-ctl --all" the user may expect the most comprehensive output, so also print the menus when listing controls. Signed-off-by: Antonio Ospite --- utils/v4l2-ctl/v4l2-ctl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/v4l2-ctl/v4l2-ctl.cpp b/utils/v4l2-ctl/v4l2-ctl.cpp index 8c52c7be..a65262f6 100644 --- a/utils/v4l2-ctl/v4l2-ctl.cpp +++ b/utils/v4l2-ctl/v4l2-ctl.cpp @@ -1255,7 +1255,7 @@ int main(int argc, char **argv) options[OptGetPriority] = 1; options[OptGetSelection] = 1; options[OptGetOutputSelection] = 1; - options[OptListCtrls] = 1; + options[OptListCtrlsMenus] = 1; options[OptSilent] = 1; } -- 2.20.1
[RFC PATCH 3/5] v4l2-ctl: use a dedicated function to print the control class name
All the details about the controls are printed in the dedicated function
print_qctrl(), use a new dedicated function named print_class_name() to
print the control class name as well, this is for symmetry but it is
also in preparation for a change which aims to abstract how the controls
are printed.
Signed-off-by: Antonio Ospite
---
utils/v4l2-ctl/v4l2-ctl-common.cpp | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/utils/v4l2-ctl/v4l2-ctl-common.cpp
b/utils/v4l2-ctl/v4l2-ctl-common.cpp
index e2710335..5d41d720 100644
--- a/utils/v4l2-ctl/v4l2-ctl-common.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl-common.cpp
@@ -403,6 +403,11 @@ static void print_qctrl(int fd, struct v4l2_query_ext_ctrl
*queryctrl,
}
}
+static void print_class_name(const char *name)
+{
+ printf("\n%s\n\n", name);
+}
+
static int print_control(int fd, struct v4l2_query_ext_ctrl &qctrl, int
show_menus)
{
struct v4l2_control ctrl;
@@ -415,7 +420,7 @@ static int print_control(int fd, struct v4l2_query_ext_ctrl
&qctrl, int show_men
if (qctrl.flags & V4L2_CTRL_FLAG_DISABLED)
return 1;
if (qctrl.type == V4L2_CTRL_TYPE_CTRL_CLASS) {
- printf("\n%s\n\n", qctrl.name);
+ print_class_name(qctrl.name);
return 1;
}
ext_ctrl.id = qctrl.id;
--
2.20.1
Re: [RFC PATCH 5/5] v4l2-ctl: add an option to list controls in a machine-readable format
On Mon, 7 Jan 2019 11:18:58 +0100
Hans Verkuil wrote:
> On 01/03/2019 07:01 PM, Antonio Ospite wrote:
> > Add a new option --list-ctrls-values to list the values of controls in
> > a format which can be passed again to --set-ctrl.
> >
> > This can be useful to save and restore device settings:
> >
> > $ v4l2-ctl --list-ctrls-values >settings.txt 2>/dev/null
> > $ v4l2-ctl --set-ctrl "$(cat settings.txt)"
> >
> > The new option has been tested with the vivid driver and it works well
> > enough to be useful with a real driver as well.
> >
> > String controls are not supported for now, as they may not be parsed
> > correctly by --set-ctrl if they contain a comma or a single quote.
> >
> > Signed-off-by: Antonio Ospite
> > ---
> > utils/v4l2-ctl/v4l2-ctl-common.cpp | 72 ++
> > utils/v4l2-ctl/v4l2-ctl.1.in | 4 ++
> > utils/v4l2-ctl/v4l2-ctl.cpp| 1 +
> > utils/v4l2-ctl/v4l2-ctl.h | 1 +
> > 4 files changed, 69 insertions(+), 9 deletions(-)
> >
> > diff --git a/utils/v4l2-ctl/v4l2-ctl-common.cpp
> > b/utils/v4l2-ctl/v4l2-ctl-common.cpp
> > index b45c..b4124608 100644
> > --- a/utils/v4l2-ctl/v4l2-ctl-common.cpp
> > +++ b/utils/v4l2-ctl/v4l2-ctl-common.cpp
[...]
> > @@ -1102,13 +1146,23 @@ void common_get(cv4l_fd &_fd)
> >
> > void common_list(cv4l_fd &fd)
> > {
> > - if (options[OptListCtrls] || options[OptListCtrlsMenus]) {
> > - struct print_format classic_format = {
> > - .print_class_name = print_class_name,
> > - .print_qctrl = print_qctrl,
> > - .show_menus = options[OptListCtrlsMenus],
> > - };
> > -
> > - list_controls(fd.g_fd(), &classic_format);
> > + if (options[OptListCtrls] || options[OptListCtrlsMenus] ||
> > options[OptListCtrlsValues]) {
> > + if (options[OptListCtrlsValues]) {
> > + struct print_format machine_format = {
> > + .print_class_name = NULL,
> > + .print_qctrl = print_qctrl_values,
> > + .show_menus = 0,
> > + };
> > +
> > + list_controls(fd.g_fd(), &machine_format);
> > + } else {
> > + struct print_format classic_format = {
> > + .print_class_name = print_class_name,
> > + .print_qctrl = print_qctrl,
> > + .show_menus = options[OptListCtrlsMenus],
> > + };
> > +
> > + list_controls(fd.g_fd(), &classic_format);
> > + }
>
> I don't like this struct print_format.
>
Hi Hans,
the idea was based on two considerations:
1. decide the format once and for all, avoiding to check each time a
control is printed.
2. have at least some partial infrastructure in case some
other export formats were to be added.
But yeah, as 2. seems quite unlikely I can go with a more essential
approach for now, no problem.
> I would prefer something like this:
>
> Rename print_qctrl to print_qctrl_readable() and create a new print_qctrl:
>
> static void print_qctrl(int fd, struct v4l2_query_ext_ctrl *queryctrl,
> struct v4l2_ext_control *ctrl, int show_menus)
> {
> if (options[OptListCtrlsValues])
> print_qctrl_values(fd, queryctrl, ctrl, show_menus);
> else
> print_qctrl_readable(fd, queryctrl, ctrl, show_menus);
> }
>
Since "readable" here means "human readable", while "values" is meant
for a "machine readable" output, I'd "avoid" the word "readable" at
all and go with "details" or "description":
if (options[OptListCtrlsValues])
print_qctrl_values(fd, queryctrl, ctrl, show_menus);
else
print_qctrl_details(fd, queryctrl, ctrl, show_menus);
> And in print_control you can just skip printing the class name if
> options[OptListCtrlsValues] is set.
>
OK.
> I would like to see string controls being supported. I would recommend
> to just write the string as a hexdump. It avoids having to escape characters.
>
> The same can be done for compound/array controls. In fact, you could write
> all controls that way. It would simplify things a lot.
>
But then --set-ctrl would need to be extended to parse the hexdump,
wouldn't it? Do you already have a syntax in mind?
TBH, I kept things simple ho
[PATCH 7/7] [media] gspca: fix a v4l2-compliance failure during read()
v4l2-compliance fails with this message: fail: v4l2-test-buffers.cpp(512): Expected EBUSY, got 22 test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL Looking at the v4l2-compliance code reveals that this failure is about the read() callback. In gspca, dev_read() is calling vidioc_dqbuf() which calls frame_ready_nolock() but the latter returns -EINVAL in a case when v4l2-compliance expects -EBUSY. Fix the failure by changing the return value in frame_ready_nolock(). Signed-off-by: Antonio Ospite --- drivers/media/usb/gspca/gspca.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c index 915b6c7..de7e300 100644 --- a/drivers/media/usb/gspca/gspca.c +++ b/drivers/media/usb/gspca/gspca.c @@ -1664,7 +1664,7 @@ static int frame_ready_nolock(struct gspca_dev *gspca_dev, struct file *file, return -ENODEV; if (gspca_dev->capt_file != file || gspca_dev->memory != memory || !gspca_dev->streaming) - return -EINVAL; + return -EBUSY; /* check if a frame is ready */ return gspca_dev->fr_o != atomic_read(&gspca_dev->fr_i); -- 2.7.0 -- 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 5/7] [media] gspca: fix a v4l2-compliance failure about buffer timestamp
v4l2-compliance fails with this message: fail: v4l2-test-buffers.cpp(250): \ timestamp != V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC && \ timestamp != V4L2_BUF_FLAG_TIMESTAMP_COPY ... test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL When setting the frame time, gspca uses v4l2_get_timestamp() which uses ktime_get_ts() which uses ktime_get_ts64() which returns a monotonic timestamp, so it's safe to initialize the buffer flags to V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC to fix the failure. Signed-off-by: Antonio Ospite --- drivers/media/usb/gspca/gspca.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c index 61cb16d..84b0d6a 100644 --- a/drivers/media/usb/gspca/gspca.c +++ b/drivers/media/usb/gspca/gspca.c @@ -522,7 +522,7 @@ static int frame_alloc(struct gspca_dev *gspca_dev, struct file *file, frame = &gspca_dev->frame[i]; frame->v4l2_buf.index = i; frame->v4l2_buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; - frame->v4l2_buf.flags = 0; + frame->v4l2_buf.flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; frame->v4l2_buf.field = V4L2_FIELD_NONE; frame->v4l2_buf.length = frsz; frame->v4l2_buf.memory = memory; -- 2.7.0 -- 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 1/7] [media] gspca: ov534/topro: use a define for the default framerate
When writing the change in commit dcc7fdbec53a ("[media] gspca:
ov534/topro: prevent a division by 0") I used magic numbers for the
default framerate to minimize the code footprint to make it easier to
backport the patch to the stable trees.
However it's better if the default framerate has its own define to avoid
risking using different values in different places, and for readability.
While at it also remove some trivial comments about the framerates which
don't add much to the code anymore.
Signed-off-by: Antonio Ospite
---
drivers/media/usb/gspca/ov534.c | 7 +++
drivers/media/usb/gspca/topro.c | 6 --
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/media/usb/gspca/ov534.c b/drivers/media/usb/gspca/ov534.c
index bfff1d1..9266a5c 100644
--- a/drivers/media/usb/gspca/ov534.c
+++ b/drivers/media/usb/gspca/ov534.c
@@ -51,6 +51,7 @@
#define OV534_OP_READ_20xf9
#define CTRL_TIMEOUT 500
+#define DEFAULT_FRAME_RATE 30
MODULE_AUTHOR("Antonio Ospite ");
MODULE_DESCRIPTION("GSPCA/OV534 USB Camera Driver");
@@ -1061,7 +1062,7 @@ static int sd_config(struct gspca_dev *gspca_dev,
cam->cam_mode = ov772x_mode;
cam->nmodes = ARRAY_SIZE(ov772x_mode);
- sd->frame_rate = 30;
+ sd->frame_rate = DEFAULT_FRAME_RATE;
return 0;
}
@@ -1492,10 +1493,8 @@ static void sd_set_streamparm(struct gspca_dev
*gspca_dev,
struct sd *sd = (struct sd *) gspca_dev;
if (tpf->numerator == 0 || tpf->denominator == 0)
- /* Set default framerate */
- sd->frame_rate = 30;
+ sd->frame_rate = DEFAULT_FRAME_RATE;
else
- /* Set requested framerate */
sd->frame_rate = tpf->denominator / tpf->numerator;
if (gspca_dev->streaming)
diff --git a/drivers/media/usb/gspca/topro.c b/drivers/media/usb/gspca/topro.c
index c028a5c..15eb069 100644
--- a/drivers/media/usb/gspca/topro.c
+++ b/drivers/media/usb/gspca/topro.c
@@ -175,6 +175,8 @@ static const u8 jpeg_q[17] = {
#error "USB buffer too small"
#endif
+#define DEFAULT_FRAME_RATE 30
+
static const u8 rates[] = {30, 20, 15, 10, 7, 5};
static const struct framerates framerates[] = {
{
@@ -4020,7 +4022,7 @@ static int sd_config(struct gspca_dev *gspca_dev,
gspca_dev->cam.mode_framerates = sd->bridge == BRIDGE_TP6800 ?
framerates : framerates_6810;
- sd->framerate = 30; /* default: 30 fps */
+ sd->framerate = DEFAULT_FRAME_RATE;
return 0;
}
@@ -4803,7 +4805,7 @@ static void sd_set_streamparm(struct gspca_dev *gspca_dev,
int fr, i;
if (tpf->numerator == 0 || tpf->denominator == 0)
- sd->framerate = 30;
+ sd->framerate = DEFAULT_FRAME_RATE;
else
sd->framerate = tpf->denominator / tpf->numerator;
--
2.7.0
--
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 0/7] gspca: pass all v4l2-compliance tests + minor fixes
Hi, after applying this patchset gspca passes all v4l2-compliance tests, at least it does with a PS3 Eye. - Patch 1 removes some magic numbers in subdrivers. - Patch 2 is a correctness fix, but it does not bring any functional changes. - Patch 3 is a readability improvement by itself, but it's also a preliminary change for patch 4. - Patches from 4 to 7 are the actual compliance fixes. More details are in the commit messages. Thanks, Antonio Antonio Ospite (7): [media] gspca: ov534/topro: use a define for the default framerate [media] gspca: fix setting frame interval type in vidioc_enum_frameintervals() [media] gspca: rename wxh_to_mode() to wxh_to_nearest_mode() [media] gspca: fix a v4l2-compliance failure about VIDIOC_ENUM_FRAMEINTERVALS [media] gspca: fix a v4l2-compliance failure about buffer timestamp [media] gspca: fix a v4l2-compliance failure during VIDIOC_REQBUFS [media] gspca: fix a v4l2-compliance failure during read() drivers/media/usb/gspca/gspca.c | 36 +++- drivers/media/usb/gspca/ov534.c | 7 +++ drivers/media/usb/gspca/topro.c | 6 -- 3 files changed, 30 insertions(+), 19 deletions(-) -- Antonio Ospite http://ao2.it A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? -- 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 2/7] [media] gspca: fix setting frame interval type in vidioc_enum_frameintervals()
Set the frame _interval_ type to V4L2_FRMIVAL_TYPE_DISCRETE instead of
using V4L2_FRMSIZE_TYPE_DISCRETE which is meant for frame _size_.
The old and new values happen to be the same so there is no functional
change.
Signed-off-by: Antonio Ospite
---
drivers/media/usb/gspca/gspca.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index af5cd82..9c8990f 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -1246,7 +1246,7 @@ static int vidioc_enum_frameintervals(struct file *filp,
void *priv,
for (i = 0; i < gspca_dev->cam.mode_framerates[mode].nrates; i++) {
if (fival->index == i) {
- fival->type = V4L2_FRMSIZE_TYPE_DISCRETE;
+ fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
fival->discrete.numerator = 1;
fival->discrete.denominator =
gspca_dev->cam.mode_framerates[mode].rates[i];
--
2.7.0
--
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 6/7] [media] gspca: fix a v4l2-compliance failure during VIDIOC_REQBUFS
When calling VIDIOC_REQBUFS v4l2-compliance fails with this message:
fail: v4l2-test-buffers.cpp(476): q.reqbufs(node, 1)
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
By looking at the v4l2-compliance code the failure happens when trying
to request V4L2_MEMORY_USERPTR buffers without freeing explicitly the
previously allocated V4L2_MEMORY_MMAP buffers.
This would suggest that when changing the memory field in struct
v4l2_requestbuffers the driver is supposed to free automatically any
previous allocated buffers, and looking for inspiration at the code in
drivers/media/v4l2-core/videobuf2-core.c::vb2_core_reqbufs() seems to
confirm this interpretation; however gspca is just returning -EBUSY in
this case.
Removing the special handling for the case of a different memory value
fixes the compliance failure.
Signed-off-by: Antonio Ospite
---
This should be safe, but I'd really like a comment from someone with a more
global knowledge of v4l2.
If my interpretation about how drivers should behave when the value of the
memory field changes is correct, I could send also a documentation update for
Documentation/DocBook/media/v4l/vidioc-reqbufs.xml
Just let me know.
Thanks,
Antonio
drivers/media/usb/gspca/gspca.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index 84b0d6a..915b6c7 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -1402,13 +1402,6 @@ static int vidioc_reqbufs(struct file *file, void *priv,
if (mutex_lock_interruptible(&gspca_dev->queue_lock))
return -ERESTARTSYS;
- if (gspca_dev->memory != GSPCA_MEMORY_NO
- && gspca_dev->memory != GSPCA_MEMORY_READ
- && gspca_dev->memory != rb->memory) {
- ret = -EBUSY;
- goto out;
- }
-
/* only one file may do the capture */
if (gspca_dev->capt_file != NULL
&& gspca_dev->capt_file != file) {
--
2.7.0
--
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 3/7] [media] gspca: rename wxh_to_mode() to wxh_to_nearest_mode()
The name wxh_to_nearest_mode() reflects better what the function does.
Signed-off-by: Antonio Ospite
---
drivers/media/usb/gspca/gspca.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index 9c8990f..1bb7901 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -991,7 +991,7 @@ static void gspca_set_default_mode(struct gspca_dev
*gspca_dev)
v4l2_ctrl_handler_setup(gspca_dev->vdev.ctrl_handler);
}
-static int wxh_to_mode(struct gspca_dev *gspca_dev,
+static int wxh_to_nearest_mode(struct gspca_dev *gspca_dev,
int width, int height)
{
int i;
@@ -1125,8 +1125,8 @@ static int try_fmt_vid_cap(struct gspca_dev *gspca_dev,
PDEBUG_MODE(gspca_dev, D_CONF, "try fmt cap",
fmt->fmt.pix.pixelformat, w, h);
- /* search the closest mode for width and height */
- mode = wxh_to_mode(gspca_dev, w, h);
+ /* search the nearest mode for width and height */
+ mode = wxh_to_nearest_mode(gspca_dev, w, h);
/* OK if right palette */
if (gspca_dev->cam.cam_mode[mode].pixelformat
@@ -1233,7 +1233,7 @@ static int vidioc_enum_frameintervals(struct file *filp,
void *priv,
struct v4l2_frmivalenum *fival)
{
struct gspca_dev *gspca_dev = video_drvdata(filp);
- int mode = wxh_to_mode(gspca_dev, fival->width, fival->height);
+ int mode = wxh_to_nearest_mode(gspca_dev, fival->width, fival->height);
__u32 i;
if (gspca_dev->cam.mode_framerates == NULL ||
--
2.7.0
--
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 4/7] [media] gspca: fix a v4l2-compliance failure about VIDIOC_ENUM_FRAMEINTERVALS
According to v4l2-compliance VIDIOC_ENUM_FRAMEINTERVALS should fail for
unsupported frame sizes, but gspca is too tolerant and tries to find
the frame intervals for the frame size nearest to the requested one.
This makes v4l2-compliance fail with this message:
fail: v4l2-test-formats.cpp(123): \
found frame intervals for invalid size 321x240
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: FAIL
Fix this by using an exact match for the frame size when enumerating
frame intervals, and retuning an error if the frame size for which the
frame intervals have been asked is not supported.
Signed-off-by: Antonio Ospite
---
drivers/media/usb/gspca/gspca.c | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index 1bb7901..61cb16d 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -991,6 +991,19 @@ static void gspca_set_default_mode(struct gspca_dev
*gspca_dev)
v4l2_ctrl_handler_setup(gspca_dev->vdev.ctrl_handler);
}
+static int wxh_to_mode(struct gspca_dev *gspca_dev,
+ int width, int height)
+{
+ int i;
+
+ for (i = 0; i < gspca_dev->cam.nmodes; i++) {
+ if (width == gspca_dev->cam.cam_mode[i].width
+ && height == gspca_dev->cam.cam_mode[i].height)
+ return i;
+ }
+ return -EINVAL;
+}
+
static int wxh_to_nearest_mode(struct gspca_dev *gspca_dev,
int width, int height)
{
@@ -1233,9 +1246,13 @@ static int vidioc_enum_frameintervals(struct file *filp,
void *priv,
struct v4l2_frmivalenum *fival)
{
struct gspca_dev *gspca_dev = video_drvdata(filp);
- int mode = wxh_to_nearest_mode(gspca_dev, fival->width, fival->height);
+ int mode;
__u32 i;
+ mode = wxh_to_mode(gspca_dev, fival->width, fival->height);
+ if (mode < 0)
+ return -EINVAL;
+
if (gspca_dev->cam.mode_framerates == NULL ||
gspca_dev->cam.mode_framerates[mode].nrates == 0)
return -EINVAL;
--
2.7.0
--
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 6/7] [media] gspca: fix a v4l2-compliance failure during VIDIOC_REQBUFS
On Thu, 10 Mar 2016 15:54:37 +0100
Hans de Goede wrote:
> Hi,
>
> On 09-03-16 17:03, Antonio Ospite wrote:
> > When calling VIDIOC_REQBUFS v4l2-compliance fails with this message:
> >
> >fail: v4l2-test-buffers.cpp(476): q.reqbufs(node, 1)
> >test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
> >
> > By looking at the v4l2-compliance code the failure happens when trying
> > to request V4L2_MEMORY_USERPTR buffers without freeing explicitly the
> > previously allocated V4L2_MEMORY_MMAP buffers.
> >
> > This would suggest that when changing the memory field in struct
> > v4l2_requestbuffers the driver is supposed to free automatically any
> > previous allocated buffers, and looking for inspiration at the code in
> > drivers/media/v4l2-core/videobuf2-core.c::vb2_core_reqbufs() seems to
> > confirm this interpretation; however gspca is just returning -EBUSY in
> > this case.
> >
> > Removing the special handling for the case of a different memory value
> > fixes the compliance failure.
> >
> > Signed-off-by: Antonio Ospite
> > ---
> >
> > This should be safe, but I'd really like a comment from someone with a more
> > global knowledge of v4l2.
> >
> > If my interpretation about how drivers should behave when the value of the
> > memory field changes is correct, I could send also a documentation update
> > for
> > Documentation/DocBook/media/v4l/vidioc-reqbufs.xml
> >
> > Just let me know.
> >
> > Thanks,
> > Antonio
> >
> >
> > drivers/media/usb/gspca/gspca.c | 7 ---
> > 1 file changed, 7 deletions(-)
> >
> > diff --git a/drivers/media/usb/gspca/gspca.c
> > b/drivers/media/usb/gspca/gspca.c
> > index 84b0d6a..915b6c7 100644
> > --- a/drivers/media/usb/gspca/gspca.c
> > +++ b/drivers/media/usb/gspca/gspca.c
> > @@ -1402,13 +1402,6 @@ static int vidioc_reqbufs(struct file *file, void
> > *priv,
> > if (mutex_lock_interruptible(&gspca_dev->queue_lock))
> > return -ERESTARTSYS;
> >
> > - if (gspca_dev->memory != GSPCA_MEMORY_NO
> > - && gspca_dev->memory != GSPCA_MEMORY_READ
> > - && gspca_dev->memory != rb->memory) {
> > - ret = -EBUSY;
> > - goto out;
> > - }
> > -
>
> reqbufs is used internally and this change will allow changing
> gspca_dev->memory from USERPTR / MMAP to GSPCA_MEMORY_READ
> please replace this check with a check to only allow
> rb->memory to be GSPCA_MEMORY_READ when coming from GSPCA_MEMORY_NO
> or GSPCA_MEMORY_READ
>
OK, thanks, I'll take a look again later this week.
In the meantime, if patches from 1 to 5 are OK, can we have them merged
so I will just resubmit the last two in the set?
Ciao,
Antonio
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
--
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 7/7] [media] gspca: fix a v4l2-compliance failure during read()
On Thu, 10 Mar 2016 16:59:45 +0100
Hans de Goede wrote:
> Hi,
>
> On 09-03-16 17:03, Antonio Ospite wrote:
> > v4l2-compliance fails with this message:
> >
> >fail: v4l2-test-buffers.cpp(512): Expected EBUSY, got 22
> >test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
> >
> > Looking at the v4l2-compliance code reveals that this failure is about
> > the read() callback.
> >
> > In gspca, dev_read() is calling vidioc_dqbuf() which calls
> > frame_ready_nolock() but the latter returns -EINVAL in a case when
> > v4l2-compliance expects -EBUSY.
> >
> > Fix the failure by changing the return value in frame_ready_nolock().
> >
> > Signed-off-by: Antonio Ospite
> > ---
> > drivers/media/usb/gspca/gspca.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/usb/gspca/gspca.c
> > b/drivers/media/usb/gspca/gspca.c
> > index 915b6c7..de7e300 100644
> > --- a/drivers/media/usb/gspca/gspca.c
> > +++ b/drivers/media/usb/gspca/gspca.c
> > @@ -1664,7 +1664,7 @@ static int frame_ready_nolock(struct gspca_dev
> > *gspca_dev, struct file *file,
> > return -ENODEV;
> > if (gspca_dev->capt_file != file || gspca_dev->memory != memory ||
> > !gspca_dev->streaming)
> > - return -EINVAL;
> > + return -EBUSY;
> >
> > /* check if a frame is ready */
> > return gspca_dev->fr_o != atomic_read(&gspca_dev->fr_i);
>
> I'm not sure that this is the right fix:
>
>
> 1) !gspca_dev->streaming should result in -EINVAL, this is the same as what
> videobuf2 is doing
> 2) gspca_dev->memory != memory should result in -EINVAL
> 3) gspca_dev->capt_file != file means calling dqbuf without having done
> reqbufs (through the same fd)
> which certainly seemes like -EINVAL to me.
>
> The actual problem is that dev_read() is not catching that mmap is being in
> use:
>
> static ssize_t dev_read(struct file *file, char __user *data,
> size_t count, loff_t *ppos)
> {
> ...
> if (gspca_dev->memory == GSPCA_MEMORY_NO) { /* first time ? */
> ret = read_alloc(gspca_dev, file);
> if (ret != 0)
> return ret;
> }
>
> It will skip the read_alloc since gspca_dev->memory is USERPTR or MMAP
> and then do a dqbuf with memory == GSPCA_MEMORY_READ, triggering the
> gspca_dev->memory != memory check.
>
> There are a couple of issues here:
>
> 1) gspca_dev->memory check without holding usb_lock, the taking and
> releasing of usb_lock should be moved from read_alloc() into dev_read()
> itself.
>
> 2) dev_read() should not assume that reading is ok if
> gspca_dev->memory == GSPCA_MEMORY_NO, it needs a:
>
> if (gspca_dev->memory != GSPCA_MEMORY_NO &&
> gspca_dev->memory != GSPCA_MEMORY_READ)
> return -EBUSY;
>
> (while holding the usb_lock so the above is wrong)
>
> 3) If gspca_dev->memory == GSPCA_MEMORY_READ already the
> stream could have been stopped. so we need to check
> gspca_dev->streaming (while holding the usb_lock)
> and do a streamon if it is not set (and then we can
> remove the streamon from read_alloc())
>
> So TL;DR: dev_read needs some love.
>
I'll try to take a look at this too later this week.
> Regards,
>
> Hans
>
>
> p.s.
>
> If you've time to work on v4l2 stuff what gspca really needs
> is to just have its buffer handling ripped out and be rewritten
> to use videobuf2. I would certainly love to see a patch for that.
>
It'd be an interesting tasklet but I don't know when I'll be able to do
that.
Ciao,
Antonio
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
--
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: gstreamer and v4l2
On Tue, 15 Mar 2016 09:10:59 +0200 Ran Shalit wrote: > Hello, > > This is a bit offtopic, so I will understand if you rather not discuss that... > > I am trying to use gstreamer with v4l2 vivi device, > I first check the capabilities with > > gst-launch-1.0 --gst-debug=v4l2src:5 v4l2src device="/dev/video0" ! > fakesink 2>&1 > > and it gives many capabilities such as the following: > > video/x-raw-yuv, format=(string)YUY2, framerate=(fraction)[1/1000, > 1000/1], width=(int) 640, height=(int)180, interlaced=(boolean)true > A cleaner way to enumerate capabilities of a video device in GStreamer is like that: gst-device-monitor-1.0 Video on Debian distributions gst-device-monitor-1.0 is in the gstreamer1.0-plugins-base-apps package. > So I tried to run as following: > > gst-launch-0.10 v4l2src device="/dev/video0" ! > video/x-raw,width=640,height=180,framerate=30 ! autovideosink > > But it keeps giving me auto negotiation error -4. > Trying to give other values did not help neither. BTW, the need for videoconvert is more likely because of the pixelformat rather than the frame dimensions. Ciao ciao, Antonio -- Antonio Ospite http://ao2.it A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? -- 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: [GIT PATCHES FOR 2.6.38] gspca for_2.6.38
On Thu, 13 Jan 2011 11:59:53 +0100 Jean-Francois Moine wrote: > The following changes since commit > 353b61709a555fab9745cb7aea18e1c376c413ce: > > [media] radio-si470x: Always report support for RDS (2011-01-11 14:44:28 > -0200) > > are available in the git repository at: > git://linuxtv.org/jfrancois/gspca.git for_2.6.38 > Hi Jean-François, I had a look at the ov534 changes. > Jean-François Moine (9): [...] > gspca - ov534: Use the new video control mechanism In this commit, is there a reason why you didn't rename also sd_setagc() into setagc() like for the other functions? I am going to test the changes and report back if there's anything more, I like the cleanup tho. Thanks, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? pgp2QgWA1sUVW.pgp Description: PGP signature
Re: [GIT PATCHES FOR 2.6.38] gspca for_2.6.38
On Thu, 13 Jan 2011 17:30:21 +0100 Jean-Francois Moine wrote: > On Thu, 13 Jan 2011 12:38:04 +0100 > Antonio Ospite wrote: > > > > Jean-François Moine (9): > > [...] > > > gspca - ov534: Use the new video control mechanism > > > > In this commit, is there a reason why you didn't rename also > > sd_setagc() into setagc() like for the other functions? > > > > I am going to test the changes and report back if there's anything > > more, I like the cleanup tho. > > Hi Antonio, > > With the new video control mechanism, the '.set_control' function is > called only when capture is active. Otherwise, the '.set' function is > called in any case, and here, it activates/inactivates the auto white > balance control... Oh, I forgot to disable the awb when the agc is > disabled! > So the convention you used for function names is: .set = sd_setFOO; and .set_control = setBAR; right? Tested with guvcview, when toggling the Autoexposure control I get this message: control id: 0x009a0901 failed to set (error -1) control id: 0x009a0901 failed to get value (error -1) Similar error with qv4l2: Error Auto Exposure (1): Invalid argument And because of that the manual Exposure control does not work either. However I verified these errors are also in 2.6.35, so I think your conversion is fine, there must be something else going on; maybe I should open another thread, as there is also the pending issue of changing framerates "live", unrelated to the control changes as well. I hope to be able to look at these issues soon, if nobody else does before. Thanks, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? pgplS6ExHdskm.pgp Description: PGP signature
[PATCH] Add a 10 bpp packed greyscale format.
Add a 10 bits per pixel greyscale format in a packed array representation,
naming it Y10P. Such pixel format is supplied for instance by the Kinect
sensor device.
Signed-off-by: Antonio Ospite
---
Hi, this version should look better than the previous one.
It's not marked as RFC anymore, is it in a submittable state?
Comments from native English speakers always appreciated.
A rendered version of the documentation is _temporarily_ here:
http://shell.studenti.unina.it/~ospite/tmp/V4L2-PIX-FMT-Y10P.html
The main recipient is linux-media, but OpenKinect is on Cc so someone there
could double check the information is actually true.
Regards,
Antonio Ospite
http://ao2.it
Documentation/DocBook/media-entities.tmpl |1 +
Documentation/DocBook/v4l/pixfmt-y10p.xml | 43 +
Documentation/DocBook/v4l/pixfmt.xml |1 +
Documentation/DocBook/v4l/videodev2.h.xml |1 +
include/linux/videodev2.h |1 +
5 files changed, 47 insertions(+), 0 deletions(-)
create mode 100644 Documentation/DocBook/v4l/pixfmt-y10p.xml
diff --git a/Documentation/DocBook/media-entities.tmpl
b/Documentation/DocBook/media-entities.tmpl
index be34dcb..2b18de5 100644
--- a/Documentation/DocBook/media-entities.tmpl
+++ b/Documentation/DocBook/media-entities.tmpl
@@ -253,6 +253,7 @@
+
diff --git a/Documentation/DocBook/v4l/pixfmt-y10p.xml
b/Documentation/DocBook/v4l/pixfmt-y10p.xml
new file mode 100644
index 000..5323ffe
--- /dev/null
+++ b/Documentation/DocBook/v4l/pixfmt-y10p.xml
@@ -0,0 +1,43 @@
+
+
+V4L2_PIX_FMT_Y10P ('Y10P')
+&manvol;
+
+
+V4L2_PIX_FMT_Y10P
+Grey-scale image as a packed array
+
+
+Description
+
+This is a packed grey-scale image format with a depth of 10 bits per
+ pixel. Pixels are stored in a packed array of 10bit bits per pixel, with
+ no padding between them and with the most significant bits coming first
+ from the left.
+
+
+ V4L2_PIX_FMT_Y10P 4 pixel data stream taking
5 bytes
+
+
+ Packed representation
+ pixels cross the byte boundary and have a ratio of 5 bytes for
each 4
+ pixels.
+
+
+
+
+
+ Y'00[9:2]
+
Y'00[1:0]Y'01[9:4]
+
Y'01[3:0]Y'02[9:6]
+
Y'02[5:0]Y'03[9:8]
+ Y'03[7:0]
+
+
+
+
+
+
+
+
+
diff --git a/Documentation/DocBook/v4l/pixfmt.xml
b/Documentation/DocBook/v4l/pixfmt.xml
index d7c4671..3682701 100644
--- a/Documentation/DocBook/v4l/pixfmt.xml
+++ b/Documentation/DocBook/v4l/pixfmt.xml
@@ -592,6 +592,7 @@ information.
&sub-packed-yuv;
&sub-grey;
&sub-y10;
+&sub-y10p;
&sub-y16;
&sub-yuyv;
&sub-uyvy;
diff --git a/Documentation/DocBook/v4l/videodev2.h.xml
b/Documentation/DocBook/v4l/videodev2.h.xml
index 325b23b..773496c 100644
--- a/Documentation/DocBook/v4l/videodev2.h.xml
+++ b/Documentation/DocBook/v4l/videodev2.h.xml
@@ -289,6 +289,7 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_Y4
v4l2_fourcc('Y', '0', '4', ' ') /* 4 Greyscale */
#define V4L2_PIX_FMT_Y6
v4l2_fourcc('Y', '0', '6', ' ') /* 6 Greyscale */
#define V4L2_PIX_FMT_Y10
v4l2_fourcc('Y', '1', '0', ' ') /* 10 Greyscale */
+#define V4L2_PIX_FMT_Y10P
v4l2_fourcc('Y', '1', '0', 'P') /* 10 Greyscale as a packed array */
#define V4L2_PIX_FMT_Y16
v4l2_fourcc('Y', '1', '6', ' ') /* 16 Greyscale */
/* Palette formats */
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 5f6f470..7682581 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -288,6 +288,7 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_Y4 v4l2_fourcc('Y', '0', '4', ' ') /* 4 Greyscale
*/
#define V4L2_PIX_FMT_Y6 v4l2_fourcc('Y', '0', '6', ' ') /* 6 Greyscale
*/
#define V4L2_PIX_FMT_Y10 v4l2_fourcc('Y', '1', '0', ' ') /* 10 Greyscale
*/
+#define V4L2_PIX_FMT_Y10Pv4l2_fourcc('Y', '1', '0', 'P') /* 10 Greyscale
as a packed array */
#define V4L2_PIX_FMT_Y16 v4l2_fourcc('Y', '1', '6', ' ') /* 16 Greyscale
*/
/* Palette formats */
--
1.7.2.3
--
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 v2] Add a 10 bpp packed greyscale format.
Add a 10 bits per pixel greyscale format in a packed array representation,
naming it Y10P. Such pixel format is supplied for instance by the Kinect
sensor device.
Signed-off-by: Antonio Ospite
---
Hi,
Changes since v1:
* Fixed a trailing space, I forgot to run ./scripts/checkpatch.pl on v1
I also added maintainers from ./scripts/get_maintainer.pl to CC, this should
increase the chance for the patch to be noticed.
Please comment on that, so I can send the final version if needed and start
adding support for the format to libv4l.
Thanks,
Antonio Ospite
http://ao2.it
Documentation/DocBook/media-entities.tmpl |1 +
Documentation/DocBook/v4l/pixfmt-y10p.xml | 43 +
Documentation/DocBook/v4l/pixfmt.xml |1 +
Documentation/DocBook/v4l/videodev2.h.xml |1 +
include/linux/videodev2.h |1 +
5 files changed, 47 insertions(+), 0 deletions(-)
create mode 100644 Documentation/DocBook/v4l/pixfmt-y10p.xml
diff --git a/Documentation/DocBook/media-entities.tmpl
b/Documentation/DocBook/media-entities.tmpl
index be34dcb..2b18de5 100644
--- a/Documentation/DocBook/media-entities.tmpl
+++ b/Documentation/DocBook/media-entities.tmpl
@@ -253,6 +253,7 @@
+
diff --git a/Documentation/DocBook/v4l/pixfmt-y10p.xml
b/Documentation/DocBook/v4l/pixfmt-y10p.xml
new file mode 100644
index 000..5323ffe
--- /dev/null
+++ b/Documentation/DocBook/v4l/pixfmt-y10p.xml
@@ -0,0 +1,43 @@
+
+
+V4L2_PIX_FMT_Y10P ('Y10P')
+&manvol;
+
+
+V4L2_PIX_FMT_Y10P
+Grey-scale image as a packed array
+
+
+Description
+
+This is a packed grey-scale image format with a depth of 10 bits per
+ pixel. Pixels are stored in a packed array of 10bit bits per pixel, with
+ no padding between them and with the most significant bits coming first
+ from the left.
+
+
+ V4L2_PIX_FMT_Y10P 4 pixel data stream taking
5 bytes
+
+
+ Packed representation
+ pixels cross the byte boundary and have a ratio of 5 bytes for
each 4
+ pixels.
+
+
+
+
+
+ Y'00[9:2]
+
Y'00[1:0]Y'01[9:4]
+
Y'01[3:0]Y'02[9:6]
+
Y'02[5:0]Y'03[9:8]
+ Y'03[7:0]
+
+
+
+
+
+
+
+
+
diff --git a/Documentation/DocBook/v4l/pixfmt.xml
b/Documentation/DocBook/v4l/pixfmt.xml
index d7c4671..3682701 100644
--- a/Documentation/DocBook/v4l/pixfmt.xml
+++ b/Documentation/DocBook/v4l/pixfmt.xml
@@ -592,6 +592,7 @@ information.
&sub-packed-yuv;
&sub-grey;
&sub-y10;
+&sub-y10p;
&sub-y16;
&sub-yuyv;
&sub-uyvy;
diff --git a/Documentation/DocBook/v4l/videodev2.h.xml
b/Documentation/DocBook/v4l/videodev2.h.xml
index 325b23b..eb39f6b 100644
--- a/Documentation/DocBook/v4l/videodev2.h.xml
+++ b/Documentation/DocBook/v4l/videodev2.h.xml
@@ -289,6 +289,7 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_Y4
v4l2_fourcc('Y', '0', '4', ' ') /* 4 Greyscale */
#define V4L2_PIX_FMT_Y6
v4l2_fourcc('Y', '0', '6', ' ') /* 6 Greyscale */
#define V4L2_PIX_FMT_Y10
v4l2_fourcc('Y', '1', '0', ' ') /* 10 Greyscale */
+#define V4L2_PIX_FMT_Y10P
v4l2_fourcc('Y', '1', '0', 'P') /* 10 Greyscale as a packed array */
#define V4L2_PIX_FMT_Y16
v4l2_fourcc('Y', '1', '6', ' ') /* 16 Greyscale */
/* Palette formats */
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 5f6f470..7682581 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -288,6 +288,7 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_Y4 v4l2_fourcc('Y', '0', '4', ' ') /* 4 Greyscale
*/
#define V4L2_PIX_FMT_Y6 v4l2_fourcc('Y', '0', '6', ' ') /* 6 Greyscale
*/
#define V4L2_PIX_FMT_Y10 v4l2_fourcc('Y', '1', '0', ' ') /* 10 Greyscale
*/
+#define V4L2_PIX_FMT_Y10Pv4l2_fourcc('Y', '1', '0', 'P') /* 10 Greyscale
as a packed array */
#define V4L2_PIX_FMT_Y16 v4l2_fourcc('Y', '1', '6', ' ') /* 16 Greyscale
*/
/* Palette formats */
--
1.7.2.3
--
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 v3 1/4] v4l: add V4L2_PIX_FMT_Y12 format
On Fri, 11 Mar 2011 09:05:46 +0100 Michael Jones wrote: > Signed-off-by: Michael Jones > --- > Documentation/DocBook/v4l/pixfmt-y12.xml | 79 > ++ > include/linux/videodev2.h|1 + > 2 files changed, 80 insertions(+), 0 deletions(-) > create mode 100644 Documentation/DocBook/v4l/pixfmt-y12.xml > Hi Michael, are you going to release also Y12 conversion routines for libv4lconvert? Regards, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? pgphcaBRmszVs.pgp Description: PGP signature
Re: [PATCH v3 1/4] v4l: add V4L2_PIX_FMT_Y12 format
On Fri, 11 Mar 2011 10:38:08 +0100 Michael Jones wrote: > On 03/11/2011 10:21 AM, Antonio Ospite wrote: > > Hi Michael, > > > > are you going to release also Y12 conversion routines for libv4lconvert? > > > > Regards, > >Antonio > > > > Hi Antonio, > > As I am neither a user nor developer of libv4lconvert, I am not planning > on adding Y12 conversion routines there. Hopefully somebody else will > step up. Maybe you? > I asked just for curiosity as I don't have any device producing this Y12 format, however I _might_ play with it if you can provide some Y12 (or Y10) raw frames. I am playing with some compressed variant of Y10 and I am exploring different ways to add support for those formats to libv4l. Regards, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? pgpnjrBzC4wkU.pgp Description: PGP signature
gspca, audio and ov534: regression.
Hi, with 2.6.36-rc6 I can't use the ov534 gspca subdriver (with PS3 eye) anymore, when I try to capture video in dmesg I get: gspca: no transfer endpoint found If I revert commit 35680ba I can make video capture work again but I still don't get the audio device in pulseaudio, it shows up in alsamixer but if I try to select it, on the console I get: cannot load mixer controls: Invalid argument I'll test with latest Jean-François tree, and if it still fails I'll try to find a solution, but I wanted to report it quickly first, I hope we fix this before 2.6.36. Thanks, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? pgpXAx1oGhpGx.pgp Description: PGP signature
Re: gspca, audio and ov534: regression.
On Wed, 6 Oct 2010 13:48:55 +0200
Jean-Francois Moine wrote:
> On Wed, 6 Oct 2010 12:33:21 +0200
> Antonio Ospite wrote:
>
> > with 2.6.36-rc6 I can't use the ov534 gspca subdriver (with PS3 eye)
> > anymore, when I try to capture video in dmesg I get:
> > gspca: no transfer endpoint found
> >
> > If I revert commit 35680ba I can make video capture work again but I
> > still don't get the audio device in pulseaudio, it shows up in
> > alsamixer but if I try to select it, on the console I get:
> > cannot load mixer controls: Invalid argument
> >
[...]
>
> I think I see why the commit prevents the webcam to work: as it is
> done, the choice of the alternate setting does not work with bulk
> transfer. A simple fix could be to also check bulk transfer when
> skipping an alt setting in the function get_ep().
>
Thanks, the following change fixes it, was this what you had in mind?
diff --git a/drivers/media/video/gspca/gspca.c
b/drivers/media/video/gspca/gspca.c
index b984610..30e0b32 100644
--- a/drivers/media/video/gspca/gspca.c
+++ b/drivers/media/video/gspca/gspca.c
@@ -651,7 +651,7 @@ static struct usb_host_endpoint *get_ep(struct gspca_dev
*gspca_dev)
: USB_ENDPOINT_XFER_ISOC;
i = gspca_dev->alt; /* previous alt setting */
if (gspca_dev->cam.reverse_alts) {
- if (gspca_dev->audio)
+ if (gspca_dev->audio && !gspca_dev->cam.bulk)
i++;
while (++i < gspca_dev->nbalt) {
ep = alt_xfer(&intf->altsetting[i], xfer);
@@ -659,7 +659,7 @@ static struct usb_host_endpoint *get_ep(struct gspca_dev
*gspca_dev)
break;
}
} else {
- if (gspca_dev->audio)
+ if (gspca_dev->audio && !gspca_dev->cam.bulk)
i--;
while (--i >= 0) {
ep = alt_xfer(&intf->altsetting[i], xfer);
> About audio stream, I do not see how it can have been broken.
>
PS3 Eye audio is working with linux-2.6.33.7 it is broken in
linux-2.6.35.7 already, I'll try to further narrow down the interval.
Ah, alsamixer doesn't work even when the device is OK in pulseaudio...
> Might you send me the full USB information of your webcam?
>
You can find lsusb output attached.
Thanks,
Antonio
--
Antonio Ospite
http://ao2.it
PGP public key ID: 0x4553B001
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
lsusb_pseye.log
Description: Binary data
pgpkUfYBytPEL.pgp
Description: PGP signature
Re: gspca, audio and ov534: regression.
On Wed, 6 Oct 2010 16:04:41 +0200 Antonio Ospite wrote: > On Wed, 6 Oct 2010 13:48:55 +0200 > Jean-Francois Moine wrote: > > > On Wed, 6 Oct 2010 12:33:21 +0200 > > Antonio Ospite wrote: > > > > > with 2.6.36-rc6 I can't use the ov534 gspca subdriver (with PS3 eye) > > > anymore, when I try to capture video in dmesg I get: > > > gspca: no transfer endpoint found > > > > > > If I revert commit 35680ba I can make video capture work again but I > > > still don't get the audio device in pulseaudio, it shows up in > > > alsamixer but if I try to select it, on the console I get: > > > cannot load mixer controls: Invalid argument > > > [...] > > About audio stream, I do not see how it can have been broken. > > > > PS3 Eye audio is working with linux-2.6.33.7 it is broken in > linux-2.6.35.7 already, I'll try to further narrow down the interval. > Ah, alsamixer doesn't work even when the device is OK in pulseaudio... > I was wrong, the audio part works even in 2.6.36-rc6 but _only_ when the webcam is plugged in from boot, could this have to do with the order gspca and snd-usb-audio are loaded? Regards, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? pgpjmnZzEvzUY.pgp Description: PGP signature
Re: gspca, audio and ov534: regression.
On Thu, 7 Oct 2010 19:44:01 +0200 Jean-Francois Moine wrote: > On Wed, 6 Oct 2010 16:53:37 +0200 > Antonio Ospite wrote: > > > > PS3 Eye audio is working with linux-2.6.33.7 it is broken in > > > linux-2.6.35.7 already, I'll try to further narrow down the > > > interval. Ah, alsamixer doesn't work even when the device is OK in > > > pulseaudio... > > > > I was wrong, the audio part works even in 2.6.36-rc6 but _only_ when > > the webcam is plugged in from boot, could this have to do with the > > order gspca and snd-usb-audio are loaded? > > Hi Antonio, > > If you still have a kernel 2.6.33, may you try my test version (tarball > in my web page)? As it contain only the gspca stuff, this may tell if > the problem is in gspca or elsewhere in the kernel. > JF I suspect the device not showing up in pulseaudio has nothing to do with gspca at all. I can actually record audio using alsa even if pulseaudio does not see the device, so it must be a pulseaudio issue. Thanks, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? pgpVDUyCny86q.pgp Description: PGP signature
Re: gspca, audio and ov534: regression.
On Wed, 6 Oct 2010 20:35:53 +0200 Jean-Francois Moine wrote: > On Wed, 6 Oct 2010 16:04:41 +0200 > Antonio Ospite wrote: > > > Thanks, the following change fixes it, was this what you had in mind? > > > > diff --git a/drivers/media/video/gspca/gspca.c [...] > > Yes, but, after thought, as there is only one alternate setting, the > tests could be: > if (gspca_dev->audio && i < gspca_dev->nbalt - 1) > and > if (gspca_dev->audio && i > 0) > > This should work also for isochronous transfers. JF this change as is does not work for me, if I change the second check to if (gspca_dev->audio && i > 1) it does, but I don't know if this breaks anything else. In my case I have: get_ep: gspca_dev->cam.reverse_alts: 0 get_ep: gspca_dev->alt: 1 get_ep: gspca_dev->nbalt: 1 get_ep: gspca_dev->audio: 1 get_ep: gspca_dev->cam.bulk: 1 Thanks, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? pgpUEGfNvIJAZ.pgp Description: PGP signature
Re: gspca, audio and ov534: regression.
On Sun, 10 Oct 2010 12:21:29 +0200 Jean-Francois Moine wrote: > On Sun, 10 Oct 2010 12:02:50 +0200 > Antonio Ospite wrote: > > > JF this change as is does not work for me, if I change the second > > check to > > if (gspca_dev->audio && i > 1) > > > > it does, but I don't know if this breaks anything else. > > Hi Antonio, > > You are right, this is the way the test must be. > > I'll try to have this in the kernel 2.6.36. > Thanks, feel free to add Reported-by: Antonio Ospite or Tested-by or whatever-by you consider appropriate. Regards, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? pgpNHCzcxdcNu.pgp Description: PGP signature
Re: [GIT PATCHES FOR 2.6.36] gspca for_2.6.36
On Sun, 10 Oct 2010 13:24:47 +0200 Jean-Francois Moine wrote: > The following changes since commit > d65728875a85ac7c8b7d6eb8d51425bacc188980: > > V4L/DVB: v4l: radio: si470x: fix unneeded free_irq() call (2010-09-30 > 07:35:12 -0300) > > are available in the git repository at: > git://linuxtv.org/jfrancois/gspca.git for_2.6.36 > > Jean-François Moine (1): > gspca - main: Fix a regression with the PS3 Eye webcam > Hi, this is not in 2.6.36-rc8, any chance we can make it for 2.6.36? Thanks, Antonio > drivers/media/video/gspca/gspca.c |4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > -- > Ken ar c'hentañ | ** Breizh ha Linux atav! ** > Jef | http://moinejf.free.fr/ > -- > 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 > -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? pgpz9ZBF3yqXo.pgp Description: PGP signature
Re: [GIT PATCHES FOR 2.6.36] gspca for_2.6.36
On Fri, 15 Oct 2010 09:41:48 +0200 Antonio Ospite wrote: > On Sun, 10 Oct 2010 13:24:47 +0200 > Jean-Francois Moine wrote: > > > The following changes since commit > > d65728875a85ac7c8b7d6eb8d51425bacc188980: > > > > V4L/DVB: v4l: radio: si470x: fix unneeded free_irq() call (2010-09-30 > > 07:35:12 -0300) > > > > are available in the git repository at: > > git://linuxtv.org/jfrancois/gspca.git for_2.6.36 > > > > Jean-François Moine (1): > > gspca - main: Fix a regression with the PS3 Eye webcam > > > > Hi, this is not in 2.6.36-rc8, any chance we can make it for 2.6.36? Ping. -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? pgpfwps0JSK1U.pgp Description: PGP signature
[RFC, PATCH] gspca: implement vidioc_enum_frameintervals
40x480 */
+ .rates = vga_rates,
+ .nrates = ARRAY_SIZE(vga_rates),
+ },
+};
+
static const struct v4l2_pix_format ov965x_mode[] = {
{320, 240, V4L2_PIX_FMT_JPEG, V4L2_FIELD_NONE,
.bytesperline = 320,
@@ -1411,6 +1425,7 @@
if (sd->sensor == SENSOR_OV772X) {
cam->cam_mode = ov772x_mode;
cam->nmodes = ARRAY_SIZE(ov772x_mode);
+ cam->mode_framerates = ov772x_framerates;
cam->bulk = 1;
cam->bulk_size = 16384;
--
Antonio Ospite
http://ao2.it
PGP public key ID: 0x4553B001
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?
pgpVAX7oIGnxc.pgp
Description: PGP signature
[PATCH 0/3] pxa_camera: remove init() callback
Hi, this series removes the init() callback from pxa_camera_platform_data, and fixes its users to do initialization statically at machine init time. Guennadi requested this change because there seems to be no use cases for dynamic initialization. I also believe that the current semantics for this init() callback is ambiguous anyways, it is invoked in pxa_camera_activate(), hence at device node open, but its users use it like a generic initialization to be done at module init time (configure MFP, request GPIOs for *sensor* control). So removing it is definitely good. As a side note, If we were really exposing some dynamic and generic initialization, this could be done in soc-camera itself, not in pxa_camera anyways. Thanks, Antonio Antonio Ospite (3): em-x270: don't use pxa_camera init() callback pcm990-baseboard: don't use pxa_camera init() callback pxa_camera: remove init() callback arch/arm/mach-pxa/em-x270.c |9 + arch/arm/mach-pxa/include/mach/camera.h |2 -- arch/arm/mach-pxa/pcm990-baseboard.c|8 +--- drivers/media/video/pxa_camera.c| 10 -- 4 files changed, 6 insertions(+), 23 deletions(-) -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? -- 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 1/3] em-x270: don't use pxa_camera init() callback
pxa_camera init() is going to be removed.
Signed-off-by: Antonio Ospite
---
arch/arm/mach-pxa/em-x270.c |9 +
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-pxa/em-x270.c b/arch/arm/mach-pxa/em-x270.c
index aec7f42..f71f34c 100644
--- a/arch/arm/mach-pxa/em-x270.c
+++ b/arch/arm/mach-pxa/em-x270.c
@@ -967,7 +967,7 @@ static inline void em_x270_init_gpio_keys(void) {}
#if defined(CONFIG_VIDEO_PXA27x) || defined(CONFIG_VIDEO_PXA27x_MODULE)
static struct regulator *em_x270_camera_ldo;
-static int em_x270_sensor_init(struct device *dev)
+static int em_x270_sensor_init(void)
{
int ret;
@@ -996,7 +996,6 @@ static int em_x270_sensor_init(struct device *dev)
}
struct pxacamera_platform_data em_x270_camera_platform_data = {
- .init = em_x270_sensor_init,
.flags = PXA_CAMERA_MASTER | PXA_CAMERA_DATAWIDTH_8 |
PXA_CAMERA_PCLK_EN | PXA_CAMERA_MCLK_EN,
.mclk_10khz = 2600,
@@ -1049,8 +1048,10 @@ static struct platform_device em_x270_camera = {
static void __init em_x270_init_camera(void)
{
- pxa_set_camera_info(&em_x270_camera_platform_data);
- platform_device_register(&em_x270_camera);
+ if (em_x270_sensor_init() == 0) {
+ pxa_set_camera_info(&em_x270_camera_platform_data);
+ platform_device_register(&em_x270_camera);
+ }
}
#else
static inline void em_x270_init_camera(void) {}
--
1.6.5.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
[PATCH 2/3] pcm990-baseboard: don't use pxa_camera init() callback
pxa_camera init() is going to be removed.
Configure PXA CIF pins directly in machine init function.
Signed-off-by: Antonio Ospite
---
arch/arm/mach-pxa/pcm990-baseboard.c |8 +---
1 files changed, 1 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-pxa/pcm990-baseboard.c
b/arch/arm/mach-pxa/pcm990-baseboard.c
index bbda570..d5255ae 100644
--- a/arch/arm/mach-pxa/pcm990-baseboard.c
+++ b/arch/arm/mach-pxa/pcm990-baseboard.c
@@ -359,19 +359,12 @@ static unsigned long pcm990_camera_pin_config[] = {
GPIO44_CIF_LV,
};
-static int pcm990_pxacamera_init(struct device *dev)
-{
- pxa2xx_mfp_config(ARRAY_AND_SIZE(pcm990_camera_pin_config));
- return 0;
-}
-
/*
* CICR4: PCLK_EN: Pixel clock is supplied by the sensor
* MCLK_EN:Master clock is generated by PXA
* PCP:Data sampled on the falling edge of pixel clock
*/
struct pxacamera_platform_data pcm990_pxacamera_platform_data = {
- .init = pcm990_pxacamera_init,
.flags = PXA_CAMERA_MASTER | PXA_CAMERA_DATAWIDTH_8 |
PXA_CAMERA_DATAWIDTH_10 |
PXA_CAMERA_PCLK_EN | PXA_CAMERA_MCLK_EN/* | PXA_CAMERA_PCP*/,
.mclk_10khz = 1000,
@@ -532,6 +525,7 @@ void __init pcm990_baseboard_init(void)
pxa_set_ac97_info(NULL);
#if defined(CONFIG_VIDEO_PXA27x) || defined(CONFIG_VIDEO_PXA27x_MODULE)
+ pxa2xx_mfp_config(ARRAY_AND_SIZE(pcm990_camera_pin_config));
pxa_set_camera_info(&pcm990_pxacamera_platform_data);
i2c_register_board_info(0, ARRAY_AND_SIZE(pcm990_i2c_devices));
--
1.6.5.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
[PATCH 3/3] pxa_camera: remove init() callback
pxa_camera init() callback is sometimes abused to setup MFP for PXA CIF, or
even to request GPIOs to be used by the camera *sensor*. These initializations
can be performed statically in machine init functions.
The current semantics for this init() callback is ambiguous anyways, it is
invoked in pxa_camera_activate(), hence at device node open, but its users use
it like a generic initialization to be done at module init time (configure
MFP, request GPIOs for *sensor* control).
Signed-off-by: Antonio Ospite
---
arch/arm/mach-pxa/include/mach/camera.h |2 --
drivers/media/video/pxa_camera.c| 10 --
2 files changed, 0 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mach-pxa/include/mach/camera.h
b/arch/arm/mach-pxa/include/mach/camera.h
index 31abe6d..6709b1c 100644
--- a/arch/arm/mach-pxa/include/mach/camera.h
+++ b/arch/arm/mach-pxa/include/mach/camera.h
@@ -35,8 +35,6 @@
#define PXA_CAMERA_VSP 0x400
struct pxacamera_platform_data {
- int (*init)(struct device *);
-
unsigned long flags;
unsigned long mclk_10khz;
};
diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
index 51b683c..49f2bf9 100644
--- a/drivers/media/video/pxa_camera.c
+++ b/drivers/media/video/pxa_camera.c
@@ -882,18 +882,8 @@ static void recalculate_fifo_timeout(struct pxa_camera_dev
*pcdev,
static void pxa_camera_activate(struct pxa_camera_dev *pcdev)
{
- struct pxacamera_platform_data *pdata = pcdev->pdata;
- struct device *dev = pcdev->soc_host.v4l2_dev.dev;
u32 cicr4 = 0;
- dev_dbg(dev, "Registered platform device at %p data %p\n",
- pcdev, pdata);
-
- if (pdata && pdata->init) {
- dev_dbg(dev, "%s: Init gpios\n", __func__);
- pdata->init(dev);
- }
-
/* disable all interrupts */
__raw_writel(0x3ff, pcdev->base + CICR0);
--
1.6.5.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 1/3] em-x270: don't use pxa_camera init() callback
On Wed, 18 Nov 2009 11:10:06 +0100 (CET) Guennadi Liakhovetski wrote: > On Tue, 17 Nov 2009, Antonio Ospite wrote: > > > pxa_camera init() is going to be removed. > > My nitpick here would be - I would put it the other way round. We do not > remove .init() in platforms, because it is going to be removed, but rather > we perform initialisation statically, because we think this is better so, > and then .init becomes useless and gets removed. > TBH, I am persuaded that the current use of init() is ambiguous /per se/ and so we'd just better not use it at all. If static initialization for sensor GPIOs is better, well I just trust you on that. However, the point here is not about static/dynamic initialization, it is more about pxa_camera init() used one time to configure MFP pins, and another time to request resources for the *sensor*, and in both cases (mis)used as it was going to be called at _module_init_ time only, which it wasn't. So, can you see why I consider these changes (patches 1 and 2) as merely functional to the removal of init() from pxa_camera? Anyhow, if you don't like references to a future change without an explanation I can arrange something in commit messages for the first two patches :) Regards, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? pgpkSb3XVKB6i.pgp Description: PGP signature
Re: [RFC, PATCH] gspca: implement vidioc_enum_frameintervals
On Thu, 19 Nov 2009 09:08:22 +0100 Hans de Goede wrote: > Hi, > Hi, thanks for commenting on this. > On 11/17/2009 11:41 AM, Antonio Ospite wrote: > > Hi, > > > > gspca does not implement vidioc_enum_frameintervals yet, so even if a > > camera can support multiple frame rates (or frame intervals) there is > > still no way to enumerate them from userspace. > > > > The following is just a quick and dirty implementation to show the > > problem and to have something to base the discussion on. In the patch > > there is also a working example of use with the ov534 subdriver. > > > > Someone with a better knowledge of gspca and v4l internals can suggest > > better solutions. > > > > > Does the ov534 driver actually support selecting a framerate from the > list this patch adds, and does it then honor the selection ? > Yes it does, it can set framerates as per the list I added (in fact I got the list looking at what the driver supports), and I can also see it honors the framerate setting, from guvcview fps counter in the capture window title. So only framerate enumeration is missing. > In my experience framerates with webcams are varying all the time, as > the lighting conditions change and the cam needs to change its exposure > setting to match, resulting in changed framerates. > > So to me this does not seem very useful for webcams. > As long as the chips involved (bridge, ISP, sensor) are powerful or smart enough then the camera won't have problems. I guess that for ov534/ov538 the usb bandwidth is the limiting factor for the framerates, as we are using a raw format. > Regards, > > Hans Btw, did you take a look at the patch anyway? Can you suggest a better place where to put the structures needed for this functionality? Regards, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? pgpcBdhd80CUR.pgp Description: PGP signature
Re: [PATCH 3/3] pxa_camera: remove init() callback
On Fri, 27 Nov 2009 15:06:53 +0100 (CET) Guennadi Liakhovetski wrote: > On Tue, 17 Nov 2009, Antonio Ospite wrote: > > > pxa_camera init() callback is sometimes abused to setup MFP for PXA CIF, or > > even to request GPIOs to be used by the camera *sensor*. These > > initializations > > can be performed statically in machine init functions. > > > > The current semantics for this init() callback is ambiguous anyways, it is > > invoked in pxa_camera_activate(), hence at device node open, but its users > > use > > it like a generic initialization to be done at module init time (configure > > MFP, request GPIOs for *sensor* control). > > > > Signed-off-by: Antonio Ospite > > Antonio, to make the merging easier and avoid imposing extra dependencies, > I would postpone this to 2.6.34, and just remove uses of .init() by > pxa-camera users as per your other two patches. Would this be ok with you? > > Thanks > Guennadi > Perfectly fine with me. Feel also free to anticipate me and edit the commit messages to whatever you want in the first two patches. Now that we aren't removing init() immediately after these it makes even more sense to change the phrasing from a future referencing "init() is going to be removed" to a more present focused "better not to use init() at all" form. Thanks, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? pgpWQ1eKqkrY5.pgp Description: PGP signature
Re: [PATCH 3/3] pxa_camera: remove init() callback
On Fri, 27 Nov 2009 15:37:19 +0100 (CET) Guennadi Liakhovetski wrote: > On Fri, 27 Nov 2009, Antonio Ospite wrote: > > > On Fri, 27 Nov 2009 15:06:53 +0100 (CET) > > Guennadi Liakhovetski wrote: > > > > > On Tue, 17 Nov 2009, Antonio Ospite wrote: > > > > > > > pxa_camera init() callback is sometimes abused to setup MFP for PXA > > > > CIF, or > > > > even to request GPIOs to be used by the camera *sensor*. These > > > > initializations > > > > can be performed statically in machine init functions. > > > > > > > > The current semantics for this init() callback is ambiguous anyways, it > > > > is > > > > invoked in pxa_camera_activate(), hence at device node open, but its > > > > users use > > > > it like a generic initialization to be done at module init time > > > > (configure > > > > MFP, request GPIOs for *sensor* control). > > > > > > > > Signed-off-by: Antonio Ospite > > > > > > Antonio, to make the merging easier and avoid imposing extra > > > dependencies, > > > I would postpone this to 2.6.34, and just remove uses of .init() by > > > pxa-camera users as per your other two patches. Would this be ok with you? > > > > > > Thanks > > > Guennadi > > > > > > > Perfectly fine with me. > > > > Feel also free to anticipate me and edit the commit messages to > > whatever you want in the first two patches. Now that we aren't removing > > init() immediately after these it makes even more sense to change the > > phrasing from a future referencing > > "init() is going to be removed" > > to a more present focused > > "better not to use init() at all" > > form. > > I cannot edit those subject lines, because I will not be handling those > patches, they will go via the PXA tree, that's why it is easier to wait > with the pxa patch. > I see, I am sending a v2 for the first two patches with changed commit messages in some hours then. Sorry for the delay. > Thanks > Guennadi > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ Regards, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? pgpELo6MV1WoT.pgp Description: PGP signature
[PATCH 1/3 v2] em-x270: don't use pxa_camera init() callback
pxa_camera init() is ambiguous, it's better to statically configure the sensor.
Signed-off-by: Antonio Ospite
Acked-by: Mike Rapoport
---
The only change from previous version is the commit message, we don't want to
mention .init() removal yet. Since the code is not changed the ack from Mike
Rapoport still holds.
arch/arm/mach-pxa/em-x270.c |9 +
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-pxa/em-x270.c b/arch/arm/mach-pxa/em-x270.c
index aec7f42..f71f34c 100644
--- a/arch/arm/mach-pxa/em-x270.c
+++ b/arch/arm/mach-pxa/em-x270.c
@@ -967,7 +967,7 @@ static inline void em_x270_init_gpio_keys(void) {}
#if defined(CONFIG_VIDEO_PXA27x) || defined(CONFIG_VIDEO_PXA27x_MODULE)
static struct regulator *em_x270_camera_ldo;
-static int em_x270_sensor_init(struct device *dev)
+static int em_x270_sensor_init(void)
{
int ret;
@@ -996,7 +996,6 @@ static int em_x270_sensor_init(struct device *dev)
}
struct pxacamera_platform_data em_x270_camera_platform_data = {
- .init = em_x270_sensor_init,
.flags = PXA_CAMERA_MASTER | PXA_CAMERA_DATAWIDTH_8 |
PXA_CAMERA_PCLK_EN | PXA_CAMERA_MCLK_EN,
.mclk_10khz = 2600,
@@ -1049,8 +1048,10 @@ static struct platform_device em_x270_camera = {
static void __init em_x270_init_camera(void)
{
- pxa_set_camera_info(&em_x270_camera_platform_data);
- platform_device_register(&em_x270_camera);
+ if (em_x270_sensor_init() == 0) {
+ pxa_set_camera_info(&em_x270_camera_platform_data);
+ platform_device_register(&em_x270_camera);
+ }
}
#else
static inline void em_x270_init_camera(void) {}
--
1.6.5.3
--
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 2/3 v2] pcm990-baseboard: don't use pxa_camera init() callback
pxa_camera init() is ambiguous, it's better to configure PXA CIF pins
statically in machine init function.
Signed-off-by: Antonio Ospite
---
The only change from previous version is the commit message, we don't want to
mention .init() removal yet.
arch/arm/mach-pxa/pcm990-baseboard.c |8 +---
1 files changed, 1 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-pxa/pcm990-baseboard.c
b/arch/arm/mach-pxa/pcm990-baseboard.c
index bbda570..d5255ae 100644
--- a/arch/arm/mach-pxa/pcm990-baseboard.c
+++ b/arch/arm/mach-pxa/pcm990-baseboard.c
@@ -359,19 +359,12 @@ static unsigned long pcm990_camera_pin_config[] = {
GPIO44_CIF_LV,
};
-static int pcm990_pxacamera_init(struct device *dev)
-{
- pxa2xx_mfp_config(ARRAY_AND_SIZE(pcm990_camera_pin_config));
- return 0;
-}
-
/*
* CICR4: PCLK_EN: Pixel clock is supplied by the sensor
* MCLK_EN:Master clock is generated by PXA
* PCP:Data sampled on the falling edge of pixel clock
*/
struct pxacamera_platform_data pcm990_pxacamera_platform_data = {
- .init = pcm990_pxacamera_init,
.flags = PXA_CAMERA_MASTER | PXA_CAMERA_DATAWIDTH_8 |
PXA_CAMERA_DATAWIDTH_10 |
PXA_CAMERA_PCLK_EN | PXA_CAMERA_MCLK_EN/* | PXA_CAMERA_PCP*/,
.mclk_10khz = 1000,
@@ -532,6 +525,7 @@ void __init pcm990_baseboard_init(void)
pxa_set_ac97_info(NULL);
#if defined(CONFIG_VIDEO_PXA27x) || defined(CONFIG_VIDEO_PXA27x_MODULE)
+ pxa2xx_mfp_config(ARRAY_AND_SIZE(pcm990_camera_pin_config));
pxa_set_camera_info(&pcm990_pxacamera_platform_data);
i2c_register_board_info(0, ARRAY_AND_SIZE(pcm990_i2c_devices));
--
1.6.5.3
--
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 0/3] pxa_camera: remove init() callback
On Tue, 17 Nov 2009 23:04:20 +0100 Antonio Ospite wrote: > Hi, > > this series removes the init() callback from pxa_camera_platform_data, and > fixes its users to do initialization statically at machine init time. > [...] > Antonio Ospite (3): > em-x270: don't use pxa_camera init() callback > pcm990-baseboard: don't use pxa_camera init() callback Eric, if Guennadi ACKs v2 for these two please apply them only, we are postponing the third one, hence you can discard it. > pxa_camera: remove init() callback > Thanks, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? pgpuOgQu287IB.pgp Description: PGP signature
[PATCH] gspca: implement vidioc_enum_frameintervals
Some drivers support multiple frameintervals (framerates), make gspca able to
enumerate them.
Signed-off-by: Antonio Ospite
---
Changes since the initial RFC version:
- 'i' is now a __u32 and the variable 'index' has been dropped.
- some more comments in gspca.h
Thanks to Hans de Goede for the review.
For now I am postponing the patch to ov534 which uses this one, because I am
verifying what the actual framerates supported by the subdriver are.
Thanks,
Antonio Ospite
diff -r 39c1be9a2ff8 -r ef8cca705478 linux/drivers/media/video/gspca/gspca.c
--- a/linux/drivers/media/video/gspca/gspca.c Tue Dec 01 11:20:34 2009 +0100
+++ b/linux/drivers/media/video/gspca/gspca.c Tue Dec 01 13:15:39 2009 +0100
@@ -998,6 +998,34 @@
return -EINVAL;
}
+static int vidioc_enum_frameintervals(struct file *filp, void *priv,
+ struct v4l2_frmivalenum *fival)
+{
+ struct gspca_dev *gspca_dev = priv;
+ int mode = wxh_to_mode(gspca_dev, fival->width, fival->height);
+ __u32 i;
+
+ if (gspca_dev->cam.mode_framerates == NULL ||
+ gspca_dev->cam.mode_framerates[mode].nrates == 0)
+ return -EINVAL;
+
+ if (fival->pixel_format !=
+ gspca_dev->cam.cam_mode[mode].pixelformat)
+ return -EINVAL;
+
+ for (i = 0; i < gspca_dev->cam.mode_framerates[mode].nrates; i++) {
+ if (fival->index == i) {
+ fival->type = V4L2_FRMSIZE_TYPE_DISCRETE;
+ fival->discrete.numerator = 1;
+ fival->discrete.denominator =
+ gspca_dev->cam.mode_framerates[mode].rates[i];
+ return 0;
+ }
+ }
+
+ return -EINVAL;
+}
+
static void gspca_release(struct video_device *vfd)
{
struct gspca_dev *gspca_dev = container_of(vfd, struct gspca_dev, vdev);
@@ -1990,6 +2018,7 @@
.vidioc_g_parm = vidioc_g_parm,
.vidioc_s_parm = vidioc_s_parm,
.vidioc_enum_framesizes = vidioc_enum_framesizes,
+ .vidioc_enum_frameintervals = vidioc_enum_frameintervals,
#ifdef CONFIG_VIDEO_ADV_DEBUG
.vidioc_g_register = vidioc_g_register,
.vidioc_s_register = vidioc_s_register,
diff -r 39c1be9a2ff8 -r ef8cca705478 linux/drivers/media/video/gspca/gspca.h
--- a/linux/drivers/media/video/gspca/gspca.h Tue Dec 01 11:20:34 2009 +0100
+++ b/linux/drivers/media/video/gspca/gspca.h Tue Dec 01 13:15:39 2009 +0100
@@ -45,11 +45,20 @@
/* image transfers */
#define MAX_NURBS 4/* max number of URBs */
+
+/* used to list framerates supported by a camera mode (resolution) */
+struct framerates {
+ int *rates;
+ int nrates;
+};
+
/* device information - set at probe time */
struct cam {
int bulk_size; /* buffer size when image transfer by bulk */
const struct v4l2_pix_format *cam_mode; /* size nmodes */
char nmodes;
+ const struct framerates *mode_framerates; /* must have size nmode,
+ * just like cam_mode */
__u8 bulk_nurbs;/* number of URBs in bulk mode
* - cannot be > MAX_NURBS
* - when 0 and bulk_size != 0 means
--
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] ov534: allow enumerating supported framerates
Signed-off-by: Antonio Ospite
---
Historical note:
This has been re-tested on a reliable machine and it works from guvcview for
all the framerates; on my old PC I am still having problems with 640x...@60fps
_regardless_ of this change, so it must be a USB problem.
Thanks,
Antonio
Index: gspca/linux/drivers/media/video/gspca/ov534.c
===
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -282,6 +282,21 @@
.priv = 0},
};
+static const int qvga_rates[] = {125, 100, 75, 60, 50, 40, 30};
+static const int vga_rates[] = {60, 50, 40, 30, 15};
+
+static const struct framerates ov772x_framerates[] = {
+ { /* 320x240 */
+ .rates = qvga_rates,
+ .nrates = ARRAY_SIZE(qvga_rates),
+ },
+ { /* 640x480 */
+ .rates = vga_rates,
+ .nrates = ARRAY_SIZE(vga_rates),
+ },
+};
+
+
static const u8 bridge_init[][2] = {
{ 0xc2, 0x0c },
{ 0x88, 0xf8 },
@@ -799,6 +814,7 @@
cam->cam_mode = ov772x_mode;
cam->nmodes = ARRAY_SIZE(ov772x_mode);
+ cam->mode_framerates = ov772x_framerates;
cam->bulk = 1;
cam->bulk_size = 16384;
--
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] ov534: allow enumerating supported framerates
On Sat, 9 Jan 2010 01:41:31 +0100 Antonio Ospite wrote: > Signed-off-by: Antonio Ospite > > --- > > Historical note: > > This has been re-tested on a reliable machine and it works from guvcview for > all the framerates; on my old PC I am still having problems with 640x...@60fps > _regardless_ of this change, so it must be a USB problem. > > Thanks, > Antonio Ping? Jean-Francois. -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? pgpA8KwH8LYS8.pgp Description: PGP signature
Re: [PATCH] ov534: allow enumerating supported framerates
On Sat, 9 Jan 2010 01:41:31 +0100
Antonio Ospite wrote:
> Index: gspca/linux/drivers/media/video/gspca/ov534.c
> ===
> --- gspca.orig/linux/drivers/media/video/gspca/ov534.c
> +++ gspca/linux/drivers/media/video/gspca/ov534.c
> @@ -282,6 +282,21 @@
>.priv = 0},
> };
>
> +static const int qvga_rates[] = {125, 100, 75, 60, 50, 40, 30};
> +static const int vga_rates[] = {60, 50, 40, 30, 15};
> +
Hmm, after double checking compilation messages, having these as 'const'
produces two:
warning: initialization discards qualifiers from pointer target type
in the assignments below.
If I remove the 'const' qualifiers here, the messages go away, so I'd
say we can do also without them. If that's ok I'll send a v2 soon,
sorry.
Thanks,
Antonio
> +static const struct framerates ov772x_framerates[] = {
> + { /* 320x240 */
> + .rates = qvga_rates,
> + .nrates = ARRAY_SIZE(qvga_rates),
> + },
> + { /* 640x480 */
> + .rates = vga_rates,
> + .nrates = ARRAY_SIZE(vga_rates),
> + },
> +};
> +
> +
> static const u8 bridge_init[][2] = {
> { 0xc2, 0x0c },
> { 0x88, 0xf8 },
> @@ -799,6 +814,7 @@
>
> cam->cam_mode = ov772x_mode;
> cam->nmodes = ARRAY_SIZE(ov772x_mode);
> + cam->mode_framerates = ov772x_framerates;
>
> cam->bulk = 1;
> cam->bulk_size = 16384;
--
Antonio Ospite
http://ao2.it
PGP public key ID: 0x4553B001
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?
pgp4D1q0L3AVD.pgp
Description: PGP signature
Re: [PATCH] ov534: allow enumerating supported framerates
On Sat, 16 Jan 2010 17:47:49 +0100 Jean-Francois Moine wrote: > Hi Antonio, > > I recoded your patch with some changes, mainly in gspca.h. If it is > OK for you, may you sign it? > Ok, that's even better. Signed-off-by: Antonio Ospite > Best regards. > > -- > Ken ar c'hentañ | ** Breizh ha Linux atav! ** > Jef | http://moinejf.free.fr/ Thanks, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? pgpY6YNSsoTvG.pgp Description: PGP signature
[PATCH] ov534: fix end of frame handling, make the camera work again.
Fix a regression, probably introduced in the driver split, which made
the ov534 driver unusable: every last packet was discarded because we
were mis-calculating the frame size before actually adding the packet.
Plus, the debug message should reflect that we discard also packets
beyond the expected frame size.
Signed-off-by: Antonio Ospite
Cc: Max Thrun
---
Max, can you test this as well? This should be better than removing all
the discard logic at once. After this is in I'll keep working on your
changes.
Jean-Francois, if this is proven to be the right thing to do, it should
go mainline along with the driver split.
Thanks,
Antonio Ospite
Index: gspca/linux/drivers/media/video/gspca/ov534.c
===
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -992,9 +992,9 @@
frame = gspca_get_i_frame(gspca_dev);
if (frame == NULL)
goto discard;
- if (frame->data_end - frame->data !=
+ if (frame->data_end - frame->data + (len - 12) !=
gspca_dev->width * gspca_dev->height * 2) {
- PDEBUG(D_PACK, "short frame");
+ PDEBUG(D_PACK, "wrong sized frame");
goto discard;
}
gspca_frame_add(gspca_dev, LAST_PACKET,
--
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 1/2] Add Y10B, a 10 bpp bit-packed greyscale format.
Add a 10 bits per pixel greyscale format in a bit-packed array representation,
naming it Y10B. Such pixel format is supplied for instance by the Kinect
sensor device.
Cc: Steven Toth
Signed-off-by: Antonio Ospite
---
Documentation/DocBook/media-entities.tmpl |1 +
Documentation/DocBook/v4l/pixfmt-y10b.xml | 43 +
Documentation/DocBook/v4l/pixfmt.xml |1 +
Documentation/DocBook/v4l/videodev2.h.xml |3 ++
include/linux/videodev2.h |3 ++
5 files changed, 51 insertions(+), 0 deletions(-)
create mode 100644 Documentation/DocBook/v4l/pixfmt-y10b.xml
diff --git a/Documentation/DocBook/media-entities.tmpl
b/Documentation/DocBook/media-entities.tmpl
index 5d259c6..2dead20 100644
--- a/Documentation/DocBook/media-entities.tmpl
+++ b/Documentation/DocBook/media-entities.tmpl
@@ -294,6 +294,7 @@
+
diff --git a/Documentation/DocBook/v4l/pixfmt-y10b.xml
b/Documentation/DocBook/v4l/pixfmt-y10b.xml
new file mode 100644
index 000..adb0ad8
--- /dev/null
+++ b/Documentation/DocBook/v4l/pixfmt-y10b.xml
@@ -0,0 +1,43 @@
+
+
+V4L2_PIX_FMT_Y10BPACK ('Y10B')
+&manvol;
+
+
+V4L2_PIX_FMT_Y10BPACK
+Grey-scale image as a bit-packed array
+
+
+Description
+
+This is a packed grey-scale image format with a depth of 10 bits per
+ pixel. Pixels are stored in a bit-packed array of 10bit bits per pixel,
+ with no padding between them and with the most significant bits coming
+ first from the left.
+
+
+ V4L2_PIX_FMT_Y10BPACK 4 pixel data stream
taking 5 bytes
+
+
+ Bit-packed representation
+ pixels cross the byte boundary and have a ratio of 5 bytes for
each 4
+ pixels.
+
+
+
+
+
+ Y'00[9:2]
+
Y'00[1:0]Y'01[9:4]
+
Y'01[3:0]Y'02[9:6]
+
Y'02[5:0]Y'03[9:8]
+ Y'03[7:0]
+
+
+
+
+
+
+
+
+
diff --git a/Documentation/DocBook/v4l/pixfmt.xml
b/Documentation/DocBook/v4l/pixfmt.xml
index c6fdcbb..2e824a3 100644
--- a/Documentation/DocBook/v4l/pixfmt.xml
+++ b/Documentation/DocBook/v4l/pixfmt.xml
@@ -696,6 +696,7 @@ information.
&sub-packed-yuv;
&sub-grey;
&sub-y10;
+&sub-y10b;
&sub-y16;
&sub-yuyv;
&sub-uyvy;
diff --git a/Documentation/DocBook/v4l/videodev2.h.xml
b/Documentation/DocBook/v4l/videodev2.h.xml
index 2b796a2..937acf5 100644
--- a/Documentation/DocBook/v4l/videodev2.h.xml
+++ b/Documentation/DocBook/v4l/videodev2.h.xml
@@ -311,6 +311,9 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_Y10
v4l2_fourcc('Y', '1', '0', ' ') /* 10 Greyscale */
#define V4L2_PIX_FMT_Y16
v4l2_fourcc('Y', '1', '6', ' ') /* 16 Greyscale */
+/* Grey bit-packed formats */
+#define V4L2_PIX_FMT_Y10BPACK
v4l2_fourcc('Y', '1', '0', 'B') /* 10 Greyscale bit-packed */
+
/* Palette formats */
#define V4L2_PIX_FMT_PAL8
v4l2_fourcc('P', 'A', 'L', '8') /* 8 8-bit palette */
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index aa6c393..38575ae 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -310,6 +310,9 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_Y10 v4l2_fourcc('Y', '1', '0', ' ') /* 10 Greyscale
*/
#define V4L2_PIX_FMT_Y16 v4l2_fourcc('Y', '1', '6', ' ') /* 16 Greyscale
*/
+/* Grey bit-packed formats */
+#define V4L2_PIX_FMT_Y10BPACKv4l2_fourcc('Y', '1', '0', 'B') /* 10
Greyscale bit-packed */
+
/* Palette formats */
#define V4L2_PIX_FMT_PAL8v4l2_fourcc('P', 'A', 'L', '8') /* 8 8-bit
palette */
--
1.7.4.1
--
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 0/2] Add a GSPCA subdriver for the Microsoft Kinect sensor
Hi, This patchset adds support for using the Kinect[1] sensor device as a regular Webcam or as a IR camera. [1] http://en.wikipedia.org/wiki/Kinect The first patch adds a new Y10B pixelformat used to expose the raw IR data the sensor can provide, the second patch adds a gspca subdriver for the Kinect sensor. There was some positive feedback about calling the new format Y10B from Hans Verkuil, Mauro Carvalho Chehab and Guennadi Liakhovetski. Any comment is appreciated. Thanks, Antonio Antonio Ospite (2): Add Y10B, a 10 bpp bit-packed greyscale format. gspca: add a subdriver for the Microsoft Kinect sensor device Documentation/DocBook/media-entities.tmpl |1 + Documentation/DocBook/v4l/pixfmt-y10b.xml | 43 +++ Documentation/DocBook/v4l/pixfmt.xml |1 + Documentation/DocBook/v4l/videodev2.h.xml |3 + drivers/media/video/gspca/Kconfig |9 + drivers/media/video/gspca/Makefile|2 + drivers/media/video/gspca/kinect.c| 427 + include/linux/videodev2.h |3 + 8 files changed, 489 insertions(+), 0 deletions(-) create mode 100644 Documentation/DocBook/v4l/pixfmt-y10b.xml create mode 100644 drivers/media/video/gspca/kinect.c -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? -- 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 2/2] gspca: add a subdriver for the Microsoft Kinect sensor device
The Kinect sensor is a device used by Microsoft for its Kinect project,
which is a system for controller-less Human-Computer interaction
targeted for Xbox 360.
In the Kinect device, RGBD data is captured from two distinct sensors: a
regular RGB sensor and a monochrome sensor which, with the aid of a IR
structured light, captures what is finally exposed as a depth map; so
what we have is basically a Structured-light 3D scanner.
The Kinect gspca subdriver just supports the video stream for now,
exposing the output from the RGB sensor or the unprocessed output from
the monochrome sensor; it does not deal with the processed depth stream
yet, but it allows using the sensor as a Webcam or as an IR camera (an
external source of IR light might be needed for this use).
The low level implementation is based on code from the OpenKinect
project (http://openkinect.org).
Cc: Steven Toth
Cc: OpenKinect
Signed-off-by: Antonio Ospite
---
drivers/media/video/gspca/Kconfig |9 +
drivers/media/video/gspca/Makefile |2 +
drivers/media/video/gspca/kinect.c | 427
3 files changed, 438 insertions(+), 0 deletions(-)
create mode 100644 drivers/media/video/gspca/kinect.c
diff --git a/drivers/media/video/gspca/Kconfig
b/drivers/media/video/gspca/Kconfig
index eb04e8b..34ae2c2 100644
--- a/drivers/media/video/gspca/Kconfig
+++ b/drivers/media/video/gspca/Kconfig
@@ -77,6 +77,15 @@ config USB_GSPCA_JEILINJ
To compile this driver as a module, choose M here: the
module will be called gspca_jeilinj.
+config USB_GSPCA_KINECT
+ tristate "Kinect sensor device USB Camera Driver"
+ depends on VIDEO_V4L2 && USB_GSPCA
+ help
+ Say Y here if you want support for the Microsoft Kinect sensor device.
+
+ To compile this driver as a module, choose M here: the
+ module will be called gspca_kinect.
+
config USB_GSPCA_KONICA
tristate "Konica USB Camera V4L2 driver"
depends on VIDEO_V4L2 && USB_GSPCA
diff --git a/drivers/media/video/gspca/Makefile
b/drivers/media/video/gspca/Makefile
index 855fbc8..802fbe1 100644
--- a/drivers/media/video/gspca/Makefile
+++ b/drivers/media/video/gspca/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_USB_GSPCA_CPIA1)+= gspca_cpia1.o
obj-$(CONFIG_USB_GSPCA_ETOMS)+= gspca_etoms.o
obj-$(CONFIG_USB_GSPCA_FINEPIX) += gspca_finepix.o
obj-$(CONFIG_USB_GSPCA_JEILINJ) += gspca_jeilinj.o
+obj-$(CONFIG_USB_GSPCA_KINECT) += gspca_kinect.o
obj-$(CONFIG_USB_GSPCA_KONICA) += gspca_konica.o
obj-$(CONFIG_USB_GSPCA_MARS) += gspca_mars.o
obj-$(CONFIG_USB_GSPCA_MR97310A) += gspca_mr97310a.o
@@ -46,6 +47,7 @@ gspca_cpia1-objs:= cpia1.o
gspca_etoms-objs:= etoms.o
gspca_finepix-objs := finepix.o
gspca_jeilinj-objs := jeilinj.o
+gspca_kinect-objs := kinect.o
gspca_konica-objs := konica.o
gspca_mars-objs := mars.o
gspca_mr97310a-objs := mr97310a.o
diff --git a/drivers/media/video/gspca/kinect.c
b/drivers/media/video/gspca/kinect.c
new file mode 100644
index 000..f85e746
--- /dev/null
+++ b/drivers/media/video/gspca/kinect.c
@@ -0,0 +1,427 @@
+/*
+ * kinect sensor device camera, gspca driver
+ *
+ * Copyright (C) 2011 Antonio Ospite
+ *
+ * Based on the OpenKinect project and libfreenect
+ * http://openkinect.org/wiki/Init_Analysis
+ *
+ * Special thanks to Steven Toth and kernellabs.com for sponsoring a Kinect
+ * sensor device which I tested the driver on.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#define MODULE_NAME "kinect"
+
+#include "gspca.h"
+
+#define CTRL_TIMEOUT 500
+
+MODULE_AUTHOR("Antonio Ospite ");
+MODULE_DESCRIPTION("GSPCA/Kinect Sensor Device USB Camera Driver");
+MODULE_LICENSE("GPL");
+
+#ifdef DEBUG
+int gspca_debug = D_ERR | D_PROBE | D_CONF | D_STREAM | D_FRAM | D_PACK |
+ D_USBI | D_USBO | D_V4L2;
+#endif
+
+struct pkt_hdr {
+ uint8_t magic[2];
+ uint8_t pad;
+ uint8_t flag;
+ uint8_t unk1;
+ uint8_t seq;
+ uint8_t unk2;
+ uint8_t unk3;
+ uint32_t timestamp;
+};
+
+struct cam_hdr {
+ uint8_t magic[2];
+ uint16_t len;
+ uint16_t cmd;
+ uint16_t tag;
+};
+
+/* specific webcam descriptor */
+struct sd {
+
[PATCH] libv4lconvert-priv.h: indent with tabs, not spaces
Indent wrapped lines with tabs, just like it is done for the other functions in the same file. Signed-off-by: Antonio Ospite --- lib/libv4lconvert/libv4lconvert-priv.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/lib/libv4lconvert/libv4lconvert-priv.h b/lib/libv4lconvert/libv4lconvert-priv.h index 30d1cfe..84c706e 100644 --- a/lib/libv4lconvert/libv4lconvert-priv.h +++ b/lib/libv4lconvert/libv4lconvert-priv.h @@ -131,7 +131,7 @@ void v4lconvert_grey_to_rgb24(const unsigned char *src, unsigned char *dest, int width, int height); void v4lconvert_grey_to_yuv420(const unsigned char *src, unsigned char *dest, -const struct v4l2_format *src_fmt); + const struct v4l2_format *src_fmt); void v4lconvert_rgb565_to_rgb24(const unsigned char *src, unsigned char *dest, int width, int height); -- 1.7.4.1 -- 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
[RFC, PATCH] libv4lconvert: Add support for Y10B grey format (V4L2_PIX_FMT_Y10BPACK)
Y10B is a 10 bits per pixel greyscale format in a bit-packed array
representation. Such pixel format is supplied for instance by the Kinect
sensor device.
Signed-off-by: Antonio Ospite
---
Hi,
this is a very first attempt about supporting Y10B in libv4lconvert, the
doubts I have are about the conversion routines which need to unpack a frame
before doing the actual pixelformat conversion, and maybe this can be handled
in some conversion layer in libv4l.
I don't know libv4l yet, so I am asking for advice providing some code to
discuss on; looking at the last hunk of the patch: can I allocate a temporary
buffer only once per device (and not per frame as I am horribly doing now) and
reuse it in the conversion routines? Or is the unpacking better be done even
before conversion, feeding the conversion routines with already unpacked
buffers?
Thanks,
Antonio Ospite
http://ao2.it
include/linux/videodev2.h |3 +
lib/libv4lconvert/libv4lconvert-priv.h |6 +++
lib/libv4lconvert/libv4lconvert.c | 20
lib/libv4lconvert/rgbyuv.c | 76
4 files changed, 105 insertions(+), 0 deletions(-)
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 51788a6..559d5f3 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -289,6 +289,9 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_Y10 v4l2_fourcc('Y', '1', '0', ' ') /* 10 Greyscale
*/
#define V4L2_PIX_FMT_Y16 v4l2_fourcc('Y', '1', '6', ' ') /* 16 Greyscale
*/
+/* Grey bit-packed formats */
+#define V4L2_PIX_FMT_Y10BPACKv4l2_fourcc('Y', '1', '0', 'B') /* 10
Greyscale bit-packed */
+
/* Palette formats */
#define V4L2_PIX_FMT_PAL8v4l2_fourcc('P', 'A', 'L', '8') /* 8 8-bit
palette */
diff --git a/lib/libv4lconvert/libv4lconvert-priv.h
b/lib/libv4lconvert/libv4lconvert-priv.h
index 84c706e..470a869 100644
--- a/lib/libv4lconvert/libv4lconvert-priv.h
+++ b/lib/libv4lconvert/libv4lconvert-priv.h
@@ -133,6 +133,12 @@ void v4lconvert_grey_to_rgb24(const unsigned char *src,
unsigned char *dest,
void v4lconvert_grey_to_yuv420(const unsigned char *src, unsigned char *dest,
const struct v4l2_format *src_fmt);
+void v4lconvert_y10b_to_rgb24(const unsigned char *src, unsigned char *dest,
+ int width, int height);
+
+void v4lconvert_y10b_to_yuv420(const unsigned char *src, unsigned char *dest,
+ const struct v4l2_format *src_fmt);
+
void v4lconvert_rgb565_to_rgb24(const unsigned char *src, unsigned char *dest,
int width, int height);
diff --git a/lib/libv4lconvert/libv4lconvert.c
b/lib/libv4lconvert/libv4lconvert.c
index e4863fd..631d912 100644
--- a/lib/libv4lconvert/libv4lconvert.c
+++ b/lib/libv4lconvert/libv4lconvert.c
@@ -48,6 +48,7 @@ static const struct v4lconvert_pixfmt supported_src_pixfmts[]
= {
{ V4L2_PIX_FMT_YVYU, 0 },
{ V4L2_PIX_FMT_UYVY, 0 },
{ V4L2_PIX_FMT_RGB565, 0 },
+ { V4L2_PIX_FMT_Y10BPACK, 0 },
{ V4L2_PIX_FMT_SN9C20X_I420, V4LCONVERT_NEEDS_CONVERSION },
{ V4L2_PIX_FMT_SBGGR8, V4LCONVERT_NEEDS_CONVERSION },
{ V4L2_PIX_FMT_SGBRG8, V4LCONVERT_NEEDS_CONVERSION },
@@ -862,6 +863,25 @@ static int v4lconvert_convert_pixfmt(struct
v4lconvert_data *data,
result = -1;
}
break;
+
+ case V4L2_PIX_FMT_Y10BPACK:
+ switch (dest_pix_fmt) {
+ case V4L2_PIX_FMT_RGB24:
+ case V4L2_PIX_FMT_BGR24:
+ v4lconvert_y10b_to_rgb24(src, dest, width, height);
+ break;
+ case V4L2_PIX_FMT_YUV420:
+ case V4L2_PIX_FMT_YVU420:
+ v4lconvert_y10b_to_yuv420(src, dest, fmt);
+ break;
+ }
+ if (src_size < (width * height * 10 / 8)) {
+ V4LCONVERT_ERR("short y10b data frame\n");
+ errno = EPIPE;
+ result = -1;
+ }
+ break;
+
case V4L2_PIX_FMT_RGB565:
switch (dest_pix_fmt) {
case V4L2_PIX_FMT_RGB24:
diff --git a/lib/libv4lconvert/rgbyuv.c b/lib/libv4lconvert/rgbyuv.c
index 2ee7e58..23fe8f3 100644
--- a/lib/libv4lconvert/rgbyuv.c
+++ b/lib/libv4lconvert/rgbyuv.c
@@ -603,3 +603,79 @@ void v4lconvert_grey_to_yuv420(const unsigned char *src,
unsigned char *dest,
/* Clear U/V */
memset(dest, 0x80, src_fmt->fmt.pix.width * src_fmt->fmt.pix.height /
2);
}
+
+#include
+#include
+/* Unpack buffer of (vw bit) data into padded 16bit buffer. */
+static inline void convert_packed_to_16bit(uint8_t *raw, uint16_t *unpac
Re: [RFC, PATCH] libv4lconvert: Add support for Y10B grey format (V4L2_PIX_FMT_Y10BPACK)
On Mon, 11 Apr 2011 23:07:36 +0200 Hans de Goede wrote: [...] > > I don't know libv4l yet, so I am asking for advice providing some code to > > discuss on; looking at the last hunk of the patch: can I allocate a > > temporary > > buffer only once per device (and not per frame as I am horribly doing now) > > and > > reuse it in the conversion routines? > > libv4l has a mechanism for doing this, you can "simply" do: > > unpacked_buffer = v4lconvert_alloc_buffer(width * height * sizeof(unsigned > short), >&data->convert_pixfmt_buf, >&data->convert_pixfmt_buf_size); > > v4lconvert_alloc_buffer will remember the buffer (and its size) and return the > same buffer each call. Freeing it on closing of the device is also taken care > of. You should still check for a NULL return. > Thanks that works fine: I am still not sure I like passing 'v4l2convert_data' to the pixelformat conversion routines but we'll discuss that on the next review round. > What has me worried more, is how libv4l will decide between asking > Y10B grey versus raw bayer from the device when an app is asking for say > RGB24. > libv4l normally does this automatically on a best match basis (together with > preferring compressed formats over uncompressed for high resolutions). But > this > won't work in the kinect case. If we prioritize one over the other we will > always end up giving the app the one we prioritize. > Mmh, I tried to materialize your worries, these are the native modes supported: - GRBG mode at 640x480 and 1280x1024 - UYVY mode ay 640x480 - Y10B mode at 640x488 and 1280x1024 ^ and this is the behavior I am observing in qv4l2 when in _wrapped_ mode: - If I choose the RGB3 output format all the three different resolutions are selectable: + at 640x480 I get the color image, as there is no greyscale format at the same resolution, + at 640x488 I get the grayscale image, as there is no color format at the same resolution, + if I choose 1280x1024 I get the grayscale image indeed, and I loose the possibility of using the color image. Everything works fine in _raw_ mode of course where only the native formats are shown. Ah, a strange thing (to me at least) happens in _wrapped_ mode even for GRBG (which is supposed to be a _native_ color format for the device): I get the grayscale image at 1280x1024 instead of the color image; can this just be a bug somewhere in qv4l2 or lib4vl? > The only thing I can think of is adding a v4l2 control (like a brightness > control) for choosing which format to prioritize... > and this control would be created by libv4l when in wrapped mode? Thanks, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? pgpiqJ4ep4hg4.pgp Description: PGP signature
Re: [GIT PATCHES FOR 2.6.40] gspca for_v2.6.40
On Tue, 19 Apr 2011 20:20:29 +0200 Jean-Francois Moine wrote: > The following changes since commit > d58307d6a1e2441ebaf2d924df4346309ff84c7d: > > [media] anysee: add more info about known board configs (2011-04-19 > 10:35:37 -0300) > > are available in the git repository at: > git://linuxtv.org/jfrancois/gspca.git for_v2.6.40 > > Antonio Ospite (2): > Add Y10B, a 10 bpp bit-packed greyscale format. > gspca - kinect: New subdriver for Microsoft Kinect > Ah glad to see that, so there was no major concern on the code, was there? Thanks Jean-Francois, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? pgpnTLdnOtL3H.pgp Description: PGP signature
[PATCH 1/3] gspca - kinect: move communications buffers out of stack
From: Drew Fisher
Move large communications buffers out of stack and into device
structure. This prevents the frame size from being >1kB and fixes a
compiler warning when CONFIG_FRAME_WARN=1024:
drivers/media/video/gspca/kinect.c: In function ‘send_cmd.clone.0’:
drivers/media/video/gspca/kinect.c:202: warning: the frame size of 1548 bytes
is larger than 1024 bytes
Reported-by: Mauro Carvalho Chehab
Signed-off-by: Drew Fisher
Signed-off-by: Antonio Ospite
---
drivers/media/video/gspca/kinect.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/media/video/gspca/kinect.c
b/drivers/media/video/gspca/kinect.c
index f85e746..79c4ef5 100644
--- a/drivers/media/video/gspca/kinect.c
+++ b/drivers/media/video/gspca/kinect.c
@@ -62,6 +62,8 @@ struct sd {
struct gspca_dev gspca_dev; /* !! must be the first item */
uint16_t cam_tag; /* a sequence number for packets */
uint8_t stream_flag;/* to identify different steram types */
+ uint8_t obuf[0x400];/* output buffer for control commands */
+ uint8_t ibuf[0x200];/* input buffer for control commands */
};
/* V4L2 controls supported by the driver */
@@ -133,8 +135,8 @@ static int send_cmd(struct gspca_dev *gspca_dev, uint16_t
cmd, void *cmdbuf,
struct sd *sd = (struct sd *) gspca_dev;
struct usb_device *udev = gspca_dev->dev;
int res, actual_len;
- uint8_t obuf[0x400];
- uint8_t ibuf[0x200];
+ uint8_t *obuf = sd->obuf;
+ uint8_t *ibuf = sd->ibuf;
struct cam_hdr *chdr = (void *)obuf;
struct cam_hdr *rhdr = (void *)ibuf;
--
1.7.4.4
--
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 3/3] gspca - kinect: fix comments referring to color camera
Use the expression "video stream" instead of "color camera" which is more correct as the driver supports the RGB and IR image on the same endpoint. Signed-off-by: Antonio Ospite --- drivers/media/video/gspca/kinect.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/video/gspca/kinect.c b/drivers/media/video/gspca/kinect.c index b4f9e2b..2028c64 100644 --- a/drivers/media/video/gspca/kinect.c +++ b/drivers/media/video/gspca/kinect.c @@ -233,7 +233,7 @@ static int sd_config(struct gspca_dev *gspca_dev, sd->cam_tag = 0; - /* Only color camera is supported for now, + /* Only video stream is supported for now, * which has stream flag = 0x80 */ sd->stream_flag = 0x80; @@ -243,7 +243,7 @@ static int sd_config(struct gspca_dev *gspca_dev, cam->nmodes = ARRAY_SIZE(video_camera_mode); #if 0 - /* Setting those values is not needed for color camera */ + /* Setting those values is not needed for video stream */ cam->npkt = 15; gspca_dev->pkt_size = 960 * 2; #endif -- 1.7.4.4 -- 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 0/3] gspca_kinect fixup
Hi, some incremental fixes for the gspca kinect driver, the first patch in the series by Drew Fisher addresses the issue Mauro was pointing out. Thanks, Antonio Antonio Ospite (1): gspca - kinect: fix comments referring to color camera Drew Fisher (2): gspca - kinect: move communications buffers out of stack gspca - kinect: fix a typo s/steram/stream/ drivers/media/video/gspca/kinect.c | 12 +++- 1 files changed, 7 insertions(+), 5 deletions(-) -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? -- 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 2/3] gspca - kinect: fix a typo s/steram/stream/
From: Drew Fisher
Signed-off-by: Drew Fisher
Signed-off-by: Antonio Ospite
---
drivers/media/video/gspca/kinect.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/gspca/kinect.c
b/drivers/media/video/gspca/kinect.c
index 79c4ef5..b4f9e2b 100644
--- a/drivers/media/video/gspca/kinect.c
+++ b/drivers/media/video/gspca/kinect.c
@@ -61,7 +61,7 @@ struct cam_hdr {
struct sd {
struct gspca_dev gspca_dev; /* !! must be the first item */
uint16_t cam_tag; /* a sequence number for packets */
- uint8_t stream_flag;/* to identify different steram types */
+ uint8_t stream_flag;/* to identify different stream types */
uint8_t obuf[0x400];/* output buffer for control commands */
uint8_t ibuf[0x200];/* input buffer for control commands */
};
--
1.7.4.4
--
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: [git:v4l-dvb/for_v2.6.40] [media] gspca - kinect: move communications buffers out of stack
On Fri, 29 Apr 2011 16:42:04 +0200 Mauro Carvalho Chehab wrote: > This is an automatic generated email to let you know that the following patch > were queued at the > http://git.linuxtv.org/media_tree.git tree: > > Subject: [media] gspca - kinect: move communications buffers out of stack > Author: Antonio Ospite > Date:Thu Apr 21 06:51:34 2011 -0300 > Hi Mauro, actually this one is from Drew Fisher as well, git-am should have picked up the additional From header: http://www.spinics.net/lists/linux-media/msg31576.html > Move large communications buffers out of stack and into device > structure. This prevents the frame size from being >1kB and fixes a > compiler warning when CONFIG_FRAME_WARN=1024: > > drivers/media/video/gspca/kinect.c: In function ‘send_cmd.clone.0’: > drivers/media/video/gspca/kinect.c:202: warning: the frame size of 1548 bytes > is larger than 1024 bytes > > Reported-by: Mauro Carvalho Chehab > Signed-off-by: Drew Fisher > Signed-off-by: Antonio Ospite > Signed-off-by: Mauro Carvalho Chehab > > drivers/media/video/gspca/kinect.c |6 -- > 1 files changed, 4 insertions(+), 2 deletions(-) > -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? pgplF4P7VDf3p.pgp Description: PGP signature
Re: [git:v4l-dvb/for_v2.6.40] [media] gspca - kinect: move communications buffers out of stack
On Fri, 29 Apr 2011 12:43:55 -0300 Mauro Carvalho Chehab wrote: > Em 29-04-2011 12:27, Antonio Ospite escreveu: > > On Fri, 29 Apr 2011 16:42:04 +0200 > > Mauro Carvalho Chehab wrote: > > > >> This is an automatic generated email to let you know that the following > >> patch were queued at the > >> http://git.linuxtv.org/media_tree.git tree: > >> > >> Subject: [media] gspca - kinect: move communications buffers out of stack > >> Author: Antonio Ospite > >> Date:Thu Apr 21 06:51:34 2011 -0300 > >> > > > > Hi Mauro, actually this one is from Drew Fisher as well, git-am should > > have picked up the additional From header: > > http://www.spinics.net/lists/linux-media/msg31576.html > > Gah! > > Patchwork suffered a crash. Patches got recovered yesterday, but all of them > missed > the e-mail body: > https://patchwork.kernel.org/patch/724331/ > > I'm needing to manually edit each patch before applying due to that. > Just FYI, gmane stores a raw representation of messages which can be used with git-am, take: http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/31735 and add /raw at the end of the URL. > I'll revert the patch and re-apply it with the proper authorship. > Thanks a lot. Best regards, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? pgpVF64w0Mya8.pgp Description: PGP signature
Re: [GIT PATCHES FOR 2.6.40] gspca for_v2.6.40
On Tue, 17 May 2011 10:54:17 +0200 Jean-Francois Moine wrote: > The following changes since commit > f9b51477fe540fb4c65a05027fdd6f2ecce4db3b: > > [media] DVB: return meaningful error codes in dvb_frontend (2011-05-09 > 05:47:20 +0200) > > are available in the git repository at: > git://linuxtv.org/jfrancois/gspca.git for_v2.6.40 > [...] Hi Jean-Francois, sometimes it is useful to add also a "why" section to commit messages so others can follow your thoughts, and even learn from them. I have this very simple scheme: a summary of the "what" goes into the short commit message and the "why" and "how" go into the long commit message when they are not immediately trivial from the code; for instance the "why" of the USB trace changes in this series wasn't trivial to me. Thanks, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? -- 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: [GIT PATCHES FOR 2.6.40] gspca for_v2.6.40
On Sat, 21 May 2011 08:59:33 +0200
Jean-Francois Moine wrote:
> The following changes since commit
> f9b51477fe540fb4c65a05027fdd6f2ecce4db3b:
>
> [media] DVB: return meaningful error codes in dvb_frontend (2011-05-09
> 05:47:20 +0200)
>
> are available in the git repository at:
> git://linuxtv.org/jfrancois/gspca.git for_v2.6.40
>
> Jean-François Moine (9):
[...]
> gspca - main: Remove USB traces
OK, now I got it, thanks a lot :)
BTW there are still a lot of messages using D_USBI and D_USBO in gspca
subdrivers which are now muted unconditionally by that change, however
I notice that they are mostly about telling what registers and values
are being set and got, we could call that "device specific" messages and
they are kind of independent form the USB transport indeed; do you
think it is worth to have a new debug level to replace D_USB{I,O} in
order to keep those messages? I am not sure about the name tho: D_COMM,
D_DEV, D_REGS, D_IC, D_HW?
Thanks,
Antonio
--
Antonio Ospite
http://ao2.it
PGP public key ID: 0x4553B001
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
pgp0QmEuOzmTG.pgp
Description: PGP signature
Re: [PATCH] [media] gspca/kinect: wrap gspca_debug with GSPCA_DEBUG
On Thu, 26 May 2011 08:49:12 +0200
Jean-Francois Moine wrote:
> On Wed, 25 May 2011 17:34:32 -0400
> Jarod Wilson wrote:
>
> > diff --git a/drivers/media/video/gspca/kinect.c
> > b/drivers/media/video/gspca/kinect.c
> > index 66671a4..26fc206 100644
> > --- a/drivers/media/video/gspca/kinect.c
> > +++ b/drivers/media/video/gspca/kinect.c
> > @@ -34,7 +34,7 @@ MODULE_AUTHOR("Antonio Ospite
> > ");
> > MODULE_DESCRIPTION("GSPCA/Kinect Sensor Device USB Camera Driver");
> > MODULE_LICENSE("GPL");
> >
> > -#ifdef DEBUG
> > +#ifdef GSPCA_DEBUG
> > int gspca_debug = D_ERR | D_PROBE | D_CONF | D_STREAM | D_FRAM | D_PACK |
> > D_USBI | D_USBO | D_V4L2;
> > #endif
>
> Hi Jarod,
>
> Sorry, it is not the right fix. In fact, the variable gspca_debug must
> not be defined in gspca subdrivers:
>
> --- a/drivers/media/video/gspca/kinect.c
> +++ b/drivers/media/video/gspca/kinect.c
> @@ -34,11 +34,6 @@
> MODULE_DESCRIPTION("GSPCA/Kinect Sensor Device USB Camera Driver");
> MODULE_LICENSE("GPL");
>
> -#ifdef DEBUG
> -int gspca_debug = D_ERR | D_PROBE | D_CONF | D_STREAM | D_FRAM | D_PACK |
> - D_USBI | D_USBO | D_V4L2;
> -#endif
> -
> struct pkt_hdr {
> uint8_t magic[2];
> uint8_t pad;
>
OK.
Thanks,
Antonio
--
Antonio Ospite
http://ao2.it
PGP public key ID: 0x4553B001
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
pgpno9l4XQh3l.pgp
Description: PGP signature
pxa_camera: Oops in pxa_camera_probe.
Hi, I get this with pxa-camera in linux-2.6.31-rc1. If this could be useful, I haven't converted my board code to the new platform_data style yet. i2c /dev entries driver Linux video capture interface: v2.00 Unable to handle kernel NULL pointer dereference at virtual address 0060 pgd = c0004000 [0060] *pgd= Internal error: Oops: f5 [#1] PREEMPT Modules linked in: CPU: 0Tainted: GW (2.6.31-rc1-ezxdev #34) PC is at dev_driver_string+0x0/0x38 LR is at pxa_camera_probe+0x144/0x428 pc : []lr : []psr: 2013 sp : cc81feb0 ip : cc81e000 fp : c0382400 r10: c0381dc0 r9 : r8 : c0381dc8 r7 : 0632ea00 r6 : 018cba80 r5 : 02faf080 r4 : cc878a60 r3 : 0020 r2 : 28a0 r1 : 0632ea00 r0 : Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel Control: 397f Table: a0004000 DAC: 0017 Process swapper (pid: 1, stack limit = 0xcc81e278) Stack: (0xcc81feb0 to 0xcc82) fea0: 0001 c00e5528 cc871878 0021 fec0: c0381dc8 c039bb08 c039bb08 c039bb08 c0398d28 fee0: c016c4a0 c039bb08 c016b658 c0381dc8 c039bb08 c0381dfc c016b76c ff00: cc81ff10 c016b70c c016aa70 cc823eb4 cc865b8c c039bb08 ff20: c039bb08 cc9784c0 c016b030 c0312ba0 c012e0f8 c039bb08 ff40: 0001 c001cb98 c016ba68 c03a4eec ff60: c001cb98 c00282d4 cc81ff88 ff80: c00de0a0 c028ec24 cc81ffc6 c03190e8 c0888f00 0140 cc81ffc6 cc847140 ffa0: cc81ffc6 00b8 c0888f94 c00de200 c00754d0 cc8471c0 c038dff4 c00754f0 ffc0: 38312c20 0034 c00245ac ffe0: c00086fc c0029e7c 41e5ced4 c1a4d9f9 [] (dev_driver_string+0x0/0x38) from [] (pxa_camera_probe+0x144/0x428) [] (pxa_camera_probe+0x144/0x428) from [] (platform_drv_probe+0x1c/0x24) [] (platform_drv_probe+0x1c/0x24) from [] (driver_probe_device+0xc0/0x174) [] (driver_probe_device+0xc0/0x174) from [] (__driver_attach+0x60/0x84) [] (__driver_attach+0x60/0x84) from [] (bus_for_each_dev+0x48/0x80) [] (bus_for_each_dev+0x48/0x80) from [] (bus_add_driver+0xa0/0x224) [] (bus_add_driver+0xa0/0x224) from [] (driver_register+0xac/0x138) [] (driver_register+0xac/0x138) from [] (do_one_initcall+0x4c/0x184) [] (do_one_initcall+0x4c/0x184) from [] (kernel_init+0x8c/0x104) [] (kernel_init+0x8c/0x104) from [] (kernel_thread_exit+0x0/0x8) Code: e8bd80f0 2710 0001a36e 000f423f (e5903060) ---[ end trace 1b75b31a2719ed1d ]--- Regards, Antonio -- A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 pgpmFTfyuUzir.pgp Description: PGP signature
pxa_camera: Oops in pxa_camera_probe.
Hi, I get this with pxa-camera in mainline linux (from today). I haven't touched my board code which used to work in 2.6.30 Linux video capture interface: v2.00 Unable to handle kernel NULL pointer dereference at virtual address 0060 pgd = c0004000 [0060] *pgd= Internal error: Oops: f5 [#1] PREEMPT Modules linked in: CPU: 0Tainted: GW (2.6.31-rc1-ezxdev #1) PC is at dev_driver_string+0x0/0x38 LR is at pxa_camera_probe+0x144/0x428 pc : []lr : []psr: 2013 sp : cc81feb0 ip : cc81e000 fp : c0382360 r10: c0381d20 r9 : r8 : c0381d28 r7 : 0632ea00 r6 : 018cba80 r5 : 02faf080 r4 : cc8dea60 r3 : 0020 r2 : 28a0 r1 : 0632ea00 r0 : Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel Control: 397f Table: a0004000 DAC: 0017 Process swapper (pid: 1, stack limit = 0xcc81e278) Stack: (0xcc81feb0 to 0xcc82) fea0: 0001 c00e55e4 cc84f878 0021 fec0: c0381d28 c039ba68 c039ba68 c039ba68 c0398c88 fee0: c016c588 c039ba68 c016b740 c0381d28 c039ba68 c0381d5c c016b854 ff00: cc81ff10 c016b7f4 c016ab58 cc823eb4 cc865b8c c039ba68 ff20: c039ba68 cc9204c0 c016b118 c0312e28 c012e1d8 c039ba68 ff40: 0001 c001cb98 c016bb50 c03a4e4c ff60: c001cb98 c00282f4 cc81ff88 ff80: c00de15c c028ed7c cc81ffc6 c0319370 c0888e00 0140 cc81ffc6 cc847140 ffa0: cc81ffc6 00b8 c0888ef4 c00de2bc c00754f0 cc8471c0 c038df54 c0075510 ffc0: 38312c20 0034 c00245ac ffe0: c00086fc c0029e9c 55aa55aa 55aa55aa [] (dev_driver_string+0x0/0x38) from [] (pxa_camera_probe+0x144/0x428) [] (pxa_camera_probe+0x144/0x428) from [] (platform_drv_probe+0x1c/0x24) [] (platform_drv_probe+0x1c/0x24) from [] (driver_probe_device+0xc0/0x174) [] (driver_probe_device+0xc0/0x174) from [] (__driver_attach+0x60/0x84) [] (__driver_attach+0x60/0x84) from [] (bus_for_each_dev+0x48/0x80) [] (bus_for_each_dev+0x48/0x80) from [] (bus_add_driver+0xa0/0x224) [] (bus_add_driver+0xa0/0x224) from [] (driver_register+0xac/0x138) [] (driver_register+0xac/0x138) from [] (do_one_initcall+0x4c/0x184) [] (do_one_initcall+0x4c/0x184) from [] (kernel_init+0x8c/0x104) [] (kernel_init+0x8c/0x104) from [] (kernel_thread_exit+0x0/0x8) Code: e8bd80f0 2710 0001a36e 000f423f (e5903060) ---[ end trace 1b75b31a2719ed1d ]--- Regards, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? pgpD5x3OjLhW4.pgp Description: PGP signature
Re: pxa_camera: Oops in pxa_camera_probe.
On Wed, 1 Jul 2009 20:43:25 +0200 Antonio Ospite wrote: > Hi, > > I get this with pxa-camera in mainline linux (from today). > I haven't touched my board code which used to work in 2.6.30 > I think I've tracked down the cause. The board code is triggering a bug in pxa_camera. The same should apply to mioa701 as well. > Linux video capture interface: v2.00 > Unable to handle kernel NULL pointer dereference at virtual address 0060 > pgd = c0004000 > [0060] *pgd= > Internal error: Oops: f5 [#1] PREEMPT > Modules linked in: > CPU: 0Tainted: GW (2.6.31-rc1-ezxdev #1) > PC is at dev_driver_string+0x0/0x38 > LR is at pxa_camera_probe+0x144/0x428 The offending dev_driver_str() here is the one in the dev_warn() call in mclk_get_divisor(). This is what is happening: in struct pxacamera_platform_data I have: .mclk_10khz = 5000, which makes the > test in mclk_get_divisor() succeed calling dev_warn to report that the clock has been limited, but pcdev->soc_host.dev is still uninitialized at this time. I could lower the value in my platform data and avoid the bug, but it would be good to have this fixed ASAP anyway. The attached rough patch fixes the problem, but you will surely come out with a better one :) Thanks, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? mclk_get_divisor uses pcdev->soc_host.dev, make sure it is initialized. Signed-off-by: Antonio Ospite diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c index 46e0d8a..e048d25 100644 --- a/drivers/media/video/pxa_camera.c +++ b/drivers/media/video/pxa_camera.c @@ -1579,6 +1579,7 @@ static int __devinit pxa_camera_probe(struct platform_device *pdev) pcdev->mclk = 2000; } + pcdev->soc_host.dev = &pdev->dev; pcdev->mclk_divisor = mclk_get_divisor(pcdev); INIT_LIST_HEAD(&pcdev->capture); @@ -1644,7 +1645,6 @@ static int __devinit pxa_camera_probe(struct platform_device *pdev) pcdev->soc_host.drv_name = PXA_CAM_DRV_NAME; pcdev->soc_host.ops = &pxa_soc_camera_host_ops; pcdev->soc_host.priv = pcdev; - pcdev->soc_host.dev = &pdev->dev; pcdev->soc_host.nr = pdev->id; err = soc_camera_host_register(&pcdev->soc_host); pgpFWUTYnD6jS.pgp Description: PGP signature
Re: pxa_camera: Oops in pxa_camera_probe.
On Fri, 3 Jul 2009 22:03:27 +0200 (CEST) Guennadi Liakhovetski wrote: > On Fri, 3 Jul 2009, Antonio Ospite wrote: > > > > Linux video capture interface: v2.00 > > > Unable to handle kernel NULL pointer dereference at virtual address > > > 0060 > > > pgd = c0004000 > > > [0060] *pgd= > > > Internal error: Oops: f5 [#1] PREEMPT > > > Modules linked in: > > > CPU: 0Tainted: GW (2.6.31-rc1-ezxdev #1) > > > PC is at dev_driver_string+0x0/0x38 > > > LR is at pxa_camera_probe+0x144/0x428 > > > > The offending dev_driver_str() here is the one in the dev_warn() call in > > mclk_get_divisor(). > > > > This is what is happening: in struct pxacamera_platform_data I have: > > .mclk_10khz = 5000, > > > > which makes the > test in mclk_get_divisor() succeed calling dev_warn > > to report that the clock has been limited, but pcdev->soc_host.dev is > > still uninitialized at this time. > > > > I could lower the value in my platform data and avoid the bug, but it > > would be good to have this fixed ASAP anyway. > > > > The attached rough patch fixes the problem, but you will surely come > > out with a better one :) > > Why should I? Your patch seems correct to me so far, thanks. I'll push it > for 2.6.31. Please, next time inline your patch as described in > Documentation/SubmittingPatches. > Well, it should be correct, I just thought it could be considered unpretty with the pcdev->soc_host initializations scattered here and there, that's what I was referring to. But, if this is ok to you, it's ok to me too :) Ciao, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? pgpA5hJKv0j2w.pgp Description: PGP signature
Re: pxa_camera: Oops in pxa_camera_probe.
On Sat, 04 Jul 2009 21:35:22 +0200 Robert Jarzmik wrote: > >> > The offending dev_driver_str() here is the one in the dev_warn() call in > >> > mclk_get_divisor(). > >> > > >> > This is what is happening: in struct pxacamera_platform_data I have: > >> > .mclk_10khz = 5000, > >> > > >> > which makes the > test in mclk_get_divisor() succeed calling dev_warn > >> > to report that the clock has been limited, but pcdev->soc_host.dev is > >> > still uninitialized at this time. > > Antonio, > > Would you check [1] and see if your stack does correspond to the one I > reported > some time ago ? As this is fresh in your memory, you'll be far quicker that > me. > ... > [1] http://osdir.com/ml/linux-media/2009-04/msg00874.html Yes, I think that is it. The offsets are different of course but the call stack is pretty much the same. Regards, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? pgpl24u7bRtMt.pgp Description: PGP signature
[BUG] pxa_camera: possible recursive locking detected
Hi,
verified to be present in linux-2.6.31-rc5, here's some info dumped
from RAM, since the machine hangs, sorry if it is not complete but I
couldn't get anything better for now, nothing is printed on
the screen.
The userspace app is capture-example from v4l2-apps/test
and the command which should be triggering the bug is:
xioctl(fd, VIDIOC_STREAMON, &type)
=
[ INFO: possible recursive locking detected ]
2.6.31-rc5-ezxdev #53
-
capture-example/967 is trying to acquire lock:
(&pcdev->lock {..}, at: [] pxa_videobuf_queue+0x28/0xc4
but task is already holding lock:
(&pcdev->lock {..} buf_streamon+0x40/0xc0
other info that might help us debug this:
held by capture-example/967:
#0: o_lock
soc_camera_streamon+0x40/0x70
#1: lock
videobuf_streamon+0x14/0xc0
#2:
eobuf_streamon+0x40/0xc0
The stack backtrace I managed to get is even worse, something like:
---
stack backtrace: [] 0x0/0xe0)
idate_chain+0x5b0/0xd84)
89.995951] []
e_chain+0x5b0/0xd84)
5a60>]
from []
0x5c/0x70)
00668e8>]
_irqsave+0x4c/0x60)
6230] []
rqsave+0x4c/0x60)
c>]
eamon+0x70/0xc0)
3] []
on+0x70/0xc0)
(soc_camera_streamon+0x58/0x70)
[ 89.996488] []
soc_camera_streamon+0x58/0x70)
rom []
tl+0x14e0/0x3404)
-
With another build with debug enabled I extracted this sequence:
[ 104.385424] camera 0-0: PXA Camera driver attached to camera 0
[ 104.385513] pxa27x-camera pxa27x-camera.0: Registered platform device at
cc923d60 data c0316fe0
[ 104.385554] pxa27x-camera pxa27x-camera.0: pxa_camera_activate: Init gpios
[ 104.447596] camera 0-0: set width: 640 height: 480
[ 104.447642] camera 0-0: camera device open
[ 104.502178] camera 0-0: set width: 640 height: 480
[ 104.502663] camera 0-0: soc_camera_reqbufs: 1
[ 104.502725] camera 0-0: count=4, size=0
[ 104.508618] camera 0-0: mmap called, vma=0xcc07fc28
[ 104.508926] camera 0-0: vma start=0x40144000, size=614400, ret=0
[ 104.542879] camera 0-0: mmap called, vma=0xcc05b1d8
[ 104.542990] camera 0-0: vma start=0x401da000, size=614400, ret=0
[ 104.546148] camera 0-0: mmap called, vma=0xcc05b6a8
[ 104.546243] camera 0-0: vma start=0x4027, size=614400, ret=0
[ 104.549401] camera 0-0: mmap called, vma=0xcc05b4f0
[ 104.549509] camera 0-0: vma start=0x40306000, size=614400, ret=0
[ 104.550380] camera 0-0: pxa_videobuf_prepare (vb=0xcc91e760) 0x40144000
614400
[ 104.714301] pxa27x-camera pxa27x-camera.0: DMA: sg_first=cd83e000,
sglen=150, ofs=0, dma.desc=acb94000
[ 104.715766] camera 0-0: pxa_videobuf_prepare (vb=0xcc91e560) 0x401da000
614400
[ 104.782840] pxa27x-camera pxa27x-camera.0: DMA: sg_first=cd852000,
sglen=150, ofs=0, dma.desc=acde7000
[ 104.783988] camera 0-0: pxa_videobuf_prepare (vb=0xcc91e660) 0x4027
614400
[ 104.841132] pxa27x-camera pxa27x-camera.0: DMA: sg_first=cd855000,
sglen=150, ofs=0, dma.desc=ac09
[ 104.863313] camera 0-0: pxa_videobuf_prepare (vb=0xcc91e860) 0x40306000
614400
[ 104.960047] pxa27x-camera pxa27x-camera.0: DMA: sg_first=cd858000,
sglen=150, ofs=0, dma.desc=acdd2000
[ 104.960922] camera 0-0: soc_camera_streamon
[ 104.961840] camera 0-0: pxa_videobuf_queue (vb=0xcc91e760) 0x40144000 614400
active=(null)
maybe some more pxa_videobuf_queue lines are missing,
but again I was not able to extract them from RAM.
Thanks,
Antonio
--
Antonio Ospite
http://ao2.it
PGP public key ID: 0x4553B001
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?
pgpuvBN5nLexm.pgp
Description: PGP signature
Re: [PATCH] soc-camera: fix recursive locking in .buf_queue()
On Tue, 4 Aug 2009 10:30:47 +0200 (CEST) Guennadi Liakhovetski wrote: > On Tue, 4 Aug 2009, Antonio Ospite wrote: > > > verified to be present in linux-2.6.31-rc5, here's some info dumped > > from RAM, since the machine hangs, sorry if it is not complete but I > > couldn't get anything better for now, nothing is printed on > > the screen. > > You're right, thanks for the report. Does the patch below fix the problem? > It only gets a bit tricky in mx3_camera.c, will have to test. > Yes, the patch fixes the problem. Many thanks. The current patch applies with some fuzzes on vanilla kernels, and it even FAILS to apply for drivers/media/video/sh_mobile_ceu_camera.c in one hunk. I hope that this one and also http://patchwork.kernel.org/patch/33960/ will hit mainline soon. Ciao, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? pgpKr9xknJ74D.pgp Description: PGP signature
Re: [PATCH] soc-camera: fix recursive locking in .buf_queue()
On Tue, 4 Aug 2009 11:31:24 +0200 (CEST) Guennadi Liakhovetski wrote: > On Tue, 4 Aug 2009, Antonio Ospite wrote: > > > The current patch applies with some fuzzes on vanilla kernels, and it > > even FAILS to apply for drivers/media/video/sh_mobile_ceu_camera.c in > > one hunk. > > Yes, I'll produce one against vanilla for submission. > Just to make sure you notice: the now unused 'flags' variables can go away as well. Thanks, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? pgpDMjrgGvhrI.pgp Description: PGP signature
Re: [REVIEW] v4l2 loopback
On Tue, 14 Apr 2009 15:53:00 +0300 vas...@gmail.com wrote: > On Tue, Apr 14, 2009 at 3:12 PM, Mauro Carvalho Chehab > wrote: > > > The issue I see is that the V4L drivers are meant to support real devices. > > This > > driver that is a loopback for some userspace driver. I don't discuss its > > value > > for testing purposes or other random usage, but I can't see why this should > > be > > at upstream kernel. > > > > So, I'm considering to add it at v4l-dvb tree, but as an out-of-tree driver > > only. For this to happen, probably, we'll need a few adjustments at v4l > > build. > > > > Cheers, > > Mauro > > > > Mauro, > > ok, let it be out-of -tree driver, this is also good as I do not have > to adapt the driver to each new kernel, but I want to argue alittle > about Inclusion of the driver into upstream kernel. > > Main reason for inclusion to the kernel is ease of use, as I > understand installing the out-of-tree driver for some kernel needs > downloading of the whole v4l-dvb tree(am I right?). > > Loopback gives one opportunities to do many fun things with video > streams and when it needs just one step to begin using it chances that > someone will do something useful with the driver are higher. > I, as a target user of vloopback, agree that having it in mainline would be really handy. Think that with a stable vloopback solution, with device detection and parameter setting, we can really make PTP digicams as webcams[1] useful, right now this is tricky and very uncomfortable on kernel update. > Awareness that there is such thing as loopback is also. If the driver > is in upstream tree - more people will see it and more chances that > more people will participate in loopback getiing better. > > vivi is an upstream driver :-) > Even vivi can be seen as a particular case of a vloopback device, can't it? Regards, Antonio [1] http://shell.studenti.unina.it/~ospite/section/it/dev/canoncam.html#English -- A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? Web site: http://www.studenti.unina.it/~ospite Public key: http://www.studenti.unina.it/~ospite/aopubkey.asc pgpEeET9DTixU.pgp Description: PGP signature
Re: [REVIEW] v4l2 loopback
re_param->capturemode = 0;
> - capture_param->extendedmode = 0;
> - capture_param->readbuffers = max_buffers_number;
> - capture_param->timeperframe.numerator = 1;
> - capture_param->timeperframe.denominator = 30;
> -}
> -
> -/* init loopback main structure */
> -static int v4l2_loopback_init(struct v4l2_loopback_device *dev)
> -{
> - dev->vdev = video_device_alloc();
> - if (dev->vdev == NULL)
> - return -ENOMEM;
> - init_vdev(dev->vdev);
> - init_capture_param(&dev->capture_param);
> - dev->buffers_number = max_buffers_number;
> - atomic_set(&dev->open_count, 0);
> - dev->ready_for_capture = 0;
> - dev->buffer_size = 0;
> - dev->image = NULL;
> - /* kfree on module release */
> - dev->buffers =
> - kzalloc(sizeof(*dev->buffers) * dev->buffers_number,
> - GFP_KERNEL);
> - if (dev->buffers == NULL)
> - return -ENOMEM;
> - init_waitqueue_head(&dev->read_event);
> - return 0;
> -};
> -
> -/* LINUX KERNEL */
> -static const struct v4l2_file_operations v4l2_loopback_fops = {
> - .owner = THIS_MODULE,
> - .open = v4l_loopback_open,
> - .release = v4l_loopback_close,
> - .read = v4l_loopback_read,
> - .write = v4l_loopback_write,
> - .poll = v4l2_loopback_poll,
> - .mmap = v4l2_loopback_mmap,
> - .ioctl = video_ioctl2,
> -};
> -
align the equals signs using spaces here and below.
> -static const struct v4l2_ioctl_ops v4l2_loopback_ioctl_ops = {
> - .vidioc_querycap = &vidioc_querycap,
> - .vidioc_enum_fmt_vid_cap = &vidioc_enum_fmt_cap,
> - .vidioc_enum_input = &vidioc_enum_input,
> - .vidioc_g_input = &vidioc_g_input,
> - .vidioc_s_input = &vidioc_s_input,
> - .vidioc_g_fmt_vid_cap = &vidioc_g_fmt_cap,
> - .vidioc_s_fmt_vid_cap = &vidioc_s_fmt_cap,
> - .vidioc_s_fmt_vid_out = &vidioc_s_fmt_video_output,
> - .vidioc_try_fmt_vid_cap = &vidioc_try_fmt_cap,
> - .vidioc_try_fmt_vid_out = &vidioc_try_fmt_video_output,
> - .vidioc_s_std = &vidioc_s_std,
> - .vidioc_g_parm = &vidioc_g_parm,
> - .vidioc_s_parm = &vidioc_s_parm,
> - .vidioc_reqbufs = &vidioc_reqbufs,
> - .vidioc_querybuf = &vidioc_querybuf,
> - .vidioc_qbuf = &vidioc_qbuf,
> - .vidioc_dqbuf = &vidioc_dqbuf,
> - .vidioc_streamon = &vidioc_streamon,
> - .vidioc_streamoff = &vidioc_streamoff,
> -#ifdef CONFIG_VIDEO_V4L1_COMPAT
> - .vidiocgmbuf = &vidiocgmbuf,
> -#endif
> -};
> -
> -int __init init_module()
> -{
> - int ret;
> -
> - dprintk("entering init_module()\n");
> - /* kfree on module release */
> - dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> - if (dev == NULL)
> - return -ENOMEM;
> - ret = v4l2_loopback_init(dev);
> - if (ret < 0)
> - return ret;
> - /* register the device -> it creates /dev/video* */
> - if (video_register_device(dev->vdev, VFL_TYPE_GRABBER, -1) < 0) {
> - video_device_release(dev->vdev);
> - printk(KERN_INFO "failed video_register_device()\n");
> - return -EFAULT;
> - }
> - printk(KERN_INFO "v4l2-loopback module installed\n");
> - return 0;
> -}
> -
> -void __exit cleanup_module()
> -{
> - dprintk("entering cleanup_module()\n");
> - /* unregister the device -> it deletes /dev/video* */
> - video_unregister_device(dev->vdev);
> - kfree(dev->buffers);
> - kfree(dev);
> - printk(KERN_INFO "v4l2-loopback module removed\n");
> -}
> \ В конце файла нет новой строки
Does this mean that there is a missing newline character at the of the
file? :)
> diff -uprN v4l-dvb.my.p/linux/drivers/media/video/v4l2loopback.h
> v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.h
> --- v4l-dvb.my.p/linux/drivers/media/video/v4l2loopback.h 2009-04-27
> 02:10:54.0 +0300
> +++ v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.h 1970-01-01
> 03:00:00.0 +0300
> @@ -1,56 +0,0 @@
> -/*
> - * v4l2loopback.h -- video 4 linux loopback driver
> - *
> - * Copyright (C) 2005-2009
> - * Vasily Levin (vas...@gmail.com)
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - */
> -
If there is no other user of v4l2oopback.h maybe you can merge this file
into v4l2loopback.c
> -#ifndef _V4L2LOOPBACK_H
> -#define _V4L2LOOPBACK_H
> -
> -#include
> -#include
> -
> -/* TODO(vasaka) use typenames which are common to kernel, but first find out
> if
> - * it is needed */
> -/* struct keeping state and settings of loopback device */
> -struct v4l2_loopback_device {
> - struct video_device *vdev;
> - /* pixel and stream format */
> - struct v4l2_pix_format pix_format;
> - struct v4l2_captureparm capture_param;
> - /* buffers stuff */
> - u8 *image; /* pointer to actual buffers data */
> - int buffers_number; /* should not be big, 4 is a good choice */
> - struct v4l2_buffer *buffers;/* inner driver buffers */
> - int write_position; /* number of last written frame + 1 */
> - long buffer_size;
> - /* sync stuff */
> - atomic_t open_count;
> - int ready_for_capture;/* set to true when at least one writer opened
> - * device and negotiated format */
> - wait_queue_head_t read_event;
> -};
> -
> -/* types of opener shows what opener wants to do with loopback */
> -enum opener_type {
> - UNNEGOTIATED = 0,
> - READER = 1,
> - WRITER = 2,
> -};
> -
> -/* struct keeping state and type of opener */
> -struct v4l2_loopback_opener {
> - enum opener_type type;
> - int buffers_number;
> - int position; /* number of last processed frame + 1 or
> -* write_position - 1 if reader went out of sync */
> - struct v4l2_buffer *buffers;
> -};
> -#endif /* _V4L2LOOPBACK_H */
> --
> 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
Regards,
Antonio Ospite
--
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?
Web site: http://www.studenti.unina.it/~ospite
Public key: http://www.studenti.unina.it/~ospite/aopubkey.asc
pgp6ZFiPxZWoB.pgp
Description: PGP signature
UVC gadget driver for linux.
Hi, (I am not subscribed to linux-uvc-devel, so please CC me on reply) I found this UVC gadget driver for linux developed by Motorola for the EzX platform, it should be the very same driver which makes the Motorola ROCKR E6[1,1a] show up as a UVC webcam. It's for linux-2.4, I haven't looked at it very deeply and I can't comment on its status, but I just thought to report about it because someone might want to port it to 2.6 and v4l2. The original tarball is at opensource.motorola.com[2]. You can find a browsable copy of it at git.openezx.org[3]. the relevant filenames are: ./drivers/media/video/camera4uvc.c ./drivers/media/video/camera4uvc.h ./drivers/usbd/UVC_fd ./drivers/usbd/UVC_fd/uvc.c ./drivers/usbd/UVC_fd/uvc.h Regards, Antonio Ospite [1] http://www.motorola.com/motoinfo/product/details.jsp?globalObjectId=175 [1a] http://lists.berlios.de/pipermail/linux-uvc-devel/2007-October/002305.html [2] https://opensource.motorola.com/sf/frs/do/downloadFile/projects.a1200/frs.a1200e_latam.r541l7_g_11_10_11r/frs6837?dl=1 [3] http://git.openezx.org/?p=motorola-2.4.git;a=tree;h=1627c6705140336fdd043af3141b57e752fbbdd2;hb=53e66c3b1448c5679cb34d64e18cca65830a25b3 -- A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? Web site: http://www.studenti.unina.it/~ospite Public key: http://www.studenti.unina.it/~ospite/aopubkey.asc pgpKHRXUukC3X.pgp Description: PGP signature
Re: [REVIEW] v4l2 loopback
On Thu, 7 May 2009 02:54:00 +0300
Vasily wrote:
> This patch introduces v4l2 loopback module
>
Hi Vasily, next time it would be useful to summarize what you changed
from the previous version, and put a revision number in the Subject,
like [PATCH v2] [PATCH v3], etc.
Also, the patch has some style problems reported by checkpatch.pl,
please fix those. Even if the driver wouldn't go mainline you do want to
follow the style guidelines.
And some more typos I spotted, see inlined comments.
I am not a native English speaker either, so I learned to use spell
checkers even in source code comments :)
Anyhow, finally I tested the driver, and I have only a question as a
user: couldn't it be possible to make a default input enabled by
default? This way, if a writer knows the format to use it doesn't have
to setup the device format on its own, and can treat the device as a
normal file.
Thanks,
Antonio
> From: Vasily Levin
>
> This is v4l2 loopback driver which can be used to make available any userspace
> video as v4l2 device. Initialy it was written to make videoeffects available
Typo: Initialy -> Initially
> to Skype, but in fact it have many more uses.
>
> Priority: normal
>
> Signed-off-by: Vasily Levin
>
> diff -uprN v4l-dvb.orig/linux/drivers/media/video/Kconfig
> v4l-dvb.my/linux/drivers/media/video/Kconfig
> --- v4l-dvb.orig/linux/drivers/media/video/Kconfig2009-04-25
> 04:41:20.0 +0300
> +++ v4l-dvb.my/linux/drivers/media/video/Kconfig 2009-05-07
> 01:49:38.0 +0300
> @@ -479,6 +479,17 @@ config VIDEO_VIVI
> Say Y here if you want to test video apps or debug V4L devices.
> In doubt, say N.
>
> +config VIDEO_V4L2_LOOPBACK
> + tristate "v4l2 loopback driver"
> + depends on VIDEO_V4L2 && VIDEO_DEV
> + help
> + Say Y if you want to use v4l2 loopback driver.
> + Looback driver allows it's user to present any userspace
typo: it's -> its
> + video as a v4l2 device that can be handy for tesring purpose,
typo: tesring -> testing
> + or for fixing bugs like upside down image, or for adding
> + nice effects to videochats
> + This driver can be compiled as a module, called v4l2loopback.
> +
> source "drivers/media/video/bt8xx/Kconfig"
>
> config VIDEO_PMS
> diff -uprN v4l-dvb.orig/linux/drivers/media/video/Makefile
> v4l-dvb.my/linux/drivers/media/video/Makefile
> --- v4l-dvb.orig/linux/drivers/media/video/Makefile 2009-05-07
> 01:31:32.0 +0300
> +++ v4l-dvb.my/linux/drivers/media/video/Makefile 2009-05-07
> 01:50:11.0 +0300
> @@ -132,6 +132,7 @@ obj-$(CONFIG_VIDEO_IVTV) += ivtv/
> obj-$(CONFIG_VIDEO_CX18) += cx18/
>
> obj-$(CONFIG_VIDEO_VIVI) += vivi.o
> +obj-$(CONFIG_VIDEO_V4L2_LOOPBACK) += v4l2loopback.o
> obj-$(CONFIG_VIDEO_CX23885) += cx23885/
>
> obj-$(CONFIG_VIDEO_OMAP2)+= omap2cam.o
> diff -uprN v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.c
> v4l-dvb.my/linux/drivers/media/video/v4l2loopback.c
> --- v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.c 1970-01-01
> 03:00:00.0 +0300
> +++ v4l-dvb.my/linux/drivers/media/video/v4l2loopback.c 2009-05-07
> 02:30:08.0 +0300
> @@ -0,0 +1,775 @@
> +/*
> + * v4l2loopback.c -- video 4 linux loopback driver
> + *
> + * Copyright (C) 2005-2009
> + * Vasily Levin (vas...@gmail.com)
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
Usually the includes with the same prefix come all near one another,
you could move #include one line up.
> +#define YAVLD_STREAMING
> +
> +MODULE_DESCRIPTION("V4L2 loopback video device");
> +MODULE_VERSION("0.1.1");
> +MODULE_AUTHOR("Vasily Levin");
> +MODULE_LICENSE("GPL");
> +
> +/* module structures */
> +/* TODO(vasaka) use typenames which are common to kernel, but first find out
> if
> + * it is needed */
> +/* struct keeping state and settings of loopback device */
> +struct v4l2_loopback_device {
> + struct video_device *vdev;
> + /* pixel and stream format */
> + struct v4l2_pix_format pix_format;
> + struct v4l2_captureparm capture_param;
> + /* buffers stuff */
> + u8 *image; /* pointer to actual buffers data */
> + int buffers_number; /* should not be big, 4 is a good choice */
> + struct v4l2_buffer *buffers;/* inner driver buffers */
> + int write_position; /* number of last written frame + 1 */
> + long buffer_size;
> + /* sync stuff */
> + atomic_t open_count;
> + int ready_for_capture;/* set to true when at least one writer opened
> + * device and negotiated format */
> + wait_queue_head_t read_event;
[PATCH 06/11] ov534: Fix Auto White Balance control
From: Max Thrun
Set only the needed bits for AWB, and enable it by default.
Signed-off-by: Max Thrun
Signed-off-by: Antonio Ospite
---
linux/drivers/media/video/gspca/ov534.c | 21 +
1 file changed, 13 insertions(+), 8 deletions(-)
Index: gspca/linux/drivers/media/video/gspca/ov534.c
===
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -171,7 +171,7 @@
.minimum = 0,
.maximum = 1,
.step= 1,
-#define AWB_DEF 0
+#define AWB_DEF 1
.default_value = AWB_DEF,
},
.set = sd_setawb,
@@ -718,10 +718,17 @@
{
struct sd *sd = (struct sd *) gspca_dev;
- if (sd->awb)
- sccb_reg_write(gspca_dev, 0x63, 0xe0); /* AWB on */
- else
- sccb_reg_write(gspca_dev, 0x63, 0xaa); /* AWB off */
+ if (sd->awb) {
+ sccb_reg_write(gspca_dev, 0x13,
+ sccb_reg_read(gspca_dev, 0x13) | 0x02);
+ sccb_reg_write(gspca_dev, 0x63,
+ sccb_reg_read(gspca_dev, 0x63) | 0xc0);
+ } else {
+ sccb_reg_write(gspca_dev, 0x13,
+ sccb_reg_read(gspca_dev, 0x13) & ~0x02);
+ sccb_reg_write(gspca_dev, 0x63,
+ sccb_reg_read(gspca_dev, 0x63) & ~0xc0);
+ }
}
static void setaec(struct gspca_dev *gspca_dev)
@@ -800,9 +807,7 @@
#else
gspca_dev->ctrl_inac |= (1 << AWB_IDX);
#endif
-#if AWB_DEF != 0
- sd->awb = AWB_DEF
-#endif
+ sd->awb = AWB_DEF;
sd->aec = AEC_DEF;
#if SHARPNESS_DEF != 0
sd->sharpness = SHARPNESS_DEF;
--
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 10/11] ov534: Add Powerline Frequency control
From: Mosalam Ebrahimi
Note that setting this options to 50Hz can reduce the framerate, so the
default is still 60Hz.
Signed-off-by: Mosalam Ebrahimi
Signed-off-by: Antonio Ospite
---
linux/drivers/media/video/gspca/ov534.c | 71 +++-
1 file changed, 70 insertions(+), 1 deletion(-)
Index: gspca/linux/drivers/media/video/gspca/ov534.c
===
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -66,7 +66,7 @@
s8 sharpness;
u8 hflip;
u8 vflip;
-
+ u8 freqfltr;
};
/* V4L2 controls supported by the driver */
@@ -90,6 +90,10 @@
static int sd_getbrightness(struct gspca_dev *gspca_dev, __s32 *val);
static int sd_setcontrast(struct gspca_dev *gspca_dev, __s32 val);
static int sd_getcontrast(struct gspca_dev *gspca_dev, __s32 *val);
+static int sd_setfreqfltr(struct gspca_dev *gspca_dev, __s32 val);
+static int sd_getfreqfltr(struct gspca_dev *gspca_dev, __s32 *val);
+static int sd_querymenu(struct gspca_dev *gspca_dev,
+ struct v4l2_querymenu *menu);
static const struct ctrl sd_ctrls[] = {
{ /* 0 */
@@ -233,6 +237,20 @@
.set = sd_setvflip,
.get = sd_getvflip,
},
+{ /* 10 */
+ {
+ .id = V4L2_CID_POWER_LINE_FREQUENCY,
+ .type= V4L2_CTRL_TYPE_MENU,
+ .name= "Light Frequency Filter",
+ .minimum = 0,
+ .maximum = 1,
+ .step= 1,
+#define FREQFLTR_DEF 1
+ .default_value = FREQFLTR_DEF,
+ },
+ .set = sd_setfreqfltr,
+ .get = sd_getfreqfltr,
+},
};
static const struct v4l2_pix_format ov772x_mode[] = {
@@ -779,6 +797,17 @@
sccb_reg_read(gspca_dev, 0x0c) & ~0x80);
}
+static void setfreqfltr(struct gspca_dev *gspca_dev)
+{
+ struct sd *sd = (struct sd *) gspca_dev;
+
+ if (sd->freqfltr == 0)
+ sccb_reg_write(gspca_dev, 0x2b, 0x9e);
+ else
+ sccb_reg_write(gspca_dev, 0x2b, 0x00);
+}
+
+
/* this function is called at probe time */
static int sd_config(struct gspca_dev *gspca_dev,
const struct usb_device_id *id)
@@ -812,6 +841,7 @@
sd->sharpness = SHARPNESS_DEF;
sd->hflip = HFLIP_DEF;
sd->vflip = VFLIP_DEF;
+ sd->freqfltr = FREQFLTR_DEF;
return 0;
}
@@ -881,6 +911,7 @@
setsharpness(gspca_dev);
setvflip(gspca_dev);
sethflip(gspca_dev);
+ setfreqfltr(gspca_dev);
ov534_set_led(gspca_dev, 1);
ov534_reg_write(gspca_dev, 0xe0, 0x00);
@@ -1174,6 +1205,43 @@
return 0;
}
+static int sd_setfreqfltr(struct gspca_dev *gspca_dev, __s32 val)
+{
+ struct sd *sd = (struct sd *) gspca_dev;
+
+ sd->freqfltr = val;
+ if (gspca_dev->streaming)
+ setfreqfltr(gspca_dev);
+ return 0;
+}
+
+static int sd_getfreqfltr(struct gspca_dev *gspca_dev, __s32 *val)
+{
+ struct sd *sd = (struct sd *) gspca_dev;
+
+ *val = sd->freqfltr;
+ return 0;
+}
+
+static int sd_querymenu(struct gspca_dev *gspca_dev,
+ struct v4l2_querymenu *menu)
+{
+ switch (menu->id) {
+ case V4L2_CID_POWER_LINE_FREQUENCY:
+ switch (menu->index) {
+ case 0: /* V4L2_CID_POWER_LINE_FREQUENCY_50HZ */
+ strcpy((char *) menu->name, "50 Hz");
+ return 0;
+ case 1: /* V4L2_CID_POWER_LINE_FREQUENCY_60HZ */
+ strcpy((char *) menu->name, "60 Hz");
+ return 0;
+ }
+ break;
+ }
+
+ return -EINVAL;
+}
+
/* get stream parameters (framerate) */
static int sd_get_streamparm(struct gspca_dev *gspca_dev,
struct v4l2_streamparm *parm)
@@ -1225,6 +1293,7 @@
.start= sd_start,
.stopN= sd_stopN,
.pkt_scan = sd_pkt_scan,
+ .querymenu = sd_querymenu,
.get_streamparm = sd_get_streamparm,
.set_streamparm = sd_set_streamparm,
};
--
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 01/11] ov534: Remove ambiguous controls
From: Max Thrun
Remove Blue/Red Channel Target Value, they are meant for Black Level
Calibration but it is not completely clear how to use them.
Signed-off-by: Max Thrun
Signed-off-by: Antonio Ospite
---
linux/drivers/media/video/gspca/ov534.c | 100 +---
1 file changed, 6 insertions(+), 94 deletions(-)
Index: gspca/linux/drivers/media/video/gspca/ov534.c
===
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -60,8 +60,6 @@
u8 contrast;
u8 gain;
u8 exposure;
- u8 redblc;
- u8 blueblc;
u8 hue;
u8 autogain;
u8 awb;
@@ -76,10 +74,6 @@
static int sd_getgain(struct gspca_dev *gspca_dev, __s32 *val);
static int sd_setexposure(struct gspca_dev *gspca_dev, __s32 val);
static int sd_getexposure(struct gspca_dev *gspca_dev, __s32 *val);
-static int sd_setredblc(struct gspca_dev *gspca_dev, __s32 val);
-static int sd_getredblc(struct gspca_dev *gspca_dev, __s32 *val);
-static int sd_setblueblc(struct gspca_dev *gspca_dev, __s32 val);
-static int sd_getblueblc(struct gspca_dev *gspca_dev, __s32 *val);
static int sd_setautogain(struct gspca_dev *gspca_dev, __s32 val);
static int sd_getautogain(struct gspca_dev *gspca_dev, __s32 *val);
static int sd_setsharpness(struct gspca_dev *gspca_dev, __s32 val);
@@ -156,34 +150,6 @@
},
{ /* 4 */
{
- .id = V4L2_CID_RED_BALANCE,
- .type= V4L2_CTRL_TYPE_INTEGER,
- .name= "Red Balance",
- .minimum = 0,
- .maximum = 255,
- .step= 1,
-#define RED_BALANCE_DEF 128
- .default_value = RED_BALANCE_DEF,
- },
- .set = sd_setredblc,
- .get = sd_getredblc,
-},
-{ /* 5 */
- {
- .id = V4L2_CID_BLUE_BALANCE,
- .type= V4L2_CTRL_TYPE_INTEGER,
- .name= "Blue Balance",
- .minimum = 0,
- .maximum = 255,
- .step= 1,
-#define BLUE_BALANCE_DEF 128
- .default_value = BLUE_BALANCE_DEF,
- },
- .set = sd_setblueblc,
- .get = sd_getblueblc,
-},
-{ /* 6 */
- {
.id = V4L2_CID_HUE,
.type= V4L2_CTRL_TYPE_INTEGER,
.name= "Hue",
@@ -196,7 +162,7 @@
.set = sd_sethue,
.get = sd_gethue,
},
-{ /* 7 */
+{ /* 5 */
{
.id = V4L2_CID_AUTOGAIN,
.type= V4L2_CTRL_TYPE_BOOLEAN,
@@ -210,8 +176,8 @@
.set = sd_setautogain,
.get = sd_getautogain,
},
-#define AWB_IDX 8
-{ /* 8 */
+#define AWB_IDX 6
+{ /* 6 */
{
.id = V4L2_CID_AUTO_WHITE_BALANCE,
.type= V4L2_CTRL_TYPE_BOOLEAN,
@@ -225,7 +191,7 @@
.set = sd_setawb,
.get = sd_getawb,
},
-{ /* 9 */
+{ /* 7 */
{
.id = V4L2_CID_SHARPNESS,
.type= V4L2_CTRL_TYPE_INTEGER,
@@ -239,7 +205,7 @@
.set = sd_setsharpness,
.get = sd_getsharpness,
},
-{ /* 10 */
+{ /* 8 */
{
.id = V4L2_CID_HFLIP,
.type= V4L2_CTRL_TYPE_BOOLEAN,
@@ -253,7 +219,7 @@
.set = sd_sethflip,
.get = sd_gethflip,
},
-{ /* 11 */
+{ /* 9 */
{
.id = V4L2_CID_VFLIP,
.type= V4L2_CTRL_TYPE_BOOLEAN,
@@ -722,20 +688,6 @@
sccb_reg_write(gspca_dev, 0x10, val << 1);
}
-static void setredblc(struct gspca_dev *gspca_dev)
-{
- struct sd *sd = (struct sd *) gspca_dev;
-
- sccb_reg_write(gspca_dev, 0x43, sd->redblc);
-}
-
-static void setblueblc(struct gspca_dev *gspca_dev)
-{
- struct sd *sd = (struct sd *) gspca_dev;
-
- sccb_reg_write(gspca_dev, 0x42, sd->blueblc);
-}
-
static void sethue(struct gspca_dev *gspca_dev)
{
struct sd *sd = (struct sd *) gspca_dev;
@@ -825,8 +777,6 @@
sd->contrast = CONTRAST_DEF;
sd->gain = GAIN_DEF;
sd->exposure = EXPO_DEF;
- sd->redblc = RED_BALANCE_DEF;
- sd->blueblc = BLUE_BALANCE_DEF;
sd->hue = HUE_DEF;
#if AUTOGAIN_DEF != 0
sd->autogain =
[PATCH 07/11] ov534: Fixes for sharpness control
From: Max Thrun * Adjust comments for sharpness control * Set default value unconditionally, for readability Signed-off-by: Max Thrun Signed-off-by: Antonio Ospite --- linux/drivers/media/video/gspca/ov534.c |6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) Index: gspca/linux/drivers/media/video/gspca/ov534.c === --- gspca.orig/linux/drivers/media/video/gspca/ov534.c +++ gspca/linux/drivers/media/video/gspca/ov534.c @@ -751,8 +751,8 @@ u8 val; val = sd->sharpness; - sccb_reg_write(gspca_dev, 0x91, val); /* vga noise */ - sccb_reg_write(gspca_dev, 0x8e, val); /* qvga noise */ + sccb_reg_write(gspca_dev, 0x91, val); /* Auto de-noise threshold */ + sccb_reg_write(gspca_dev, 0x8e, val); /* De-noise threshold */ } static void sethflip(struct gspca_dev *gspca_dev) @@ -809,9 +809,7 @@ #endif sd->awb = AWB_DEF; sd->aec = AEC_DEF; -#if SHARPNESS_DEF != 0 sd->sharpness = SHARPNESS_DEF; -#endif #if HFLIP_DEF != 0 sd->hflip = HFLIP_DEF; #endif -- 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 11/11] ov534: Update copyright info
From: Max Thrun Signed-off-by: Max Thrun Signed-off-by: Antonio Ospite --- linux/drivers/media/video/gspca/ov534.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Index: gspca/linux/drivers/media/video/gspca/ov534.c === --- gspca.orig/linux/drivers/media/video/gspca/ov534.c +++ gspca/linux/drivers/media/video/gspca/ov534.c @@ -10,8 +10,8 @@ * https://jim.sh/svn/jim/devl/playstation/ps3/eye/test/ * * PS3 Eye camera enhanced by Richard Kaswy http://kaswy.free.fr - * PS3 Eye camera, brightness, contrast, hue, AWB control added - * by Max Thrun + * PS3 Eye camera - brightness, contrast, awb, agc, aec controls + * added by Max Thrun * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by -- 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 02/11] ov534: Remove hue control
Hue control doesn't work and the sensor datasheet is not clear about how
to set hue properly.
Signed-off-by: Antonio Ospite
---
linux/drivers/media/video/gspca/ov534.c | 54 ++--
1 file changed, 5 insertions(+), 49 deletions(-)
Index: gspca/linux/drivers/media/video/gspca/ov534.c
===
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -60,7 +60,6 @@
u8 contrast;
u8 gain;
u8 exposure;
- u8 hue;
u8 autogain;
u8 awb;
s8 sharpness;
@@ -82,8 +81,6 @@
static int sd_gethflip(struct gspca_dev *gspca_dev, __s32 *val);
static int sd_setvflip(struct gspca_dev *gspca_dev, __s32 val);
static int sd_getvflip(struct gspca_dev *gspca_dev, __s32 *val);
-static int sd_sethue(struct gspca_dev *gspca_dev, __s32 val);
-static int sd_gethue(struct gspca_dev *gspca_dev, __s32 *val);
static int sd_setawb(struct gspca_dev *gspca_dev, __s32 val);
static int sd_getawb(struct gspca_dev *gspca_dev, __s32 *val);
static int sd_setbrightness(struct gspca_dev *gspca_dev, __s32 val);
@@ -150,20 +147,6 @@
},
{ /* 4 */
{
- .id = V4L2_CID_HUE,
- .type= V4L2_CTRL_TYPE_INTEGER,
- .name= "Hue",
- .minimum = 0,
- .maximum = 255,
- .step= 1,
-#define HUE_DEF 143
- .default_value = HUE_DEF,
- },
- .set = sd_sethue,
- .get = sd_gethue,
-},
-{ /* 5 */
- {
.id = V4L2_CID_AUTOGAIN,
.type= V4L2_CTRL_TYPE_BOOLEAN,
.name= "Autogain",
@@ -176,8 +159,8 @@
.set = sd_setautogain,
.get = sd_getautogain,
},
-#define AWB_IDX 6
-{ /* 6 */
+#define AWB_IDX 5
+{ /* 5 */
{
.id = V4L2_CID_AUTO_WHITE_BALANCE,
.type= V4L2_CTRL_TYPE_BOOLEAN,
@@ -191,7 +174,7 @@
.set = sd_setawb,
.get = sd_getawb,
},
-{ /* 7 */
+{ /* 6 */
{
.id = V4L2_CID_SHARPNESS,
.type= V4L2_CTRL_TYPE_INTEGER,
@@ -205,7 +188,7 @@
.set = sd_setsharpness,
.get = sd_getsharpness,
},
-{ /* 8 */
+{ /* 7 */
{
.id = V4L2_CID_HFLIP,
.type= V4L2_CTRL_TYPE_BOOLEAN,
@@ -219,7 +202,7 @@
.set = sd_sethflip,
.get = sd_gethflip,
},
-{ /* 9 */
+{ /* 8 */
{
.id = V4L2_CID_VFLIP,
.type= V4L2_CTRL_TYPE_BOOLEAN,
@@ -688,13 +671,6 @@
sccb_reg_write(gspca_dev, 0x10, val << 1);
}
-static void sethue(struct gspca_dev *gspca_dev)
-{
- struct sd *sd = (struct sd *) gspca_dev;
-
- sccb_reg_write(gspca_dev, 0x01, sd->hue);
-}
-
static void setautogain(struct gspca_dev *gspca_dev)
{
struct sd *sd = (struct sd *) gspca_dev;
@@ -777,7 +753,6 @@
sd->contrast = CONTRAST_DEF;
sd->gain = GAIN_DEF;
sd->exposure = EXPO_DEF;
- sd->hue = HUE_DEF;
#if AUTOGAIN_DEF != 0
sd->autogain = AUTOGAIN_DEF;
#else
@@ -857,7 +832,6 @@
setautogain(gspca_dev);
setawb(gspca_dev);
setgain(gspca_dev);
- sethue(gspca_dev);
setexposure(gspca_dev);
setbrightness(gspca_dev);
setcontrast(gspca_dev);
@@ -1040,24 +1014,6 @@
return 0;
}
-static int sd_sethue(struct gspca_dev *gspca_dev, __s32 val)
-{
- struct sd *sd = (struct sd *) gspca_dev;
-
- sd->hue = val;
- if (gspca_dev->streaming)
- sethue(gspca_dev);
- return 0;
-}
-
-static int sd_gethue(struct gspca_dev *gspca_dev, __s32 *val)
-{
- struct sd *sd = (struct sd *) gspca_dev;
-
- *val = sd->hue;
- return 0;
-}
-
static int sd_setautogain(struct gspca_dev *gspca_dev, __s32 val)
{
struct sd *sd = (struct sd *) gspca_dev;
--
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 00/11] ov534: Fixes and updates
Hi Jean-Francois, will you review/apply these changes please? Several fixes and updates: * Removed some controls because their state was not good enough * Added AEC * AGC and AWB enabled by default * Fixed setting/unsetting registers for - exposure - sharpness - hflip - vflip * Fixed coding style * Added Powerline Frequency control The only big behavioural change should be AGC enabled by default, if users want autogain disabled by default we can rediscuss this again. Special thanks to Max Thrun and Mosalam Ebrahimi. Regards, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? -- 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 03/11] ov534: Fix autogain control, enable it by default
From: Max Thrun
* Use 'agc' instead of 'autogain' in the code so to align the naming
as in AEC/AWB.
* Tweak brightness and contrast default values.
* Fix setting/resetting registers values for AGC.
* Set actual gain back when disabling AGC.
* Skip setting GAIN register when AGC is enabled.
* Enable AGC by default.
Note that as Auto Gain Control is now enabled by default, if you are
using the driver for visual computing applications you might need to
disable it explicitly in your software.
Signed-off-by: Max Thrun
Signed-off-by: Antonio Ospite
---
linux/drivers/media/video/gspca/ov534.c | 53 ++--
1 file changed, 30 insertions(+), 23 deletions(-)
Index: gspca/linux/drivers/media/video/gspca/ov534.c
===
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -60,7 +60,7 @@
u8 contrast;
u8 gain;
u8 exposure;
- u8 autogain;
+ u8 agc;
u8 awb;
s8 sharpness;
u8 hflip;
@@ -73,8 +73,8 @@
static int sd_getgain(struct gspca_dev *gspca_dev, __s32 *val);
static int sd_setexposure(struct gspca_dev *gspca_dev, __s32 val);
static int sd_getexposure(struct gspca_dev *gspca_dev, __s32 *val);
-static int sd_setautogain(struct gspca_dev *gspca_dev, __s32 val);
-static int sd_getautogain(struct gspca_dev *gspca_dev, __s32 *val);
+static int sd_setagc(struct gspca_dev *gspca_dev, __s32 val);
+static int sd_getagc(struct gspca_dev *gspca_dev, __s32 *val);
static int sd_setsharpness(struct gspca_dev *gspca_dev, __s32 val);
static int sd_getsharpness(struct gspca_dev *gspca_dev, __s32 *val);
static int sd_sethflip(struct gspca_dev *gspca_dev, __s32 val);
@@ -97,7 +97,7 @@
.minimum = 0,
.maximum = 255,
.step= 1,
-#define BRIGHTNESS_DEF 20
+#define BRIGHTNESS_DEF 0
.default_value = BRIGHTNESS_DEF,
},
.set = sd_setbrightness,
@@ -111,7 +111,7 @@
.minimum = 0,
.maximum = 255,
.step= 1,
-#define CONTRAST_DEF 37
+#define CONTRAST_DEF 32
.default_value = CONTRAST_DEF,
},
.set = sd_setcontrast,
@@ -149,15 +149,15 @@
{
.id = V4L2_CID_AUTOGAIN,
.type= V4L2_CTRL_TYPE_BOOLEAN,
- .name= "Autogain",
+ .name= "Auto Gain",
.minimum = 0,
.maximum = 1,
.step= 1,
-#define AUTOGAIN_DEF 0
- .default_value = AUTOGAIN_DEF,
+#define AGC_DEF 1
+ .default_value = AGC_DEF,
},
- .set = sd_setautogain,
- .get = sd_getautogain,
+ .set = sd_setagc,
+ .get = sd_getagc,
},
#define AWB_IDX 5
{ /* 5 */
@@ -639,6 +639,9 @@
struct sd *sd = (struct sd *) gspca_dev;
u8 val;
+ if (sd->agc)
+ return;
+
val = sd->gain;
switch (val & 0x30) {
case 0x00:
@@ -671,18 +674,22 @@
sccb_reg_write(gspca_dev, 0x10, val << 1);
}
-static void setautogain(struct gspca_dev *gspca_dev)
+static void setagc(struct gspca_dev *gspca_dev)
{
struct sd *sd = (struct sd *) gspca_dev;
- if (sd->autogain) {
- sccb_reg_write(gspca_dev, 0x13, 0xf7); /* AGC,AEC,AWB ON */
+ if (sd->agc) {
+ sccb_reg_write(gspca_dev, 0x13,
+ sccb_reg_read(gspca_dev, 0x13) | 0x04);
sccb_reg_write(gspca_dev, 0x64,
sccb_reg_read(gspca_dev, 0x64) | 0x03);
} else {
- sccb_reg_write(gspca_dev, 0x13, 0xf0); /* AGC,AEC,AWB OFF */
+ sccb_reg_write(gspca_dev, 0x13,
+ sccb_reg_read(gspca_dev, 0x13) & ~0x04);
sccb_reg_write(gspca_dev, 0x64,
- sccb_reg_read(gspca_dev, 0x64) & 0xfc);
+ sccb_reg_read(gspca_dev, 0x64) & ~0x03);
+
+ setgain(gspca_dev);
}
}
@@ -753,8 +760,8 @@
sd->contrast = CONTRAST_DEF;
sd->gain = GAIN_DEF;
sd->exposure = EXPO_DEF;
-#if AUTOGAIN_DEF != 0
- sd->autogain = AUTOGAIN_DEF;
+#if AGC_DEF != 0
+ sd->agc = AGC_DEF;
#else
gspca_dev->ctrl_inac |= (1 << AWB_IDX);
#endif
@@ -829,7 +836,7 @@
}
set_frame_rate(gspca_dev);
- setautogain(gspca_dev);
+ setagc(gspca_dev);
setawb(gspca_dev);
setgain(gspca_dev);
setexposure(gspca_dev);
@@ -1014,11 +1021,11 @@
return 0;
}
-static int sd_setautogain(struct gspca_dev *gspca_dev, __s32 val)
+static int sd_setagc(struct gspca_dev *gspca_dev, __s32 val)
{
struct sd *sd = (struct sd *)
[PATCH 05/11] ov534: Fix setting manual exposure
Exposure is now a u16 value, both MSB and LSB are set, but values in the v4l2
control are limited to the interval [0,506] as 0x01fa (506) is the maximum
observed value with AEC enabled.
Skip setting exposure when AEC is enabled.
Signed-off-by: Antonio Ospite
---
linux/drivers/media/video/gspca/ov534.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
Index: gspca/linux/drivers/media/video/gspca/ov534.c
===
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -59,7 +59,7 @@
u8 brightness;
u8 contrast;
u8 gain;
- u8 exposure;
+ u16 exposure;
u8 agc;
u8 awb;
u8 aec;
@@ -140,7 +140,7 @@
.type= V4L2_CTRL_TYPE_INTEGER,
.name= "Exposure",
.minimum = 0,
- .maximum = 255,
+ .maximum = 506,
.step= 1,
#define EXPO_DEF 120
.default_value = EXPO_DEF,
@@ -684,11 +684,15 @@
static void setexposure(struct gspca_dev *gspca_dev)
{
struct sd *sd = (struct sd *) gspca_dev;
- u8 val;
+ u16 val;
+
+ if (sd->aec)
+ return;
val = sd->exposure;
- sccb_reg_write(gspca_dev, 0x08, val >> 7);
- sccb_reg_write(gspca_dev, 0x10, val << 1);
+ sccb_reg_write(gspca_dev, 0x08, val >> 8);
+ sccb_reg_write(gspca_dev, 0x10, val & 0xff);
+
}
static void setagc(struct gspca_dev *gspca_dev)
--
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 09/11] ov534: Cosmetics: fix indentation and hex digits
* Indent with tabs, not with spaces.
* Less indentation for controls index comments.
* Use lowercase hex digits.
Signed-off-by: Antonio Ospite
---
linux/drivers/media/video/gspca/ov534.c | 128
1 file changed, 64 insertions(+), 64 deletions(-)
Index: gspca/linux/drivers/media/video/gspca/ov534.c
===
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -92,7 +92,7 @@
static int sd_getcontrast(struct gspca_dev *gspca_dev, __s32 *val);
static const struct ctrl sd_ctrls[] = {
-{ /* 0 */
+{ /* 0 */
{
.id = V4L2_CID_BRIGHTNESS,
.type= V4L2_CTRL_TYPE_INTEGER,
@@ -105,8 +105,8 @@
},
.set = sd_setbrightness,
.get = sd_getbrightness,
-},
-{ /* 1 */
+},
+{ /* 1 */
{
.id = V4L2_CID_CONTRAST,
.type= V4L2_CTRL_TYPE_INTEGER,
@@ -119,51 +119,51 @@
},
.set = sd_setcontrast,
.get = sd_getcontrast,
-},
-{ /* 2 */
+},
+{ /* 2 */
{
- .id = V4L2_CID_GAIN,
- .type= V4L2_CTRL_TYPE_INTEGER,
- .name= "Main Gain",
- .minimum = 0,
- .maximum = 63,
- .step= 1,
+ .id = V4L2_CID_GAIN,
+ .type= V4L2_CTRL_TYPE_INTEGER,
+ .name= "Main Gain",
+ .minimum = 0,
+ .maximum = 63,
+ .step= 1,
#define GAIN_DEF 20
- .default_value = GAIN_DEF,
+ .default_value = GAIN_DEF,
},
.set = sd_setgain,
.get = sd_getgain,
-},
-{ /* 3 */
+},
+{ /* 3 */
{
- .id = V4L2_CID_EXPOSURE,
- .type= V4L2_CTRL_TYPE_INTEGER,
- .name= "Exposure",
- .minimum = 0,
- .maximum = 506,
- .step= 1,
+ .id = V4L2_CID_EXPOSURE,
+ .type= V4L2_CTRL_TYPE_INTEGER,
+ .name= "Exposure",
+ .minimum = 0,
+ .maximum = 506,
+ .step= 1,
#define EXPO_DEF 120
- .default_value = EXPO_DEF,
+ .default_value = EXPO_DEF,
},
.set = sd_setexposure,
.get = sd_getexposure,
-},
-{ /* 4 */
+},
+{ /* 4 */
{
- .id = V4L2_CID_AUTOGAIN,
- .type= V4L2_CTRL_TYPE_BOOLEAN,
- .name= "Auto Gain",
- .minimum = 0,
- .maximum = 1,
- .step= 1,
+ .id = V4L2_CID_AUTOGAIN,
+ .type= V4L2_CTRL_TYPE_BOOLEAN,
+ .name= "Auto Gain",
+ .minimum = 0,
+ .maximum = 1,
+ .step= 1,
#define AGC_DEF 1
- .default_value = AGC_DEF,
+ .default_value = AGC_DEF,
},
.set = sd_setagc,
.get = sd_getagc,
-},
+},
#define AWB_IDX 5
-{ /* 5 */
+{ /* 5 */
{
.id = V4L2_CID_AUTO_WHITE_BALANCE,
.type= V4L2_CTRL_TYPE_BOOLEAN,
@@ -176,8 +176,8 @@
},
.set = sd_setawb,
.get = sd_getawb,
-},
-{ /* 6 */
+},
+{ /* 6 */
{
.id = V4L2_CID_EXPOSURE_AUTO,
.type= V4L2_CTRL_TYPE_BOOLEAN,
@@ -190,49 +190,49 @@
},
.set = sd_setaec,
.get = sd_getaec,
-},
-{ /* 7 */
+},
+{ /* 7 */
{
- .id = V4L2_CID_SHARPNESS,
- .type= V4L2_CTRL_TYPE_INTEGER,
- .name= "Sharpness",
- .minimum = 0,
- .maximum = 63,
- .step= 1,
+ .id = V4L2_CID_SHARPNESS,
+ .type= V4L2_CTRL_TYPE_INTEGER,
+ .name= "Sharpness",
+ .minimum = 0,
+ .maximum = 63,
+ .step= 1,
#define SHARPNESS_DEF 0
- .default_value = SHARPNESS_DEF,
+ .default_value = SHARPNESS_DEF,
},
.set = sd_setsharpness,
.get = sd_getsharpness,
-},
-{ /* 8 */
+},
+{ /* 8 */
{
- .id = V4L2_CID_HFLIP,
- .type= V4L2_CTRL_TYPE_BOOLEAN,
- .name= "HFlip",
- .minimum = 0,
-
[PATCH 04/11] ov534: Add Auto Exposure
From: Max Thrun
This also makes manual exposure actually work: it never worked before
because AEC was always enabled.
Signed-off-by: Max Thrun
Signed-off-by: Antonio Ospite
---
linux/drivers/media/video/gspca/ov534.c | 55 ++--
1 file changed, 53 insertions(+), 2 deletions(-)
Index: gspca/linux/drivers/media/video/gspca/ov534.c
===
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -62,6 +62,7 @@
u8 exposure;
u8 agc;
u8 awb;
+ u8 aec;
s8 sharpness;
u8 hflip;
u8 vflip;
@@ -83,6 +84,8 @@
static int sd_getvflip(struct gspca_dev *gspca_dev, __s32 *val);
static int sd_setawb(struct gspca_dev *gspca_dev, __s32 val);
static int sd_getawb(struct gspca_dev *gspca_dev, __s32 *val);
+static int sd_setaec(struct gspca_dev *gspca_dev, __s32 val);
+static int sd_getaec(struct gspca_dev *gspca_dev, __s32 *val);
static int sd_setbrightness(struct gspca_dev *gspca_dev, __s32 val);
static int sd_getbrightness(struct gspca_dev *gspca_dev, __s32 *val);
static int sd_setcontrast(struct gspca_dev *gspca_dev, __s32 val);
@@ -176,6 +179,20 @@
},
{ /* 6 */
{
+ .id = V4L2_CID_EXPOSURE_AUTO,
+ .type= V4L2_CTRL_TYPE_BOOLEAN,
+ .name= "Auto Exposure",
+ .minimum = 0,
+ .maximum = 1,
+ .step= 1,
+#define AEC_DEF 1
+ .default_value = AEC_DEF,
+ },
+ .set = sd_setaec,
+ .get = sd_getaec,
+},
+{ /* 7 */
+ {
.id = V4L2_CID_SHARPNESS,
.type= V4L2_CTRL_TYPE_INTEGER,
.name= "Sharpness",
@@ -188,7 +205,7 @@
.set = sd_setsharpness,
.get = sd_getsharpness,
},
-{ /* 7 */
+{ /* 8 */
{
.id = V4L2_CID_HFLIP,
.type= V4L2_CTRL_TYPE_BOOLEAN,
@@ -202,7 +219,7 @@
.set = sd_sethflip,
.get = sd_gethflip,
},
-{ /* 8 */
+{ /* 9 */
{
.id = V4L2_CID_VFLIP,
.type= V4L2_CTRL_TYPE_BOOLEAN,
@@ -703,6 +720,20 @@
sccb_reg_write(gspca_dev, 0x63, 0xaa); /* AWB off */
}
+static void setaec(struct gspca_dev *gspca_dev)
+{
+ struct sd *sd = (struct sd *) gspca_dev;
+
+ if (sd->aec)
+ sccb_reg_write(gspca_dev, 0x13,
+ sccb_reg_read(gspca_dev, 0x13) | 0x01);
+ else {
+ sccb_reg_write(gspca_dev, 0x13,
+ sccb_reg_read(gspca_dev, 0x13) & ~0x01);
+ setexposure(gspca_dev);
+ }
+}
+
static void setsharpness(struct gspca_dev *gspca_dev)
{
struct sd *sd = (struct sd *) gspca_dev;
@@ -768,6 +799,7 @@
#if AWB_DEF != 0
sd->awb = AWB_DEF
#endif
+ sd->aec = AEC_DEF;
#if SHARPNESS_DEF != 0
sd->sharpness = SHARPNESS_DEF;
#endif
@@ -838,6 +870,7 @@
setagc(gspca_dev);
setawb(gspca_dev);
+ setaec(gspca_dev);
setgain(gspca_dev);
setexposure(gspca_dev);
setbrightness(gspca_dev);
@@ -1066,6 +1099,24 @@
return 0;
}
+static int sd_setaec(struct gspca_dev *gspca_dev, __s32 val)
+{
+ struct sd *sd = (struct sd *) gspca_dev;
+
+ sd->aec = val;
+ if (gspca_dev->streaming)
+ setaec(gspca_dev);
+ return 0;
+}
+
+static int sd_getaec(struct gspca_dev *gspca_dev, __s32 *val)
+{
+ struct sd *sd = (struct sd *) gspca_dev;
+
+ *val = sd->aec;
+ return 0;
+}
+
static int sd_setsharpness(struct gspca_dev *gspca_dev, __s32 val)
{
struct sd *sd = (struct sd *) gspca_dev;
--
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 08/11] ov534: Fix unsetting hflip and vflip bits
From: Max Thrun Also set default values unconditionally, for readability. Signed-off-by: Max Thrun Signed-off-by: Antonio Ospite --- linux/drivers/media/video/gspca/ov534.c |8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) Index: gspca/linux/drivers/media/video/gspca/ov534.c === --- gspca.orig/linux/drivers/media/video/gspca/ov534.c +++ gspca/linux/drivers/media/video/gspca/ov534.c @@ -764,7 +764,7 @@ sccb_reg_read(gspca_dev, 0x0c) | 0x40); else sccb_reg_write(gspca_dev, 0x0c, - sccb_reg_read(gspca_dev, 0x0c) & 0xbf); + sccb_reg_read(gspca_dev, 0x0c) & ~0x40); } static void setvflip(struct gspca_dev *gspca_dev) @@ -776,7 +776,7 @@ sccb_reg_read(gspca_dev, 0x0c) | 0x80); else sccb_reg_write(gspca_dev, 0x0c, - sccb_reg_read(gspca_dev, 0x0c) & 0x7f); + sccb_reg_read(gspca_dev, 0x0c) & ~0x80); } /* this function is called at probe time */ @@ -810,12 +810,8 @@ sd->awb = AWB_DEF; sd->aec = AEC_DEF; sd->sharpness = SHARPNESS_DEF; -#if HFLIP_DEF != 0 sd->hflip = HFLIP_DEF; -#endif -#if VFLIP_DEF != 0 sd->vflip = VFLIP_DEF; -#endif return 0; } -- 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 05/11] ov534: Fix setting manual exposure
On Sun, 28 Feb 2010 19:38:14 +0100 Jean-Francois Moine wrote: > On Sat, 27 Feb 2010 21:20:22 +0100 > Antonio Ospite wrote: > > > Exposure is now a u16 value, both MSB and LSB are set, but values in > > the v4l2 control are limited to the interval [0,506] as 0x01fa (506) > > is the maximum observed value with AEC enabled. > [snip] > > .type= V4L2_CTRL_TYPE_INTEGER, > > .name= "Exposure", > > .minimum = 0, > > - .maximum = 255, > > + .maximum = 506, > > .step= 1, > > #define EXPO_DEF 120 > > .default_value = EXPO_DEF, > > Hi Antonio, > > Do we need a so high precision for the exposure? Just setting the > maximum value to 253 should solve the problem. > JF, the intent here is to cover all the range of values available in Auto Exposure mode too, doesn't this make sense to you? I could set .maximum to 253 to limit the "UI" control precision but then I should use 2*value when setting the registers in order to cover the actual max value, this looks a little unclean. Anyhow, let me know what you prefer, I have no strong feelings on that. If you want to save a byte, I'll agree. Regards, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? pgp9LFJ2MkYGn.pgp Description: PGP signature
Re: [PATCH 09/11] ov534: Cosmetics: fix indentation and hex digits
On Sun, 28 Feb 2010 19:46:36 +0100 Jean-Francois Moine wrote: > On Sat, 27 Feb 2010 21:20:26 +0100 > Antonio Ospite wrote: > > > * Indent with tabs, not with spaces. > > * Less indentation for controls index comments. > [snip] > > -}, > > +}, > > }; > > I had preferred one more TAB for all controls. I found it redundant, but I am preparing a v2 patch as per your request now. I'll also need to refresh patch 10, will send a v2 for it too after discussing your comments on that one. Regards, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? pgp3BMRUIwUY1.pgp Description: PGP signature
Re: [PATCH 10/11] ov534: Add Powerline Frequency control
On Sun, 28 Feb 2010 19:49:51 +0100
Jean-Francois Moine wrote:
> On Sat, 27 Feb 2010 21:20:27 +0100
> Antonio Ospite wrote:
>
> > +static int sd_querymenu(struct gspca_dev *gspca_dev,
> > + struct v4l2_querymenu *menu)
> > +{
> > + switch (menu->id) {
> > + case V4L2_CID_POWER_LINE_FREQUENCY:
> > + switch (menu->index) {
> > + case 0: /*
> > V4L2_CID_POWER_LINE_FREQUENCY_50HZ */
> > + strcpy((char *) menu->name, "50 Hz");
> > + return 0;
> > + case 1: /*
> > V4L2_CID_POWER_LINE_FREQUENCY_60HZ */
> > + strcpy((char *) menu->name, "60 Hz");
> > + return 0;
> > + }
> > + break;
> > + }
> > +
> > + return -EINVAL;
> > +}
>
> In videodev2.h, there is:
>
> V4L2_CID_POWER_LINE_FREQUENCY_50HZ = 1,
> V4L2_CID_POWER_LINE_FREQUENCY_60HZ = 2,
>
Maybe we could just use
V4L2_CID_POWER_LINE_FREQUENCY_DISABLED = 0,
V4L2_CID_POWER_LINE_FREQUENCY_50HZ = 1,
It looks like the code matches the DISABLED state (writing 0 to the
register). Mosalam?
Regards,
Antonio
--
Antonio Ospite
http://ao2.it
PGP public key ID: 0x4553B001
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?
pgp2X3jNIt1Rk.pgp
Description: PGP signature
Re: [PATCH 05/11] ov534: Fix setting manual exposure
On Sun, 28 Feb 2010 19:54:25 +0100 Antonio Ospite wrote: > On Sun, 28 Feb 2010 19:38:14 +0100 > Jean-Francois Moine wrote: > > > On Sat, 27 Feb 2010 21:20:22 +0100 > > Antonio Ospite wrote: > > > > > Exposure is now a u16 value, both MSB and LSB are set, but values in > > > the v4l2 control are limited to the interval [0,506] as 0x01fa (506) > > > is the maximum observed value with AEC enabled. > > [snip] > > > .type= V4L2_CTRL_TYPE_INTEGER, > > > .name= "Exposure", > > > .minimum = 0, > > > - .maximum = 255, > > > + .maximum = 506, > > > .step= 1, > > > #define EXPO_DEF 120 > > > .default_value = EXPO_DEF, > > > > Hi Antonio, > > > > Do we need a so high precision for the exposure? Just setting the > > maximum value to 253 should solve the problem. > > > > JF, the intent here is to cover all the range of values available in > Auto Exposure mode too, doesn't this make sense to you? > > I could set .maximum to 253 to limit the "UI" control precision but then > I should use 2*value when setting the registers in order to cover the > actual max value, this looks a little unclean. > Ok, I now see that that's exactly what current code does, sorry for the noise. The patch then degenerates to a simpler one with some documentation added, so others don't overlook the code like I did. Sending it in a min. Regards, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? pgpW3oPfs6eYu.pgp Description: PGP signature
[PATCH v2 05/11] ov534: Fix and document setting manual exposure
Document that even if the state is a u8 value, both MSB and LSB are set as sd->exposure represents half of the value we are going to set into registers. Skip setting exposure when AEC is enabled. Signed-off-by: Antonio Ospite --- The code was already doing the right thing, I just overlooked it. Regards, Antonio linux/drivers/media/video/gspca/ov534.c |9 + 1 file changed, 9 insertions(+) Index: gspca/linux/drivers/media/video/gspca/ov534.c === --- gspca.orig/linux/drivers/media/video/gspca/ov534.c +++ gspca/linux/drivers/media/video/gspca/ov534.c @@ -686,6 +686,15 @@ struct sd *sd = (struct sd *) gspca_dev; u8 val; + if (sd->aec) + return; + + /* 'val' is one byte and represents half of the exposure value we are +* going to set into registers, a two bytes value: +* +*MSB: ((u16) val << 1) >> 8 == val >> 7 +*LSB: ((u16) val << 1) & 0xff == val << 1 +*/ val = sd->exposure; sccb_reg_write(gspca_dev, 0x08, val >> 7); sccb_reg_write(gspca_dev, 0x10, val << 1); -- 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 v2 09/11] ov534: Cosmetics: fix indentation and hex digits
* Indent with tabs, not with spaces, nor with mixed style.
* Less indentation for controls index comments.
* Use lowercase hex digits.
Signed-off-by: Antonio Ospite
---
Changes since v1:
Indent controls by one more level as JF requested.
Note that the controls "index comments" are still less indented than before,
it is more readable this way.
Thanks,
Antonio
linux/drivers/media/video/gspca/ov534.c | 260
1 file changed, 130 insertions(+), 130 deletions(-)
Index: gspca/linux/drivers/media/video/gspca/ov534.c
===
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -92,147 +92,147 @@
static int sd_getcontrast(struct gspca_dev *gspca_dev, __s32 *val);
static const struct ctrl sd_ctrls[] = {
-{ /* 0 */
- {
- .id = V4L2_CID_BRIGHTNESS,
- .type= V4L2_CTRL_TYPE_INTEGER,
- .name= "Brightness",
- .minimum = 0,
- .maximum = 255,
- .step= 1,
+ { /* 0 */
+ {
+ .id = V4L2_CID_BRIGHTNESS,
+ .type= V4L2_CTRL_TYPE_INTEGER,
+ .name= "Brightness",
+ .minimum = 0,
+ .maximum = 255,
+ .step= 1,
#define BRIGHTNESS_DEF 0
- .default_value = BRIGHTNESS_DEF,
- },
- .set = sd_setbrightness,
- .get = sd_getbrightness,
-},
-{ /* 1 */
- {
- .id = V4L2_CID_CONTRAST,
- .type= V4L2_CTRL_TYPE_INTEGER,
- .name= "Contrast",
- .minimum = 0,
- .maximum = 255,
- .step= 1,
+ .default_value = BRIGHTNESS_DEF,
+ },
+ .set = sd_setbrightness,
+ .get = sd_getbrightness,
+ },
+ { /* 1 */
+ {
+ .id = V4L2_CID_CONTRAST,
+ .type= V4L2_CTRL_TYPE_INTEGER,
+ .name= "Contrast",
+ .minimum = 0,
+ .maximum = 255,
+ .step= 1,
#define CONTRAST_DEF 32
- .default_value = CONTRAST_DEF,
- },
- .set = sd_setcontrast,
- .get = sd_getcontrast,
-},
-{ /* 2 */
- {
- .id = V4L2_CID_GAIN,
- .type= V4L2_CTRL_TYPE_INTEGER,
- .name= "Main Gain",
- .minimum = 0,
- .maximum = 63,
- .step= 1,
+ .default_value = CONTRAST_DEF,
+ },
+ .set = sd_setcontrast,
+ .get = sd_getcontrast,
+ },
+ { /* 2 */
+ {
+ .id = V4L2_CID_GAIN,
+ .type= V4L2_CTRL_TYPE_INTEGER,
+ .name= "Main Gain",
+ .minimum = 0,
+ .maximum = 63,
+ .step= 1,
#define GAIN_DEF 20
- .default_value = GAIN_DEF,
- },
- .set = sd_setgain,
- .get = sd_getgain,
-},
-{ /* 3 */
- {
- .id = V4L2_CID_EXPOSURE,
- .type= V4L2_CTRL_TYPE_INTEGER,
- .name= "Exposure",
- .minimum = 0,
- .maximum = 255,
- .step= 1,
+ .default_value = GAIN_DEF,
+ },
+ .set = sd_setgain,
+ .get = sd_getgain,
+ },
+ { /* 3 */
+ {
+ .id = V4L2_CID_EXPOSURE,
+ .type= V4L2_CTRL_TYPE_INTEGER,
+ .name= "Exposure",
+ .minimum = 0,
+ .maximum = 255,
+ .step= 1,
#define EXPO_DEF 120
- .default_value = EXPO_DEF,
- },
- .set = sd_setexposure,
- .get = sd_getexposure,
-},
-{ /* 4 */
- {
- .id = V4L2_CID_AUTOGAIN,
- .type= V4L2_CTRL_TYPE_BOOLEAN,
- .name= "Auto Gain",
- .minimum = 0,
- .maximum = 1,
- .step= 1,
+ .default_value = EXPO_DEF,
+ },
+ .set = sd_setexposure,
+ .get = sd_getexposure,
+ },
+ { /* 4 */
+ {
+ .id = V4L2_CI
Re: [PATCH 10/11] ov534: Add Powerline Frequency control
On Tue, 2 Mar 2010 11:26:15 + "M.Ebrahimi" wrote: > On 28 February 2010 19:55, Jean-Francois Moine wrote: > > On Sun, 28 Feb 2010 20:18:50 +0100 > > Antonio Ospite wrote: > > > >> Maybe we could just use > >> V4L2_CID_POWER_LINE_FREQUENCY_DISABLED = 0, > >> V4L2_CID_POWER_LINE_FREQUENCY_50HZ = 1, > >> > >> It looks like the code matches the DISABLED state (writing 0 to the > >> register). Mosalam? > > > > I don't know the ov772x sensor. I think it should look like the ov7670 > > where there are 3 registers to control the light frequency: one > > register tells if light frequency filter must be used, and which > > frequency 50Hz or 60Hz; the two other ones give the filter values for > > each frequency. > > > > I think it's safe to go with disabled/50hz. Perhaps later if needed > can patch it to control the filter values. Since it seems there is no > flickering in the 60hz regions at available frame rates, and this > register almost perfectly removes light flickers in the 50hz regions > (by modifying exposure/frame rate). > > Mosalam > Mosalam did you spot the register from a PS3 usb dump or by looking at the sensor datasheet? Thanks, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? pgp3a5qpZ7Lsn.pgp Description: PGP signature
Re: [PATCH 10/11] ov534: Add Powerline Frequency control
On Wed, 3 Mar 2010 02:27:38 + "M.Ebrahimi" wrote: > On 2 March 2010 16:06, Max Thrun wrote: > > > > > > On Tue, Mar 2, 2010 at 10:39 AM, Antonio Ospite > > wrote: [...] > >> Mosalam did you spot the register from a PS3 usb dump or by looking at > >> the sensor datasheet? > > None, I got that register from sniffing a Windows driver for another > camera that turned out to be using ov7620 or something similar, though > I thought it has the same sensor. I double checked, this register is > for frame rate adjustment (decreasing frame rate / increasing > exposure) . And this has been used in some other drivers (e.g. > gspca_sonixb) to remove light flicker as well. > I see. It would be interesting to see how Powerline Frequency filtering is done on PS3. I added Jim Paris on CC. > > > > I'd also like to know where you got the 2b register from, cause someone else > > also said 2b was filtering but the datasheet says it LSB of dummy pixel... > > > >- Max Thrun > > Definitely it is adjusting the frame rate (see the ov7620 DS for the > description how the register value is used, for instance). I have no > idea why the ov7720 datasheet says otherwise. > > Since this patch does not use the banding filter registers mentioned > in the datasheet maybe should be discarded. I am working on 75 FPS at > VGA, when I get that working well I can get back to this again. > > Thanks for the comments. > Mosalam > Ok, so Jean-Francois can you apply the patches except 10/11, please? We are keeping this one for another round. Thanks, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? pgpGyiZTE1GPH.pgp Description: PGP signature
Re: [PATCH 10/11] ov534: Add Powerline Frequency control
On Wed, 3 Mar 2010 09:37:44 +0100 Jean-Francois Moine wrote: > On Wed, 3 Mar 2010 09:00:08 +0100 > Antonio Ospite wrote: > > > Ok, so Jean-Francois can you apply the patches except 10/11, please? > > We are keeping this one for another round. > > Hello ov534 team, > > Actually, I have problems with the mercurial tree. I will apply your > changes as soon as everything will be resolved.. > No problem. Just a question, will you switch to git anytime soon? Regards, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? pgpiP56zkn0TX.pgp Description: PGP signature
