v4l2 ioctls

v4l2 ioctls

[Shuah Khan]

Hi Mauro/Hans,

I am working on adding sharing construct to dvb-core and v4l2-core.
In the case of dvb I have clean start and stop points to acquire the
tuner and release it. Tuner is acquired from dvb_frontend_start() and
released from dvb_frontend_thread() when thread exits. This works very
well.

The problem with analog case is there are no clear entry and exit
points. Instead of changing ioctls, it will be cleaner to change
the main ioctl entry routine __video_do_ioctl(). Is there an easy
way to tell which ioctls are query only and which are set?

So far I changed the following to check check for tuner token
before they invoke v4l2_ioctl_ops:

v4l_g_tuner()
v4l_s_tuner()
v4l_s_modulator()
v4l_s_frequency()
v4l_s_hw_freq_seek()

This isn't enough looks like, since I see tuner_s_std() getting
invoked and cutting off the dvb stream. I am currently releasing
the tuner from v4l2_fh_exit(), but I don't think that is a good
idea since all these ioctls are independent control paths. Each
ioctl might have to acquire and release it at the end. More on
this below.

For example, xawtv makes several ioctls before it even touches the
tuner to set frequency and starting the stream. What I am looking
for is an ioctl that would signal the intent to hold the tuner.
Is that possible?

The question is can we identify a clean start and stop points
for analog case for tuner ownership??

Would it make sense to treat all these ioctls as independent and
make them acquire and release lock or hold the tuner in shared
mode? Shared doesn't really make sense to me since two user-space
analog apps can interfere with each other.

I am trying avoid changing tuner-core and if at all possible.

I can send the code I have now for review if you like. I have the
locking construct in a good state at the moment. dvb is in good
shape.

-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

Re: v4l2 ioctls

[Hans Verkuil]

On 09/12/2014 03:26 AM, Shuah Khan wrote:
> Hi Mauro/Hans,
> 
> I am working on adding sharing construct to dvb-core and v4l2-core.
> In the case of dvb I have clean start and stop points to acquire the
> tuner and release it. Tuner is acquired from dvb_frontend_start() and
> released from dvb_frontend_thread() when thread exits. This works very
> well.
> 
> The problem with analog case is there are no clear entry and exit
> points. Instead of changing ioctls, it will be cleaner to change
> the main ioctl entry routine __video_do_ioctl(). Is there an easy
> way to tell which ioctls are query only and which are set?
> 
> So far I changed the following to check check for tuner token
> before they invoke v4l2_ioctl_ops:
> 
> v4l_g_tuner()

G_TUNER should work, even if the tuner is in a different mode. See my
slides on that topic:

http://hverkuil.home.xs4all.nl/presentations/ambiguities2.odp

> v4l_s_tuner()
> v4l_s_modulator()
> v4l_s_frequency()
> v4l_s_hw_freq_seek()

Other ioctls that should claim the tuner are:

S_STD
S_INPUT
STREAMON
QUERYSTD (depends on the hardware)

Strictly speaking these ioctls only need to claim the tuner if
they capture from the tuner input, but I think in most cases you aren't
able to use a radio tuner at the same time as capturing from an S-Video
or Composite input. Usually due to the audio muxing part that switches
to a line-in jack when you start capturing video.

Once an application takes ownership of a tuner (and multiple apps can
own a tuner as long as they all want the same tuner mode), then that
application stays owner for as long as the filehandle remains open.

A tuner owner can switch tuner mode as long as there are no other owners.

And opening a radio device should take tuner ownership immediately.
Although, as I mentioned before, I think we should try to fix radio
applications so that this is no longer necessary. It's very ugly
behavior even though it is part of the V4L2 spec.

> 
> This isn't enough looks like, since I see tuner_s_std() getting
> invoked and cutting off the dvb stream.

You are right, I forgot about those ioctls. Calling S_STD, S_INPUT or
STREAMON clearly indicates that you want to switch to TV mode.

> I am currently releasing
> the tuner from v4l2_fh_exit(), but I don't think that is a good
> idea since all these ioctls are independent control paths. Each
> ioctl might have to acquire and release it at the end. More on
> this below.
> 
> For example, xawtv makes several ioctls before it even touches the
> tuner to set frequency and starting the stream. What I am looking
> for is an ioctl that would signal the intent to hold the tuner.
> Is that possible?
> 
> The question is can we identify a clean start and stop points
> for analog case for tuner ownership??

