All of lore.kernel.org
 help / color / mirror / Atom feed
* RFC: Core + Radio profile
@ 2012-08-22  9:40 Hans Verkuil
  2012-08-22  9:47 ` Hans de Goede
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Hans Verkuil @ 2012-08-22  9:40 UTC (permalink / raw)
  To: linux-media

Hi all!

The v4l2-compliance utility checks whether device drivers follows the V4L2
specification. What is missing are 'profiles' detailing what device drivers
should and shouldn't implement for particular device classes.

This has been discussed several times, but until now no attempt was made to
actually write this down.

This RFC is my attempt to start this process by describing three profiles:
the core profile that all drivers must adhere to, a profile for a radio
receiver and a profile for a radio transmitter.

Missing in this RFC is a description of how tuner ownership is handled if a
tuner is shared between a radio and video driver. This will be discussed
during the workshop next week.

I am not certain where these profile descriptions should go. Should we add
this to DocBook, or describe it in Documentation/video4linux? Or perhaps
somehow add it to v4l2-compliance, since that tool effectively verifies
the profile.

Also note that the core profile description is more strict than the spec. For
example, G/S_PRIORITY are currently optional, but I feel that all new drivers
should implement this, especially since it is very easy to add provided you
use struct v4l2_fh. Similar with respect to the control requirements which
assume you use the control framework.

Note that these profiles are primarily meant for driver developers. The V4L2
specification is leading for application developers.

While writing down these profiles I noticed one thing that was very much
missing for radio devices: there is no capability bit to tell the application
whether there is an associated ALSA device or whether the audio goes through
a line-in/out. Personally I think that this should be fixed.

Comments are welcome!

Regards,

	Hans


Core Profile
============

All V4L2 device nodes must support the core profile.

VIDIOC_QUERYCAP must be supported and must set the device_caps field.
bus_info must be a unique string and must not be empty (pending result of
the upcoming workshop).

VIDIOC_G/S_PRIORITY must be supported.

If you have controls then both the VIDIOC_G/S_CTRL and the VIDIOC_G/S/TRY_EXT_CTRLS
ioctls must be supported, together with poll(), VIDIOC_DQEVENT and
VIDIOC_(UN)SUBSCRIBE_EVENT to handle control events. Use the control framework
to implement this.

VIDIOC_LOG_STATUS is optional, but recommended for complex drivers.

VIDIOC_DBG_G_CHIP_IDENT, VIDIOC_DBG_G/S_REGISTER are optional and are
primarily used to debug drivers.

Video, Radio and VBI nodes are for input or output only. The only exception
are video nodes that have the V4L2_CAP_VIDEO_M2M(_MPLANE) capability set.

Video nodes only control video, radio nodes only control radio and RDS, vbi
nodes only control VBI (raw and/or sliced).

Streaming I/O is not supported by radio nodes.

Radio Receiver Profile
======================

Radio devices have a tuner and (usually) a demodulator. The audio is either
send out to a line-out connector or uses ALSA to stream the audio.

It implements VIDIOC_G/S_TUNER, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS.

If hardware seek is supported, then VIDIOC_S_HW_FREQ_SEEK is implemented.

It does *not* implement VIDIOC_ENUM/G/S_INPUT or VIDIOC_ENUM/G/S_AUDIO.

If RDS_BLOCK_IO is supported, then read() and poll() are implemented as well.

If a mute control exists and the audio is output using a line-out, then the
audio must be muted on driver load and unload.

On driver load the frequency must be initialized to a valid frequency.

Note: There are a few drivers that use a radio (pvrusb2) or video (ivtv)
node to stream the audio. These are legacy exceptions to the rule.

FM Transmitter Profile
======================

FM transmitter devices have a transmitter and modulator. The audio is
transferred using ALSA or a line-in connector.

It implements VIDIOC_G/S_MODULATOR, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS.

It does *not* implement VIDIOC_ENUM/G/S_OUTPUT or VIDIOC_ENUM/G/S_AUDOUT.

If RDS_BLOCK_IO is supported, then write() and poll() are implemented
as well.

On driver load the frequency must be initialized to a valid frequency.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: RFC: Core + Radio profile
  2012-08-22  9:40 RFC: Core + Radio profile Hans Verkuil
@ 2012-08-22  9:47 ` Hans de Goede
  2012-08-22 10:11 ` Hans Verkuil
  2012-08-25  0:37 ` Andy Walls
  2 siblings, 0 replies; 14+ messages in thread
From: Hans de Goede @ 2012-08-22  9:47 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media

Hi,

On 08/22/2012 11:40 AM, Hans Verkuil wrote:
> Hi all!
>

<Snip>

> While writing down these profiles I noticed one thing that was very much
> missing for radio devices: there is no capability bit to tell the application
> whether there is an associated ALSA device or whether the audio goes through
> a line-in/out. Personally I think that this should be fixed.

The question with generic tuner drivers like the tea57XX series is do we always
know this ?

One could argue to proper way to find this out for applications is by looking
at the device topology, either through the media controller framework, or
through sysfs. This is for example what xawtv currently does. We need a better
library to handle this, which also hides from the user whether the media controller
or sysfs is used.

 > Comments are welcome!

Looks good!

Regards,

Hans

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: RFC: Core + Radio profile
  2012-08-22  9:40 RFC: Core + Radio profile Hans Verkuil
  2012-08-22  9:47 ` Hans de Goede
@ 2012-08-22 10:11 ` Hans Verkuil
  2012-08-22 13:42   ` Mauro Carvalho Chehab
  2012-08-25  0:37 ` Andy Walls
  2 siblings, 1 reply; 14+ messages in thread
From: Hans Verkuil @ 2012-08-22 10:11 UTC (permalink / raw)
  To: linux-media

I've added some more core profile requirements.

Regards,

	Hans

On Wed August 22 2012 11:40:25 Hans Verkuil wrote:
> Hi all!
> 
> The v4l2-compliance utility checks whether device drivers follows the V4L2
> specification. What is missing are 'profiles' detailing what device drivers
> should and shouldn't implement for particular device classes.
> 
> This has been discussed several times, but until now no attempt was made to
> actually write this down.
> 
> This RFC is my attempt to start this process by describing three profiles:
> the core profile that all drivers must adhere to, a profile for a radio
> receiver and a profile for a radio transmitter.
> 
> Missing in this RFC is a description of how tuner ownership is handled if a
> tuner is shared between a radio and video driver. This will be discussed
> during the workshop next week.
> 
> I am not certain where these profile descriptions should go. Should we add
> this to DocBook, or describe it in Documentation/video4linux? Or perhaps
> somehow add it to v4l2-compliance, since that tool effectively verifies
> the profile.
> 
> Also note that the core profile description is more strict than the spec. For
> example, G/S_PRIORITY are currently optional, but I feel that all new drivers
> should implement this, especially since it is very easy to add provided you
> use struct v4l2_fh. Similar with respect to the control requirements which
> assume you use the control framework.
> 
> Note that these profiles are primarily meant for driver developers. The V4L2
> specification is leading for application developers.
> 
> While writing down these profiles I noticed one thing that was very much
> missing for radio devices: there is no capability bit to tell the application
> whether there is an associated ALSA device or whether the audio goes through
> a line-in/out. Personally I think that this should be fixed.
> 
> Comments are welcome!
> 
> Regards,
> 
> 	Hans
> 
> 
> Core Profile
> ============
> 
> All V4L2 device nodes must support the core profile.
> 
> VIDIOC_QUERYCAP must be supported and must set the device_caps field.
> bus_info must be a unique string and must not be empty (pending result of
> the upcoming workshop).
> 
> VIDIOC_G/S_PRIORITY must be supported.
> 
> If you have controls then both the VIDIOC_G/S_CTRL and the VIDIOC_G/S/TRY_EXT_CTRLS
> ioctls must be supported, together with poll(), VIDIOC_DQEVENT and
> VIDIOC_(UN)SUBSCRIBE_EVENT to handle control events. Use the control framework
> to implement this.
> 
> VIDIOC_LOG_STATUS is optional, but recommended for complex drivers.
> 
> VIDIOC_DBG_G_CHIP_IDENT, VIDIOC_DBG_G/S_REGISTER are optional and are
> primarily used to debug drivers.
> 
> Video, Radio and VBI nodes are for input or output only. The only exception
> are video nodes that have the V4L2_CAP_VIDEO_M2M(_MPLANE) capability set.
> 
> Video nodes only control video, radio nodes only control radio and RDS, vbi
> nodes only control VBI (raw and/or sliced).
> 
> Streaming I/O is not supported by radio nodes.

It should be possible to open device nodes more than once. Just opening a
node and querying information from the driver should not change the state
of the driver.

The file handle that called VIDIOC_REQBUFS, VIDIOC_CREATE_BUFS, read/write
or select (for reading or writing) first is the only one that can do I/O.
Attempts by other file handles to call these ioctls/file operations will
get an EBUSY error.

This file handle remains the owner of the I/O until the file handle is
closed, or until VIDIOC_REQBUFS is called with count == 0.

> 
> Radio Receiver Profile
> ======================
> 
> Radio devices have a tuner and (usually) a demodulator. The audio is either
> send out to a line-out connector or uses ALSA to stream the audio.
> 
> It implements VIDIOC_G/S_TUNER, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS.
> 
> If hardware seek is supported, then VIDIOC_S_HW_FREQ_SEEK is implemented.
> 
> It does *not* implement VIDIOC_ENUM/G/S_INPUT or VIDIOC_ENUM/G/S_AUDIO.
> 
> If RDS_BLOCK_IO is supported, then read() and poll() are implemented as well.
> 
> If a mute control exists and the audio is output using a line-out, then the
> audio must be muted on driver load and unload.
> 
> On driver load the frequency must be initialized to a valid frequency.
> 
> Note: There are a few drivers that use a radio (pvrusb2) or video (ivtv)
> node to stream the audio. These are legacy exceptions to the rule.
> 
> FM Transmitter Profile
> ======================
> 
> FM transmitter devices have a transmitter and modulator. The audio is
> transferred using ALSA or a line-in connector.
> 
> It implements VIDIOC_G/S_MODULATOR, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS.
> 
> It does *not* implement VIDIOC_ENUM/G/S_OUTPUT or VIDIOC_ENUM/G/S_AUDOUT.
> 
> If RDS_BLOCK_IO is supported, then write() and poll() are implemented
> as well.
> 
> On driver load the frequency must be initialized to a valid frequency.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: RFC: Core + Radio profile
  2012-08-22 10:11 ` Hans Verkuil
@ 2012-08-22 13:42   ` Mauro Carvalho Chehab
  2012-08-22 15:19     ` Mike Isely
  2012-08-24 12:31     ` Hans Verkuil
  0 siblings, 2 replies; 14+ messages in thread
From: Mauro Carvalho Chehab @ 2012-08-22 13:42 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, Mike Isely, Andy Walls

Em 22-08-2012 07:11, Hans Verkuil escreveu:
> I've added some more core profile requirements.
> 
> Regards,
> 
> 	Hans
> 
> On Wed August 22 2012 11:40:25 Hans Verkuil wrote:
>> Hi all!
>>
>> The v4l2-compliance utility checks whether device drivers follows the V4L2
>> specification. What is missing are 'profiles' detailing what device drivers
>> should and shouldn't implement for particular device classes.
>>
>> This has been discussed several times, but until now no attempt was made to
>> actually write this down.
>>
>> This RFC is my attempt to start this process by describing three profiles:
>> the core profile that all drivers must adhere to, a profile for a radio
>> receiver and a profile for a radio transmitter.
>>
>> Missing in this RFC is a description of how tuner ownership is handled if a
>> tuner is shared between a radio and video driver. This will be discussed
>> during the workshop next week.

DVB x V4L2 tunership discussion is also needed. It also makes sense to 
document it somewhere.

>> I am not certain where these profile descriptions should go. Should we add
>> this to DocBook, or describe it in Documentation/video4linux? Or perhaps
>> somehow add it to v4l2-compliance, since that tool effectively verifies
>> the profile.

The strongest document we have, IMO, is the DocBook API one, as both Kernel
and userspace developers use it. As we want userspace apps to be aware and
benefit of the profiles, this should be there at DocBook.

>> Also note that the core profile description is more strict than the spec.

IMO, that's the right approach: The specs should define the several API
aspects; the profile should mandate what's optional and what's mandatory.

>> For example, G/S_PRIORITY are currently optional,

After putting the profiles there, we should remove "optional" tags elsewhere,
as the profiles section will tell what's mandatory and what is optional, as
different profiles require different mandatory arguments.

>> but I feel that all new drivers should implement this, especially since it
>> is very easy to add provided you use struct v4l2_fh. 

I agree on making G/S_PRIORITY mandatory.

>> Similar with respect to the control requirements which
>> assume you use the control framework.

IMO, we should split the internal API requirements (use the control framework)
from the external API ones, as they're addressed to different audiences, e. g.
external API requirements are needed by all application developers, while
the internal ones are only for Kernel hackers.

The internal API requirements should be together with the internal API descriptions,
so Documentation/video4linux is probably the best place for them.

>> Note that these profiles are primarily meant for driver developers. The V4L2
>> specification is leading for application developers.

This is where we diverge ;) We need profiles primary for application developers,
for them to know what is expected to be there on any media driver of a certain
kind.

Ok, internal driver-developer profiles is also needed, in order for them to
use the right internal API's and internal core functions.

>> While writing down these profiles I noticed one thing that was very much
>> missing for radio devices: there is no capability bit to tell the application
>> whether there is an associated ALSA device or whether the audio goes through
>> a line-in/out. Personally I think that this should be fixed.

Yes, this is one bit that it is missing. Well, this is currently handled this
at xawtv3 "radio" aplication by using the libmedia_dev API there.

It likely makes sense, for a version 2 of the profiles (e. g. after we finish
merging the basic stuff there), to add a chapter at  the profiles section 
recommending the usage of the libraries provided by v4l-utils, with some 
description or code examples.

>>
>> Comments are welcome!
>>
>> Regards,
>>
>> 	Hans
>>
>>
>> Core Profile
>> ============
>>
>> All V4L2 device nodes must support the core profile.
>>
>> VIDIOC_QUERYCAP must be supported and must set the device_caps field.
>> bus_info must be a unique string and must not be empty (pending result of
>> the upcoming workshop).
>>
>> VIDIOC_G/S_PRIORITY must be supported.
>>
>> If you have controls then both the VIDIOC_G/S_CTRL and the VIDIOC_G/S/TRY_EXT_CTRLS
>> ioctls must be supported, together with poll(), VIDIOC_DQEVENT and
>> VIDIOC_(UN)SUBSCRIBE_EVENT to handle control events. 

