Skip to contentSkip to conversation

hdpvr: fix interrupted recording

RFC PATCH Superseded Jonathan Sims 2 participants 2 messages Jan 26, 2017 – Feb 8, 2017

Subsystems (2)
Media (V4L/DVB) Hdpvr USB video encoder driver

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Jonathan Sims

author
#0 lore raw

RFC PATCH v3 1/1 hdpvr: fix interrupted recording

This is a reworking of a patch originally submitted by Ryley Angus, modified by Hans Verkuil and then seemingly forgotten before changes suggested by Keith Pyle here:

http://www.mail-archive.com/[email protected]/msg75163.html

were made and tested.

I have implemented the suggested changes and have been testing for several months. I am no longer experiencing lockups while recording (with blue light on, requiring power cycling) which had been a long standing problem with the HD-PVR. I have not noticed any other problems since applying the patch.

Signed-off-by Jonathan Sims <[email protected]>

---

Changes in v3:
- Actually sleep for 4 seconds after the hdpvr_stop_streaming call.

 drivers/media/usb/hdpvr/hdpvr-video.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)
drivers/media/usb/hdpvr/hdpvr-video.c +22 -3
index 2a3a8b4..dec8496 100644
@@ -454,6 +454,7 @@ static ssize_t hdpvr_read(struct file *file, char __user *buffer, size_t count,
 
 		if (buf->status != BUFSTAT_READY &&
 		    dev->status != STATUS_DISCONNECTED) {
+			int err;
 			/* return nonblocking */
 			if (file->f_flags & O_NONBLOCK) {
 				if (!ret)

@@ -461,9 +462,27 @@ static ssize_t hdpvr_read(struct file *file, char __user *buffer, size_t count,
 				goto err;
 			}
 
-			if (wait_event_interruptible(dev->wait_data,
-					      buf->status == BUFSTAT_READY))
-				return -ERESTARTSYS;
+			err = wait_event_interruptible_timeout(dev->wait_data,
+				buf->status == BUFSTAT_READY,
+				msecs_to_jiffies(1000));
+			if (err < 0) {
+				ret = err;
+				goto err;
+			}
+			if (!err) {
+				int delay;
+
+				v4l2_dbg(MSG_INFO, hdpvr_debug, &dev->v4l2_dev,
+					"timeout: restart streaming\n");
+				hdpvr_stop_streaming(dev);
+				delay = msecs_to_jiffies(4000);
+				schedule_timeout(delay);
+				err = hdpvr_start_streaming(dev);
+				if (err) {
+					ret = err;
+					goto err;
+				}
+			}
 		}
 
 		if (buf->status != BUFSTAT_READY)
Signature · 1 line
2.10.1
#1 lore raw

Em Thu, 26 Jan 2017 07:17:22 -0500
Jonathan Sims <[email protected]> escreveu:

Quoted 22 lines

This is a reworking of a patch originally submitted by Ryley Angus, modified by Hans Verkuil and then seemingly forgotten before changes suggested by Keith Pyle here:

http://www.mail-archive.com/[email protected]/msg75163.html

were made and tested.

I have implemented the suggested changes and have been testing for several months. I am no longer experiencing lockups while recording (with blue light on, requiring power cycling) which had been a long standing problem with the HD-PVR. I have not noticed any other problems since applying the patch.

Signed-off-by Jonathan Sims <[email protected]>

---

Changes in v3:
- Actually sleep for 4 seconds after the hdpvr_stop_streaming call.

 drivers/media/usb/hdpvr/hdpvr-video.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)
drivers/media/usb/hdpvr/hdpvr-video.c +0 -0
index 2a3a8b4..dec8496 100644

drivers/media/usb/hdpvr/hdpvr-video.c static ssize_t hdpvr_read(struct file *file, char __user *buffer, size_t count, · line 457

Quoted 4 lines
drivers/media/usb/hdpvr/hdpvr-video.c +1 -0
@@ -454,6 +454,7 @@ static ssize_t hdpvr_read(struct file *file, char __user *buffer, size_t count,
 
 		if (buf->status != BUFSTAT_READY &&
 		    dev->status != STATUS_DISCONNECTED) {
+			int err;

Are there any reason why not using "ret" var, instead of creating another return variable here?

Quoted 15 lines
drivers/media/usb/hdpvr/hdpvr-video.c +5 -3
 			/* return nonblocking */
 			if (file->f_flags & O_NONBLOCK) {
 				if (!ret)

@@ -461,9 +462,27 @@ static ssize_t hdpvr_read(struct file *file, char __user *buffer, size_t count,
 				goto err;
 			}
 
-			if (wait_event_interruptible(dev->wait_data,
-					      buf->status == BUFSTAT_READY))
-				return -ERESTARTSYS;
+			err = wait_event_interruptible_timeout(dev->wait_data,
+				buf->status == BUFSTAT_READY,
+				msecs_to_jiffies(1000));
+			if (err < 0) {
+				ret = err;

drivers/media/usb/hdpvr/hdpvr-video.c static ssize_t hdpvr_read(struct file *file, char __user *buffer, size_t count, · line 479

Quoted 10 lines
drivers/media/usb/hdpvr/hdpvr-video.c +10 -0
+				goto err;
+			}
+			if (!err) {
+				int delay;
+
+				v4l2_dbg(MSG_INFO, hdpvr_debug, &dev->v4l2_dev,
+					"timeout: restart streaming\n");
+				hdpvr_stop_streaming(dev);
+				delay = msecs_to_jiffies(4000);
+				schedule_timeout(delay);

Why don't you just call:

	msleep(4000);

drivers/media/usb/hdpvr/hdpvr-video.c static ssize_t hdpvr_read(struct file *file, char __user *buffer, size_t count, · line 488

Quoted 9 lines
drivers/media/usb/hdpvr/hdpvr-video.c +6 -0
+				err = hdpvr_start_streaming(dev);
+				if (err) {
+					ret = err;
+					goto err;
+				}
+			}
 		}
 
 		if (buf->status != BUFSTAT_READY)

Thanks,
Mauro