diff mbox

Implement changing resolution on the fly for zr364xx driver

Message ID 200907202046.43194.lamarque@gmail.com (mailing list archive)
State RFC
Headers show

Commit Message

Lamarque Vieira Souza July 20, 2009, 11:46 p.m. UTC 
This patch implements changing resolution in zr364xx_vidioc_s_fmt_vid_cap for 
zr364xx driver. This version is synced with v4l-dvb as of 20/Jul/2009. Tested 
with Creative PC-CAM 880.

OBS: I had to increase MAX_FRAME_SIZE to prevent a hard crash in my notebook 
(caps lock blinking) when testing with mplayer, which automatically sets 
resolution to the maximum (640x480). Maybe we should add code to auto-detect 
frame size to prevent this kind of crash in the future.

Signed-off-by: Lamarque V. Souza <lamarque@gmail.com>
---


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Antoine Jacquet July 21, 2009, 2:29 p.m. UTC | #1 
Hi,

> This patch implements changing resolution in zr364xx_vidioc_s_fmt_vid_cap for 
> zr364xx driver. This version is synced with v4l-dvb as of 20/Jul/2009. Tested 
> with Creative PC-CAM 880.

Nice, I successfully tested your patch with 2 compatible webcams.
 From the users feedbacks I had before, it seems that some devices do 
not support the 640x480 resolution, but I was not able to verify this 
myself.
This is the only concern I have, since some users may think the driver 
is not working if the application automatically switches to the maximum 
resolution with an incompatible device.

> OBS: I had to increase MAX_FRAME_SIZE to prevent a hard crash in my notebook 
> (caps lock blinking) when testing with mplayer, which automatically sets 
> resolution to the maximum (640x480). Maybe we should add code to auto-detect 
> frame size to prevent this kind of crash in the future.

Yes, I also had this issue before. I don't know what is the good 
approach to determine the best size with JPEG compression.

I will push your changes to my tree and send a pull request to Mauro later.

Regards,

Antoine
Lamarque Vieira Souza July 21, 2009, 3:14 p.m. UTC | #2 
Em Terça-feira 21 Julho 2009, Antoine Jacquet escreveu:
> Hi,

	Hi,

> > This patch implements changing resolution in zr364xx_vidioc_s_fmt_vid_cap
> > for zr364xx driver. This version is synced with v4l-dvb as of
> > 20/Jul/2009. Tested with Creative PC-CAM 880.
>
> Nice, I successfully tested your patch with 2 compatible webcams.
>  From the users feedbacks I had before, it seems that some devices do
> not support the 640x480 resolution, but I was not able to verify this
> myself.
> This is the only concern I have, since some users may think the driver
> is not working if the application automatically switches to the maximum
> resolution with an incompatible device.

	Maybe we should add a quirk list to prevent such cases or if send_control_msg 
returns error in such cases I can change the code to revert the change and 
keep the old resolution.

> > OBS: I had to increase MAX_FRAME_SIZE to prevent a hard crash in my
> > notebook (caps lock blinking) when testing with mplayer, which
> > automatically sets resolution to the maximum (640x480). Maybe we should
> > add code to auto-detect frame size to prevent this kind of crash in the
> > future.
>
> Yes, I also had this issue before. I don't know what is the good
> approach to determine the best size with JPEG compression.

	The driver reads data from USB port in 4096-bytes chunks, so before adding 
the data to frm->lpvbits we could verify if frm->lpvbits has enough space to 
hold it. If there is no enough space we can dynamically increase frm->lpvbits. 
In user space that can be done using realloc, I just do not know if there is a 
similar function in kernel space. OBS: frm->lpvbits had be increased without 
loosing the old data and of course we should have a upper limit, maybe 640 x 
480 * 3 (3 bytes per pixel) = 921600 bytes.

> I will push your changes to my tree and send a pull request to Mauro later.

	Ok, thanks.
diff mbox

Patch

diff -r 1cb6f19d2c9d linux/drivers/media/video/zr364xx.c
--- a/linux/drivers/media/video/zr364xx.c	Sun Jul 19 18:03:23 2009 -0300
+++ b/linux/drivers/media/video/zr364xx.c	Mon Jul 20 20:20:21 2009 -0300
@@ -43,14 +43,14 @@ 
 
 /* Version Information */
 #define DRIVER_VERSION "v0.73"
-#define ZR364_VERSION_CODE KERNEL_VERSION(0, 7, 3)
+#define ZR364XX_VERSION_CODE KERNEL_VERSION(0, 7, 3)
 #define DRIVER_AUTHOR "Antoine Jacquet, http://royale.zerezo.com/"
 #define DRIVER_DESC "Zoran 364xx"
 
 
 /* Camera */
 #define FRAMES 1
-#define MAX_FRAME_SIZE 100000
+#define MAX_FRAME_SIZE 200000
 #define BUFFER_SIZE 0x1000
 #define CTRL_TIMEOUT 500
 