>> Use the control framework to implement this.

Now looking at the concrete case, I don't see any troubles on adding it to
the V4L2 API, but writing it as:

	"Driver developers must use the control framework to implement this."

>> VIDIOC_LOG_STATUS is optional, but recommended for complex drivers.
>>
>> VIDIOC_DBG_G_CHIP_IDENT, VIDIOC_DBG_G/S_REGISTER are optional and are
>> primarily used to debug drivers.

The above ones require a note for application developers:

	"Applications must not use VIDIOC_LOG_STATUS during its normal behaviour;
	 this is reserved for driver's debug/test.

	 The usage of VIDIOC_DBG_G_CHIP_IDENT and VIDIOC_DBG_G/S_REGISTER is solely 
	 for driver's debug. Applications should never use it."

>> Video, Radio and VBI nodes are for input or output only. 

Hmm... didn't look clear to me. I would change the first phase to something like:

	"Each video, radio or vbi node should be used by just one input or
	output streaming engine."

>> The only exception
>> are video nodes that have the V4L2_CAP_VIDEO_M2M(_MPLANE) capability set.

Again, not clear what it should be expected on a device with the M2M capabilities.

Better to write it like:

	"Video nodes with memory to memory capabilities (V4L2_CAP_VIDEO_M2M and
V4L2_CAP_VIDEO_M2M_MPLANE) must implement both one input streaming engine and
one output streaming engine at the same node."

>> Video nodes only control video, radio nodes only control radio and RDS, vbi
>> nodes only control VBI (raw and/or sliced).

I would use "must" word for everything mandatory at the profiles section (
and "may" for optional ones), like:

	"Video nodes must only control the video functionality.

	 Radio nodes must only control the radio functionality.

	 VBI nodes must only control the Vertical Blank Interface functionality. The
	 same VBI node must be used by raw VBI and sliced VBI api's, when both are
	 supported."

>>
>> Streaming I/O is not supported by radio nodes.

	Hmm... pvrusb2/ivtv? Ok, it makes sense to move it to use the alsa
mpeg API there. If we're enforcing it, we should deprecate the current way
there, and make it use ALSA.

(more comments about it below)

> It should be possible to open device nodes more than once. Just opening a
> node and querying information from the driver should not change the state
> of the driver.

Hmm... locking between DVB and V4L should likely be added here: e. g. is it
allowed to open both DVB and V4L nodes for the same tuner/streaming engine?

Currently, this is not allowed. There are also known issues when applications
change too fast between them (open/close V4L, then open/close dvb or vice-versa).

> The file handle that called VIDIOC_REQBUFS, VIDIOC_CREATE_BUFS, read/write
> or select (for reading or writing) first is the only one that can do I/O.
> Attempts by other file handles to call these ioctls/file operations will
> get an EBUSY error.
>
> This file handle remains the owner of the I/O until the file handle is
> closed, or until VIDIOC_REQBUFS is called with count == 0.

Please change it to use "must". Also, I think that doing s/called/calls/
is better.

It is likely better to say it as:

	The streaming engine ownership must be taken by the first handle that calls
	VIDIOC_REQBUFS, VIDIOC_CREATE_BUFS, read/write or select (for reading or writing).

	Only such file handler must do streaming I/O. Any attempts by other file handlers
	to do I/O must return EBUSY.

	The I/O streaming ownership must be released when the file handler is closed
	or when VIDIOC_REQBUFS is called with count equal to zero.

>>
>> Radio Receiver Profile
>> ======================
>>
>> Radio devices have a tuner and (usually) a demodulator. The audio is either
>> send out to a line-out connector or uses ALSA to stream the audio.

Hmm... are there anything mandatory here, or are you just defining what a radio is?
In the latter, the above text looks ok.

>>
>> It implements VIDIOC_G/S_TUNER, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS.

	In addition to the mandatory items at the core profile (I would add an hyperlink here),
	all drivers must implement VIDIOC_G/S_TUNER, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS.

>> If hardware seek is supported, then VIDIOC_S_HW_FREQ_SEEK is implemented.

	A radio device may implement VIDIOC_S_HW_FREQ_SEEK, when audio station seek is
	supported by radio.

>> It does *not* implement VIDIOC_ENUM/G/S_INPUT or VIDIOC_ENUM/G/S_AUDIO.

A radio node must *not* implement VIDIOC_ENUM/G/S_INPUT or VIDIOC_ENUM/G/S_AUDIO.

>> If RDS_BLOCK_IO is supported, then read() and poll() are implemented as well.

Radio drivers that support RDS must report RDS_BLOCK_IO. In this case, radio
nodes must implement read()/poll() syscalls.

>> If a mute control exists and the audio is output using a line-out, then the
>> audio must be muted on driver load and unload.
>>
>> On driver load the frequency must be initialized to a valid frequency.

This is not nice, and not always possible: devices with xc3028 tuner (and other
Xceive tuners) have firmwares for radio and for TV. Initializing for radio
makes TV uninitialized, and vice-versa. Also, some devices take up to 30 seconds
to load a firmware (tm6000).

I think you're likely trying to say that the initial frequency should be
a valid value. If so, please prepend it with a "driver developer's note".

>>
>> Note: There are a few drivers that use a radio (pvrusb2) or video (ivtv)
>> node to stream the audio. These are legacy exceptions to the rule.

What an application developer should do with that???

If this should not be supported anymore, then we need instead to either
fix the drivers that aren't compliant with the specs or move them to
staging, in order to let them to be either fixed or dropped, if none
cares enough to fix.

>>
>> FM Transmitter Profile
>> ======================
>>
>> FM transmitter devices have a transmitter and modulator. The audio is
>> transferred using ALSA or a line-in connector.

Hmm... are there anything mandatory here, or are you just defining what a radio is?
In the latter, the above text looks ok.

>>
>> It implements VIDIOC_G/S_MODULATOR, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS.
>>
>> It does *not* implement VIDIOC_ENUM/G/S_OUTPUT or VIDIOC_ENUM/G/S_AUDOUT.
>>
>> If RDS_BLOCK_IO is supported, then write() and poll() are implemented
>> as well.
>>
>> On driver load the frequency must be initialized to a valid frequency.

Same notes from the "radio" profile applies here.

--

That's said, I think that the RFC proposal is going on the right direction.
It is very good to see it moving forward!

Thanks!
Mauro

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: RFC: Core + Radio profile
  2012-08-22 13:42   ` Mauro Carvalho Chehab
@ 2012-08-22 15:19     ` Mike Isely
  2012-08-22 18:09       ` Mauro Carvalho Chehab
  2012-08-24 12:31     ` Hans Verkuil
  1 sibling, 1 reply; 14+ messages in thread
From: Mike Isely @ 2012-08-22 15:19 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: Hans Verkuil, linux-media, Andy Walls

On Wed, 22 Aug 2012, Mauro Carvalho Chehab wrote:

> Em 22-08-2012 07:11, Hans Verkuil escreveu:
> > I've added some more core profile requirements.
> 
> >>
> >> Streaming I/O is not supported by radio nodes.
> 
> 	Hmm... pvrusb2/ivtv? Ok, it makes sense to move it to use the alsa
> mpeg API there. If we're enforcing it, we should deprecate the current way
> there, and make it use ALSA.

I am unaware of any ALSA MPEG API.  It's entirely likely that this is 
because I haven't been paying attention.  Nevertheless, can you please 
point me at any documentation on this so I can get up to speed?

Currently the pvrusb2 driver does not attempt to perform any processing 
or filtering of the data stream, so radio data is just the same mpeg 
stream as video (but without any real embedded video data).  If I have 
to get into the business of processing the MPEG data in order to adhere 
to this proposal, then that will be a very big deal for this driver.

  -Mike

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: RFC: Core + Radio profile
  2012-08-22 15:19     ` Mike Isely
@ 2012-08-22 18:09       ` Mauro Carvalho Chehab
  2012-08-23 11:12         ` Andy Walls
  0 siblings, 1 reply; 14+ messages in thread
From: Mauro Carvalho Chehab @ 2012-08-22 18:09 UTC (permalink / raw)
  To: Mike Isely at pobox; +Cc: Mike Isely, Hans Verkuil, linux-media, Andy Walls

Em 22-08-2012 12:19, Mike Isely escreveu:
> On Wed, 22 Aug 2012, Mauro Carvalho Chehab wrote:
> 
>> Em 22-08-2012 07:11, Hans Verkuil escreveu:
>>> I've added some more core profile requirements.
>>
>>>>
>>>> Streaming I/O is not supported by radio nodes.
>>
>> 	Hmm... pvrusb2/ivtv? Ok, it makes sense to move it to use the alsa
>> mpeg API there. If we're enforcing it, we should deprecate the current way
>> there, and make it use ALSA.
> 
> I am unaware of any ALSA MPEG API.  It's entirely likely that this is 
> because I haven't been paying attention.  Nevertheless, can you please 
> point me at any documentation on this so I can get up to speed?


I don't know much about that. A grep at sound might help:

$ git grep -i mpeg sound/
sound/core/oss/pcm_oss.c: case AFMT_MPEG:         return SNDRV_PCM_FORMAT_MPEG;
sound/core/oss/pcm_oss.c: case SNDRV_PCM_FORMAT_MPEG:             return AFMT_MPEG;
sound/core/pcm.c: FORMAT(MPEG),
sound/core/pcm.c: case AFMT_MPEG:
sound/core/pcm.c:         return "MPEG";
sound/core/pcm_misc.c:    [SNDRV_PCM_FORMAT_MPEG] = {
sound/drivers/vx/vx_cmd.h:#define MASK_VALID_PIPE_MPEG_PARAM      0x000040
sound/drivers/vx/vx_cmd.h:#define MASK_SET_PIPE_MPEG_PARAM        0x000002
sound/drivers/vx/vx_cmd.h:#define P_PREPARE_FOR_MPEG3_MASK        
sound/drivers/vx/vx_core.c:       if (chip->audio_info & VX_AUDIO_INFO_MPEG1)
sound/drivers/vx/vx_core.c:               snd_iprintf(buffer, " mpeg1");
sound/drivers/vx/vx_core.c:       if (chip->audio_info & VX_AUDIO_INFO_MPEG2)
sound/drivers/vx/vx_core.c:               snd_iprintf(buffer, " mpeg2");
sound/pci/asihpi/asihpi.c:        -1,                     /* HPI_FORMAT_MPEG_L1              3 */
sound/pci/asihpi/asihpi.c:        SNDRV_PCM_FORMAT_MPEG,  /* HPI_FORMAT_MPEG_L2              4 */
sound/pci/asihpi/asihpi.c:        SNDRV_PCM_FORMAT_MPEG,  /* HPI_FORMAT_MPEG_L3              5 */
sound/pci/asihpi/hpi.h:/** MPEG-1 Layer-1. */
sound/pci/asihpi/hpi.h:   HPI_FORMAT_MPEG_L1 = 3,
sound/pci/asihpi/hpi.h:/** MPEG-1 Layer-2.
sound/pci/asihpi/hpi.h:Windows equivalent is WAVE_FORMAT_MPEG.
sound/pci/asihpi/hpi.h:   HPI_FORMAT_MPEG_L2 = 4,
sound/pci/asihpi/hpi.h:/** MPEG-1 Layer-3.
sound/pci/asihpi/hpi.h:Windows equivalent is WAVE_FORMAT_MPEG.
sound/pci/asihpi/hpi.h:   HPI_FORMAT_MPEG_L3 = 5,
sound/pci/asihpi/hpi.h:#define HPI_CAPABILITY_MPEG_LAYER3      (1)
sound/pci/asihpi/hpi.h:/** MPEG Ancillary Data modes
sound/pci/asihpi/hpi.h:enum HPI_MPEG_ANC_MODES {
sound/pci/asihpi/hpi.h:   /** the MPEG frames have energy information stored in them (5 bytes per stereo frame, 3 per mono) */
sound/pci/asihpi/hpi.h:   HPI_MPEG_ANC_HASENERGY = 0,
sound/pci/asihpi/hpi.h:   HPI_MPEG_ANC_RAW = 1
sound/pci/asihpi/hpi.h:enum HPI_ISTREAM_MPEG_ANC_ALIGNS {
sound/pci/asihpi/hpi.h:   HPI_MPEG_ANC_ALIGN_LEFT = 0,
sound/pci/asihpi/hpi.h:   HPI_MPEG_ANC_ALIGN_RIGHT = 1
sound/pci/asihpi/hpi.h:/** MPEG modes
sound/pci/asihpi/hpi.h:MPEG modes - can be used optionally for HPI_FormatCreate()
sound/pci/asihpi/hpi.h:Using any mode setting other than HPI_MPEG_MODE_DEFAULT
sound/pci/asihpi/hpi.h:enum HPI_MPEG_MODES {
sound/pci/asihpi/hpi.h:/** Causes the MPEG-1 Layer II bitstream to be recorded
sound/pci/asihpi/hpi.h:   HPI_MPEG_MODE_DEFAULT = 0,
sound/pci/asihpi/hpi.h:   HPI_MPEG_MODE_STEREO = 1,
sound/pci/asihpi/hpi.h:   HPI_MPEG_MODE_JOINTSTEREO = 2,
sound/pci/asihpi/hpi.h:   HPI_MPEG_MODE_DUALCHANNEL = 3
sound/pci/asihpi/hpi.h:/** maximum number of ancillary bytes per MPEG frame */
sound/pci/asihpi/hpi.h:   u32 bit_rate;             /**< for MPEG */
sound/pci/asihpi/hpi.h:   u16 format;       /**< HPI_FORMAT_PCM16, _MPEG etc. see #HPI_FORMATS. */
sound/pci/asihpi/hpi_internal.h:  u32 bit_rate; /**< for MPEG */
sound/pci/asihpi/hpi_internal.h:  u16 format; /**< HPI_FORMAT_PCM16, _MPEG etc. see \ref HPI_FORMATS. */
sound/pci/asihpi/hpifunc.c:       case HPI_FORMAT_MPEG_L1:
sound/pci/asihpi/hpifunc.c:       case HPI_FORMAT_MPEG_L2:
sound/pci/asihpi/hpifunc.c:       case HPI_FORMAT_MPEG_L3:
sound/pci/asihpi/hpifunc.c:       case HPI_FORMAT_MPEG_L1:
sound/pci/asihpi/hpifunc.c:       case HPI_FORMAT_MPEG_L2:
sound/pci/asihpi/hpifunc.c:       case HPI_FORMAT_MPEG_L3:
sound/pci/asihpi/hpifunc.c:       case HPI_FORMAT_MPEG_L2:
sound/pci/asihpi/hpifunc.c:                       && (attributes != HPI_MPEG_MODE_DEFAULT)) {
sound/pci/asihpi/hpifunc.c:                       attributes = HPI_MPEG_MODE_DEFAULT;
sound/pci/asihpi/hpifunc.c:               } else if (attributes > HPI_MPEG_MODE_DUALCHANNEL) {
sound/pci/asihpi/hpifunc.c:                       attributes = HPI_MPEG_MODE_DEFAULT;
sound/pci/asihpi/hpifunc.c:       case HPI_FORMAT_MPEG_L1:
sound/pci/asihpi/hpifunc.c:       case HPI_FORMAT_MPEG_L2:
sound/pci/asihpi/hpifunc.c:       case HPI_FORMAT_MPEG_L3:
sound/pci/bt87x.c: * (DVB cards use the audio function to transfer MPEG data) */
sound/pci/ens1370.c:#define   ES_MSFMTSEL         (1<<15)         /* MPEG serial data format; 0 = SONY, 1 = I2S */
sound/pci/ens1370.c:#define   ES_1370_M_SBB               (1<<14)         /* clock source for DAC - 0 = clock generator; 1 = MPEG clocks */
sound/pci/ens1370.c:#define   ES_1370_M_CB                (1<<9)          /* capture clock source; 0 = ADC; 1 = MPEG */
sound/pci/hda/hda_eld.c:  AUDIO_CODING_TYPE_MPEG1                 =  3,
sound/pci/hda/hda_eld.c:  AUDIO_CODING_TYPE_MPEG2                 =  5,
sound/pci/hda/hda_eld.c:  AUDIO_CODING_TYPE_MPEG_SURROUND         = 17,
sound/pci/hda/hda_eld.c:  AUDIO_CODING_XTYPE_MPEG_SURROUND        = 3,
sound/pci/hda/hda_eld.c:  /*  3 */ "MPEG1",
sound/pci/hda/hda_eld.c:  /*  5 */ "MPEG2",
sound/pci/hda/hda_eld.c:  /* 17 */ "MPEG Surround",
sound/pci/hda/hda_eld.c:  case AUDIO_CODING_TYPE_MPEG1:
sound/pci/hda/hda_eld.c:  case AUDIO_CODING_TYPE_MPEG2:
sound/pci/mixart/mixart_core.h:   CT_MPEG_L1,
sound/pci/mixart/mixart_core.h:   CT_MPEG_L2,
sound/pci/mixart/mixart_core.h:   CT_MPEG_L3,
sound/pci/mixart/mixart_core.h:   CT_MPEG_L3_LSF,
sound/pci/mixart/mixart_core.h:                   u32 mpeg_layer;
sound/pci/mixart/mixart_core.h:                   u32 mpeg_mode;
sound/pci/mixart/mixart_core.h:                   u32 mpeg_mode_extension;
sound/pci/mixart/mixart_core.h:                   u32 mpeg_pre_emphasis;
sound/pci/mixart/mixart_core.h:                   u32 mpeg_has_padding_bit;
sound/pci/mixart/mixart_core.h:                   u32 mpeg_has_crc;
sound/pci/mixart/mixart_core.h:                   u32 mpeg_has_extension;
sound/pci/mixart/mixart_core.h:                   u32 mpeg_is_original;
sound/pci/mixart/mixart_core.h:                   u32 mpeg_has_copyright;
sound/pci/mixart/mixart_core.h:           } mpeg_format_info;
sound/usb/format.c:       case UAC_FORMAT_TYPE_II_MPEG:
sound/usb/format.c:               fp->formats = SNDRV_PCM_FMTBIT_MPEG;
sound/usb/format.c:               snd_printd(KERN_INFO "%d:%u:%d : unknown format tag %#x is detected.  processed as MPEG.\n",
sound/usb/format.c:               fp->formats = SNDRV_PCM_FMTBIT_MPEG;


> 
> Currently the pvrusb2 driver does not attempt to perform any processing 
> or filtering of the data stream, so radio data is just the same mpeg 
> stream as video (but without any real embedded video data).  If I have 
> to get into the business of processing the MPEG data in order to adhere 
> to this proposal, then that will be a very big deal for this driver.

I _suspect_ that it is just a matter of adding something like em28xx-audio
at pvrusb2, saying that the format is MPEG, instead of raw PCM. In-kernel
processing is likely not needed/wanted.

We may try to double check with Takashi during the KS media workshop.

Regards,
Mauro

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: RFC: Core + Radio profile
  2012-08-22 18:09       ` Mauro Carvalho Chehab
