* [REVIEWv2 PATCH 0/3] hdpvr: various fixes
@ 2013-05-29 7:55 Hans Verkuil
2013-05-29 7:55 ` [REVIEWv2 PATCH 1/3] hdpvr: fix querystd 'unknown format' return Hans Verkuil
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Hans Verkuil @ 2013-05-29 7:55 UTC (permalink / raw)
To: linux-media; +Cc: leo
The first patch fixes a bug in querystd: if there is no signal, then
querystd should return V4L2_STD_UNKNOWN. There are more drivers that
return the wrong value here, I have a patch series pending to fix that
and also to improve the spec.
The second does a code cleanup that improves readability, but it doesn't
change the logic.
The third patch is based on a patch from Mauro and a patch from Leo:
https://patchwork.linuxtv.org/patch/18573/
https://linuxtv.org/patch/18399/
This improves the error handling in case usb_control_msg() returns an
error.
Changes since v1:
- Also return the low-level usb_control_msg error in the vidioc_g_fmt_vid_cap
case.
Regards,
Hans
^ permalink raw reply [flat|nested] 4+ messages in thread
* [REVIEWv2 PATCH 1/3] hdpvr: fix querystd 'unknown format' return.
2013-05-29 7:55 [REVIEWv2 PATCH 0/3] hdpvr: various fixes Hans Verkuil
@ 2013-05-29 7:55 ` Hans Verkuil
2013-05-29 7:55 ` [REVIEWv2 PATCH 2/3] hdpvr: code cleanup Hans Verkuil
2013-05-29 7:55 ` [REVIEWv2 PATCH 3/3] hdpvr: improve error handling Hans Verkuil
2 siblings, 0 replies; 4+ messages in thread
From: Hans Verkuil @ 2013-05-29 7:55 UTC (permalink / raw)
To: linux-media; +Cc: leo, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
If no format has been detected, then querystd should return V4L2_STD_UNKNOWN,
not V4L2_STD_ALL.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/usb/hdpvr/hdpvr-video.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c b/drivers/media/usb/hdpvr/hdpvr-video.c
index 2d02b49..81018c4 100644
--- a/drivers/media/usb/hdpvr/hdpvr-video.c
+++ b/drivers/media/usb/hdpvr/hdpvr-video.c
@@ -613,7 +613,7 @@ static int vidioc_querystd(struct file *file, void *_fh, v4l2_std_id *a)
struct hdpvr_fh *fh = _fh;
int ret;
- *a = V4L2_STD_ALL;
+ *a = V4L2_STD_UNKNOWN;
if (dev->options.video_input == HDPVR_COMPONENT)
return fh->legacy_mode ? 0 : -ENODATA;
ret = get_video_info(dev, &vid_info);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [REVIEWv2 PATCH 2/3] hdpvr: code cleanup
2013-05-29 7:55 [REVIEWv2 PATCH 0/3] hdpvr: various fixes Hans Verkuil
2013-05-29 7:55 ` [REVIEWv2 PATCH 1/3] hdpvr: fix querystd 'unknown format' return Hans Verkuil
@ 2013-05-29 7:55 ` Hans Verkuil
2013-05-29 7:55 ` [REVIEWv2 PATCH 3/3] hdpvr: improve error handling Hans Verkuil
2 siblings, 0 replies; 4+ messages in thread
From: Hans Verkuil @ 2013-05-29 7:55 UTC (permalink / raw)
To: linux-media; +Cc: leo, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
Remove an unnecessary 'else' and invert a condition which makes the code
more readable.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/usb/hdpvr/hdpvr-video.c | 54 ++++++++++++++++-----------------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c b/drivers/media/usb/hdpvr/hdpvr-video.c
index 81018c4..e947105 100644
--- a/drivers/media/usb/hdpvr/hdpvr-video.c
+++ b/drivers/media/usb/hdpvr/hdpvr-video.c
@@ -281,43 +281,43 @@ static int hdpvr_start_streaming(struct hdpvr_device *dev)
if (dev->status == STATUS_STREAMING)
return 0;
- else if (dev->status != STATUS_IDLE)
+ if (dev->status != STATUS_IDLE)
return -EAGAIN;
ret = get_video_info(dev, &vidinf);
+ if (ret) {
+ msleep(250);
+ v4l2_dbg(MSG_INFO, hdpvr_debug, &dev->v4l2_dev,
+ "no video signal at input %d\n", dev->options.video_input);
+ return -EAGAIN;
+ }
- if (!ret) {
- v4l2_dbg(MSG_BUFFER, hdpvr_debug, &dev->v4l2_dev,
- "video signal: %dx%d@%dhz\n", vidinf.width,
- vidinf.height, vidinf.fps);
+ v4l2_dbg(MSG_BUFFER, hdpvr_debug, &dev->v4l2_dev,
+ "video signal: %dx%d@%dhz\n", vidinf.width,
+ vidinf.height, vidinf.fps);
- /* start streaming 2 request */
- ret = usb_control_msg(dev->udev,
- usb_sndctrlpipe(dev->udev, 0),
- 0xb8, 0x38, 0x1, 0, NULL, 0, 8000);
- v4l2_dbg(MSG_BUFFER, hdpvr_debug, &dev->v4l2_dev,
- "encoder start control request returned %d\n", ret);
- if (ret < 0)
- return ret;
+ /* start streaming 2 request */
+ ret = usb_control_msg(dev->udev,
+ usb_sndctrlpipe(dev->udev, 0),
+ 0xb8, 0x38, 0x1, 0, NULL, 0, 8000);
+ v4l2_dbg(MSG_BUFFER, hdpvr_debug, &dev->v4l2_dev,
+ "encoder start control request returned %d\n", ret);
+ if (ret < 0)
+ return ret;
- ret = hdpvr_config_call(dev, CTRL_START_STREAMING_VALUE, 0x00);
- if (ret)
- return ret;
+ ret = hdpvr_config_call(dev, CTRL_START_STREAMING_VALUE, 0x00);
+ if (ret)
+ return ret;
- dev->status = STATUS_STREAMING;
+ dev->status = STATUS_STREAMING;
- INIT_WORK(&dev->worker, hdpvr_transmit_buffers);
- queue_work(dev->workqueue, &dev->worker);
+ INIT_WORK(&dev->worker, hdpvr_transmit_buffers);
+ queue_work(dev->workqueue, &dev->worker);
- v4l2_dbg(MSG_BUFFER, hdpvr_debug, &dev->v4l2_dev,
- "streaming started\n");
+ v4l2_dbg(MSG_BUFFER, hdpvr_debug, &dev->v4l2_dev,
+ "streaming started\n");
- return 0;
- }
- msleep(250);
- v4l2_dbg(MSG_INFO, hdpvr_debug, &dev->v4l2_dev,
- "no video signal at input %d\n", dev->options.video_input);
- return -EAGAIN;
+ return 0;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [REVIEWv2 PATCH 3/3] hdpvr: improve error handling
2013-05-29 7:55 [REVIEWv2 PATCH 0/3] hdpvr: various fixes Hans Verkuil
2013-05-29 7:55 ` [REVIEWv2 PATCH 1/3] hdpvr: fix querystd 'unknown format' return Hans Verkuil
2013-05-29 7:55 ` [REVIEWv2 PATCH 2/3] hdpvr: code cleanup Hans Verkuil
@ 2013-05-29 7:55 ` Hans Verkuil
2 siblings, 0 replies; 4+ messages in thread
From: Hans Verkuil @ 2013-05-29 7:55 UTC (permalink / raw)
To: linux-media; +Cc: leo, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
get_video_info() should never return EFAULT, instead it should return
the low-level usb_control_msg() error. Add a valid field to the hdpvr_video_info
struct so the driver can easily check if a valid format was detected.
Whenever get_video_info is called and it returns an error (e.g. usb_control_msg
failed), then return that error to userspace as well.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/usb/hdpvr/hdpvr-control.c | 21 +++++++++------------
drivers/media/usb/hdpvr/hdpvr-video.c | 18 +++++++++++-------
drivers/media/usb/hdpvr/hdpvr.h | 1 +
3 files changed, 21 insertions(+), 19 deletions(-)
diff --git a/drivers/media/usb/hdpvr/hdpvr-control.c b/drivers/media/usb/hdpvr/hdpvr-control.c
index df6bcb5..6053661 100644
--- a/drivers/media/usb/hdpvr/hdpvr-control.c
+++ b/drivers/media/usb/hdpvr/hdpvr-control.c
@@ -49,6 +49,7 @@ int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vidinf)
{
int ret;
+ vidinf->valid = false;
mutex_lock(&dev->usbc_mutex);
ret = usb_control_msg(dev->udev,
usb_rcvctrlpipe(dev->udev, 0),
@@ -56,11 +57,6 @@ int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vidinf)
0x1400, 0x0003,
dev->usbc_buf, 5,
1000);
- if (ret == 5) {
- vidinf->width = dev->usbc_buf[1] << 8 | dev->usbc_buf[0];
- vidinf->height = dev->usbc_buf[3] << 8 | dev->usbc_buf[2];
- vidinf->fps = dev->usbc_buf[4];
- }
#ifdef HDPVR_DEBUG
if (hdpvr_debug & MSG_INFO) {
@@ -73,14 +69,15 @@ int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vidinf)
#endif
mutex_unlock(&dev->usbc_mutex);
- if ((ret > 0 && ret != 5) ||/* fail if unexpected byte count returned */
- !vidinf->width || /* preserve original behavior - */
- !vidinf->height || /* fail if no signal is detected */
- !vidinf->fps) {
- ret = -EFAULT;
- }
+ if (ret < 0)
+ return ret;
- return ret < 0 ? ret : 0;
+ vidinf->width = dev->usbc_buf[1] << 8 | dev->usbc_buf[0];
+ vidinf->height = dev->usbc_buf[3] << 8 | dev->usbc_buf[2];
+ vidinf->fps = dev->usbc_buf[4];
+ vidinf->valid = vidinf->width && vidinf->height && vidinf->fps;
+
+ return 0;
}
int get_input_lines_info(struct hdpvr_device *dev)
diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c b/drivers/media/usb/hdpvr/hdpvr-video.c
index e947105..4f8567a 100644
--- a/drivers/media/usb/hdpvr/hdpvr-video.c
+++ b/drivers/media/usb/hdpvr/hdpvr-video.c
@@ -285,7 +285,10 @@ static int hdpvr_start_streaming(struct hdpvr_device *dev)
return -EAGAIN;
ret = get_video_info(dev, &vidinf);
- if (ret) {
+ if (ret < 0)
+ return ret;
+
+ if (!vidinf.valid) {
msleep(250);
v4l2_dbg(MSG_INFO, hdpvr_debug, &dev->v4l2_dev,
"no video signal at input %d\n", dev->options.video_input);
@@ -617,15 +620,12 @@ static int vidioc_querystd(struct file *file, void *_fh, v4l2_std_id *a)
if (dev->options.video_input == HDPVR_COMPONENT)
return fh->legacy_mode ? 0 : -ENODATA;
ret = get_video_info(dev, &vid_info);
- if (ret)
- return 0;
- if (vid_info.width == 720 &&
+ if (vid_info.valid && vid_info.width == 720 &&
(vid_info.height == 480 || vid_info.height == 576)) {
*a = (vid_info.height == 480) ?
V4L2_STD_525_60 : V4L2_STD_625_50;
}
-
- return 0;
+ return ret;
}
static int vidioc_s_dv_timings(struct file *file, void *_fh,
@@ -679,6 +679,8 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh,
return -ENODATA;
ret = get_video_info(dev, &vid_info);
if (ret)
+ return ret;
+ if (!vid_info.valid)
return -ENOLCK;
interlaced = vid_info.fps <= 30;
for (i = 0; i < ARRAY_SIZE(hdpvr_dv_timings); i++) {
@@ -1008,7 +1010,9 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *_fh,
struct hdpvr_video_info vid_info;
ret = get_video_info(dev, &vid_info);
- if (ret)
+ if (ret < 0)
+ return ret;
+ if (!vid_info.valid)
return -EFAULT;
f->fmt.pix.width = vid_info.width;
f->fmt.pix.height = vid_info.height;
diff --git a/drivers/media/usb/hdpvr/hdpvr.h b/drivers/media/usb/hdpvr/hdpvr.h
index 808ea7a..dc685d4 100644
--- a/drivers/media/usb/hdpvr/hdpvr.h
+++ b/drivers/media/usb/hdpvr/hdpvr.h
@@ -154,6 +154,7 @@ struct hdpvr_video_info {
u16 width;
u16 height;
u8 fps;
+ bool valid;
};
enum {
--
1.7.10.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-05-29 7:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-29 7:55 [REVIEWv2 PATCH 0/3] hdpvr: various fixes Hans Verkuil
2013-05-29 7:55 ` [REVIEWv2 PATCH 1/3] hdpvr: fix querystd 'unknown format' return Hans Verkuil
2013-05-29 7:55 ` [REVIEWv2 PATCH 2/3] hdpvr: code cleanup Hans Verkuil
2013-05-29 7:55 ` [REVIEWv2 PATCH 3/3] hdpvr: improve error handling Hans Verkuil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).