The clean start points are the ioctls listed above. The stop point is
when the filehandle is closed.

> 
> Would it make sense to treat all these ioctls as independent and
> make them acquire and release lock or hold the tuner in shared
> mode?

I don't follow what you mean.

> Shared doesn't really make sense to me since two user-space
> analog apps can interfere with each other.

This is allowed by the API. If you want to prevent other apps from
making changes, then an application should raise its priority using
S_PRIORITY. It's quite often very handy to have one application to
do the streaming and another application to switch channels/inputs.

> 
> I am trying avoid changing tuner-core and if at all possible.
> 
> I can send the code I have now for review if you like. I have the
> locking construct in a good state at the moment. dvb is in good
> shape.

I'm happy to look at it.

Regards,

	Hans

Re: v4l2 ioctls

[Mauro Carvalho Chehab]

Hi Shuah,

See my comments below.

Regards,
Mauro

Em Fri, 12 Sep 2014 10:07:55 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 09/12/2014 03:26 AM, Shuah Khan wrote:
> > Hi Mauro/Hans,
> > 
> > I am working on adding sharing construct to dvb-core and v4l2-core.
> > In the case of dvb I have clean start and stop points to acquire the
> > tuner and release it. Tuner is acquired from dvb_frontend_start() and
> > released from dvb_frontend_thread() when thread exits. This works very
> > well.
> > 
> > The problem with analog case is there are no clear entry and exit
> > points. Instead of changing ioctls, it will be cleaner to change
> > the main ioctl entry routine __video_do_ioctl(). Is there an easy
> > way to tell which ioctls are query only and which are set?
> > 
> > So far I changed the following to check check for tuner token
> > before they invoke v4l2_ioctl_ops:
> > 
> > v4l_g_tuner()
> 
> G_TUNER should work, even if the tuner is in a different mode. See my
> slides on that topic:
> 
> http://hverkuil.home.xs4all.nl/presentations/ambiguities2.odp
> 
> > v4l_s_tuner()
> > v4l_s_modulator()
> > v4l_s_frequency()
> > v4l_s_hw_freq_seek()
> 
> Other ioctls that should claim the tuner are:
> 
> S_STD
> S_INPUT
> STREAMON

> QUERYSTD (depends on the hardware)

I have doubts about this one. I don't think that just doing a
QUERYSTD is enough to keep the tuner owned. perhaps the logic
here should be, instead:

	- take ownership (returning error if DVB mode is active)
	- Run the query
	- release ownership

> 
> Strictly speaking these ioctls only need to claim the tuner if
> they capture from the tuner input, but I think in most cases you aren't
> able to use a radio tuner at the same time as capturing from an S-Video
> or Composite input. Usually due to the audio muxing part that switches
> to a line-in jack when you start capturing video.
> 
> Once an application takes ownership of a tuner (and multiple apps can
> own a tuner as long as they all want the same tuner mode), then that
> application stays owner for as long as the filehandle remains open.
> 
> A tuner owner can switch tuner mode as long as there are no other owners.
> 
> And opening a radio device should take tuner ownership immediately.

I don't agree here. While S_TUNER is not called at a radio device,
there's no problem. So, if one just opens the radio device, calls G_TUNER
and does nothing else, there's no need to take tuner ownership.

> Although, as I mentioned before, I think we should try to fix radio
> applications so that this is no longer necessary. It's very ugly
> behavior even though it is part of the V4L2 spec.

Yeah, the behavior of open radio, call S_TUNER and close, keeping
using the radio is a ugly behavior, but before we touch Kernel,
applications need to be fixed.

> > 
> > This isn't enough looks like, since I see tuner_s_std() getting
> > invoked and cutting off the dvb stream.
> 
> You are right, I forgot about those ioctls. Calling S_STD, S_INPUT or
> STREAMON clearly indicates that you want to switch to TV mode.
> 
> > I am currently releasing
> > the tuner from v4l2_fh_exit(), but I don't think that is a good
> > idea since all these ioctls are independent control paths. Each
> > ioctl might have to acquire and release it at the end. More on
> > this below.
> > 
> > For example, xawtv makes several ioctls before it even touches the
> > tuner to set frequency and starting the stream. What I am looking
> > for is an ioctl that would signal the intent to hold the tuner.
> > Is that possible?
> > 
> > The question is can we identify a clean start and stop points
> > for analog case for tuner ownership??
> 
> The clean start points are the ioctls listed above. The stop point is
> when the filehandle is closed.
> 
> > 
> > Would it make sense to treat all these ioctls as independent and
> > make them acquire and release lock or hold the tuner in shared
> > mode?

