[Ksummit-2012-discuss] Media Workshop notes - part 1

I finished reviewing the etherpad notes for the first 3 topics from the
media workshop discussions:
	- V4L2 Ambiguities
	- V4L2 compliance
	- V4L2 and ALSA

Those themes covers what it was discussed during the morning. 
We ended by not having "ARM and needed features for V4L/DVB" discussions there, due
to the lack of ARM people in the room. We also moved the Media Controller library
discussions for the afternoon, as the above themes are the ones that required more
work and are the ones that have more impact at the Kernel subsystem.

I probably won't have time to post the review of the afternoon topics before 
the end of the next week. Anyway, I'll post here the results of the remaining
sessions when we finish reviewing the taken Etherpad notes.

Regards,
Mauro

---

1. V4L2 Ambiguities

The first topic is with regards to V4L2 API ambiguities that need to be addressed, in order to
avoid inconsistent behavior between drivers.
1.1. Split off controls
Split off the control part from videodev2.h. Controls are almost 30% of videodev2.h (and growing).
Maintaining controls would be easier if they are moved to e.g. linux/v4l2-controls.h which is
included by videodev2.h
    
Results: OK, split them from videodev2.h to v4l2-controls.h

1.2. Driver-specific controls

Should driver-specific controls have a unique CID, or can the CIDs overlap ?

Currently there are three types of controls: standard controls, controls that are specific to a
chipset (e.g. cx2341x, mfc51) and driver-specific controls. The controls of the first two types
have well defined and unique IDs. For driver-specific controls however there are no clear rules.

It all depends on one question: should driver-specific controls have a unique control ID as well,
or can they overlap with other drivers?

If the answer is that they should be unique as well, then all driver-specific controls will have to
be defined in a single header so you can be certain that there is no overlap.

Results:

New controls should not overlap.
Having all driver-specific controls in a single header file would probably be overkill.
 We can instead reserve a range of CIDs for each driver, and define the range base CID only in a
common header file. 
Driver-specific CIDs themselves would be defined in driver-specific headers.

1.3. STREAMON/STREAMOFF

What should VIDIOC_STREAMON/OFF do if the stream is already started/stopped? I believe they should
do nothing and just return 0. The main reason for that is it can be useful if an application can
just call VIDIOC_STREAMOFF without having to check whether streaming is in progress.
Results: STREAMON/STREAMOFF should return success (0) if the stream is already started/stopped, to
avoid potentially breaking existing applications.

1.4. Unsupported formats in TRY_FMT/S_FMT

What should a driver return in TRY_FMT/S_FMT if the requested format is not supported (possible
behaviors include returning the currently selected format or a default format).
The spec says this: "Drivers should not return an error code unless the input is ambiguous", but it
does not explain what constitutes an ambiguous input. In my opinion TRY/S_FMT should never return
an error other than EINVAL (if the buffer type is unsupported) or EBUSY (for S_FMT if streaming is
in progress).
Should we make a recommendation whether the currently selected format or a default format should be
returned?
One proposal is to just return a default format if the requested pixelformat is unsupported.
Returning the currently selected format leads to inconsistent results.

Results:
TRY_FMT/S_FMT should never return an error when the requested format is not supported. Drivers
should always return a valid format, preferably a format that is as widely supported by
applications as possible.
Both TRY_FMT and S_FMT should have the same behaviour. Drivers should not return different formats
when getting the same input parameters.
The format returned should be a driver default format if possible (stateless behaviour) but can be
stateful if needed.
The API spec should let clear that format retuns may be different when different video inputs (or
outputs) are selected.

1.5. Empty VIDIOC_QUERYCAP bus_info

VIDIOC_QUERYCAP allows bus_info to be empty. Since the purpose of bus_info is to distinguish
multiple identical devices this makes no sense. I propose to make the spec more strict and require
that bus_info is always filled in with a unique string.

Results: bus_info should never be empty. The spec should specify the syntax of the field contents
for common busses, like PCI e USB.

1.6. Deprecate V4L2_BUF_TYPE_PRIVATE

None of the kernel drivers use it, and I cannot see any good use-case for this. If some new type of
buffer is needed, then that should be added instead of allowing someone to abuse this buffer type.

Results: Not used upstream, should be deprecated.

1.7. Buftype checks against node type