@ 2012-08-23 11:12         ` Andy Walls
  0 siblings, 0 replies; 14+ messages in thread
From: Andy Walls @ 2012-08-23 11:12 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Mike Isely at pobox, Mike Isely, Hans Verkuil, linux-media

On Wed, 2012-08-22 at 15:09 -0300, Mauro Carvalho Chehab wrote:
> Em 22-08-2012 12:19, Mike Isely escreveu:
> > On Wed, 22 Aug 2012, Mauro Carvalho Chehab wrote:
> > 
> >> Em 22-08-2012 07:11, Hans Verkuil escreveu:
> >>> I've added some more core profile requirements.
> >>
> >>>>
> >>>> Streaming I/O is not supported by radio nodes.
> >>
> >> 	Hmm... pvrusb2/ivtv? Ok, it makes sense to move it to use the alsa
> >> mpeg API there. If we're enforcing it, we should deprecate the current way
> >> there, and make it use ALSA.
> > 
> > I am unaware of any ALSA MPEG API.  It's entirely likely that this is 
> > because I haven't been paying attention.  Nevertheless, can you please 
> > point me at any documentation on this so I can get up to speed?
> 
> 
> I don't know much about that. A grep at sound might help:
> 
> $ git grep -i mpeg sound/
> sound/core/oss/pcm_oss.c: case AFMT_MPEG:         return SNDRV_PCM_FORMAT_MPEG;
> sound/core/oss/pcm_oss.c: case SNDRV_PCM_FORMAT_MPEG:             return AFMT_MPEG;
> sound/core/pcm.c: FORMAT(MPEG),
> sound/core/pcm.c: case AFMT_MPEG:
> sound/core/pcm.c:         return "MPEG";
> sound/core/pcm_misc.c:    [SNDRV_PCM_FORMAT_MPEG] = {

> sound/usb/format.c:       case UAC_FORMAT_TYPE_II_MPEG:
> sound/usb/format.c:               fp->formats = SNDRV_PCM_FMTBIT_MPEG;
> sound/usb/format.c:               snd_printd(KERN_INFO "%d:%u:%d : unknown format tag %#x is detected.  processed as MPEG.\n",
> sound/usb/format.c:               fp->formats = SNDRV_PCM_FMTBIT_MPEG;
> 
> 
> > 
> > Currently the pvrusb2 driver does not attempt to perform any processing 
> > or filtering of the data stream, so radio data is just the same mpeg 
> > stream as video (but without any real embedded video data).  If I have 
> > to get into the business of processing the MPEG data in order to adhere 
> > to this proposal, then that will be a very big deal for this driver.
> 
> I _suspect_ that it is just a matter of adding something like em28xx-audio
> at pvrusb2, saying that the format is MPEG, instead of raw PCM. In-kernel
> processing is likely not needed/wanted.

The ivtv and cx18 drivers ask the CX2341[568] to produce a raw PCM audio
stream and provide that PCM audio via the legacy /dev/video24 device,
and via ALSA as well in the case of cx18.

>From what I can tell, the pvrusb2 driver does not use the raw PCM stream
the CX23416/7 is able to produce.  Instead, pvrusb2 appears to "mute"
the video and provides then whole MPEG-2 PS in radio mode.  That means
there is a full MPEG-2 PS container stream with both the audio ES and
video ES in it.

If something in user-space needs to get PCM or MPEG-1 Layer 3 audio from
that pvrusb2 "audio" stream via ALSA, something is going to need to
demux the MPEG-2 PS and possibly convert to PCM.  I'll bet ALSA
currently does not support that except maybe via some ALSA plug-in which
likely does not exist.

At that point, it might be easier just to use the PCM stream from the
CX23416 and add a pvrusb2-alsa module for the ALSA interface.

Regards,
Andy

> We may try to double check with Takashi during the KS media workshop.
> 
> Regards,
> Mauro



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: RFC: Core + Radio profile
  2012-08-22 13:42   ` Mauro Carvalho Chehab
  2012-08-22 15:19     ` Mike Isely
@ 2012-08-24 12:31     ` Hans Verkuil
  2012-08-24 14:51       ` Mauro Carvalho Chehab
  2012-08-26 22:56       ` Andy Walls
  1 sibling, 2 replies; 14+ messages in thread
From: Hans Verkuil @ 2012-08-24 12:31 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-media, Mike Isely, Andy Walls

Hi Mauro,

Thanks for your review!

On Wed August 22 2012 15:42:26 Mauro Carvalho Chehab wrote:
> Em 22-08-2012 07:11, Hans Verkuil escreveu:
> > I've added some more core profile requirements.
> > 
> > Regards,
> > 
> > 	Hans
> > 
> > On Wed August 22 2012 11:40:25 Hans Verkuil wrote:
> >> Hi all!
> >>
> >> The v4l2-compliance utility checks whether device drivers follows the V4L2
> >> specification. What is missing are 'profiles' detailing what device drivers
> >> should and shouldn't implement for particular device classes.
> >>
> >> This has been discussed several times, but until now no attempt was made to
> >> actually write this down.
> >>
> >> This RFC is my attempt to start this process by describing three profiles:
> >> the core profile that all drivers must adhere to, a profile for a radio
> >> receiver and a profile for a radio transmitter.
> >>
> >> Missing in this RFC is a description of how tuner ownership is handled if a
> >> tuner is shared between a radio and video driver. This will be discussed
> >> during the workshop next week.
> 
> DVB x V4L2 tunership discussion is also needed. It also makes sense to 
> document it somewhere.

Absolutely. But as I know nothing about DVB someone will have to help with
that.

> >> I am not certain where these profile descriptions should go. Should we add
> >> this to DocBook, or describe it in Documentation/video4linux? Or perhaps
> >> somehow add it to v4l2-compliance, since that tool effectively verifies
> >> the profile.
> 
> The strongest document we have, IMO, is the DocBook API one, as both Kernel
> and userspace developers use it. As we want userspace apps to be aware and
> benefit of the profiles, this should be there at DocBook.

Sounds reasonable.

> >> Also note that the core profile description is more strict than the spec.
> 
> IMO, that's the right approach: The specs should define the several API
> aspects; the profile should mandate what's optional and what's mandatory.
> 
> >> For example, G/S_PRIORITY are currently optional,
> 
> After putting the profiles there, we should remove "optional" tags elsewhere,
> as the profiles section will tell what's mandatory and what is optional, as
> different profiles require different mandatory arguments.
> 
> >> but I feel that all new drivers should implement this, especially since it
> >> is very easy to add provided you use struct v4l2_fh. 
> 
> I agree on making G/S_PRIORITY mandatory.
> 
> >> Similar with respect to the control requirements which
> >> assume you use the control framework.
> 
> IMO, we should split the internal API requirements (use the control framework)
> from the external API ones, as they're addressed to different audiences, e. g.
> external API requirements are needed by all application developers, while
> the internal ones are only for Kernel hackers.
> 
> The internal API requirements should be together with the internal API descriptions,
> so Documentation/video4linux is probably the best place for them.
> 
> >> Note that these profiles are primarily meant for driver developers. The V4L2
> >> specification is leading for application developers.
> 
> This is where we diverge ;) We need profiles primary for application developers,
> for them to know what is expected to be there on any media driver of a certain
> kind.
> 
> Ok, internal driver-developer profiles is also needed, in order for them to
> use the right internal API's and internal core functions.

Well, it is all very nice to just say e.g. 'G/S_PRIORITY is mandatory' in the
profile section, but the reality is that it is hit and miss whether or not it
is implemented. Even if we manage to convert all drivers to implement G/S_PRIO,
then it is still not something an app developer can rely on because many users
use older kernel versions.

Which is why IMHO the spec should be leading for app development. The profile
sections are a handy summary of what you can expect from drivers for specific
types of hardware, but it can't be leading except when it comes to new driver
development or driver changes in areas covered by the profile(s).

> >> While writing down these profiles I noticed one thing that was very much
> >> missing for radio devices: there is no capability bit to tell the application
> >> whether there is an associated ALSA device or whether the audio goes through
> >> a line-in/out. Personally I think that this should be fixed.
> 
> Yes, this is one bit that it is missing. Well, this is currently handled this
> at xawtv3 "radio" aplication by using the libmedia_dev API there.
> 
> It likely makes sense, for a version 2 of the profiles (e. g. after we finish
> merging the basic stuff there), to add a chapter at  the profiles section 
> recommending the usage of the libraries provided by v4l-utils, with some 
> description or code examples.

Good idea.

> >>
> >> Comments are welcome!
> >>
> >> Regards,
> >>
> >> 	Hans
> >>
> >>
> >> Core Profile
> >> ============
> >>
> >> All V4L2 device nodes must support the core profile.
> >>
> >> VIDIOC_QUERYCAP must be supported and must set the device_caps field.
> >> bus_info must be a unique string and must not be empty (pending result of
> >> the upcoming workshop).
> >>
> >> VIDIOC_G/S_PRIORITY must be supported.
> >>
> >> If you have controls then both the VIDIOC_G/S_CTRL and the VIDIOC_G/S/TRY_EXT_CTRLS
> >> ioctls must be supported, together with poll(), VIDIOC_DQEVENT and
> >> VIDIOC_(UN)SUBSCRIBE_EVENT to handle control events. 
> 
> >> Use the control framework to implement this.
> 
> Now looking at the concrete case, I don't see any troubles on adding it to
> the V4L2 API, but writing it as:
> 
> 	"Driver developers must use the control framework to implement this."

