v4l2 ioctls

DISCUSSION Shuah Khan 3 participants 9 messages Sep 12, 2014 – Sep 16, 2014 Media (V4L/DVB)

Share & tools

Shuah Khan

author

2014-09-12 01:35 UTC #0 lore raw

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

Signature · 4 lines

Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
[email protected] | (970) 217-8978

Hans Verkuil

2014-09-12 08:08 UTC #1 lore raw

Quoted 13 lines · 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?

Quoted 4 lines

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

Quoted 4 lines

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.

Quoted 3 lines

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.

Quoted 6 lines

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.

Quoted 7 lines

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.

Quoted 4 lines

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.

Quoted 2 lines

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.

Quoted 6 lines

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

Mauro Carvalho Chehab

2014-09-12 15:19 UTC #2 lore raw

Hi Shuah,

See my comments below.

Regards,
Mauro

Em Fri, 12 Sep 2014 10:07:55 +0200
Hans Verkuil <[email protected]> escreveu:

Quoted 29 lines · Shuah Khan

Quoted 13 lines
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?
Quoted 4 lines
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
Quoted 4 lines
v4l_s_tuner()
v4l_s_modulator()
v4l_s_frequency()
v4l_s_hw_freq_seek()

Quoted 7 lines

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

Quoted 7 lines

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.

Quoted 7 lines

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.

Quoted 3 lines

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.

Quoted 18 lines

Quoted 3 lines
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.
Quoted 11 lines
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?

Quoted 10 lines

Quoted 2 lines
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.
Quoted 4 lines
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.

Quoted 23 lines

I don't follow what you mean.
Quoted 2 lines
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.
Quoted 6 lines
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

Shuah Khan

author

2014-09-13 00:37 UTC #3 lore raw

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

Signature · 4 lines

Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
[email protected] | (970) 217-8978

Hans Verkuil

2014-09-13 08:49 UTC #4 lore raw

Quoted 7 lines · Shuah Khan

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.

Quoted 1 line

S_MODULATOR

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

Quoted 8 lines

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

Quoted 5 lines

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.

Quoted 4 lines

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.

Quoted 7 lines

- 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

Mauro Carvalho Chehab

2014-09-15 11:55 UTC #5 lore raw

In reply to Shuah Khan

Hi Shuah,

Em Fri, 12 Sep 2014 18:37:13 -0600
Shuah Khan <[email protected]> escreveu:

Quoted 4 lines

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.

Quoted 12 lines

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

Quoted 8 lines

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.

Quoted 6 lines

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

Quoted 7 lines

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

Shuah Khan

author

2014-09-15 23:15 UTC #6 lore raw

Quoted 5 lines · Mauro Carvalho Chehab

Hi Shuah,
Em Fri, 12 Sep 2014 18:37:13 -0600
Shuah Khan <[email protected]> escreveu:

Quoted 7 lines

Quoted 4 lines
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.

Quoted 7 lines

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

Signature · 4 lines

Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
[email protected] | (970) 217-8978

Mauro Carvalho Chehab

2014-09-15 23:53 UTC #7 lore raw

Em Mon, 15 Sep 2014 17:15:52 -0600
Shuah Khan <[email protected]> escreveu:

Quoted 11 lines · Mauro Carvalho Chehab

Quoted 10 lines
Hi Shuah,
Em Fri, 12 Sep 2014 18:37:13 -0600
Shuah Khan <[email protected]> escreveu:
Quoted 4 lines
Mauro/Hans,
Thanks for both for your replies. I finally have it working with the following:

Quoted 6 lines

Quoted 2 lines
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.

Quoted 13 lines

Quoted 7 lines
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

author

2014-09-16 00:23 UTC #8 lore raw

Quoted 17 lines · Mauro Carvalho Chehab

Em Mon, 15 Sep 2014 17:15:52 -0600
Shuah Khan <[email protected]> escreveu:
Quoted 14 lines · Mauro Carvalho Chehab
Quoted 12 lines
Hi Shuah,
Em Fri, 12 Sep 2014 18:37:13 -0600
Shuah Khan <[email protected]> escreveu:
Quoted 4 lines
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.

Quoted 7 lines

Quoted 3 lines
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

Signature · 4 lines

Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
[email protected] | (970) 217-8978