Hi !
I would like to add the support for GL860 based webcams within the
GSPCA framework.
A patch (116KB) for that can be found at :
http://launchpadlibrarian.net/31182405/patchu_gl860g.diff
This is not a final version, some improvement in the auto detection
of sensor will be done. Before that I'm waiting for comments about
what should changed in this patch in order to be accepted.
Basically there is four managed sensors so that this patch add a new
directory in the gspca one, it contains the main part of the driver
and the four sub-drivers.
Here are some remarks:
- in gl860/gl860.h, there are complex macros. Please, use functions
instead.
Done
- in gl860/gl860.c
. don't change the returned values of the virtual functions as:
static s32 sd_init(struct gspca_dev *gspca_dev);
(should be int and not s32)
. more generally, it is a bad idea to have s32 variables.
I changed the returned value to int instead of s32.
There is still s32 for the control parameters.
. why are the module parameters read only? (see below)
Properties changed to 644 instead of 444.
I discarded all parameters but sensor and AC power frequency.
. some initialization are unuseful as:
static char sensor[7] = "";
Removed with some initialisation to zero for static variables.
. why is the video control table not static? (if some controls are not
available for some webcams, just set gspca_dev->ctl_dis)
It's a kind of trade-off. Control characteristics are hold in the
sensor specific files but because of "const" the control table is
still in the main
file. I can surely improve that.
. in the function gl860_guess_sensor, there is
if (product_id == 0xf191)
sd->vsettings.sensor = ID_MI1320;
This information could be in the device_table, and also, in the
declaration of this table, '.driver_info = 0' is not useful.
Driver_info removed. I need the sensor ID in functions which not benefit from
the device table so that it is useful in the sd structure.
. in the function sd_config, there is no need to set values to 0 as:
sd->vsettings.mirrorMask = 0;
. in the same function,
gspca_dev->alt = 3 + 1;
is not useful (the value will be reset at streaming start).
Removed
. in the function sd_pkt_scan, the line
switch (*(s16 *)data) {
may not work either with BE or LE machines.
I add a comment about the fact that only 0x0202 is checked.
May a "if" instead of the "case" could be used as I don't think any other
values will be tested.
. in the function sd_mod_init, why are the static module parameters
moved to the variable vsettings?
. about this same variable, it should be better to set the device
settings from the module parameters at connect time instead of at
module load time. This permits to have different webcam types active
at the same time...
Right! Old stuff no more needed. Changed.
- in the other .c files
. the use of static variables prevents to have more than one active
webcam.
Changed. Also, the byte arrays whose one byte is a control value are no more
static.
. there are values >= 0x80 in 'char' tables. These ones should be 's8'
or 'u8' ('char' may be unsigned).
Changed to u8.
. using strings to handle binary values is less readable than simple
hexadecimal values.
There are still there. Because these binary values are not human
understandable, I prefer the strings as they are more compact.
The improved patch is there :
http://launchpadlibrarian.net/31723472/patchu_gl860g.diff
Regards,
Olivier Lorin
----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