A driver that has both a video node and a vbi node using the same struct v4l2_ioctl_ops for both
nodes will have to check in e.g. vidioc_g_fmt_vid_cap or vidioc_g_fmt_vbi_cap whether it is called
from the correct node (video or vbi) and return an error if it isn't.
Should such checks be added to the V4L2 core? And if so, should we add some additional VFL types?
Currently we have GRABBER (video nodes), VBI, RADIO and SUBDEV. But if we want to do proper core
checks, then we would also need OUTPUT, VBI_OUT and M2M.
We could also refuse ioctls based on the VFL_TYPE: e.g. VIDIOC_ENCODER_CMD makes no sense for
radio/vbi nodes. Easy to implement with the lookup table.

Results: It should first be needed to check whether this could break applications, before enforcing
it at the core.

1.8. Remove experimental tag from old drivers

Remove the experimental tag from the following old drivers:
        VIDEO_TLV320AIC23B
        USB_STKWEBCAM
        VIDEO_CX18
        VIDEO_CX18_ALSA
        VIDEO_ZORAN_AVS6EYES
        DVB_USB_AF9005
        MEDIA_TUNER_TEA5761
Removing this tag from these drivers might be too soon, though:
        VIDEO_NOON010PC30
        VIDEO_OMAP3

Results: 
Remove the tag from the first set of old drivers. 
It is too soon for VIDEO_OMAP3.
With regards to VIDEO_NOON010PC30, it was marked as experimental only because it
used VIDEO_V4L2_SUBDEV_API, which in turn depends on EXPERIMENTAL.
Otherwise it's pretty standard and simple sensor driver. Hence the experimental tag could
probably just be removed.

1.9. Remove ioctl's experimental tag

Remove the experimental tag from the V4L2 API elements:
Video Output Overlay (OSD) Interface (from the section “Video Output Overlay Interface”).
V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY.
V4L2_CAP_VIDEO_OUTPUT_OVERLAY.
VIDIOC_ENUM_FRAMESIZES and VIDIOC_ENUM_FRAMEINTERVALS ioctls.
VIDIOC_G_ENC_INDEX ioctl.
VIDIOC_ENCODER_CMD and VIDIOC_TRY_ENCODER_CMD ioctls.
VIDIOC_DECODER_CMD and VIDIOC_TRY_DECODER_CMD ioctls.

Results: Remove experimental tag from listed ioctls.

1.10. Wrong timings API use for input

What should VIDIOC_G_STD/DV_PRESET/DV_TIMINGS return if the current input or output does not
support that particular timing approach? EINVAL? ENODATA?
This is relevant for the case where a driver has multiple inputs/outputs where some are SDTV (and
support the STD API) and others are HDTV (and support the DV_TIMINGS API).
I propose ENODATA.

Results: EINVAL is not a good error code, ENODATA is a better possibility. Let's stick with the
latest one.

1.11. New defines for DV timing API

Add these new defines:
#define V4L2_IN_CAP_TIMINGS V4L2_IN_CAP_CUSTOM_TIMINGS
#define V4L2_OUT_CAP_TIMINGS V4L2_OUT_CAP_CUSTOM_TIMINGS
Since DV_TIMINGS is now used for any HDTV timings and no longer just for custom, non-standard
timings, the word "CUSTOM" is no longer appropriate.
Results: 
Rename them to V4L2_IN_CAP_TIMINGS and V4L2_OUT_CAP_TIMINGS
Turn V4L2_(IN|OUT)_CAP_CUSTOM_TIMINGS into aliases for V4L2_(IN|OUT)_CAP_TIMINGS
Deprecate V4L2_(IN|OUT)_CAP_CUSTOM_TIMINGS.

1.12. Sequence and timestamp for output

What should video output drivers do with the sequence and timestamp fields when they return a
v4l2_buffer from VIDIOC_DQBUF?
I think the spec is clear with respect to the timestamp: "The driver stores the time at which the
first data byte was actually sent out in the timestamp field." For sequence the spec just says:
"Set by the driver, counting the frames in the sequence."
So I think that output drivers should indeed set both sequence and timestamp.
Results:
For capture and output, the behaviour is OK, the documentation should be clarified.
For mem2mem, we need to specify the behaviour. Timestamp are not actually needed, but it is needed
a way to match buffers on both sides, as the codec can reorder them, and timestamps, together with
sequence numbers are used for it. An RFC should be sent to the list.

1.13. Const arg for write-only ioctls

Make the argument of write-only ioctls const in v4l2-ioctls.h. This makes it obvious to drivers
that they shouldn't change the contents of the input struct since it won't make it back to
userspace. It also simplifies v4l2-ioctl.c since it can rely on the fact that after the ioctl call
the contents of the struct hasn't changed. Right now the struct contents is logged (if debugging is
on) before the ioctl call for write-only ioctls.
Results: Yes.