@@ -734,7 +734,7 @@ 
 	strlcpy(cap->card, cam->udev->product, sizeof(cap->card));
 	strlcpy(cap->bus_info, dev_name(&cam->udev->dev),
 		sizeof(cap->bus_info));
-	cap->version = ZR364_VERSION_CODE;
+	cap->version = ZR364XX_VERSION_CODE;
 	cap->capabilities = V4L2_CAP_VIDEO_CAPTURE |
 			    V4L2_CAP_READWRITE |
 			    V4L2_CAP_STREAMING;
@@ -874,9 +874,14 @@ 
 		return -EINVAL;
 	}
 
+	if (!(f->fmt.pix.width == 160 && f->fmt.pix.height == 120) &&
+	    !(f->fmt.pix.width == 640 && f->fmt.pix.height == 480)) {
+		f->fmt.pix.width = 320;
+		f->fmt.pix.height = 240;
+	}
+
 	f->fmt.pix.field = V4L2_FIELD_NONE;
-	f->fmt.pix.width = cam->width;
-	f->fmt.pix.height = cam->height;
+	f->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
 	f->fmt.pix.bytesperline = f->fmt.pix.width * 2;
 	f->fmt.pix.sizeimage = f->fmt.pix.height * f->fmt.pix.bytesperline;
 	f->fmt.pix.colorspace = 0;
@@ -907,7 +912,6 @@ 
 	return 0;
 }
 
-/* Lamarque TODO: implement changing resolution on the fly */
 static int zr364xx_vidioc_s_fmt_vid_cap(struct file *file, void *priv,
 				    struct v4l2_format *f)
 {
@@ -915,6 +919,7 @@ 
 	struct videobuf_queue *q = &cam->vb_vidq;
 	char pixelformat_name[5];
 	int ret = zr364xx_vidioc_try_fmt_vid_cap(file, cam, f);
+	int i;
 
 	if (ret < 0)
 		return ret;
@@ -927,15 +932,55 @@ 
 		goto out;
 	}
 
-	f->fmt.pix.field = V4L2_FIELD_NONE;
-	f->fmt.pix.width = cam->width;
-	f->fmt.pix.height = cam->height;
+	if (res_check(cam)) {
+		DBG("%s can't change format after started\n", __func__);
+		ret = -EBUSY;
+		goto out;
+	}
+
+	cam->width = f->fmt.pix.width;
+	cam->height = f->fmt.pix.height;
+	dev_info(&cam->udev->dev, "%s: %dx%d mode selected\n", __func__,
+		 cam->width, cam->height);
 	f->fmt.pix.bytesperline = f->fmt.pix.width * 2;
 	f->fmt.pix.sizeimage = f->fmt.pix.height * f->fmt.pix.bytesperline;
 	f->fmt.pix.colorspace = 0;
 	f->fmt.pix.priv = 0;
 	cam->vb_vidq.field = f->fmt.pix.field;
 	cam->mode.color = V4L2_PIX_FMT_JPEG;
+
+	if (f->fmt.pix.width == 160 && f->fmt.pix.height == 120)
+		mode = 1;
+	else if (f->fmt.pix.width == 640 && f->fmt.pix.height == 480)
+		mode = 2;
+	else
+		mode = 0;
+
+	m0d1[0] = mode;
+	m1[2].value = 0xf000 + mode;
+	m2[1].value = 0xf000 + mode;
+	header2[437] = cam->height / 256;
+	header2[438] = cam->height % 256;
+	header2[439] = cam->width / 256;
+	header2[440] = cam->width % 256;
+
+	for (i = 0; init[cam->method][i].size != -1; i++) {
+		ret =
+		    send_control_msg(cam->udev, 1, init[cam->method][i].value,
+				     0, init[cam->method][i].bytes,
+				     init[cam->method][i].size);
+		if (ret < 0) {
+			dev_err(&cam->udev->dev,
+			   "error during resolution change sequence: %d\n", i);
+			goto out;
+		}
+	}
+
+	/* Added some delay here, since opening/closing the camera quickly,
+	 * like Ekiga does during its startup, can crash the webcam
+	 */
+	mdelay(100);
+	cam->skip = 2;
 	ret = 0;
 
 out:
@@ -1123,6 +1168,7 @@ 
 		cam->buffer.frame[j].ulState = ZR364XX_READ_IDLE;
 		cam->buffer.frame[j].cur_size = 0;
 	}
+	cam->b_acquire = 1;
 	return 0;
 }
 
@@ -1165,7 +1211,6 @@ 
 	res = videobuf_streamon(&cam->vb_vidq);
 	if (res == 0) {
 		zr364xx_start_acquire(cam);
-		cam->b_acquire = 1;
 	} else {
 		res_free(cam);
 	}