I wrote this profile fairly quickly, so it is quite concise and needs to be
expanded. It's just an RFC to get a discussion started :-)

> >> VIDIOC_LOG_STATUS is optional, but recommended for complex drivers.
> >>
> >> VIDIOC_DBG_G_CHIP_IDENT, VIDIOC_DBG_G/S_REGISTER are optional and are
> >> primarily used to debug drivers.
> 
> The above ones require a note for application developers:
> 
> 	"Applications must not use VIDIOC_LOG_STATUS during its normal behaviour;
> 	 this is reserved for driver's debug/test.
> 
> 	 The usage of VIDIOC_DBG_G_CHIP_IDENT and VIDIOC_DBG_G/S_REGISTER is solely 
> 	 for driver's debug. Applications should never use it."
> 
> >> Video, Radio and VBI nodes are for input or output only. 
> 
> Hmm... didn't look clear to me. I would change the first phase to something like:
> 
> 	"Each video, radio or vbi node should be used by just one input or
> 	output streaming engine."
> 
> >> The only exception
> >> are video nodes that have the V4L2_CAP_VIDEO_M2M(_MPLANE) capability set.
> 
> Again, not clear what it should be expected on a device with the M2M capabilities.
> 
> Better to write it like:
> 
> 	"Video nodes with memory to memory capabilities (V4L2_CAP_VIDEO_M2M and
> V4L2_CAP_VIDEO_M2M_MPLANE) must implement both one input streaming engine and
> one output streaming engine at the same node."
> 
> >> Video nodes only control video, radio nodes only control radio and RDS, vbi
> >> nodes only control VBI (raw and/or sliced).
> 
> I would use "must" word for everything mandatory at the profiles section (
> and "may" for optional ones), like:
> 
> 	"Video nodes must only control the video functionality.
> 
> 	 Radio nodes must only control the radio functionality.
> 
> 	 VBI nodes must only control the Vertical Blank Interface functionality. The
> 	 same VBI node must be used by raw VBI and sliced VBI api's, when both are
> 	 supported."
> 
> >>
> >> Streaming I/O is not supported by radio nodes.
> 
> 	Hmm... pvrusb2/ivtv? Ok, it makes sense to move it to use the alsa
> mpeg API there. If we're enforcing it, we should deprecate the current way
> there, and make it use ALSA.
> 
> (more comments about it below)
> 
> > It should be possible to open device nodes more than once. Just opening a
> > node and querying information from the driver should not change the state
> > of the driver.
> 
> Hmm... locking between DVB and V4L should likely be added here: e. g. is it
> allowed to open both DVB and V4L nodes for the same tuner/streaming engine?
> 
> Currently, this is not allowed. There are also known issues when applications
> change too fast between them (open/close V4L, then open/close dvb or vice-versa).

As mentioned above, I will need help here from people who actually use and
understand DVB.

> > The file handle that called VIDIOC_REQBUFS, VIDIOC_CREATE_BUFS, read/write
> > or select (for reading or writing) first is the only one that can do I/O.
> > Attempts by other file handles to call these ioctls/file operations will
> > get an EBUSY error.
> >
> > This file handle remains the owner of the I/O until the file handle is
> > closed, or until VIDIOC_REQBUFS is called with count == 0.
> 
> Please change it to use "must". Also, I think that doing s/called/calls/
> is better.
> 
> It is likely better to say it as:
> 
> 	The streaming engine ownership must be taken by the first handle that calls
> 	VIDIOC_REQBUFS, VIDIOC_CREATE_BUFS, read/write or select (for reading or writing).
> 
> 	Only such file handler must do streaming I/O. Any attempts by other file handlers
> 	to do I/O must return EBUSY.
> 
> 	The I/O streaming ownership must be released when the file handler is closed
> 	or when VIDIOC_REQBUFS is called with count equal to zero.
> 
> >>
> >> Radio Receiver Profile
> >> ======================
> >>
> >> Radio devices have a tuner and (usually) a demodulator. The audio is either
> >> send out to a line-out connector or uses ALSA to stream the audio.
> 
> Hmm... are there anything mandatory here, or are you just defining what a radio is?
> In the latter, the above text looks ok.

What I tried to achieve here is to limit the number of possible methods of getting
the audio to just two: line-out or ALSA. This is to prevent future pvrusb2/ivtv
non-standard methods of getting the audio.

> >>
> >> It implements VIDIOC_G/S_TUNER, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS.
> 
> 	In addition to the mandatory items at the core profile (I would add an hyperlink here),
> 	all drivers must implement VIDIOC_G/S_TUNER, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS.
> 
> >> If hardware seek is supported, then VIDIOC_S_HW_FREQ_SEEK is implemented.
> 
> 	A radio device may implement VIDIOC_S_HW_FREQ_SEEK, when audio station seek is
> 	supported by radio.
> 
> >> It does *not* implement VIDIOC_ENUM/G/S_INPUT or VIDIOC_ENUM/G/S_AUDIO.
> 
> A radio node must *not* implement VIDIOC_ENUM/G/S_INPUT or VIDIOC_ENUM/G/S_AUDIO.
> 
> >> If RDS_BLOCK_IO is supported, then read() and poll() are implemented as well.
> 
> Radio drivers that support RDS must report RDS_BLOCK_IO. In this case, radio
> nodes must implement read()/poll() syscalls.
> 
> >> If a mute control exists and the audio is output using a line-out, then the
> >> audio must be muted on driver load and unload.
> >>
> >> On driver load the frequency must be initialized to a valid frequency.
> 
> This is not nice, and not always possible: devices with xc3028 tuner (and other
> Xceive tuners) have firmwares for radio and for TV. Initializing for radio
> makes TV uninitialized, and vice-versa. Also, some devices take up to 30 seconds
> to load a firmware (tm6000).
> 
> I think you're likely trying to say that the initial frequency should be
> a valid value. If so, please prepend it with a "driver developer's note".

Yes, that's what I tried to say :-)

I've seen drivers where G_FREQUENCY would return 0 after the initial module
load, which is something that should be avoided.

> >>
> >> Note: There are a few drivers that use a radio (pvrusb2) or video (ivtv)
> >> node to stream the audio. These are legacy exceptions to the rule.
> 
> What an application developer should do with that???

Nothing. A generic application cannot support audio for these devices.
You need specialized apps for that.

> If this should not be supported anymore, then we need instead to either
> fix the drivers that aren't compliant with the specs or move them to
> staging, in order to let them to be either fixed or dropped, if none
> cares enough to fix.

Well, the problem is that people are actually using the existing, nonstandard,
APIs for ivtv (and probably pvrusb2 as well).

For ivtv it is probably not too difficult to add ALSA support.

Andy, I know you implemented ALSA support for cx18, do you know how much work
it would be to do the same for ivtv?

If Andy has absolutely no time, then I can try to find time to add ALSA support
to ivtv. But the existing API should probably remain (if possible).

It's probably more work to fix this for pvrusb2, though.

> 
> >>
> >> FM Transmitter Profile
> >> ======================
> >>
> >> FM transmitter devices have a transmitter and modulator. The audio is
> >> transferred using ALSA or a line-in connector.
> 
> Hmm... are there anything mandatory here, or are you just defining what a radio is?
> In the latter, the above text looks ok.
> 
> >>
> >> It implements VIDIOC_G/S_MODULATOR, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS.
> >>
> >> It does *not* implement VIDIOC_ENUM/G/S_OUTPUT or VIDIOC_ENUM/G/S_AUDOUT.
> >>
> >> If RDS_BLOCK_IO is supported, then write() and poll() are implemented
> >> as well.
> >>
> >> On driver load the frequency must be initialized to a valid frequency.
> 
> Same notes from the "radio" profile applies here.
> 
> --
> 
> That's said, I think that the RFC proposal is going on the right direction.
> It is very good to see it moving forward!

You're welcome.

Regards,

	Hans

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: RFC: Core + Radio profile
  2012-08-24 12:31     ` Hans Verkuil
@ 2012-08-24 14:51       ` Mauro Carvalho Chehab
  2012-08-24 15:35         ` Hans Verkuil
  2012-08-26 22:56       ` Andy Walls
  1 sibling, 1 reply; 14+ messages in thread
From: Mauro Carvalho Chehab @ 2012-08-24 14:51 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, Mike Isely, Andy Walls

Em 24-08-2012 09:31, Hans Verkuil escreveu:
> Hi Mauro,
> 
> Thanks for your review!
> 
> On Wed August 22 2012 15:42:26 Mauro Carvalho Chehab wrote:
>> Em 22-08-2012 07:11, Hans Verkuil escreveu:
>>> I've added some more core profile requirements.
>>>
>>> Regards,
>>>
>>> 	Hans
>>>
>>> On Wed August 22 2012 11:40:25 Hans Verkuil wrote:
>>>> Hi all!
>>>>
>>>> The v4l2-compliance utility checks whether device drivers follows the V4L2
>>>> specification. What is missing are 'profiles' detailing what device drivers
>>>> should and shouldn't implement for particular device classes.
>>>>
>>>> This has been discussed several times, but until now no attempt was made to
>>>> actually write this down.
>>>>
>>>> This RFC is my attempt to start this process by describing three profiles:
>>>> the core profile that all drivers must adhere to, a profile for a radio
>>>> receiver and a profile for a radio transmitter.
>>>>
>>>> Missing in this RFC is a description of how tuner ownership is handled if a
>>>> tuner is shared between a radio and video driver. This will be discussed
>>>> during the workshop next week.
>>
>> DVB x V4L2 tunership discussion is also needed. It also makes sense to 
>> document it somewhere.
> 
> Absolutely. But as I know nothing about DVB someone will have to help with
> that.

Sure.

>>>> I am not certain where these profile descriptions should go. Should we add
>>>> this to DocBook, or describe it in Documentation/video4linux? Or perhaps
>>>> somehow add it to v4l2-compliance, since that tool effectively verifies
>>>> the profile.
>>
>> The strongest document we have, IMO, is the DocBook API one, as both Kernel
>> and userspace developers use it. As we want userspace apps to be aware and
>> benefit of the profiles, this should be there at DocBook.
> 
> Sounds reasonable.
> 
>>>> Also note that the core profile description is more strict than the spec.
>>
>> IMO, that's the right approach: The specs should define the several API
>> aspects; the profile should mandate what's optional and what's mandatory.
>>
>>>> For example, G/S_PRIORITY are currently optional,
>>
>> After putting the profiles there, we should remove "optional" tags elsewhere,
>> as the profiles section will tell what's mandatory and what is optional, as
>> different profiles require different mandatory arguments.
>>
>>>> but I feel that all new drivers should implement this, especially since it
>>>> is very easy to add provided you use struct v4l2_fh. 
>>
>> I agree on making G/S_PRIORITY mandatory.
>>
>>>> Similar with respect to the control requirements which
>>>> assume you use the control framework.
>>
>> IMO, we should split the internal API requirements (use the control framework)
>> from the external API ones, as they're addressed to different audiences, e. g.
>> external API requirements are needed by all application developers, while
>> the internal ones are only for Kernel hackers.
>>
>> The internal API requirements should be together with the internal API descriptions,
>> so Documentation/video4linux is probably the best place for them.
>>
>>>> Note that these profiles are primarily meant for driver developers. The V4L2
>>>> specification is leading for application developers.
>>
>> This is where we diverge ;) We need profiles primary for application developers,
>> for them to know what is expected to be there on any media driver of a certain
>> kind.
>>
>> Ok, internal driver-developer profiles is also needed, in order for them to
>> use the right internal API's and internal core functions.
> 
> Well, it is all very nice to just say e.g. 'G/S_PRIORITY is mandatory' in the
> profile section, but the reality is that it is hit and miss whether or not it
> is implemented. Even if we manage to convert all drivers to implement G/S_PRIO,
> then it is still not something an app developer can rely on because many users
> use older kernel versions.

It is up to the application developers to decide it. Imagining applications
that is G/S_PRIO-aware, some of them may require only newer kernels; other may
have some fallback code, based on V4L2 version. It is up to the application
developer to decide.

> Which is why IMHO the spec should be leading for app development. The profile
> sections are a handy summary of what you can expect from drivers for specific
> types of hardware, but it can't be leading except when it comes to new driver
> development or driver changes in areas covered by the profile(s).

Well, drivers that aren't compliant with a profile already fails, or they will fail
sooner or later with some applications, as they don't behave like the other drivers.

We need to be sure, of course, that the profile will state what's already
implemented by the vast majority (if not all) of the drivers.

So, in this particular case, IMO, we should state that the prio ioctl's are
not mandatory, updating it later when we finish the porting of all drivers to
use it.

>>>> While writing down these profiles I noticed one thing that was very much
>>>> missing for radio devices: there is no capability bit to tell the application
>>>> whether there is an associated ALSA device or whether the audio goes through
>>>> a line-in/out. Personally I think that this should be fixed.
>>
>> Yes, this is one bit that it is missing. Well, this is currently handled this
>> at xawtv3 "radio" aplication by using the libmedia_dev API there.
>>
>> It likely makes sense, for a version 2 of the profiles (e. g. after we finish
>> merging the basic stuff there), to add a chapter at  the profiles section 
>> recommending the usage of the libraries provided by v4l-utils, with some 
>> description or code examples.
> 
> Good idea.
> 
>>>>
>>>> Comments are welcome!
>>>>
>>>> Regards,
>>>>
>>>> 	Hans
>>>>
>>>>
>>>> Core Profile
>>>> ============
>>>>
>>>> All V4L2 device nodes must support the core profile.
>>>>
>>>> VIDIOC_QUERYCAP must be supported and must set the device_caps field.
>>>> bus_info must be a unique string and must not be empty (pending result of
>>>> the upcoming workshop).
>>>>
>>>> VIDIOC_G/S_PRIORITY must be supported.
>>>>
>>>> If you have controls then both the VIDIOC_G/S_CTRL and the VIDIOC_G/S/TRY_EXT_CTRLS
>>>> ioctls must be supported, together with poll(), VIDIOC_DQEVENT and
>>>> VIDIOC_(UN)SUBSCRIBE_EVENT to handle control events. 
>>
>>>> Use the control framework to implement this.
>>
>> Now looking at the concrete case, I don't see any troubles on adding it to
>> the V4L2 API, but writing it as:
>>
>> 	"Driver developers must use the control framework to implement this."
> 
> I wrote this profile fairly quickly, so it is quite concise and needs to be
> expanded. It's just an RFC to get a discussion started :-)