For some, it might make sense to not keep the lock holded, like 
on QUERYSTD, but for things like S_STD, S_INPUT, etc, I think that
the best is to hold the tuner lock.

> I don't follow what you mean.
> 
> > Shared doesn't really make sense to me since two user-space
> > analog apps can interfere with each other.
> 
> This is allowed by the API. If you want to prevent other apps from
> making changes, then an application should raise its priority using
> S_PRIORITY. It's quite often very handy to have one application to
> do the streaming and another application to switch channels/inputs.
> 
> > 
> > I am trying avoid changing tuner-core and if at all possible.
> > 
> > I can send the code I have now for review if you like. I have the
> > locking construct in a good state at the moment. dvb is in good
> > shape.
> 
> I'm happy to look at it.
> 
> Regards,
> 
> 	Hans
> 

Re: v4l2 ioctls

[Shuah Khan]

Mauro/Hans,

Thanks for both for your replies. I finally have it working with
the following:

S_INPUT
S_OUTPUT
S_MODULATOR
S_TUNER
S_STD
S_FREQUENCY
S_HW_FREQ_SEEK
S_FMT
 - get tuner in shared mode and hold it
 - i.e return with tuner held

STREAMON
 - get tuner in shared mode and hold it
 - i.e return with tuner held
STREAMOFF
 - put tuner (get is done in STREAMON)

QUERYSTD
G_TUNER (au0828 does tuner init in its g_tuner ops)
 - get tuner in shared mode and hold it
 - service request
 - put tuner

With these changes now I have digital stream not get
disrupted as soon as xawtv starts. I am working through
issues related to unbalanced nature of tuner holds in
analog mode.

-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

Re: v4l2 ioctls

[Hans Verkuil]

On 09/13/2014 02:37 AM, Shuah Khan wrote:
> Mauro/Hans,
> 
> Thanks for both for your replies. I finally have it working with
> the following:
> 
> S_INPUT
> S_OUTPUT

S_OUTPUT is not necessary. It will never be used in combination with a
modulator since we don't support TV modulators at all.

> S_MODULATOR

I think this is unnecessary as well: we only have radio modulators and
those are always stand-alone drivers.

> S_TUNER
> S_STD
> S_FREQUENCY
> S_HW_FREQ_SEEK
> S_FMT
>  - get tuner in shared mode and hold it
>  - i.e return with tuner held
> 
> STREAMON
>  - get tuner in shared mode and hold it
>  - i.e return with tuner held
> STREAMOFF
>  - put tuner (get is done in STREAMON)

I wouldn't do this. Once you start streaming you hold the tuner and it
isn't released until the filehandle closes. The V4L2 API doesn't have
an explicit 'release tuner' ioctl.

> 
> QUERYSTD
> G_TUNER (au0828 does tuner init in its g_tuner ops)
>  - get tuner in shared mode and hold it

Note that G_TUNER should still work if it can't get hold of the tuner.
I.e., it should never return an error.

>  - service request
>  - put tuner
> 
> With these changes now I have digital stream not get
> disrupted as soon as xawtv starts. I am working through
> issues related to unbalanced nature of tuner holds in
> analog mode.

Nice!

Regards,

	Hans

Re: v4l2 ioctls

[Mauro Carvalho Chehab]

Hi Shuah,

Em Fri, 12 Sep 2014 18:37:13 -0600
Shuah Khan <shuahkh@osg.samsung.com> escreveu:

> Mauro/Hans,
> 
> Thanks for both for your replies. I finally have it working with
> the following:

One additional info: While in DVB mode, opening the device in
readonly mode should not take the tuner locking.

If you need/want to test it, please use:
	$ dvb-fe-tool --femon

I implemented this functionality this weekend, so you'll need
to update your v4l-utils tool to be able to test it.

> 
> S_INPUT
> S_OUTPUT
> S_MODULATOR
> S_TUNER
> S_STD
> S_FREQUENCY
> S_HW_FREQ_SEEK
> S_FMT
>  - get tuner in shared mode and hold it
>  - i.e return with tuner held
> 
> STREAMON
>  - get tuner in shared mode and hold it
>  - i.e return with tuner held
> STREAMOFF
>  - put tuner (get is done in STREAMON)
> 
> QUERYSTD
> G_TUNER (au0828 does tuner init in its g_tuner ops)

