Re: V4L2 spec behavior for G_TUNER and T_STANDBY

2011-01-01 Thread Hans Verkuil
On Saturday, January 01, 2011 22:25:04 Devin Heitmueller wrote:
> Hi Hans,
> 
> Thanks for the feedback.
> 
> On Sat, Jan 1, 2011 at 4:18 PM, Hans Verkuil  wrote:
> >> This basically means that a video tuner will bail out, which sounds
> >> good because the rest of the function supposedly assumes a radio
> >> device.  However, as a result the has_signal() call (which returns
> >> signal strength) will never be executed for video tuners.  You
> >> wouldn't notice this if a video decoder subdev is responsible for
> >> showing signal strength, but if you're expecting the tuner to provide
> >> the info, the call will never happen.
> >
> > I am not aware of any tuner that does that. I think that for video this
> > is always done by a video decoder. That said, it isn't pretty, but a lot
> > of this is legacy code and I wouldn't want to change it.
> 
> The Xceive tuners have their analog demodulator onboard, so they make
> available a 0-100% signal strength.
> 
> > After digging some more I think that check_mode is a poor function. There 
> > are
> > two things that check_mode does: checking if the tuner support radio and/or 
> > tv
> > mode (that's fine), and if it is in standby: not so fine. That should be a
> > separate function since filling in frequency ranges can be done regardless 
> > of
> > the standby state.
> 
> Yeah, I didn't realize myself that is what check_mode was doing until
> I had this problem.
> 
> I'll see if I can cook up a patch which returns the appropriate data
> even when in standby, while trying to minimize the risk of introducing
> a regression.

Since you are working with tuner-core and xceive I wonder if you could also
make a patch to remove T_DIGITAL_TV in enum tuner_mode. T_DIGITAL_TV (and it's
public API counterpart V4L2_TUNER_DIGITAL_TV) are not used (and never were used
since digital tuning is handled through dvb).

We can't remove V4L2_TUNER_DIGITAL_TV (although we can mark it as deprecated in
videodev2.h), but T_DIGITAL_TV should definitely be removed.

The only tuner driver that uses T_DIGITAL_TV is tuner-xc2028.c, but on closer
analysis it turns out that it uses enum tuner_mode as an internal mode. So this
mode is never set outside the driver. Instead it should have used an internal
xceive_mode enum.

Everywhere else T_DIGITAL_TV can be removed.

For some reason I'm in an early spring-cleaning mode (winter-cleaning mode?)
these days :-) Getting rid of old obsolete stuff is always nice.

Regards,

Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: V4L2 spec behavior for G_TUNER and T_STANDBY

2011-01-01 Thread Devin Heitmueller
Hi Hans,

Thanks for the feedback.

On Sat, Jan 1, 2011 at 4:18 PM, Hans Verkuil  wrote:
>> This basically means that a video tuner will bail out, which sounds
>> good because the rest of the function supposedly assumes a radio
>> device.  However, as a result the has_signal() call (which returns
>> signal strength) will never be executed for video tuners.  You
>> wouldn't notice this if a video decoder subdev is responsible for
>> showing signal strength, but if you're expecting the tuner to provide
>> the info, the call will never happen.
>
> I am not aware of any tuner that does that. I think that for video this
> is always done by a video decoder. That said, it isn't pretty, but a lot
> of this is legacy code and I wouldn't want to change it.

The Xceive tuners have their analog demodulator onboard, so they make
available a 0-100% signal strength.