I don't mind adding a concise chapter ;) It can be later expanded, but the
better is to commit something as soon as possible, and keep improving it as
we find some time and need for that.

> 
>>>> VIDIOC_LOG_STATUS is optional, but recommended for complex drivers.
>>>>
>>>> VIDIOC_DBG_G_CHIP_IDENT, VIDIOC_DBG_G/S_REGISTER are optional and are
>>>> primarily used to debug drivers.
>>
>> The above ones require a note for application developers:
>>
>> 	"Applications must not use VIDIOC_LOG_STATUS during its normal behaviour;
>> 	 this is reserved for driver's debug/test.
>>
>> 	 The usage of VIDIOC_DBG_G_CHIP_IDENT and VIDIOC_DBG_G/S_REGISTER is solely 
>> 	 for driver's debug. Applications should never use it."
>>
>>>> Video, Radio and VBI nodes are for input or output only. 
>>
>> Hmm... didn't look clear to me. I would change the first phase to something like:
>>
>> 	"Each video, radio or vbi node should be used by just one input or
>> 	output streaming engine."
>>
>>>> The only exception
>>>> are video nodes that have the V4L2_CAP_VIDEO_M2M(_MPLANE) capability set.
>>
>> Again, not clear what it should be expected on a device with the M2M capabilities.
>>
>> Better to write it like:
>>
>> 	"Video nodes with memory to memory capabilities (V4L2_CAP_VIDEO_M2M and
>> V4L2_CAP_VIDEO_M2M_MPLANE) must implement both one input streaming engine and
>> one output streaming engine at the same node."
>>
>>>> Video nodes only control video, radio nodes only control radio and RDS, vbi
>>>> nodes only control VBI (raw and/or sliced).
>>
>> I would use "must" word for everything mandatory at the profiles section (
>> and "may" for optional ones), like:
>>
>> 	"Video nodes must only control the video functionality.
>>
>> 	 Radio nodes must only control the radio functionality.
>>
>> 	 VBI nodes must only control the Vertical Blank Interface functionality. The
>> 	 same VBI node must be used by raw VBI and sliced VBI api's, when both are
>> 	 supported."
>>
>>>>
>>>> Streaming I/O is not supported by radio nodes.
>>
>> 	Hmm... pvrusb2/ivtv? Ok, it makes sense to move it to use the alsa
>> mpeg API there. If we're enforcing it, we should deprecate the current way
>> there, and make it use ALSA.
>>
>> (more comments about it below)
>>
>>> It should be possible to open device nodes more than once. Just opening a
>>> node and querying information from the driver should not change the state
>>> of the driver.
>>
>> Hmm... locking between DVB and V4L should likely be added here: e. g. is it
>> allowed to open both DVB and V4L nodes for the same tuner/streaming engine?
>>
>> Currently, this is not allowed. There are also known issues when applications
>> change too fast between them (open/close V4L, then open/close dvb or vice-versa).
> 
> As mentioned above, I will need help here from people who actually use and
> understand DVB.

Maybe we can discuss it a little bit next week, if we have some time for that.

>>> The file handle that called VIDIOC_REQBUFS, VIDIOC_CREATE_BUFS, read/write
>>> or select (for reading or writing) first is the only one that can do I/O.
>>> Attempts by other file handles to call these ioctls/file operations will
>>> get an EBUSY error.
>>>
>>> This file handle remains the owner of the I/O until the file handle is
>>> closed, or until VIDIOC_REQBUFS is called with count == 0.
>>
>> Please change it to use "must". Also, I think that doing s/called/calls/
>> is better.
>>
>> It is likely better to say it as:
>>
>> 	The streaming engine ownership must be taken by the first handle that calls
>> 	VIDIOC_REQBUFS, VIDIOC_CREATE_BUFS, read/write or select (for reading or writing).
>>
>> 	Only such file handler must do streaming I/O. Any attempts by other file handlers
>> 	to do I/O must return EBUSY.
>>
>> 	The I/O streaming ownership must be released when the file handler is closed
>> 	or when VIDIOC_REQBUFS is called with count equal to zero.
>>
>>>>
>>>> Radio Receiver Profile
>>>> ======================
>>>>
>>>> Radio devices have a tuner and (usually) a demodulator. The audio is either
>>>> send out to a line-out connector or uses ALSA to stream the audio.
>>
>> Hmm... are there anything mandatory here, or are you just defining what a radio is?
>> In the latter, the above text looks ok.
> 
> What I tried to achieve here is to limit the number of possible methods of getting
> the audio to just two: line-out or ALSA. This is to prevent future pvrusb2/ivtv
> non-standard methods of getting the audio.
> 
>>>>
>>>> It implements VIDIOC_G/S_TUNER, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS.
>>
>> 	In addition to the mandatory items at the core profile (I would add an hyperlink here),
>> 	all drivers must implement VIDIOC_G/S_TUNER, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS.
>>
>>>> If hardware seek is supported, then VIDIOC_S_HW_FREQ_SEEK is implemented.
>>
>> 	A radio device may implement VIDIOC_S_HW_FREQ_SEEK, when audio station seek is
>> 	supported by radio.
>>
>>>> It does *not* implement VIDIOC_ENUM/G/S_INPUT or VIDIOC_ENUM/G/S_AUDIO.
>>
>> A radio node must *not* implement VIDIOC_ENUM/G/S_INPUT or VIDIOC_ENUM/G/S_AUDIO.
>>
>>>> If RDS_BLOCK_IO is supported, then read() and poll() are implemented as well.
>>
>> Radio drivers that support RDS must report RDS_BLOCK_IO. In this case, radio
>> nodes must implement read()/poll() syscalls.
>>
>>>> If a mute control exists and the audio is output using a line-out, then the
>>>> audio must be muted on driver load and unload.
>>>>
>>>> On driver load the frequency must be initialized to a valid frequency.
>>
>> This is not nice, and not always possible: devices with xc3028 tuner (and other
>> Xceive tuners) have firmwares for radio and for TV. Initializing for radio
>> makes TV uninitialized, and vice-versa. Also, some devices take up to 30 seconds
>> to load a firmware (tm6000).
>>
>> I think you're likely trying to say that the initial frequency should be
>> a valid value. If so, please prepend it with a "driver developer's note".
> 
> Yes, that's what I tried to say :-)

Yeah, I figured out ;)
> 
> I've seen drivers where G_FREQUENCY would return 0 after the initial module
> load, which is something that should be avoided.

Sure.

>>>>
>>>> Note: There are a few drivers that use a radio (pvrusb2) or video (ivtv)
>>>> node to stream the audio. These are legacy exceptions to the rule.
>>
>> What an application developer should do with that???
> 
> Nothing. A generic application cannot support audio for these devices.
> You need specialized apps for that.

We actually need to fix those drivers to do the right thing, or to add this
possibility to the standard.

>> If this should not be supported anymore, then we need instead to either
>> fix the drivers that aren't compliant with the specs or move them to
>> staging, in order to let them to be either fixed or dropped, if none
>> cares enough to fix.
> 
> Well, the problem is that people are actually using the existing, nonstandard,
> APIs for ivtv (and probably pvrusb2 as well).
> 
> For ivtv it is probably not too difficult to add ALSA support.
> 
> Andy, I know you implemented ALSA support for cx18, do you know how much work
> it would be to do the same for ivtv?
> 
> If Andy has absolutely no time, then I can try to find time to add ALSA support
> to ivtv. But the existing API should probably remain (if possible).
> 
> It's probably more work to fix this for pvrusb2, though.

As I said, the API should be mandatory for all drivers under drivers/media.
So, either the API specs should add support for the ivtv/pvrusb2 way, or they 
need to be moved to staging.

Of course, dropping support for mpeg audio-only on radio/video nodes require
it to follow the features to be deprecated. But all drivers should at least
implement everything that it is mandatory at the API profiles, and for sure,
the way audio IN/OUT will be provided should be properly documented.

>>>> FM Transmitter Profile
>>>> ======================
>>>>
>>>> FM transmitter devices have a transmitter and modulator. The audio is
>>>> transferred using ALSA or a line-in connector.
>>
>> Hmm... are there anything mandatory here, or are you just defining what a radio is?
>> In the latter, the above text looks ok.
>>
>>>>
>>>> It implements VIDIOC_G/S_MODULATOR, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS.
>>>>
>>>> It does *not* implement VIDIOC_ENUM/G/S_OUTPUT or VIDIOC_ENUM/G/S_AUDOUT.
>>>>
>>>> If RDS_BLOCK_IO is supported, then write() and poll() are implemented
>>>> as well.
>>>>
>>>> On driver load the frequency must be initialized to a valid frequency.
>>
>> Same notes from the "radio" profile applies here.
>>
>> --
>>
>> That's said, I think that the RFC proposal is going on the right direction.
>> It is very good to see it moving forward!
> 
> You're welcome.
> 
> Regards,
>  
> 	Hans
> 

Thanks!
Mauro


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: RFC: Core + Radio profile
  2012-08-24 14:51       ` Mauro Carvalho Chehab
@ 2012-08-24 15:35         ` Hans Verkuil
  0 siblings, 0 replies; 14+ messages in thread
From: Hans Verkuil @ 2012-08-24 15:35 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-media, Mike Isely, Andy Walls

On Fri August 24 2012 16:51:56 Mauro Carvalho Chehab wrote:
> Em 24-08-2012 09:31, Hans Verkuil escreveu:
> > Hi Mauro,
> > 
> > Thanks for your review!
> > 
> > On Wed August 22 2012 15:42:26 Mauro Carvalho Chehab wrote:
> >> Em 22-08-2012 07:11, Hans Verkuil escreveu:
> >>> I've added some more core profile requirements.
> >>>
> >>> Regards,
> >>>
> >>> 	Hans
> >>>
> >>> On Wed August 22 2012 11:40:25 Hans Verkuil wrote:
> >>>> Hi all!
> >>>>
> >>>> The v4l2-compliance utility checks whether device drivers follows the V4L2
> >>>> specification. What is missing are 'profiles' detailing what device drivers
> >>>> should and shouldn't implement for particular device classes.
> >>>>
> >>>> This has been discussed several times, but until now no attempt was made to
> >>>> actually write this down.
> >>>>
> >>>> This RFC is my attempt to start this process by describing three profiles:
> >>>> the core profile that all drivers must adhere to, a profile for a radio
> >>>> receiver and a profile for a radio transmitter.
> >>>>
> >>>> Missing in this RFC is a description of how tuner ownership is handled if a
> >>>> tuner is shared between a radio and video driver. This will be discussed
> >>>> during the workshop next week.
> >>
> >> DVB x V4L2 tunership discussion is also needed. It also makes sense to 
> >> document it somewhere.
> > 
> > Absolutely. But as I know nothing about DVB someone will have to help with
> > that.
> 
> Sure.
> 
> >>>> I am not certain where these profile descriptions should go. Should we add
> >>>> this to DocBook, or describe it in Documentation/video4linux? Or perhaps
> >>>> somehow add it to v4l2-compliance, since that tool effectively verifies
> >>>> the profile.
> >>
> >> The strongest document we have, IMO, is the DocBook API one, as both Kernel
> >> and userspace developers use it. As we want userspace apps to be aware and
> >> benefit of the profiles, this should be there at DocBook.
> > 
> > Sounds reasonable.
> > 
> >>>> Also note that the core profile description is more strict than the spec.
> >>
> >> IMO, that's the right approach: The specs should define the several API
> >> aspects; the profile should mandate what's optional and what's mandatory.
> >>
> >>>> For example, G/S_PRIORITY are currently optional,
> >>
> >> After putting the profiles there, we should remove "optional" tags elsewhere,
> >> as the profiles section will tell what's mandatory and what is optional, as
> >> different profiles require different mandatory arguments.
> >>
> >>>> but I feel that all new drivers should implement this, especially since it
> >>>> is very easy to add provided you use struct v4l2_fh. 
> >>
> >> I agree on making G/S_PRIORITY mandatory.
> >>
> >>>> Similar with respect to the control requirements which
> >>>> assume you use the control framework.
> >>
> >> IMO, we should split the internal API requirements (use the control framework)
> >> from the external API ones, as they're addressed to different audiences, e. g.
> >> external API requirements are needed by all application developers, while
> >> the internal ones are only for Kernel hackers.
> >>
> >> The internal API requirements should be together with the internal API descriptions,
> >> so Documentation/video4linux is probably the best place for them.
> >>
> >>>> Note that these profiles are primarily meant for driver developers. The V4L2
> >>>> specification is leading for application developers.
> >>
> >> This is where we diverge ;) We need profiles primary for application developers,
> >> for them to know what is expected to be there on any media driver of a certain
> >> kind.
> >>
> >> Ok, internal driver-developer profiles is also needed, in order for them to
> >> use the right internal API's and internal core functions.
> > 
> > Well, it is all very nice to just say e.g. 'G/S_PRIORITY is mandatory' in the
> > profile section, but the reality is that it is hit and miss whether or not it
> > is implemented. Even if we manage to convert all drivers to implement G/S_PRIO,
> > then it is still not something an app developer can rely on because many users
> > use older kernel versions.
> 
> It is up to the application developers to decide it. Imagining applications
> that is G/S_PRIO-aware, some of them may require only newer kernels; other may
> have some fallback code, based on V4L2 version. It is up to the application
> developer to decide.
> 
> > Which is why IMHO the spec should be leading for app development. The profile
> > sections are a handy summary of what you can expect from drivers for specific
> > types of hardware, but it can't be leading except when it comes to new driver
> > development or driver changes in areas covered by the profile(s).
> 
> Well, drivers that aren't compliant with a profile already fails, or they will fail
> sooner or later with some applications, as they don't behave like the other drivers.
> 
> We need to be sure, of course, that the profile will state what's already
> implemented by the vast majority (if not all) of the drivers.
> 
> So, in this particular case, IMO, we should state that the prio ioctl's are
> not mandatory, updating it later when we finish the porting of all drivers to
> use it.

I would phrase it something like this:

'The VIDIOC_G/S_PRIORITY ioctls are mandatory. However, not all drivers are
compliant. Until all drivers have been made to comply application developers
should be aware that these ioctls may be missing.'