1.14. How to set the timestamp?

What is the right/best way to set the timestamp? The spec says gettimeofday, but is it my
understanding that ktime_get_ts is much more efficient. Some drivers are already using
ktime_get_ts.
Options:
all drivers must comply to the spec and use gettimeofday
we change the spec and all drivers must use the more efficient ktime_get_ts
we add a buffer flag V4L2_BUF_FLAG_MONOTONIC to tell userspace that a monotonic clock like
ktime_get_ts is used and all drivers that use ktime_get_ts should set that flag.
If we go for c, then we should add a recommendation to use one or the other as the preferred
timestamp for new drivers.

Results:
ktime_get_ts() should be used.
Change the spec, and require all drivers to be fixed. 
A new driver capability flag is needed to tell applications that the driver uses the monotonic
clock.

1.15. Device specified clock

Some devices provide their own timestamp in device clock units. How should that be passed on to
userspace?
Proposal: extend the struct v4l2_timecode in v4l2_buffer, either by just adding a new type, or by
creating an anonymous union.
Currently there are no drivers that use v4l2_timecode.
It is questionable for v4l2_timecode to be in v4l2_buffer at all since such information will
typically be carried over VBI.

Results: Reuse v4l2_timecode to pass the device clock timestamp to userspace (making it an
anonymous union is an option, as it would allow different kind of timestamps later) and add a
capability flag to inform applications.

1.16. V4L2_CAP_VIDEO_CAPTURE

If a driver supports only formats with more than one plane, should V4L2_CAP_VIDEO_CAPTURE still be
defined? And if a driver also supports single-plane formats in addition to >1 plane formats, should
V4L2_CAP_VIDEO_CAPTURE be compulsary?
The consensus seems to be 'No' and 'No'. The CAPs refer to whether the single and/or multiplanar
API is implemented, not whether single and/or multiplanar formats are available.

Results: Flags should reflect what it is visible/usable by the userspace API; reviewing v4l-utils
with that regards may be needed, due to emulation code that could be there.

1.17. V4L2_CROPCAP

VIDIOC_CROPCAP: the spec says that CROPCAP must be implemented by all capture and output devices
(Section "Image Cropping, Inserting and Scaling"). In reality only a subset of the drivers support
cropcap.
Should cropcap really be compulsory? Or only for drivers that can scale? And in that case, should
we make a default implementation for those drivers that do not support it? (E.g.: call g_fmt and
use the width/height as the default and bounds rectangles, and set the pixel aspect to 1/1)

Results: the ioctls should not be mandatory if the driver doesn't support crop or scale.

1.18. Pixel aspect ratio

Pixel aspect: currently this is only available through VIDIOC_CROPCAP. It never really belonged to
VIDIOC_CROPCAP IMHO. It's just not a property of cropping/composing. It really belongs to the
input/output timings (STD or DV_TIMINGS). That's where the pixel aspect ratio is determined.
While it is possible to add it to the dv_timings struct, I see no way of cleanly adding it to
struct v4l2_standard (mostly because VIDIOC_ENUMSTD is now handled inside the V4L2 core and doesn't
call the drivers anymore).
An alternative is to add it to struct v4l2_input/output, but I don't know if it is possible to
defined a pixelaspect for inputs that are not the current input.
What I am thinking of is just to add a new ioctl for this VIDIOC_G_PIXELASPECT.

Results: TBD on the mailing list.

1.19. Tuner ownership

How to handle tuner ownership if both a video and radio node share the same tuner?
Obvious rules:
Calling S_FREQ, S_TUNER, S_MODULATOR or S_HW_FREQ_SEEK will make the filehandle the owner if
possible. EBUSY is returned if someone else owns the tuner and you would need to switch the tuner
mode.
Ditto for ioctls that expect a valid tuner configuration like QUERYSTD. This is likely to be driver
dependent, though.
Just opening a device node should not switch ownership
But it is not clear what to do when any of these ioctls are called:
G_FREQUENCY: could just return the last set frequency for radio or TV: requires that that is
remembered when switching ownership. This is what happens today, so G_FREQUENCY does not have to
switch ownership.
G_TUNER: the rxsubchans, signal and afc fields all require ownership of the tuner. So in principle
you would want to switch ownership when G_TUNER is called. On the other hand, that would mean that
calling v4l2-ctl --all -d /dev/radio0 would change tuner ownership to radio for /dev/video0. That's
rather unexpected.It is possible to just set rxsubchans, signal and afc to 0 if the device node
doesn't own the tuner. I'm inclined to do that.
Should closing a device node switch ownership? E.g. if nobody has a radio device open, should the
tuner switch back to TV mode automatically? I don't think it should.
How about hybrid tuners?