> After digging some more I think that check_mode is a poor function. There are
> two things that check_mode does: checking if the tuner support radio and/or tv
> mode (that's fine), and if it is in standby: not so fine. That should be a
> separate function since filling in frequency ranges can be done regardless of
> the standby state.

Yeah, I didn't realize myself that is what check_mode was doing until
I had this problem.

I'll see if I can cook up a patch which returns the appropriate data
even when in standby, while trying to minimize the risk of introducing
a regression.

Thanks,

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: V4L2 spec behavior for G_TUNER and T_STANDBY

2011-01-01 Thread Hans Verkuil
On Saturday, January 01, 2011 21:52:35 Devin Heitmueller wrote:
> I have been doing some application conformance for VLC, and I noticed
> something interesting with regards to the G_TUNER call.
> 
> If you have a tuner which supports sleeping, making a G_TUNER call
> essentially returns garbage.
> ===
> r...@devin-laptop2:~# v4l2-ctl -d /dev/video1 --get-tuner
> Tuner:
> Name : Auvitek tuner
> Capabilities : 62.5 kHz stereo lang1 lang2
> Frequency range  : 0.0 MHz - 0.0 MHz
> Signal strength/AFC  : 0%/0
> Current audio mode   : stereo
> Available subchannels: mono
> ===
> Note that the frequency range is zero (the capabilities and name are
> populated by the bridge or video decoder).  Some digging into the
> tuner_g_tuner() function in tuner core shows that the check_mode()
> call fails because the device's mode is T_STANDBY.  However, it does
> this despite the fact that none of values required actually interact
> with the tuner.  The capabilities and frequency ranges should be able
> to be populated regardless of whether the device is in standby.

Agreed.

> This is particularly bad because devices normally come out of standby
> when a s_freq call occurs, but some applications (such as VLC) will
> call g_tuner first to validate the target frequency is inside the
> valid frequency range.  So you have a chicken/egg problem:  The
> g_tuner won't return a valid frequency range until you do a tuning
> request to wake up the tuner, but apps like VLC won't do a tuning
> request unless it has validated the frequency range.
> 
> Further, look at the following block:
> 
> if (t->mode != V4L2_TUNER_RADIO) {
> vt->rangelow = tv_range[0] * 16;
> vt->rangehigh = tv_range[1] * 16;
> return 0;
> }
> 
> This basically means that a video tuner will bail out, which sounds
> good because the rest of the function supposedly assumes a radio
> device.  However, as a result the has_signal() call (which returns
> signal strength) will never be executed for video tuners.  You
> wouldn't notice this if a video decoder subdev is responsible for
> showing signal strength, but if you're expecting the tuner to provide
> the info, the call will never happen.

I am not aware of any tuner that does that. I think that for video this
is always done by a video decoder. That said, it isn't pretty, but a lot
of this is legacy code and I wouldn't want to change it.
 
> Are these known issues?

No.

> Am I misreading the specified behavior?  I
> don't see anything in the spec that suggests that this call should
> return invalid data if the tuner happens to be powered down.

You are correct, G_TUNER should return valid data, even when powered down.
I suspect that G_FREQUENCY has the same problem.

This should be fixed so that both work when the tuner is powered down.

And if this is fixed, then the switch_v4l2, using_v4l2 and check_v4l2 symbols
can also be removed since they are no longer in use.


After digging some more I think that check_mode is a poor function. There are
two things that check_mode does: checking if the tuner support radio and/or tv
mode (that's fine), and if it is in standby: not so fine. That should be a
separate function since filling in frequency ranges can be done regardless of
the standby state.

Regards,

Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


V4L2 spec behavior for G_TUNER and T_STANDBY

2011-01-01 Thread Devin Heitmueller
I have been doing some application conformance for VLC, and I noticed
something interesting with regards to the G_TUNER call.

If you have a tuner which supports sleeping, making a G_TUNER call
essentially returns garbage.
===
r...@devin-laptop2:~# v4l2-ctl -d /dev/video1 --get-tuner
Tuner:
Name : Auvitek tuner
Capabilities : 62.5 kHz stereo lang1 lang2
Frequency range  : 0.0 MHz - 0.0 MHz
Signal strength/AFC  : 0%/0
Current audio mode   : stereo
Available subchannels: mono
===
Note that the frequency range is zero (the capabilities and name are
populated by the bridge or video decoder).  Some digging into the
tuner_g_tuner() function in tuner core shows that the check_mode()
call fails because the device's mode is T_STANDBY.  However, it does
this despite the fact that none of values required actually interact
with the tuner.  The capabilities and frequency ranges should be able
to be populated regardless of whether the device is in standby.

This is particularly bad because devices normally come out of standby
when a s_freq call occurs, but some applications (such as VLC) will
call g_tuner first to validate the target frequency is inside the
valid frequency range.  So you have a chicken/egg problem:  The
g_tuner won't return a valid frequency range until you do a tuning
request to wake up the tuner, but apps like VLC won't do a tuning
request unless it has validated the frequency range.

Further, look at the following block:

if (t->mode != V4L2_TUNER_RADIO) {
vt->rangelow = tv_range[0] * 16;
vt->rangehigh = tv_range[1] * 16;
return 0;
}

This basically means that a video tuner will bail out, which sounds
good because the rest of the function supposedly assumes a radio
device.  However, as a result the has_signal() call (which returns
signal strength) will never be executed for video tuners.  You
wouldn't notice this if a video decoder subdev is responsible for
showing signal strength, but if you're expecting the tuner to provide
the info, the call will never happen.

Are these known issues?  Am I misreading the specified behavior?  I
don't see anything in the spec that suggests that this call should
return invalid data if the tuner happens to be powered down.

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html