I propose that once I have received my radio-cadet card and I have had some time to test it that we talk it over on irc. It seems we are close to a result. BTW, multiple tuners for a radio device should definitely be possible and we do need to clarify the spec in that respect. There isn't actually any board with multiple tuners, although the Hauppauge PVR-500 (ivtv based) comes close as it has separate TV and radio tuners instead of the usual 'one tuner fits all' approach. Regards, Hans On Sun May 20 2012 13:50:52 Hans de Goede wrote: > Hi, > > On 05/20/2012 12:23 PM, Hans Verkuil wrote: > > Hi Hans, > > > > I'm CC-ing Manjunatha Halli as well as due to his work on adding support > > for the weather band: > > > > http://www.spinics.net/lists/kernel/msg1340986.html > > > > The question is whether the weather band and the AM band should be treated > > the same. The wl128x will implement the weather band as part of the single > > tuner, the cadet currently implements the AM band as a separate tuner. > > > > Given the fact that the wl128x and the radioSHARK both have only one > > physical tuner (and that's almost certainly the case for the cadet radio > > as well) I am not so sure whether we should handle this as two tuners. The > > approach taken for the wl128x seems at first glance a better solution. > > > > However, when I have my cadet card I'd like to experiment with it first and > > see what the best approach is to solve this problem. > > I agree that since it is a single tuner, it makes sense to treat is as such :) > The problem is that it has very distinctive properties depending on whether > it is operating in AM or FM mode, ie the bands are: 87.5 - 108 Mhz and > 530 - 1710 kHz. Now we can just represent that as the tuner being capable to > tune from 530 kHz - 108 Mhz but that won't result in a good UI experience > in userspace at all. > > I still agree that since it is a single tuner, it makes sense to treat is as such, > and since there are no overlapping frequencies, treating it as one tuner is > not a problem for most of the API, the only trouble some part really is > G_TUNER, since it cannot deal with having multiple frequency ranges, nor > with different properties per frequency range. > > We could introduce a subidx concept, and a related > tuner capability. Add if that capability is present, then userspace > can query different frequency ranges and there separate properties > by calling g_tuner with the same index but a different subidx until > it returns -EINVAL. > > We can use one of the reserved fields for the subidx, or ... > > We could store the subidx in the higher 16 bits of index, since index > is way larger then we need anyways, and this also avoids the > theoretical problems with apps not clearing the reserved fields we could > use for a proper subidx again. > > I personally prefer just using a separate field for it. > > > > > > On Sat May 19 2012 20:20:57 Hans de Goede wrote: > >> Hi Hans et all, > >> > >> Currently the V4L2 API does not allow for radio devices with more then 1 tuner, > >> which is a bit of a historical oversight, since many radio devices have 2 > >> tuners/demodulators 1 for FM and one for AM. Trying to model this as 1 tuner > >> really does not work well, as they have 2 completely separate frequency bands > >> they handle, as well as different properties (the FM part usually is stereo > >> capable, the AM part is not). > >> > >> It is important to realize here that usually the AM/FM tuners are part > >> of 1 chip, and often have only 1 frequency register which is used in > >> both AM/FM modes. IOW it more or less is one tuner, but with 2 modes, > >> and from a V4L2 API pov these modes are best modeled as 2 tuners. > >> This is at least true for the radio-cadet card and the tea575x, > >> which are the only 2 AM capable radio devices we currently know about. > >> > >> Currently the V4L2 spec says the following on this subject: > >> http://linuxtv.org/downloads/v4l-dvb-apis/tuner.html > >> "Radio devices have exactly one tuner with index zero, no video inputs." > >> > >> This text can easily be changed into allowing multiple tuners, without > >> any API change from the app pov, existing apps will be limited to > >> accessing just the first tuner though (probably best to always > >> make this the FM one). > > > > I agree. This text should change. > > Well if we go with the actually it is a single tuner concept above it > does not need to change, and we avoid the problems below... > > >> > >> http://linuxtv.org/downloads/v4l-dvb-apis/vidioc-g-tuner.html > >> "... call the VIDIOC_S_TUNER ioctl. This will not change the current tuner, > >> which is determined by the current video input." > >> > >> This is a problem, video devices when they have multiple tuners often > >> do so with the purpose of being able to watch/record multiple channels > >> at the same time, and thus multiple tuners are usually connected to > >> different inputs / frame-grabbers, and the input<-> tuner mapping done > >> for video devices makes sense there. > >> > >> As the spec states, radio devices have no video inputs, so > >> VIDIOC_S_INPUT cannot be used on them. Which means we need another > >> way to get/set the active tuner (the tuner mode) for a radio device. > > > > Correct. The spec contradicts itself here for radio devices and that needs > > to be solved. > > Only if we assume there can be more then 1 tuner on a radio dev. > > >> Lets look at the getting of the active tuner first. We cannot use > >> VIDIOC_G_TUNER for this, since this is used to enumerate tuners, > >> so it should return info on the tuner with the specified index, > >> rather then the active tuner. > > > > I agree. > > > >> VIDEOC_G_FREQUENCY otoh looks like a good candidate to use for this, > >> for radio devices we can simply ignore the passed in tuner field, > >> and instead return the active tuner and the current frequency. > >> This means there will be no way to get the frequency for the non > >> active tuner (mode), this is fine, since the non active tuner > >> does not have a (valid) frequency anyways. > > > > This would mean that the spec changes for this ioctl. I'm not certain I like > > that. > > I understand, note that this is what the cadet driver is currently > doing, which is were I got my inspiration from :) > > >> If we choose for VIDIOC_G_FREQUENCY to always return info on the > >> active tuner it makes sense to use VIDIOC_S_FREQUENCY to select > >> the active tuner. So for radio devices it will not only change > >> the currently tuned frequency for the indicated tuner, but if > >> the indicated tuner was not the active tuner it will make it the > >> active tuner. > > > > Ack. > > > >> Which leaves the question of what to do with VIDIOC_S_HW_FREQ_SEEK, > >> since VIDIOC_S_HW_FREQ_SEEK needs a valid begin frequency as a pre > >> condition, and the frequency ranges differ between different > >> tuners it makes sense to only allow VIDIOC_S_HW_FREQ_SEEK on > >> the active tuner. > > > > I see S_HW_FREQ_SEEK as an extended variation on S_FREQUENCY, so I > > believe calling S_HW_FREQ_SEEK should also change the current tuner. > > Ok, this does mean caching the last set freq for AM/FM as S_FREQ does > not provide a point where to start searching ... > > >> So this leaves one last problem, what to > >> return from VIDIOC_S_HW_FREQ_SEEK if it tries to seek for > >> a non active tuner. I'm tending towards saying -EBUSY, since some > >> parts of the tuners are shared, so the non active tuner cannot > >> seek because those shared parts are otherwise used. > > > > *If* we decide that S_HW_FREQ_SEEK cannot change the current tuner, then > > -EBUSY would be a good error code. > > > > The problem is really that you are trying to use G_FREQUENCY to figure out > > what the active tuner is. That requires an API change (the tuner field now > > only contains the current active tuner instead of the tuner whose frequency > > is requested), and because of that API change you have to change S_HW_FREQ_SEEK > > as well. > > Right (more or less) > > > In my view struct v4l2_tuner should be enhanced allowing it to return a > > flag or something similar that tells the application whether the given tuner > > index is the active index. > > > > Adding a flags field might do it, or perhaps (a bit hackish though) adding a > > V4L2_TUNER_SUB_ACTIVE flag. > > > > All this is independent of the question whether we should model the AM and > > weather bands as one or multiple tuners. Strictly speaking I think this would > > depend on whether there is only one tuner or if there are two (or more) > > independent tuners. In the second case I think modelling this using two tuners > > is the right approach. In the first case keeping to one tuner seems better, but > > it leads to the question how to query the frequency range of each band. > > Right, the more I think about this, the more I tend towards treating it as a > single tuner (which at least for the tea5757 based devices, as well as for > the cadet is true). Which indeed makes the question how do we query the > different bands (and the caps per band) > > > I like much of the work done on frequency bands in Manjunatha's patches (I did > > help with that, so I'm biased :-) ), but getting the frequency ranges for each > > band wasn't addressed there. > > > > I would propose that we add a new capability: > > > > V4L2_TUNER_CAP_BANDS 0x1000 > > > > If set, then frequency bands are supported and the correct band capabilities > > from Manjunatha's patch series are set. In addition a new band field is added > > to v4l2_tuner. It is unused if V4L2_TUNER_CAP_BANDS isn't set, otherwise the > > application can set it to the band whose frequency range and capabilities it > > wants to know. > > Sounds like my subidx from above, your name is better though, +1 > > > If band == 0, or if an invalid band is passed, then the driver fills in the > > current band and frequency range and capabilities. > > Smart, this will help with old app compatiblity +1 again :) > > > If non-zero, then the frequency range and capabilities of the given band are > > returned. Fields like rxsubchans and audmode always return values for the > > current selected frequency. Hmm, this doesn't feel right. Perhaps this should > > only return correct values if the current frequency falls within the given > > band, and otherwise these fields are set to 0. > > How about rxsubchans and audmode only being valid when band == 0, iow when > querying the current active band? That seems the right thing to do to me, > as apps either want to find out info about available bands, or about the > current mode and things like currently active audmode only makes sense for > the current mode. Same for signal BTW. > > > Since S_TUNER is an IOW ioctl, none of this applies to S_TUNER. > > Right. > > > Regards, > > Hans > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