Results: TBD on the mailing list (or later this week).

2. V4L2 Compliance

An automated tool to test whether a driver complies with the spec was written, in order to allow
being sure that drivers will match the V4L2 spec.

Known missing tests at V4L2 compliance:
Cropping: VIDIOC_CROPCAP, VIDIOC_G/S_CROP, VIDIOC_G/S_SELECTION
Framebuffer: VIDIOC_S_FBUF, OVERLAY
Encoder/Decoder: VIDIOC_(TRY_)ENCODER_CMD, VIDIOC_(TRY_)DECODER_CMD, VIDIOC_G_ENC_INDEX
Streaming: VIDIOC_QBUF, DQBUF, QUERYBUF, PREPARE_BUFS, VIDIOC_STREAMON/OFF
Subdev API, MC API (but these are possibly out-of-scope anyway)

Extra tests – It requires:

G/S_PRIORITY
Normal and extended controls
Control events
videobuf2 (or an own video buffers implementation that is spec-compliant, videobuf1 isn"t)

Results:
Passing v4l2-compliance tests will be compulsory for new drivers under drivers/media.
The enforcement of the extra tests are ok. However, the usage of videobuf2 can't be mandatory yet
for all drivers, as VB2 currently lacks support for videobuf-dvb. So, hybrid drivers may use VB1
instead, while this is not fixed on VB2.
Lots of work is required to make existing drivers compliant; as almost all fails, but merging newer
drivers without passing with the tests would mean further re-work, with should really be avoided.
In order for it to work, it would also require that new tests should be added whenever a new API
appears.
The V4L2 streaming profile needs to be implemented to support complex MC drivers in
v4l2-compliance.

3. ALSA+V4L2

There are some pending issues related to the needs at ALSA by V4L2 drivers.

3.1. Compressed audio (ivtv, pvrusb2)

Currently, two audio drivers (ivtv and pvrusb2) currently provides pure audio mpeg TS streams,
either at video or radio node, in order for the cards to work with FM radio. This is an API
violation that should be fixed. Also, newer radio USB sticks might also have compressed radio
support. So, it is needed a way to move the existing devices and enforce newer ones to use ALSA api
for audio-only output.

During the discussions, Takashi Iwai (ALSA maintainer) said that the compressed audio format
support for ALSA, including sound drivers, will be supported in 3.7. Patches to add support in
driver were added on Aug, 20:
http://git.kernel.org/?p=linux/kernel/git/broonie/sound.git;... 
Yet, the userspace ALSA library currently doesn't support yet, as it currently on a separate
library.
tinycompress: http://git.alsa-project.org/?p=tinycompress.git;a=summary
Vinod Koul (vinod.koul@linux.intel.com) is working with the userspace library. It seems that
there's no technical reason why not adding it at alsalib, so it was proposed that this should be
merged at alsalib, in order to allow the drivers conversions.
The headers for the kernel API are inside the
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git tree, at:
	include/sounds/compress_params.h
	include/sounds/compress_offload.h

3.2. timestamps

Userspace needs to sync audio and video. V4L2 has timestamps already. Currently, alsalib provides
the PCM_STATUS: current position, system time. There is no device timestamp at kernelspace,
however; the time shift is calculated by knowing the sampling time.
In opposite to pure audio devices, on audio and video devices, when capture starts, the audio can
start later than video, or together with it, depending on the driver. Such time difference between
audio and video could be calculated in order to adjust the initial drift, to be used as the initial
timestamp for the start of the audio.
However, audio/video delay may not remain constant. For example, when changing inputs, the audio
can cut off while the input change is in progress, so there need to be a way to resync the audio
timestamp. Also, small time drifts may happen during audio/video stream lifetime. So, it should be
possible to have a way for the driver to periodically tag the audio stream with the time
reference.

In the case of compressed audio, the API has a timestamp already. So, no changes there should be
needed.

3.3. Usage of media API by ALSA

There's a requirement of using the media API also by ALSA, as this is needed on some embedded
devices. As there will be some discussions about that theme at LPC, on Wednesday 11:40, at
Nautilus 2 room:
http://summit.linuxplumbersconf.org/lpc-2012/meeting/80/l... 
It was decided to not talk about that on this time slot.
There is a preliminary ALSA RFC patches with such proposal at:
http://comments.gmane.org/gmane.linux.alsa.devel/100537

_______________________________________________
Ksummit-2012-discuss mailing list
Ksummit-2012-discuss@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/ksumm...