| Message ID | 1359315912-1767-7-git-send-email-linux@rainbow-software.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Em Sun, 27 Jan 2013 20:45:11 +0100 Ondrej Zary <linux@rainbow-software.org> escreveu: > Make saa7134 driver more V4L2 compliant: don't set bogus V4L2_IN_ST_NO_SYNC > flag in enum_input as it's for digital video only > > Signed-off-by: Ondrej Zary <linux@rainbow-software.org> > --- > drivers/media/pci/saa7134/saa7134-video.c | 2 -- > 1 files changed, 0 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c > index 0b42f0c..fff6735 100644 > --- a/drivers/media/pci/saa7134/saa7134-video.c > +++ b/drivers/media/pci/saa7134/saa7134-video.c > @@ -1757,8 +1757,6 @@ static int saa7134_enum_input(struct file *file, void *priv, > > if (0 != (v1 & 0x40)) > i->status |= V4L2_IN_ST_NO_H_LOCK; > - if (0 != (v2 & 0x40)) > - i->status |= V4L2_IN_ST_NO_SYNC; > if (0 != (v2 & 0x0e)) > i->status |= V4L2_IN_ST_MACROVISION; > } Hmm... I'm not sure about this one. Very few drivers use those definitions, but I suspect that this might potentially break some userspace applications. It sounds more likely that v4l-compliance is doing the wrong thing here, if it is complaining about that.
On Sun January 27 2013 20:45:11 Ondrej Zary wrote: > Make saa7134 driver more V4L2 compliant: don't set bogus V4L2_IN_ST_NO_SYNC > flag in enum_input as it's for digital video only I think there is a lot to be said for allowing this particular flag for analog as well. There are two types of flags here: flags relating the digital tuners (those should not be used) and flags relating to sync issues. V4L2_IN_ST_NO_SYNC is perfectly valid as a way to report sync problems. I think a better solution is to allow this flag and to reorganize the 'Input Status Flags' documentation: - rename the "Analog Video" header to "Video Sync" - rename the "Digital Video" header to "Digital Tuner (Deprecated)" - move V4L2_IN_ST_NO_SYNC to the "Video Sync" section Basically you have a number of video sync states: 1) NO_POWER: the device is off 2) NO_SIGNAL: the device is on, but there is no incoming signal 3) NO_SYNC: there is a signal, but we can't sync at all 4) NO_H_LOCK: there is a signal, we detect video lines, but we can't get a horizontal sync 5) NO_COLOR: there is no color signal at all 6) COLOR_KILL: we see a color signal, but we can't lock to it. Note that not a single driver uses COLOR_KILL at the moment, but I know it can be used. If you can think of a way of improving the documentation w.r.t. these sync status flags, then that would be great. Perhaps the table shouldn't be in the order of the value but in a more logical order. Regards, Hans > Signed-off-by: Ondrej Zary <linux@rainbow-software.org> > --- > drivers/media/pci/saa7134/saa7134-video.c | 2 -- > 1 files changed, 0 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c > index 0b42f0c..fff6735 100644 > --- a/drivers/media/pci/saa7134/saa7134-video.c > +++ b/drivers/media/pci/saa7134/saa7134-video.c > @@ -1757,8 +1757,6 @@ static int saa7134_enum_input(struct file *file, void *priv, > > if (0 != (v1 & 0x40)) > i->status |= V4L2_IN_ST_NO_H_LOCK; > - if (0 != (v2 & 0x40)) > - i->status |= V4L2_IN_ST_NO_SYNC; > if (0 != (v2 & 0x0e)) > i->status |= V4L2_IN_ST_MACROVISION; > } > -- 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
On Mon January 28 2013 11:30:46 Mauro Carvalho Chehab wrote: > Em Sun, 27 Jan 2013 20:45:11 +0100 > Ondrej Zary <linux@rainbow-software.org> escreveu: > > > Make saa7134 driver more V4L2 compliant: don't set bogus V4L2_IN_ST_NO_SYNC > > flag in enum_input as it's for digital video only > > > > Signed-off-by: Ondrej Zary <linux@rainbow-software.org> > > --- > > drivers/media/pci/saa7134/saa7134-video.c | 2 -- > > 1 files changed, 0 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c > > index 0b42f0c..fff6735 100644 > > --- a/drivers/media/pci/saa7134/saa7134-video.c > > +++ b/drivers/media/pci/saa7134/saa7134-video.c > > @@ -1757,8 +1757,6 @@ static int saa7134_enum_input(struct file *file, void *priv, > > > > if (0 != (v1 & 0x40)) > > i->status |= V4L2_IN_ST_NO_H_LOCK; > > - if (0 != (v2 & 0x40)) > > - i->status |= V4L2_IN_ST_NO_SYNC; > > if (0 != (v2 & 0x0e)) > > i->status |= V4L2_IN_ST_MACROVISION; > > } > > > Hmm... I'm not sure about this one. Very few drivers use those definitions, > but I suspect that this might potentially break some userspace applications. > > It sounds more likely that v4l-compliance is doing the wrong thing here, > if it is complaining about that. I agree. See my reply to this patch. I'd appreciate your input on that. Regards, Hans -- 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
diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c index 0b42f0c..fff6735 100644 --- a/drivers/media/pci/saa7134/saa7134-video.c +++ b/drivers/media/pci/saa7134/saa7134-video.c @@ -1757,8 +1757,6 @@ static int saa7134_enum_input(struct file *file, void *priv, if (0 != (v1 & 0x40)) i->status |= V4L2_IN_ST_NO_H_LOCK; - if (0 != (v2 & 0x40)) - i->status |= V4L2_IN_ST_NO_SYNC; if (0 != (v2 & 0x0e)) i->status |= V4L2_IN_ST_MACROVISION; }
Make saa7134 driver more V4L2 compliant: don't set bogus V4L2_IN_ST_NO_SYNC flag in enum_input as it's for digital video only Signed-off-by: Ondrej Zary <linux@rainbow-software.org> --- drivers/media/pci/saa7134/saa7134-video.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)