New Jeilin dual-mode camera support

PATCH In review Patrice Chotard 2 participants 3 messages Mar 16, 2011 – Mar 17, 2011

Subsystems (2)

Documentation Media (V4L/DVB)

lore

Share & tools

lore

Patrice Chotard

author

2011-03-16 20:06 UTC #0 lore raw

PATCH 1/1 New Jeilin dual-mode camera support

This is a re-submission, my previous patch has not been send in plain text.

This patch add a new jeilin dual mode camera support and some specific controls settings.

Regards

Signed-off-by Patrice CHOTARD <[email protected]>

Theodore Kilgore <[email protected]>

---
 Documentation/video4linux/gspca.txt |    1 +
 drivers/media/video/gspca/jeilinj.c |  396 ++++++++++++++++++++++++++++++-----
 2 files changed, 345 insertions(+), 52 deletions(-)

Documentation/video4linux/gspca.txt +1 -0

index 261776e..c4245d2 100644
@@ -265,6 +265,7 @@ pac7302		093a:2629	Genious iSlim 300
 pac7302		093a:262a	Webcam 300k
 pac7302		093a:262c	Philips SPC 230 NC
 jeilinj		0979:0280	Sakar 57379
+jeilinj		0979:0270	Sportscam DV15
 zc3xx		0ac8:0302	Z-star Vimicro zc0302
 vc032x		0ac8:0321	Vimicro generic vc0321
 vc032x		0ac8:0323	Vimicro Vc0323

drivers/media/video/gspca/jeilinj.c +344 ~~-52~~

501 lines not on this page · raw message

Signature · 1 line

1.7.0.4

Jean-Francois Moine

2011-03-17 10:28 UTC #1 lore raw

Quoted 2 lines · On Wed, 16 Mar 2011 21:06:13 +0100 Patrice Chotard <[email protected]>…

This patch add a new jeilin dual mode camera support and some specific controls settings.

Hi Patrice and Theodore,

Here are somme comments about Patrice's patch.

Quoted 3 lines

#include <linux/workqueue.h>
+#include <linux/delay.h>
#include <linux/slab.h>

It is not a good idea to use mdelay(): it is a loop. Better use msleep().