It comes down to what you want to achieve with profiles. My intention is that
profiles describe what should and what shouldn't be implemented for various
classes of video/radio hardware. And v4l2-compliance is the tool that verifies
the profile. In addition, it makes a very clear set of guidelines for driver
developers to follow.

As long as there are many drivers that do not yet comply we should add notes
to the profiles describing where theory and reality differ, so application
developers are aware of the pitfalls they have to work around.

Hopefully over time these notes can be removed one by one as existing drivers
are fixed. But that's going to take a *long* time.

If the profile would just state what drivers do today, then that doesn't add
much to what the current spec already does. IMHO.

> >>>> While writing down these profiles I noticed one thing that was very much
> >>>> missing for radio devices: there is no capability bit to tell the application
> >>>> whether there is an associated ALSA device or whether the audio goes through
> >>>> a line-in/out. Personally I think that this should be fixed.
> >>
> >> Yes, this is one bit that it is missing. Well, this is currently handled this
> >> at xawtv3 "radio" aplication by using the libmedia_dev API there.
> >>
> >> It likely makes sense, for a version 2 of the profiles (e. g. after we finish
> >> merging the basic stuff there), to add a chapter at  the profiles section 
> >> recommending the usage of the libraries provided by v4l-utils, with some 
> >> description or code examples.
> > 
> > Good idea.
> > 
> >>>>
> >>>> Comments are welcome!
> >>>>
> >>>> Regards,
> >>>>
> >>>> 	Hans
> >>>>
> >>>>
> >>>> Core Profile
> >>>> ============
> >>>>
> >>>> All V4L2 device nodes must support the core profile.
> >>>>
> >>>> VIDIOC_QUERYCAP must be supported and must set the device_caps field.
> >>>> bus_info must be a unique string and must not be empty (pending result of
> >>>> the upcoming workshop).
> >>>>
> >>>> VIDIOC_G/S_PRIORITY must be supported.
> >>>>
> >>>> If you have controls then both the VIDIOC_G/S_CTRL and the VIDIOC_G/S/TRY_EXT_CTRLS
> >>>> ioctls must be supported, together with poll(), VIDIOC_DQEVENT and
> >>>> VIDIOC_(UN)SUBSCRIBE_EVENT to handle control events. 
> >>
> >>>> Use the control framework to implement this.
> >>
> >> Now looking at the concrete case, I don't see any troubles on adding it to
> >> the V4L2 API, but writing it as:
> >>
> >> 	"Driver developers must use the control framework to implement this."
> > 
> > I wrote this profile fairly quickly, so it is quite concise and needs to be
> > expanded. It's just an RFC to get a discussion started :-)
> 
> I don't mind adding a concise chapter ;) It can be later expanded, but the
> better is to commit something as soon as possible, and keep improving it as
> we find some time and need for that.
> 
> > 
> >>>> VIDIOC_LOG_STATUS is optional, but recommended for complex drivers.
> >>>>
> >>>> VIDIOC_DBG_G_CHIP_IDENT, VIDIOC_DBG_G/S_REGISTER are optional and are
> >>>> primarily used to debug drivers.
> >>
> >> The above ones require a note for application developers:
> >>
> >> 	"Applications must not use VIDIOC_LOG_STATUS during its normal behaviour;
> >> 	 this is reserved for driver's debug/test.
> >>
> >> 	 The usage of VIDIOC_DBG_G_CHIP_IDENT and VIDIOC_DBG_G/S_REGISTER is solely 
> >> 	 for driver's debug. Applications should never use it."
> >>
> >>>> Video, Radio and VBI nodes are for input or output only. 
> >>
> >> Hmm... didn't look clear to me. I would change the first phase to something like:
> >>
> >> 	"Each video, radio or vbi node should be used by just one input or
> >> 	output streaming engine."
> >>
> >>>> The only exception
> >>>> are video nodes that have the V4L2_CAP_VIDEO_M2M(_MPLANE) capability set.
> >>
> >> Again, not clear what it should be expected on a device with the M2M capabilities.
> >>
> >> Better to write it like:
> >>
> >> 	"Video nodes with memory to memory capabilities (V4L2_CAP_VIDEO_M2M and
> >> V4L2_CAP_VIDEO_M2M_MPLANE) must implement both one input streaming engine and
> >> one output streaming engine at the same node."
> >>
> >>>> Video nodes only control video, radio nodes only control radio and RDS, vbi
> >>>> nodes only control VBI (raw and/or sliced).
> >>
> >> I would use "must" word for everything mandatory at the profiles section (
> >> and "may" for optional ones), like:
> >>
> >> 	"Video nodes must only control the video functionality.
> >>
> >> 	 Radio nodes must only control the radio functionality.
> >>
> >> 	 VBI nodes must only control the Vertical Blank Interface functionality. The
> >> 	 same VBI node must be used by raw VBI and sliced VBI api's, when both are
> >> 	 supported."
> >>
> >>>>
> >>>> Streaming I/O is not supported by radio nodes.
> >>
> >> 	Hmm... pvrusb2/ivtv? Ok, it makes sense to move it to use the alsa
> >> mpeg API there. If we're enforcing it, we should deprecate the current way
> >> there, and make it use ALSA.
> >>
> >> (more comments about it below)
> >>
> >>> It should be possible to open device nodes more than once. Just opening a
> >>> node and querying information from the driver should not change the state
> >>> of the driver.
> >>
> >> Hmm... locking between DVB and V4L should likely be added here: e. g. is it
> >> allowed to open both DVB and V4L nodes for the same tuner/streaming engine?
> >>
> >> Currently, this is not allowed. There are also known issues when applications
> >> change too fast between them (open/close V4L, then open/close dvb or vice-versa).
> > 
> > As mentioned above, I will need help here from people who actually use and
> > understand DVB.
> 
> Maybe we can discuss it a little bit next week, if we have some time for that.
> 
> >>> The file handle that called VIDIOC_REQBUFS, VIDIOC_CREATE_BUFS, read/write
> >>> or select (for reading or writing) first is the only one that can do I/O.
> >>> Attempts by other file handles to call these ioctls/file operations will
> >>> get an EBUSY error.
> >>>
> >>> This file handle remains the owner of the I/O until the file handle is
> >>> closed, or until VIDIOC_REQBUFS is called with count == 0.
> >>
> >> Please change it to use "must". Also, I think that doing s/called/calls/
> >> is better.
> >>
> >> It is likely better to say it as:
> >>
> >> 	The streaming engine ownership must be taken by the first handle that calls
> >> 	VIDIOC_REQBUFS, VIDIOC_CREATE_BUFS, read/write or select (for reading or writing).
> >>
> >> 	Only such file handler must do streaming I/O. Any attempts by other file handlers
> >> 	to do I/O must return EBUSY.
> >>
> >> 	The I/O streaming ownership must be released when the file handler is closed
> >> 	or when VIDIOC_REQBUFS is called with count equal to zero.
> >>
> >>>>
> >>>> Radio Receiver Profile
> >>>> ======================
> >>>>
> >>>> Radio devices have a tuner and (usually) a demodulator. The audio is either
> >>>> send out to a line-out connector or uses ALSA to stream the audio.
> >>
> >> Hmm... are there anything mandatory here, or are you just defining what a radio is?
> >> In the latter, the above text looks ok.
> > 
> > What I tried to achieve here is to limit the number of possible methods of getting
> > the audio to just two: line-out or ALSA. This is to prevent future pvrusb2/ivtv
> > non-standard methods of getting the audio.
> > 
> >>>>
> >>>> It implements VIDIOC_G/S_TUNER, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS.
> >>
> >> 	In addition to the mandatory items at the core profile (I would add an hyperlink here),
> >> 	all drivers must implement VIDIOC_G/S_TUNER, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS.
> >>
> >>>> If hardware seek is supported, then VIDIOC_S_HW_FREQ_SEEK is implemented.
> >>
> >> 	A radio device may implement VIDIOC_S_HW_FREQ_SEEK, when audio station seek is
> >> 	supported by radio.
> >>
> >>>> It does *not* implement VIDIOC_ENUM/G/S_INPUT or VIDIOC_ENUM/G/S_AUDIO.
> >>
> >> A radio node must *not* implement VIDIOC_ENUM/G/S_INPUT or VIDIOC_ENUM/G/S_AUDIO.
> >>
> >>>> If RDS_BLOCK_IO is supported, then read() and poll() are implemented as well.
> >>
> >> Radio drivers that support RDS must report RDS_BLOCK_IO. In this case, radio
> >> nodes must implement read()/poll() syscalls.
> >>
> >>>> If a mute control exists and the audio is output using a line-out, then the
> >>>> audio must be muted on driver load and unload.
> >>>>
> >>>> On driver load the frequency must be initialized to a valid frequency.
> >>
> >> This is not nice, and not always possible: devices with xc3028 tuner (and other
> >> Xceive tuners) have firmwares for radio and for TV. Initializing for radio
> >> makes TV uninitialized, and vice-versa. Also, some devices take up to 30 seconds
> >> to load a firmware (tm6000).
> >>
> >> I think you're likely trying to say that the initial frequency should be
> >> a valid value. If so, please prepend it with a "driver developer's note".
> > 
> > Yes, that's what I tried to say :-)
> 
> Yeah, I figured out ;)
> > 
> > I've seen drivers where G_FREQUENCY would return 0 after the initial module
> > load, which is something that should be avoided.
> 
> Sure.
> 
> >>>>
> >>>> Note: There are a few drivers that use a radio (pvrusb2) or video (ivtv)
> >>>> node to stream the audio. These are legacy exceptions to the rule.
> >>
> >> What an application developer should do with that???
> > 
> > Nothing. A generic application cannot support audio for these devices.
> > You need specialized apps for that.
> 
> We actually need to fix those drivers to do the right thing, or to add this
> possibility to the standard.
> 
> >> If this should not be supported anymore, then we need instead to either
> >> fix the drivers that aren't compliant with the specs or move them to
> >> staging, in order to let them to be either fixed or dropped, if none
> >> cares enough to fix.
> > 
> > Well, the problem is that people are actually using the existing, nonstandard,
> > APIs for ivtv (and probably pvrusb2 as well).
> > 
> > For ivtv it is probably not too difficult to add ALSA support.
> > 
> > Andy, I know you implemented ALSA support for cx18, do you know how much work
> > it would be to do the same for ivtv?
> > 
> > If Andy has absolutely no time, then I can try to find time to add ALSA support
> > to ivtv. But the existing API should probably remain (if possible).
> > 
> > It's probably more work to fix this for pvrusb2, though.
> 
> As I said, the API should be mandatory for all drivers under drivers/media.
> So, either the API specs should add support for the ivtv/pvrusb2 way, or they 
> need to be moved to staging.
> 
> Of course, dropping support for mpeg audio-only on radio/video nodes require
> it to follow the features to be deprecated. But all drivers should at least
> implement everything that it is mandatory at the API profiles, and for sure,
> the way audio IN/OUT will be provided should be properly documented.

Let's wait a year or two before we start taking such drastic measures. For
now the vast majority of drivers fails horribly on v4l2-compliance. It is
even hard to keep vivi compliant! Each time I add new tests to v4l2-compliance
I often uncover a new bug in vivi. Even the new test for the
VIDIOC_G_ENC_INDEX ioctl failed when tested with ivtv: and ivtv is what I
used to design and test this ioctl at the time, and yet is still didn't comply
to the spec I wrote myself! :-)

So things really need to be in better shape all around before we start to
take drastic measures for non-conforming drivers (or parts of drivers).

Regards,

	Hans

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: RFC: Core + Radio profile
  2012-08-22  9:40 RFC: Core + Radio profile Hans Verkuil
  2012-08-22  9:47 ` Hans de Goede
  2012-08-22 10:11 ` Hans Verkuil
@ 2012-08-25  0:37 ` Andy Walls
  2012-08-25  7:21   ` Hans Verkuil
  2 siblings, 1 reply; 14+ messages in thread
From: Andy Walls @ 2012-08-25  0:37 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media

On Wed, 2012-08-22 at 11:40 +0200, Hans Verkuil wrote:
> Hi all!
> 
> The v4l2-compliance utility checks whether device drivers follows the V4L2
> specification. What is missing are 'profiles' detailing what device drivers
> should and shouldn't implement for particular device classes.
>
>This has been discussed several times, but until now no attempt was made to
> actually write this down.
 
Hi Hans,

Excellent!

Profiles of what to implement from a large, complex, option-filled
standard are a great way to increase interoperability.  In our case the
"standard" is the V4L2 specification.  The interoperability we need to
increase is between user-space applications (MythTV, tvtime, VLC,
mplayer, gstreamer, cheese, Ekiga, ZoneMinder, Kaffeine, ....) and the
kernel device drivers.

I know of two viewpoints from which profiles are documented:

1. how an as-built system complies with and implement options from a
stanadrd, and

2. how future systems and systems in development should implement a
standard and its options

For linux-media purposes, profiling from viewpoint #1 is a bit of a
waste: the v4l2-compliance tool essentially auto generates the profiles
plus at least some of the existing user applications are already aware
of driver quirks.

I think documenting the profiles from the 2nd viewpoint is what we want.
(Which is what you are proposing.)

Such profiles should state unequviocally:

1. the optional features from the standard/spec that are required to be
implemented

2. any optional features from the standard/spec that are required *not*
to be implemented

3. the mandatory features from the standard/spec that are expected to be
implemented

4. any mandatory portions of the specification that are specifically not
to be used (Probably only needed for profiles trying to ensure the
broadest interoperability).

Hopefully, specifying the above in profiles removes all the guess-work
about what is to be implemented, so that application <-> kernel driver
combinations will be broadly interoperable.



> This RFC is my attempt to start this process by describing three profiles:
> the core profile that all drivers must adhere to, a profile for a radio
> receiver and a profile for a radio transmitter.

I was thinking that profiles based on applications types might be more
useful, but then I saw that applications were basically already handling
different device types differently.  So prfoiles for hardware device
types seems the reasonable choice.

MythTV seems to care about 4 classes of device
http://www.mythtv.org/wiki/Video_capture_card

	Analog Framebuffer
	Analog Hardware Encoder
	Digital Capture
	Digital Tuner

VLC seems to be similar to MythTV in terms of classes:
http://www.videolan.org/doc/streaming-howto/en/images/global-diagram.jpg

I suppose there would also be profiles to support device classes for:

	webcams
	webcams that provide video in a container format (AVI, MJPEG, whatever)
	integrated cameras (I'm thinking smart-phones, but I'm out of my depth here)


