PATCH: gspca-spc200nc-upside-down-v2

Hi,

This patch adds a V4L2_CAP_SENSOR_UPSIDE_DOWN flag to the capabilities flags, and sets this flag for the Philips SPC200NC cam (which has its sensor installed upside down). The same flag is also needed and added for the Philips SPC300NC.

Together with a patch to libv4l which adds flipping the image in software this fixes the upside down display with the SPC200NC cam.

This is version 2 of the patch which now makes the flag a capability flag rather then a format flag as suggested by Hans Verkuil.

Regards,

Hans

p.s.

Of you do not plan to apply this patch please let me know that and why, then we can discuss this further, I really believe that in cases where the upside down ness is 100% known at the kernel level we should report this in some way to userspace so that libv4l can flip the image in software. I know that for certain cases the upside down ness needs to be determined elsewhere, but not for all cases.

I believe all hardware info for a certain piece of hardware should be kept at one place, and in this case that is the driver. With upside down mounted generic laptop cam modules, the upside down ness is not module specific but laptop specific and thus this info should be stored in hal, which takes care of laptop model specific things which can differ from laptop to laptop even though they use the same lowlevel IC's. In this case however this is not system/latop specific but cam specific so this info should be stored together with the other cam specific knowledge (such as which sensor it uses) in the driver.

This patch adds a V4L2_CAP_SENSOR_UPSIDE_DOWN flag to the capabilities flags, and sets this flag for the Philips SPC200NC cam (which has its sensor installed upside down). The same flag is also needed and added for the Philips SPC300NC.

Together with a patch to libv4l which adds flipping the image in software this fixes the upside down display with the SPC200NC cam.

diff -r 1da7166882de linux/drivers/media/video/gspca/gspca.c
@@ -857,6 +857,8 @@
 	cap->capabilities = V4L2_CAP_VIDEO_CAPTURE
 			  | V4L2_CAP_STREAMING
 			  | V4L2_CAP_READWRITE;
+	if (gspca_dev->flags & GSPCA_SENSOR_UPSIDE_DOWN_FLAG)
+		cap->capabilities |= V4L2_CAP_SENSOR_UPSIDE_DOWN;
 	return 0;
 }
 

diff -r 1da7166882de linux/drivers/media/video/gspca/gspca.h
@@ -118,6 +118,9 @@
 	struct v4l2_buffer v4l2_buf;
 };
 
+/* defines for the flags member */
+#define GSPCA_SENSOR_UPSIDE_DOWN_FLAG 0x01
+
 struct gspca_dev {
 	struct video_device vdev;	/* !! must be the first item */
 	struct file_operations fops;

@@ -160,6 +163,7 @@
 	char nurbs;			/* number of allocated URBs */
 	char memory;			/* memory type (V4L2_MEMORY_xxx) */
 	__u8 nbalt;			/* number of USB alternate settings */
+	__u8 flags;                     /* see GSPCA_XXX_FLAG defines */
 };
 
 int gspca_dev_probe(struct usb_interface *intf,

diff -r 1da7166882de linux/drivers/media/video/gspca/zc3xx.c
@@ -69,6 +69,10 @@
 #define SENSOR_MAX 17
 	unsigned short chip_revision;
 };
+
+#define DRIVER_INFO(sensor, flags) .driver_info = ((sensor) << 8) | (flags)
+#define DRIVER_INFO_GET_SENSOR(driver_info) ((driver_info) >> 8)
+#define DRIVER_INFO_GET_FLAGS(driver_info)  ((driver_info) & 0xff)
 
 /* V4L2 controls supported by the driver */
 static int sd_setbrightness(struct gspca_dev *gspca_dev, __s32 val);

@@ -7020,7 +7024,8 @@
 
 	/* define some sensors from the vendor/product */
 	sd->sharpness = 2;
-	sd->sensor = id->driver_info;
+	sd->sensor = DRIVER_INFO_GET_SENSOR(id->driver_info);
+	gspca_dev->flags = DRIVER_INFO_GET_FLAGS(id->driver_info);
 	sensor = zcxx_probeSensor(gspca_dev);
 	if (sensor >= 0)
 		PDEBUG(D_PROBE, "probe sensor -> %02x", sensor);

@@ -7517,19 +7522,19 @@
 	{USB_DEVICE(0x041e, 0x041e)},
 #ifndef CONFIG_USB_ZC0301
 	{USB_DEVICE(0x041e, 0x4017)},
-	{USB_DEVICE(0x041e, 0x401c), .driver_info = SENSOR_PAS106},
+	{USB_DEVICE(0x041e, 0x401c), DRIVER_INFO(SENSOR_PAS106, 0)},
 	{USB_DEVICE(0x041e, 0x401e)},
 	{USB_DEVICE(0x041e, 0x401f)},
 #endif
 	{USB_DEVICE(0x041e, 0x4029)},
 #ifndef CONFIG_USB_ZC0301