As this is something specific for some devices, it is probably better
to implement the locking for G_TUNER inside the driver.

>  - get tuner in shared mode and hold it
>  - service request
>  - put tuner
> 


> With these changes now I have digital stream not get
> disrupted as soon as xawtv starts. I am working through
> issues related to unbalanced nature of tuner holds in
> analog mode.
> 
> -- Shuah
> 
Regards,
Mauro

Re: v4l2 ioctls

[Shuah Khan]

On 09/15/2014 05:54 AM, Mauro Carvalho Chehab wrote:
> Hi Shuah,
> 
> Em Fri, 12 Sep 2014 18:37:13 -0600
> Shuah Khan <shuahkh@osg.samsung.com> escreveu:
> 
>> Mauro/Hans,
>>
>> Thanks for both for your replies. I finally have it working with
>> the following:
> 
> One additional info: While in DVB mode, opening the device in
> readonly mode should not take the tuner locking.

That's what the code does for dvb. It gets the tuner lock in
dvb_frontend_start() which is called from dvb_frontend_open()
when dvb is opened in R/W mode.

> 
> If you need/want to test it, please use:
> 	$ dvb-fe-tool --femon
> 
> I implemented this functionality this weekend, so you'll need
> to update your v4l-utils tool to be able to test it.
> 

ok - I will update v4l-utils on my system.

-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

Re: v4l2 ioctls

[Mauro Carvalho Chehab]

Em Mon, 15 Sep 2014 17:15:52 -0600
Shuah Khan <shuahkh@osg.samsung.com> escreveu:

> On 09/15/2014 05:54 AM, Mauro Carvalho Chehab wrote:
> > Hi Shuah,
> > 
> > Em Fri, 12 Sep 2014 18:37:13 -0600
> > Shuah Khan <shuahkh@osg.samsung.com> escreveu:
> > 
> >> Mauro/Hans,
> >>
> >> Thanks for both for your replies. I finally have it working with
> >> the following:
> > 
> > One additional info: While in DVB mode, opening the device in
> > readonly mode should not take the tuner locking.
> 
> That's what the code does for dvb. It gets the tuner lock in
> dvb_frontend_start() which is called from dvb_frontend_open()
> when dvb is opened in R/W mode.

Yeah, I think that the FE kthread is only started in R/W mode, 
but it doesn't hurt to double-check it and to do some tests to
avoid regressions.

> > 
> > If you need/want to test it, please use:
> > 	$ dvb-fe-tool --femon
> > 
> > I implemented this functionality this weekend, so you'll need
> > to update your v4l-utils tool to be able to test it.
> > 
> 
> ok - I will update v4l-utils on my system.
> 
> -- Shuah
> 
> 

Re: v4l2 ioctls

[Shuah Khan]

On 09/15/2014 05:53 PM, Mauro Carvalho Chehab wrote:
> Em Mon, 15 Sep 2014 17:15:52 -0600
> Shuah Khan <shuahkh@osg.samsung.com> escreveu:
> 
>> On 09/15/2014 05:54 AM, Mauro Carvalho Chehab wrote:
>>> Hi Shuah,
>>>
>>> Em Fri, 12 Sep 2014 18:37:13 -0600
>>> Shuah Khan <shuahkh@osg.samsung.com> escreveu:
>>>
>>>> Mauro/Hans,
>>>>
>>>> Thanks for both for your replies. I finally have it working with
>>>> the following:
>>>
>>> One additional info: While in DVB mode, opening the device in
>>> readonly mode should not take the tuner locking.
>>
>> That's what the code does for dvb. It gets the tuner lock in
>> dvb_frontend_start() which is called from dvb_frontend_open()
>> when dvb is opened in R/W mode.
> 
> Yeah, I think that the FE kthread is only started in R/W mode, 
> but it doesn't hurt to double-check it and to do some tests to
> avoid regressions.

Here is the code snippet:

       if ((file->f_flags & O_ACCMODE) != O_RDONLY) {
                /* normal tune mode when opened R/W */
                fepriv->tune_mode_flags &= ~FE_TUNE_MODE_ONESHOT;
                fepriv->tone = -1;
                fepriv->voltage = -1;

                ret = dvb_frontend_start (fe);
                if (ret)
                        goto err2;

                /*  empty event queue */
                fepriv->events.eventr = fepriv->events.eventw = 0;
        }


I can do some testing to make sure

-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978