I assume you are designing the Core profile to be a minimum subset
profile upon which all other profiles build.


> Missing in this RFC is a description of how tuner ownership is handled if a
> tuner is shared between a radio and video driver. This will be discussed
> during the workshop next week.

IMO, that is a specification issue, and not an issue to be solved in a
profile.  I have a narrow view the profiles are intended to fix
interoperability problems, and they are not to levy new, unprecedented
requirements on behaviour.


> I am not certain where these profile descriptions should go. Should we add
> this to DocBook, or describe it in Documentation/video4linux? Or perhaps
> somehow add it to v4l2-compliance, since that tool effectively verifies
> the profile.

Don't bury the authoritative profile in a tool, profiles should be
documents easily accessed by implementors.

Interoperability is promoted via clearly documentation requirments for
implementors.  Interoperability is assessed with tools.


> Also note that the core profile description is more strict than the spec. For
> example, G/S_PRIORITY are currently optional, but I feel that all new drivers
> should implement this, especially since it is very easy to add provided you
> use struct v4l2_fh. Similar with respect to the control requirements which
> assume you use the control framework.
> 
> Note that these profiles are primarily meant for driver developers. The V4L2
> specification is leading for application developers.

I would contend profiles are for both groups of developers.

The V4L2 spec tells everyone the realm of possibilities.

Profiles tell people here's how to ensure your driver or application has
the greatest chance of working properly with many different applications
or kernel drivers.


> While writing down these profiles I noticed one thing that was very much
> missing for radio devices: there is no capability bit to tell the application
> whether there is an associated ALSA device or whether the audio goes through
> a line-in/out. Personally I think that this should be fixed.

You didn't say it, but I assume that is a problem you think needs to be
solved in the V4L2 spec.

> Comments are welcome!
> 
> Regards,
> 
> 	Hans
> 
> 
> Core Profile
> ============
> 
> All V4L2 device nodes must support the core profile.
> 
> VIDIOC_QUERYCAP must be supported and must set the device_caps field.
> bus_info must be a unique string and must not be empty (pending result of
> the upcoming workshop).

The "bus_info" string is too open ended.  Nail it down.

The profile needs to tell implementers how to ensure their string is
unique and any other constraints on said string.  Likewise application
developers need to know what to expect from the string.



> VIDIOC_G/S_PRIORITY must be supported.

OK.


> If you have controls then both the VIDIOC_G/S_CTRL and the VIDIOC_G/S/TRY_EXT_CTRLS
> ioctls must be supported, together with poll(), VIDIOC_DQEVENT and
> VIDIOC_(UN)SUBSCRIBE_EVENT to handle control events. Use the control framework
> to implement this.

OK.

> VIDIOC_LOG_STATUS is optional, but recommended for complex drivers.

Not OK.  I would avoid making anything in a profile optional.

Given that the output of LOG_STATUS does not enhance interoperability,
as its output is not readily readable by the calling application, I
recommend dropping mention of LOG_STATUS from the core profile.


> VIDIOC_DBG_G_CHIP_IDENT, VIDIOC_DBG_G/S_REGISTER are optional and are
> primarily used to debug drivers.

Drop mention of these from the Core profile as well.  These can be, and
usually are, compiled out of stock distro kernel with a kernel build
configuration option.

Normal end user applications cannot rely on the being there, so they do
not enhance interoperability.

If you wish to add a Debug profile that builds upon the Core profile,
them these ioctl()s and _LOG_STATUS would fit nicely in that.


> Video, Radio and VBI nodes are for input or output only. The only exception
> are video nodes that have the V4L2_CAP_VIDEO_M2M(_MPLANE) capability set.

Slight wording change:

"Video, Radio and VBI nodes are for data input only or output only; not
both from the same node.  The only exception ..."

Although I thought the V4L2 specification would have been clear on that.


> 
> Video nodes only control video, radio nodes only control radio and RDS, vbi
> nodes only control VBI (raw and/or sliced).

If that is the case, then does the profile need to say something about
multiple open of radio nodes?  (I'm thinking of the legacy /dev/video24
nodes here, which are really interoperabily losers in terms of existing
FM radio apps anyway.)

> Streaming I/O is not supported by radio nodes.


That's all I have for now.  I have to go.  I will comment on other posts
as I have time.

Regards,
Andy W.


> Radio Receiver Profile
> ======================
> 
> Radio devices have a tuner and (usually) a demodulator. The audio is either
> send out to a line-out connector or uses ALSA to stream the audio.
> 
> It implements VIDIOC_G/S_TUNER, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS.
> 
> If hardware seek is supported, then VIDIOC_S_HW_FREQ_SEEK is implemented.
> 
> It does *not* implement VIDIOC_ENUM/G/S_INPUT or VIDIOC_ENUM/G/S_AUDIO.
> 
> If RDS_BLOCK_IO is supported, then read() and poll() are implemented as well.
> 
> If a mute control exists and the audio is output using a line-out, then the
> audio must be muted on driver load and unload.
> 
> On driver load the frequency must be initialized to a valid frequency.
> 
> Note: There are a few drivers that use a radio (pvrusb2) or video (ivtv)
> node to stream the audio. These are legacy exceptions to the rule.
> 
> FM Transmitter Profile
> ======================
> 
> FM transmitter devices have a transmitter and modulator. The audio is
> transferred using ALSA or a line-in connector.
> 
> It implements VIDIOC_G/S_MODULATOR, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS.
> 
> It does *not* implement VIDIOC_ENUM/G/S_OUTPUT or VIDIOC_ENUM/G/S_AUDOUT.
> 
> If RDS_BLOCK_IO is supported, then write() and poll() are implemented
> as well.
> 
> On driver load the frequency must be initialized to a valid frequency.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: RFC: Core + Radio profile
  2012-08-25  0:37 ` Andy Walls
@ 2012-08-25  7:21   ` Hans Verkuil
  2012-08-26 22:15     ` Andy Walls
  0 siblings, 1 reply; 14+ messages in thread
From: Hans Verkuil @ 2012-08-25  7:21 UTC (permalink / raw)
  To: Andy Walls; +Cc: linux-media

On Sat August 25 2012 02:37:15 Andy Walls wrote:
> On Wed, 2012-08-22 at 11:40 +0200, Hans Verkuil wrote:
> > Hi all!
> > 
> > The v4l2-compliance utility checks whether device drivers follows the V4L2
> > specification. What is missing are 'profiles' detailing what device drivers
> > should and shouldn't implement for particular device classes.
> >
> >This has been discussed several times, but until now no attempt was made to
> > actually write this down.
>  
> Hi Hans,
> 
> Excellent!
> 
> Profiles of what to implement from a large, complex, option-filled
> standard are a great way to increase interoperability.  In our case the
> "standard" is the V4L2 specification.  The interoperability we need to
> increase is between user-space applications (MythTV, tvtime, VLC,
> mplayer, gstreamer, cheese, Ekiga, ZoneMinder, Kaffeine, ....) and the
> kernel device drivers.
> 
> I know of two viewpoints from which profiles are documented:
> 
> 1. how an as-built system complies with and implement options from a
> stanadrd, and
> 
> 2. how future systems and systems in development should implement a
> standard and its options
> 
> For linux-media purposes, profiling from viewpoint #1 is a bit of a
> waste: the v4l2-compliance tool essentially auto generates the profiles
> plus at least some of the existing user applications are already aware
> of driver quirks.
> 
> I think documenting the profiles from the 2nd viewpoint is what we want.
> (Which is what you are proposing.)
> 
> Such profiles should state unequviocally:
> 
> 1. the optional features from the standard/spec that are required to be
> implemented
> 
> 2. any optional features from the standard/spec that are required *not*
> to be implemented
> 
> 3. the mandatory features from the standard/spec that are expected to be
> implemented
> 
> 4. any mandatory portions of the specification that are specifically not
> to be used (Probably only needed for profiles trying to ensure the
> broadest interoperability).
> 
> Hopefully, specifying the above in profiles removes all the guess-work
> about what is to be implemented, so that application <-> kernel driver
> combinations will be broadly interoperable.
> 
> 
> 
> > This RFC is my attempt to start this process by describing three profiles:
> > the core profile that all drivers must adhere to, a profile for a radio
> > receiver and a profile for a radio transmitter.
> 
> I was thinking that profiles based on applications types might be more
> useful, but then I saw that applications were basically already handling
> different device types differently.  So prfoiles for hardware device
> types seems the reasonable choice.
> 
> MythTV seems to care about 4 classes of device
> http://www.mythtv.org/wiki/Video_capture_card
> 
> 	Analog Framebuffer
> 	Analog Hardware Encoder
> 	Digital Capture
> 	Digital Tuner
> 
> VLC seems to be similar to MythTV in terms of classes:
> http://www.videolan.org/doc/streaming-howto/en/images/global-diagram.jpg
> 
> I suppose there would also be profiles to support device classes for:
> 
> 	webcams
> 	webcams that provide video in a container format (AVI, MJPEG, whatever)
> 	integrated cameras (I'm thinking smart-phones, but I'm out of my depth here)

Looking at the current set of V4L drivers I come up with the following
profiles:

- Core Profile (mandatory for all)
- Radio Receiver Profile (with optional RDS support (sub-profile?))
- Radio Transmitter Profile (with optional RDS support)
- Webcam Profile
- Video Capture Profile (with optional tuner/overlay/vbi support)
- Video Output Profile (with optional vbi support)
- Memory-to-Memory Profile (for hardware codecs)
- Complex Profile (for SoCs with complex video hardware requiring the use
  of the Media Controller API and a supporting library)

I wonder if e.g. optional tuner support and similar optional features
should be defined as full profiles or as sub-profiles.

In theory it is possible to have, say, a device that just captures VBI and
no video. So there is something to be said for making it a full profile
and allow drivers to support multiple profiles (although not all combinations
are allowed).

> 
> 
> I assume you are designing the Core profile to be a minimum subset
> profile upon which all other profiles build.

Yes.

> 
> 
> > Missing in this RFC is a description of how tuner ownership is handled if a
> > tuner is shared between a radio and video driver. This will be discussed
> > during the workshop next week.
> 
> IMO, that is a specification issue, and not an issue to be solved in a
> profile.  I have a narrow view the profiles are intended to fix
> interoperability problems, and they are not to levy new, unprecedented
> requirements on behaviour.
> 
> 
> > I am not certain where these profile descriptions should go. Should we add
> > this to DocBook, or describe it in Documentation/video4linux? Or perhaps
> > somehow add it to v4l2-compliance, since that tool effectively verifies
> > the profile.
> 
> Don't bury the authoritative profile in a tool, profiles should be
> documents easily accessed by implementors.
> 
> Interoperability is promoted via clearly documentation requirments for
> implementors.  Interoperability is assessed with tools.
> 
> 
> > Also note that the core profile description is more strict than the spec. For
> > example, G/S_PRIORITY are currently optional, but I feel that all new drivers
> > should implement this, especially since it is very easy to add provided you
> > use struct v4l2_fh. Similar with respect to the control requirements which
> > assume you use the control framework.
> > 
> > Note that these profiles are primarily meant for driver developers. The V4L2
> > specification is leading for application developers.
> 
> I would contend profiles are for both groups of developers.
> 
> The V4L2 spec tells everyone the realm of possibilities.
> 
> Profiles tell people here's how to ensure your driver or application has
> the greatest chance of working properly with many different applications
> or kernel drivers.
> 
> 
> > While writing down these profiles I noticed one thing that was very much
> > missing for radio devices: there is no capability bit to tell the application
> > whether there is an associated ALSA device or whether the audio goes through
> > a line-in/out. Personally I think that this should be fixed.
> 
> You didn't say it, but I assume that is a problem you think needs to be
> solved in the V4L2 spec.

Yes.

> 
> > Comments are welcome!
> > 
> > Regards,
> > 
> > 	Hans
> > 
> > 
> > Core Profile
> > ============
> > 
> > All V4L2 device nodes must support the core profile.
> > 
> > VIDIOC_QUERYCAP must be supported and must set the device_caps field.
> > bus_info must be a unique string and must not be empty (pending result of
> > the upcoming workshop).
> 
> The "bus_info" string is too open ended.  Nail it down.
> 
> The profile needs to tell implementers how to ensure their string is
> unique and any other constraints on said string.  Likewise application
> developers need to know what to expect from the string.
> 
> 
> 
> > VIDIOC_G/S_PRIORITY must be supported.
> 
> OK.
> 
> 
> > If you have controls then both the VIDIOC_G/S_CTRL and the VIDIOC_G/S/TRY_EXT_CTRLS
> > ioctls must be supported, together with poll(), VIDIOC_DQEVENT and
> > VIDIOC_(UN)SUBSCRIBE_EVENT to handle control events. Use the control framework
> > to implement this.
> 
> OK.
> 
> > VIDIOC_LOG_STATUS is optional, but recommended for complex drivers.
> 
> Not OK.  I would avoid making anything in a profile optional.
> 
> Given that the output of LOG_STATUS does not enhance interoperability,
> as its output is not readily readable by the calling application, I
> recommend dropping mention of LOG_STATUS from the core profile.
> 
> 
> > VIDIOC_DBG_G_CHIP_IDENT, VIDIOC_DBG_G/S_REGISTER are optional and are
> > primarily used to debug drivers.
> 
> Drop mention of these from the Core profile as well.  These can be, and
> usually are, compiled out of stock distro kernel with a kernel build
> configuration option.
> 
> Normal end user applications cannot rely on the being there, so they do
> not enhance interoperability.
> 
> If you wish to add a Debug profile that builds upon the Core profile,
> them these ioctl()s and _LOG_STATUS would fit nicely in that.

A Debug Profile sounds good.

> > Video, Radio and VBI nodes are for input or output only. The only exception
> > are video nodes that have the V4L2_CAP_VIDEO_M2M(_MPLANE) capability set.
> 
> Slight wording change:
> 
> "Video, Radio and VBI nodes are for data input only or output only; not
> both from the same node.  The only exception ..."
> 
> Although I thought the V4L2 specification would have been clear on that.
> 
> 
> > 
> > Video nodes only control video, radio nodes only control radio and RDS, vbi
> > nodes only control VBI (raw and/or sliced).
> 
> If that is the case, then does the profile need to say something about
> multiple open of radio nodes?  (I'm thinking of the legacy /dev/video24
> nodes here, which are really interoperabily losers in terms of existing
> FM radio apps anyway.)

I made a follow-up to this profile that adds that.

Regards,

	Hans

> > Streaming I/O is not supported by radio nodes.
> 
> 
> That's all I have for now.  I have to go.  I will comment on other posts
> as I have time.
> 
> Regards,
> Andy W.
> 
> 
> > Radio Receiver Profile
> > ======================
> > 
> > Radio devices have a tuner and (usually) a demodulator. The audio is either
> > send out to a line-out connector or uses ALSA to stream the audio.
> > 
> > It implements VIDIOC_G/S_TUNER, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS.
> > 
> > If hardware seek is supported, then VIDIOC_S_HW_FREQ_SEEK is implemented.
> > 
> > It does *not* implement VIDIOC_ENUM/G/S_INPUT or VIDIOC_ENUM/G/S_AUDIO.
> > 
> > If RDS_BLOCK_IO is supported, then read() and poll() are implemented as well.
> > 
> > If a mute control exists and the audio is output using a line-out, then the
> > audio must be muted on driver load and unload.
> > 
> > On driver load the frequency must be initialized to a valid frequency.
> > 
> > Note: There are a few drivers that use a radio (pvrusb2) or video (ivtv)
> > node to stream the audio. These are legacy exceptions to the rule.
> > 
> > FM Transmitter Profile
> > ======================
> > 
> > FM transmitter devices have a transmitter and modulator. The audio is
> > transferred using ALSA or a line-in connector.
> > 
> > It implements VIDIOC_G/S_MODULATOR, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS.
> > 
> > It does *not* implement VIDIOC_ENUM/G/S_OUTPUT or VIDIOC_ENUM/G/S_AUDOUT.
> > 
> > If RDS_BLOCK_IO is supported, then write() and poll() are implemented
> > as well.
> > 
> > On driver load the frequency must be initialized to a valid frequency.
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-media" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: RFC: Core + Radio profile
  2012-08-25  7:21   ` Hans Verkuil