-	{USB_DEVICE(0x041e, 0x4034), .driver_info = SENSOR_PAS106},
-	{USB_DEVICE(0x041e, 0x4035), .driver_info = SENSOR_PAS106},
+	{USB_DEVICE(0x041e, 0x4034), DRIVER_INFO(SENSOR_PAS106, 0)},
+	{USB_DEVICE(0x041e, 0x4035), DRIVER_INFO(SENSOR_PAS106, 0)},
 	{USB_DEVICE(0x041e, 0x4036)},
 	{USB_DEVICE(0x041e, 0x403a)},
 #endif
-	{USB_DEVICE(0x041e, 0x4051), .driver_info = SENSOR_TAS5130C_VF0250},
-	{USB_DEVICE(0x041e, 0x4053), .driver_info = SENSOR_TAS5130C_VF0250},
+	{USB_DEVICE(0x041e, 0x4051), DRIVER_INFO(SENSOR_TAS5130C_VF0250, 0)},
+	{USB_DEVICE(0x041e, 0x4053), DRIVER_INFO(SENSOR_TAS5130C_VF0250, 0)},
 #ifndef CONFIG_USB_ZC0301
 	{USB_DEVICE(0x0458, 0x7007)},
 	{USB_DEVICE(0x0458, 0x700c)},

@@ -7555,11 +7560,13 @@
 	{USB_DEVICE(0x046d, 0x08d9)},
 	{USB_DEVICE(0x046d, 0x08d8)},
 	{USB_DEVICE(0x046d, 0x08da)},
-	{USB_DEVICE(0x046d, 0x08dd), .driver_info = SENSOR_MC501CB},
-	{USB_DEVICE(0x0471, 0x0325), .driver_info = SENSOR_PAS106},
-	{USB_DEVICE(0x0471, 0x0326), .driver_info = SENSOR_PAS106},
-	{USB_DEVICE(0x0471, 0x032d), .driver_info = SENSOR_PAS106},
-	{USB_DEVICE(0x0471, 0x032e), .driver_info = SENSOR_PAS106},
+	{USB_DEVICE(0x046d, 0x08dd), DRIVER_INFO(SENSOR_MC501CB, 0)},
+	{USB_DEVICE(0x0471, 0x0325), DRIVER_INFO(SENSOR_PAS106,
+		GSPCA_SENSOR_UPSIDE_DOWN_FLAG)},
+	{USB_DEVICE(0x0471, 0x0326), DRIVER_INFO(SENSOR_PAS106,
+		GSPCA_SENSOR_UPSIDE_DOWN_FLAG)},
+	{USB_DEVICE(0x0471, 0x032d), DRIVER_INFO(SENSOR_PAS106, 0)},
+	{USB_DEVICE(0x0471, 0x032e), DRIVER_INFO(SENSOR_PAS106, 0)},
 	{USB_DEVICE(0x055f, 0xc005)},
 #ifndef CONFIG_USB_ZC0301
 	{USB_DEVICE(0x055f, 0xd003)},

@@ -7571,7 +7578,7 @@
 	{USB_DEVICE(0x0ac8, 0x301b)},
 	{USB_DEVICE(0x0ac8, 0x303b)},
 #endif
-	{USB_DEVICE(0x0ac8, 0x305b), .driver_info = SENSOR_TAS5130C_VF0250},
+	{USB_DEVICE(0x0ac8, 0x305b), DRIVER_INFO(SENSOR_TAS5130C_VF0250, 0)},
 #ifndef CONFIG_USB_ZC0301
 	{USB_DEVICE(0x0ac8, 0x307b)},
 	{USB_DEVICE(0x10fd, 0x0128)},

diff -r 1da7166882de linux/include/linux/videodev2.h
@@ -260,6 +260,11 @@
 #define V4L2_CAP_READWRITE              0x01000000  /* read/write systemcalls */
 #define V4L2_CAP_ASYNCIO                0x02000000  /* async I/O */
 #define V4L2_CAP_STREAMING              0x04000000  /* streaming I/O ioctls */