drivers/media/video/gspca/jeilinj.c struct sd { · line 69

Quoted 4 lines

drivers/media/video/gspca/jeilinj.c +2 -2

-	u8 quality;			/* image quality */
-	u8 jpegqual;			/* webcam quality */
+	u8 camquality;			/* webcam quality */
+	u8 jpegquality;			/* jpeg quality */

The webcam (encoding) quality and the jpeg (decoding) quality must be the same. Then, looking carefully, jpegquality is not used!

drivers/media/video/gspca/jeilinj.c struct sd { · line 77

Quoted 7 lines

drivers/media/video/gspca/jeilinj.c +7 -0

+	u8 freq;
+	u8 type;
+	/* below variables are only used for SPORTSCAM_DV15 */
+	u8 autogain;
+	u8 cyan;
+	u8 magenta;
+	u8 yellow;

You should use the new control mechanism (see stk014, sonixj, zc3xx...).

drivers/media/video/gspca/jeilinj.c static int jlj_read1(struct gspca_dev *gspca_dev, unsigned char response) · line 273

Quoted 4 lines

drivers/media/video/gspca/jeilinj.c +4 -0

+#define V4L2_CID_CAMQUALITY (V4L2_CID_USER_BASE + 1)
+		.id      = V4L2_CID_CAMQUALITY,
+		.type    = V4L2_CTRL_TYPE_INTEGER,
+		.name    = "Image quality",

The JPEG quality must be get/set by the VIDIOC_G_JPEGCOMP / VIDIOC_S_JPEGCOMP ioctl's.

Quoted 1 line

+#define V4L2_CID_CYAN_BALANCE (V4L2_CID_USER_BASE + 2)

	[snip]

Quoted 1 line

+#define V4L2_CID_MAGENTA_BALANCE (V4L2_CID_USER_BASE + 3)

	[snip]

Quoted 1 line

+#define V4L2_CID_YELLOW_BALANCE (V4L2_CID_USER_BASE + 4)

These values redefine V4L2_CID_SATURATION and V4L2_CID_HUE (user_base + 4 is no more defined). You should use V4L2_CID_RED_BALANCE, V4L2_CID_BLUE_BALANCE and V4L2_CID_GAIN to set these controls.

drivers/media/video/gspca/jeilinj.c static int jlj_read1(struct gspca_dev *gspca_dev, unsigned char response) · line 382

Quoted 4 lines

drivers/media/video/gspca/jeilinj.c +4 -0

+	if (sd->type == SPORTSCAM_DV15)
+		start_commands_size = 9;
+	else
+		start_commands_size = ARRAY_SIZE(start_commands);

Don't use magic values ('9').

drivers/media/video/gspca/jeilinj.c static int jlj_read1(struct gspca_dev *gspca_dev, unsigned char response) · line 389

Quoted 1 line

drivers/media/video/gspca/jeilinj.c +1 -0
+			mdelay(start_commands[i].delay);

See above.

BTW, Theodore, as there is no USB command in the loop, there is no need to have a work queue (look at the SENSOR_OV772x in ov534).

Best regards.

Signature · 2 lines

Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

Patrice Chotard

author

2011-03-17 23:32 UTC #2 lore raw

Quoted 61 lines · Jean-Francois Moine

Quoted 2 lines · On Wed, 16 Mar 2011 21:06:13 +0100 Patrice Chotard <[email protected]>…
This patch add a new jeilin dual mode camera support and some specific controls settings.
Hi Patrice and Theodore,
Here are somme comments about Patrice's patch.
Quoted 3 lines
#include <linux/workqueue.h>
+#include <linux/delay.h>
#include <linux/slab.h>
It is not a good idea to use mdelay(): it is a loop. Better use msleep().
Review
Quoted 4 lines
(quoted diff) +2 -2
-	u8 quality;			/* image quality */
-	u8 jpegqual;			/* webcam quality */
+	u8 camquality;			/* webcam quality */
+	u8 jpegquality;			/* jpeg quality */
The webcam (encoding) quality and the jpeg (decoding) quality must be the same. Then, looking carefully, jpegquality is not used!
Review
Quoted 7 lines
(quoted diff) +7 -0
+	u8 freq;
+	u8 type;
+	/* below variables are only used for SPORTSCAM_DV15 */
+	u8 autogain;
+	u8 cyan;
+	u8 magenta;
+	u8 yellow;
You should use the new control mechanism (see stk014, sonixj, zc3xx...).
Review
Quoted 4 lines
(quoted diff) +4 -0
+#define V4L2_CID_CAMQUALITY (V4L2_CID_USER_BASE + 1)
+		.id      = V4L2_CID_CAMQUALITY,
+		.type    = V4L2_CTRL_TYPE_INTEGER,
+		.name    = "Image quality",
The JPEG quality must be get/set by the VIDIOC_G_JPEGCOMP / VIDIOC_S_JPEGCOMP ioctl's.
Quoted 1 line
+#define V4L2_CID_CYAN_BALANCE (V4L2_CID_USER_BASE + 2)
	[snip]
Quoted 1 line
+#define V4L2_CID_MAGENTA_BALANCE (V4L2_CID_USER_BASE + 3)
	[snip]
Quoted 1 line
+#define V4L2_CID_YELLOW_BALANCE (V4L2_CID_USER_BASE + 4)
These values redefine V4L2_CID_SATURATION and V4L2_CID_HUE (user_base + 4 is no more defined). You should use V4L2_CID_RED_BALANCE, V4L2_CID_BLUE_BALANCE and V4L2_CID_GAIN to set these controls.
Review
Quoted 4 lines
(quoted diff) +4 -0
+	if (sd->type == SPORTSCAM_DV15)
+		start_commands_size = 9;
+	else
+		start_commands_size = ARRAY_SIZE(start_commands);
Don't use magic values ('9').

Quoted 9 lines

Review
Quoted 1 line
(quoted diff) +1 -0
+			mdelay(start_commands[i].delay);
See above.
BTW, Theodore, as there is no USB command in the loop, there is no need to have a work queue (look at the SENSOR_OV772x in ov534).
Best regards.

Hi,

Following Jean Francois's remark, here is a new version of my previous patch for a new jeilin dual mode camera support with specific control settings.

Regards.

Signed-off-by Patrice CHOTARD <[email protected]>

Signed-off-by Theodore Kilgore <[email protected]>

---
 Documentation/video4linux/gspca.txt |    1 +
 drivers/media/video/gspca/jeilinj.c |  405 ++++++++++++++++++++++++++++++-----
 2 files changed, 354 insertions(+), 52 deletions(-)

Documentation/video4linux/gspca.txt +1 -0

index 261776e..c4245d2 100644
@@ -265,6 +265,7 @@ pac7302		093a:2629	Genious iSlim 300
 pac7302		093a:262a	Webcam 300k
 pac7302		093a:262c	Philips SPC 230 NC
 jeilinj		0979:0280	Sakar 57379
+jeilinj		0979:0270	Sportscam DV15
 zc3xx		0ac8:0302	Z-star Vimicro zc0302
 vc032x		0ac8:0321	Vimicro generic vc0321
 vc032x		0ac8:0323	Vimicro Vc0323

drivers/media/video/gspca/jeilinj.c +353 ~~-52~~

504 lines not on this page · raw message

Signature · 1 line

1.7.0.4