@ 2012-08-26 22:15     ` Andy Walls
  0 siblings, 0 replies; 14+ messages in thread
From: Andy Walls @ 2012-08-26 22:15 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media

Hi Hans,

On Sat, 2012-08-25 at 09:21 +0200, Hans Verkuil wrote:
> On Sat August 25 2012 02:37:15 Andy Walls wrote:
> > On Wed, 2012-08-22 at 11:40 +0200, Hans Verkuil wrote:
 
> > > This RFC is my attempt to start this process by describing three profiles:
> > > the core profile that all drivers must adhere to, a profile for a radio
> > > receiver and a profile for a radio transmitter.
> > 
> > I was thinking that profiles based on applications types might be more
> > useful, but then I saw that applications were basically already handling
> > different device types differently.  So prfoiles for hardware device
> > types seems the reasonable choice.
> > 
> > MythTV seems to care about 4 classes of device
> > http://www.mythtv.org/wiki/Video_capture_card
> > 
> > 	Analog Framebuffer
> > 	Analog Hardware Encoder
> > 	Digital Capture
> > 	Digital Tuner
> > 
> > VLC seems to be similar to MythTV in terms of classes:
> > http://www.videolan.org/doc/streaming-howto/en/images/global-diagram.jpg
> > 
> > I suppose there would also be profiles to support device classes for:
> > 
> > 	webcams
> > 	webcams that provide video in a container format (AVI, MJPEG, whatever)
> > 	integrated cameras (I'm thinking smart-phones, but I'm out of my depth here)
> 
> Looking at the current set of V4L drivers I come up with the following
> profiles:
> 
> - Core Profile (mandatory for all)
> - Radio Receiver Profile (with optional RDS support (sub-profile?))

I would avoid the use of the word "optional" in a profile.

I would imagine the profile would say something like: "If the device and
driver support RDS, then applications can determine this by [some method
consistent with the V4L2 spec] and the driver will implement the
following ioctl()s: [...]; and provide data in the following manner:
[...]"

So nothing is really optional, if the device and driver can support RDS.

That way you also don't get a proliferation of profiles - Radio Receiver
with RDS and also Radio Receiver w/o RDS - or subprofiles. (As a group,
numerous profiles begin to create the option inconsistency problem all
over again.) 

Again, I'm coming from the viewpoint that every "option" holds potential
for an application <-> driver interoperability problem.  Profiles need
to set clear expectations for both applications and driver.  Stating
that something is optional in a profile is undersireable, IMO.


> - Radio Transmitter Profile (with optional RDS support)
> - Webcam Profile
> - Video Capture Profile (with optional tuner/overlay/vbi support)
> - Video Output Profile (with optional vbi support)

Ditto, regarding "optional".


> - Memory-to-Memory Profile (for hardware codecs)
> - Complex Profile (for SoCs with complex video hardware requiring the use
>   of the Media Controller API and a supporting library)
> 
> I wonder if e.g. optional tuner support and similar optional features
> should be defined as full profiles or as sub-profiles.

I would suggest you avoid a proliferation of profiles.  Fewer profiles
go farther in enhancing application <-> driver inetroperability.

I like your list above.  Using your example of tuner support, you would
include that in the Video Capture Profile.  The language I would use is
again something like:

"If the device has a supported analog broadcast TV tuner, then
applications can detect this by [some method consistent with the V4L2
spec], and the following ioctl()s must be supported: [...].


> In theory it is possible to have, say, a device that just captures VBI and
> no video. So there is something to be said for making it a full profile
> and allow drivers to support multiple profiles (although not all combinations
> are allowed).

IMO, Combinations of profiles, just creates uncertainty and thus
interoperability problems.

My first inclination would be to just handle a VBI-only capture device
under the Video Capture profile, with a properly worded profile.

There is probably a similar situation with Video Capture devices that
output raw frames vs. video capture devices that output conatiner
formats (MPEG PS, MPEG TS, AVI).  I think that can be handled in a
single Video Capture profile as well.



> > > I am not certain where these profile descriptions should go. Should we add
> > > this to DocBook, or describe it in Documentation/video4linux? Or perhaps
> > > somehow add it to v4l2-compliance, since that tool effectively verifies
> > > the profile.
> > 
> > Don't bury the authoritative profile in a tool, profiles should be
> > documents easily accessed by implementors.
> > 
> > Interoperability is promoted via clearly documentation requirments for
> > implementors.  Interoperability is assessed with tools.

I would like to temper my above statements.

Given the reality of man-power to write documents for Open Source
software, my above recommendations might not be realistic.  I've been
living in a "big, up-front design" world lately.

Also Steven Toth's Viewcast Osprey PULL request has shown how a good
tool can help speed compliance after the fact.  (Although good profile
documents may have prevented the back & forth in the first place.)

 

> > > Core Profile
> > > ============
 

> > > VIDIOC_LOG_STATUS is optional, but recommended for complex drivers.
> > 
> > Not OK.  I would avoid making anything in a profile optional.
> > 
> > Given that the output of LOG_STATUS does not enhance interoperability,
> > as its output is not readily readable by the calling application, I
> > recommend dropping mention of LOG_STATUS from the core profile.
> > 
> > 
> > > VIDIOC_DBG_G_CHIP_IDENT, VIDIOC_DBG_G/S_REGISTER are optional and are
> > > primarily used to debug drivers.
> > 
> > Drop mention of these from the Core profile as well.  These can be, and
> > usually are, compiled out of stock distro kernel with a kernel build
> > configuration option.
> > 
> > Normal end user applications cannot rely on the being there, so they do
> > not enhance interoperability.
> > 
> > If you wish to add a Debug profile that builds upon the Core profile,
> > them these ioctl()s and _LOG_STATUS would fit nicely in that.
> 
> A Debug Profile sounds good.

I'm having second thoughts on this.

The only real advantage I can see is that we could recommend to
application developers to test against drivers that support the Debug
profile, to enhance troubleshooting, support requests, and bug reports.

Given a previous email I saw from you, I guess supporting USERPTR would
also be a good thing to add to a Debug profile.


> > > 
> > > Video nodes only control video, radio nodes only control radio and RDS, vbi
> > > nodes only control VBI (raw and/or sliced).
> > 
> > If that is the case, then does the profile need to say something about
> > multiple open of radio nodes?  (I'm thinking of the legacy /dev/video24
> > nodes here, which are really interoperabily losers in terms of existing
> > FM radio apps anyway.)
> 
> I made a follow-up to this profile that adds that.
> 
> Regards,
> 
> 	Hans


Regards,
Andy



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: RFC: Core + Radio profile
  2012-08-24 12:31     ` Hans Verkuil
  2012-08-24 14:51       ` Mauro Carvalho Chehab
@ 2012-08-26 22:56       ` Andy Walls
  1 sibling, 0 replies; 14+ messages in thread
From: Andy Walls @ 2012-08-26 22:56 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: Mauro Carvalho Chehab, linux-media, Mike Isely

On Fri, 2012-08-24 at 14:31 +0200, Hans Verkuil wrote:
> Hi Mauro,
> 
> Thanks for your review!
> 
> On Wed August 22 2012 15:42:26 Mauro Carvalho Chehab wrote:
> > Em 22-08-2012 07:11, Hans Verkuil escreveu:


> > >> Also note that the core profile description is more strict than the spec.
> > 
> > IMO, that's the right approach: The specs should define the several API
> > aspects; the profile should mandate what's optional and what's mandatory.
> > 
> > >> For example, G/S_PRIORITY are currently optional,
> > 
> > After putting the profiles there, we should remove "optional" tags elsewhere,
> > as the profiles section will tell what's mandatory and what is optional, as
> > different profiles require different mandatory arguments.
> > 
> > >> but I feel that all new drivers should implement this, especially since it
> > >> is very easy to add provided you use struct v4l2_fh. 
> > 
> > I agree on making G/S_PRIORITY mandatory.

 
> > >> Note that these profiles are primarily meant for driver developers. The V4L2
> > >> specification is leading for application developers.
> > 
> > This is where we diverge ;) We need profiles primary for application developers,
> > for them to know what is expected to be there on any media driver of a certain
> > kind.

+1

> > Ok, internal driver-developer profiles is also needed, in order for them to
> > use the right internal API's and internal core functions.
> 
> Well, it is all very nice to just say e.g. 'G/S_PRIORITY is mandatory' in the
> profile section, but the reality is that it is hit and miss whether or not it
> is implemented. Even if we manage to convert all drivers to implement G/S_PRIO,
> then it is still not something an app developer can rely on because many users
> use older kernel versions.


A profile should either
a. make no statement about G/S_PRIORITY,
b. should make them mandatory, or
c. should state explicitly they should not be implemented.

The V4L2 spec already implies they are optional:

"... V4L2 defines the VIDIOC_G_PRIORITY and VIDIOC_S_PRIORITY ioctls to
request and query the access priority associate with a file descriptor.
Opening a device assigns a medium priority, compatible with earlier
versions of V4L2 and drivers not supporting these ioctls."
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

If the profile make them mandatory, legacy driver non-compliance with
this should be reported.  Better still, a driver should not be claimed
to be profile compliant until the non-compliance is fixed.

We are not going to have the drivers compliant with profiles all at
once.

In the long view, applications evolve too.  As drivers move to comply
with profiles, and old kernel versions are dropped from main stream
distros, application writers can get rid of complex code to handle V4L2
quirks and exceptions.


> Which is why IMHO the spec should be leading for app development. The profile
> sections are a handy summary of what you can expect from drivers for specific
> types of hardware, but it can't be leading except when it comes to new driver
> development or driver changes in areas covered by the profile(s).

I disagree here.  I guess that is obvious by now. ;)



> > >>
> > >> Note: There are a few drivers that use a radio (pvrusb2) or video (ivtv)
> > >> node to stream the audio. These are legacy exceptions to the rule.
> > 
> > What an application developer should do with that???
> 
> Nothing. A generic application cannot support audio for these devices.
> You need specialized apps for that.
> 
> > If this should not be supported anymore, then we need instead to either
> > fix the drivers that aren't compliant with the specs or move them to
> > staging, in order to let them to be either fixed or dropped, if none
> > cares enough to fix.
> 
> Well, the problem is that people are actually using the existing, nonstandard,
> APIs for ivtv (and probably pvrusb2 as well).

Please note that the profile need not prohibit the legacy radio audio
API.  That way the legacy drivers can be compliant with the profile
without removing existing functionality.

It will be sufficient for the profile to require all drivers to
implement the ALSA interface.  No sane person is going to work to
inplement the legacy oddball audio interface in a driver after the ALSA
interface is done.


> For ivtv it is probably not too difficult to add ALSA support.
> 
> Andy, I know you implemented ALSA support for cx18, do you know how much work
> it would be to do the same for ivtv?

I implemented the cx18-alsa-* skeleton; DEvin actually got it hooked up
to ALSA and working.  There are still some aspects of it that are dead
code - i.e. the ALSA controls for volume - even though the skeleton has
them.


> If Andy has absolutely no time, then I can try to find time to add ALSA support
> to ivtv. But the existing API should probably remain (if possible).

The cx18-alsa-* code and glue is certainly copyable to ivtv.  I might
suggest one difference: don't make ivtv-alsa it's own module.  Just
build it in to the ivtv driver and reduce some module-glue complexity.
ALSA audio interface is going to be mandated by the profile anyway, so
there really isn't much point in a modular implemetation,

I might have time to do this.  But that is probably an optimistic
statement too.

> It's probably more work to fix this for pvrusb2, though.

Agree.

> > --
> > 
> > That's said, I think that the RFC proposal is going on the right direction.
> > It is very good to see it moving forward!

Ditto.

Regards,
Andy



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2012-08-26 22:57 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-22  9:40 RFC: Core + Radio profile Hans Verkuil
2012-08-22  9:47 ` Hans de Goede
2012-08-22 10:11 ` Hans Verkuil
2012-08-22 13:42   ` Mauro Carvalho Chehab
2012-08-22 15:19     ` Mike Isely
2012-08-22 18:09       ` Mauro Carvalho Chehab
2012-08-23 11:12         ` Andy Walls
2012-08-24 12:31     ` Hans Verkuil
2012-08-24 14:51       ` Mauro Carvalho Chehab
2012-08-24 15:35         ` Hans Verkuil
2012-08-26 22:56       ` Andy Walls
2012-08-25  0:37 ` Andy Walls
2012-08-25  7:21   ` Hans Verkuil
2012-08-26 22:15     ` Andy Walls

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.