On Saturday, June 25, 2011 16:17:18 Mauro Carvalho Chehab wrote:
> Em 25-06-2011 11:02, Hans Verkuil escreveu:
> > Hi all,
> >
> > The tuner-core.c implementation does this at the start of g_tuner:
> >
> > if (check_mode(t, vt->type) == -EINVAL)
> > return 0;
> > vt->type = t->mode;
> >
> > The idea is that the vt->type is set depending on whether the VIDIOC_G_TUNER
> > ioctl is called from a radio device node or a video device node. If we have
> > a
> > tuner that can do both radio and TV, then the type is set to whatever the
> > current tuner mode is.
> >
> > This seems reasonable, but it will actually run into problems when g_tuner
> > is called for audio demodulators like msp3400 or cx25840. These need to know
> > the correct vt->type in order to fill in the right fields.
> >
> > The problem here is that the tuner subdevices are not necessarily called
> > first.
> > In fact, the msp3400/cx25840 are actually called first by ivtv.
> >
> > So the msp3400 will get called with type TV, and later the tuner may change
> > that
> > to type RADIO. This causes inconsistencies. This has actually been observed
> > when
> > testing with ivtv and a PVR-500.
> >
> > There are two solutions:
> >
> > 1) Audit the drivers and ensure that the tuner subdevices are registered
> > first.
> >
> > 2) Do not allow the tuner to switch the type.
>
> (2) is the right thing to do. A VIDIOC_GET_foo should not change anything.
> They are
> supposed to be read only access.
Great!
Can you review this patch? This implements solution 2. I think it's correct, but
it's tuner code, so you never know :-)
diff --git a/drivers/media/video/tuner-core.c b/drivers/media/video/tuner-core.c
index 9006c9a..706fc11 100644
--- a/drivers/media/video/tuner-core.c
+++ b/drivers/media/video/tuner-core.c
@@ -1119,8 +1119,7 @@ static int tuner_g_frequency(struct v4l2_subdev *sd,
struct v4l2_frequency *f)
if (check_mode(t, f->type) == -EINVAL)
return 0;
- f->type = t->mode;
- if (fe_tuner_ops->get_frequency && !t->standby) {
+ if (f->type == t->mode && fe_tuner_ops->get_frequency && !t->standby) {
u32 abs_freq;
fe_tuner_ops->get_frequency(&t->fe, &abs_freq);
@@ -1128,7 +1127,7 @@ static int tuner_g_frequency(struct v4l2_subdev *sd,
struct v4l2_frequency *f)
DIV_ROUND_CLOSEST(abs_freq * 2, 125) :
DIV_ROUND_CLOSEST(abs_freq, 62500);
} else {
- f->frequency = (V4L2_TUNER_RADIO == t->mode) ?
+ f->frequency = (V4L2_TUNER_RADIO == f->type) ?
t->radio_freq : t->tv_freq;
}
return 0;
@@ -1152,32 +1151,33 @@ static int tuner_g_tuner(struct v4l2_subdev *sd, struct
v4l2_tuner *vt)
if (check_mode(t, vt->type) == -EINVAL)
return 0;
- vt->type = t->mode;
- if (analog_ops->get_afc)
+ if (vt->type == t->mode && analog_ops->get_afc)
vt->afc = analog_ops->get_afc(&t->fe);
- if (t->mode == V4L2_TUNER_ANALOG_TV)
+ if (vt->type == V4L2_TUNER_ANALOG_TV)
vt->capability |= V4L2_TUNER_CAP_NORM;
- if (t->mode != V4L2_TUNER_RADIO) {
+ if (vt->type != V4L2_TUNER_RADIO) {
vt->rangelow = tv_range[0] * 16;
vt->rangehigh = tv_range[1] * 16;
return 0;
}
/* radio mode */
- vt->rxsubchans = V4L2_TUNER_SUB_MONO | V4L2_TUNER_SUB_STEREO;
- if (fe_tuner_ops->get_status) {
- u32 tuner_status;
-
- fe_tuner_ops->get_status(&t->fe, &tuner_status);
- vt->rxsubchans =
- (tuner_status & TUNER_STATUS_STEREO) ?
- V4L2_TUNER_SUB_STEREO :
- V4L2_TUNER_SUB_MONO;
+ if (vt->type == t->mode) {
+ vt->rxsubchans = V4L2_TUNER_SUB_MONO | V4L2_TUNER_SUB_STEREO;
+ if (fe_tuner_ops->get_status) {
+ u32 tuner_status;
+
+ fe_tuner_ops->get_status(&t->fe, &tuner_status);
+ vt->rxsubchans =
+ (tuner_status & TUNER_STATUS_STEREO) ?
+ V4L2_TUNER_SUB_STEREO :
+ V4L2_TUNER_SUB_MONO;
+ }
+ if (analog_ops->has_signal)
+ vt->signal = analog_ops->has_signal(&t->fe);
+ vt->audmode = t->audmode;
}
- if (analog_ops->has_signal)
- vt->signal = analog_ops->has_signal(&t->fe);
vt->capability |= V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO;
- vt->audmode = t->audmode;
vt->rangelow = radio_range[0] * 16000;
vt->rangehigh = radio_range[1] * 16000;
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