+
+/* This flags gets set if the "sensor" is known to be upside down and this can
+   *not* be fixed using v4l2 flipx/y controls. Note that absence of this flag
+   is not a guarantee for the image not being upside down. */
+#define V4L2_CAP_SENSOR_UPSIDE_DOWN     0x10000000
 
 /*
  *	V I D E O   I M A G E   F O R M A T

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Hi Hans,

	[snip]

Well, I looked at various messages in various mail-lists talking about upside down. Sometimes, a webcam may be normal or upside down, or even just mirrored. Two times only (Vimicro 0325 and 0326), they say that the webcam is always upside down. So, is it useful to make a generic code for this specific case?

For the general case (the webcam may have H or V flip, or both - upside down). The user will see it. If she may use the HFLIP and VFLIP controls, she will get a correct image. To go further, it will be nice to have a v4l2 control program which saves and restores the video control values at system stop and start times, as it is done for sound.

BTW, if noticed a small difference in the PAS106B initialization between the actual driver (zc3xx) and the data in usbvm31b.inf. As I have no such webcams, may anybody check what happens changing the lines 4146 and 4264 of zc3xx.c from

	{0xa0, 0x00, 0x01ad},
to
	{0xa0, 0x09, 0x01ad},

This will impact on the webcams V:041e P:401c, 4034 and 4035, V:0471, P:0325, 0326, 032d and 032e.

Cheers.

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


--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Yes, as that will make these webcam work out of the box for end users. Please stop thinking as a developer for a moment and start thinking as a simple end user plugging such a cam into his asus eee pc, which is his first and only linux machine. What do you think he will like better, the upside down picture or the hey cool I plug in in this cam and it just works (tm) ?

Currently the 4 major (as in more then just a gimmick) end user v4l wewbcam programs I'm aware of are:
ekiga
cheese
flash plugin
skype

And AFAIK (didn't check skype) non of these offer a simple GUI option for the user to change vflip / hflip controls. Telling a user to go the cmdline is not *userfriendly* and in this scenario is not necessary!

Sorry to say this, but sheesh what a lot of discussion I'm only adding one simple lousy flag, all the actual rotating code is done in libv4l! So what if for now we only can use this flag for 2 webcams, we may use it for others in the future.

The reason why I'm spending tons of time on all this webcam stuff, is so that end users can just plugin their cam and have it work. If that requires a special flag for just these 2 cams so be it and I strongly believe we will encounter other cams like this in the future.

That may be usefull, but I myself prefer (atleast for laptop frame cams) a solution using a laptop BIOS DMI string database so things will just work for end users.

That register seems to influence the zc3xx autoexposure / autogain algorithm, with the values changed to 9 as suggested by you, the algorithm becomes very quick to adjust, so quick I can get it to oscilate quite easily just be moving me head a little in the picture until I've found a sweet spot for oscilating, so the 0 setting definitely is better!

I also tried writing 5 to it, but it does not seem to be a simple linear scale, after I wrote 5 to it no more autoexposure / gain was done at all, not even after changing the value back to 0, I had to unplug the cam to get autoexposure back again.

Regards,

Hans

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

	[snip]

It is possible if some system application (hal?) does the job.

You did not try v4l2ucp?

	[snip]

	[snip]

Sorry, but I am not happy the way it is done. Here is an other proposal.

In the V4L2 spec, VIDIOC_QUERYCTRL returns the controls accepted (or rejected) by the driver, and also information about these ones. As the Vimicro/Z-star has no way to change H and V flips, the driver may give these controls as READ_ONLY and set the control values according to the device type.

Now, when accessing the device, the V4L library will get the flags and values of the H and V flip. If HFLIP and VFLIP are settable, the driver does all the job. If not (HFLIP and VFLIP are READ_ONLY or INVAL), the library memorizes the control values of the driver (INVAL implies 0) and also the values asked by the application. Frame decoding is then H and/or H flipped according to (<driver value> ^ <user value>).

And now, for something completely different! I could not find a MS-win snoop of these 'upside down' devices (0471:0325 and 0471:0326). I want to check the initialization sequences and also the format of the frames. May anybody send one to me? Thank you.

Cheers.

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


--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

No I didn't and that is not installed by default on most systems, it actually isn't packaged for most distros and even it was needing to start up a seperate application is anything but userfriendly, things should just work whenever possible.

As interesting as ideas like this are, that translates to a *lott* of additional code for no gain, all we need (for this case) is a way for the kernel to tell libv4l I know 100% certain this device is upside down.

This is a boolean a simple flag is sufficient for that anything more is bloat!

Why on earth would we add fake controls for that (confusing applications like v4l2ucp), and then add more fake controls to libv4l to cover up the fake controls of the driver ????

I know you want me to add fake v4l2 controls for the software flipping in libv4l, but what is the use case scenario for that? As technically interesting this might be, I see little relevant real life use for it, I see little relevant real life use for flipping in general (which is proven by the lack of availability to control flipping from all serious webcam applications). The only use for flipping is to correct broken hardware and todo that we do not need to export the flipping functionality to the application using libv4l, so until a practical real life example which shows the use of adding fake v4l2 controls for flipping to libv4l I won't be adding them.

You see there is this little thing called time, and this whole discussion and the whole add fake v4l2 controls for flipping thing is a big waste of time!

I've actually spend many many hours the last 3 days getting the Pixart JPEG cams to work (and I'm happy to report I hope to post patches for them soon). I've even bought a pac7311 based cam to complement my pac7302 based cam so that I could test both code paths and I much rather spend my time on this (as this is actually usefull for people) then on such frivilous features as adding fake v4l2 flip controls, whats next fake contrast and brightness? The sky is the limit ...

So let me repeat the most important part of this mail:

This is a boolean a simple flag is sufficient for that anything more is bloat!

Regards,

Hans

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list