Re: MEDIA_IOC_G_TOPOLOGY and pad indices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/05/2018 12:15 PM, Mauro Carvalho Chehab wrote:
> Hi Hans,
> 
> Em Sun, 4 Feb 2018 14:06:42 +0100
> Hans Verkuil <hverkuil@xxxxxxxxx> escreveu:
> 
>> Hi Mauro,
>>
>> I'm working on adding proper compliance tests for the MC but I think something
>> is missing in the G_TOPOLOGY ioctl w.r.t. pads.
>>
>> In several v4l-subdev ioctls you need to pass the pad. There the pad is an index
>> for the corresponding entity. I.e. an entity has 3 pads, so the pad argument is
>> [0-2].
>>
>> The G_TOPOLOGY ioctl returns a pad ID, which is > 0x01000000. I can't use that
>> in the v4l-subdev ioctls, so how do I translate that to a pad index in my application?
>>
>> It seems to be a missing feature in the API. I assume this information is available
>> in the core, so then I would add a field to struct media_v2_pad with the pad index
>> for the entity.
> 
> No, this was designed this way by purpose, back in 2015, when this was
> discussed at the first MC workshop. It was a consensus on that time that
> parameters like PAD index, PAD name, etc should be implemented via the
> properties API, as proposed by Sakari [1]. We also had a few discussions 
> about that on both IRC and ML.
> 
> [1] See: https://linuxtv.org/news.php?entry=2015-08-17.mchehab
> 
> 	3.3 Action items
> 	...
> 	media_properties: RFC userspace API: Sakari
> 
> Unfortunately, Sakari never found the time to send us a patch series
> implementing a properties API, even as RFC.
> 
> One of the other missing features is to add a new version for setting
> links (I guess we called it as S_LINKS_V2 at the meetings there).
> The hole idea is to provide a way to setup the entire pipeline using
> a single ioctl, on an atomic way.
> 
> The big problem with pad indexes happen on entities that have PADs with
> different types of signal input or output, like for example a TV tuner.
> 
> On almost all PC consumers hardware supported by the Kernel, the same
> PCI/USB ID may come with different types of hardware. This may also
> happen with embedded TV sets, as, depending on the market is is
> sold, the same product may come with a different tuner.
> 
> A "classic" TV tuner usually has those types of output signals:
> 
> 	- analog TV luminance IF;
> 	- analog TV chrominance IF [1];
> 	- digital TV OFDM IF;
> 	- audio IF;
> 
> [1] As both luminance and chrominance should go to the same TV
>     demod, in practice, we can group both signals together on a
>     single pad.
> 
> More modern tuners also have an audio demod integrated, meaning that
> they also have another PAD:
> 	- digital mono or stereo audio.
> 
> At the simplest possible scenario, let's say that we have a TV device
> has those entities (among others), each with a single PAD input:
> 
> 	- entity #0: a TV tuner;
> 	- entity #1: an audio demod;
> 	- entity #2: an analog TV demod;
> 	- entity #3: a digital TV demod.
> 
> And the TV tuner has 4 output pads:
> 
> 	- pad #0 -> analog TV luminance/chrominance;
> 	- pad #1 -> digital TV IF;
> 	- pad #2 -> audio IF;
> 	- pad #3 -> digital audio.
> 
> There, pad #0 can only be connected to entity #2. You cannot
> connect any other pad to entity #2. The same apply to every other
> TV tuner output PAD.
> 
> In this specific scenario, entity #1 can only be connected to pad #2,
> and pad #3 can't be connected anywhere, as, on this model opted to
> have a separate audio demod, and didn't wired the digital audio output.
> Yet, the subdev has it.
> 
> Another TV set may have different pad numbers - placing them even on a
> different order, and opting to use the audio demod tuner, wiring the
> digital audio output.
> 
> Currently, there's no way for an userspace application to know what pads
> can be linked to each entity, as there's no way to tell userspace what
> kind of signal each pad supports.
> 
> In any case, the Kernel should either detect the tuner model or know
> it (via a different DT entry), but userspace need such information,
> in order to be able to properly set the pipelines.
> 
> So, what we really need here is passing an optional set of properties
> to userspace in order to allow it to do the right thing.

I fail to see the problem. Entities have pads. Pads have both a unique
ID and an index within the entity pad list. I.e. the pad ID and the
(entity ID + pad index) tuple both uniquely identify the pad.

Whether a pad is connected to anything or what type it is is unrelated
to this. Passing the pad index of an unconnected pad to e.g. SUBDEV_S_FMT
will just result in an error. All I need is to be able to obtain the
pad index so I can call SUBDEV_S_FMT at all!

I actually like G_TOPOLOGY, it's nice. But it just does not give all the
information needed.

> Yet, I agree with you: we should not wait anymore for a properties API,
> as it seems unlikely that we'll add support for it anytime soon.
> 
> So, let's add two fields there:
> 	- pad index number;

So should I also add a flag to signal that the pad index is set? Since
current and past kernels never set this. Obviously none of the current
applications using G_TOPOLOGY would use a pad index because there isn't
one. It would only be a problem for new applications that switch to
G_TOPOLOGY, use subdevs and assume that the kernel that is used supports
the pad index. I'm not sure if this warrants a new pad flag though.

> 	- pad type.
> 
> In order to make things easy, I guess the best would be to use a string
> for the pad type, and fill it only for entities where it is relevant
> (like TV tuners and demods).

struct media_v2_pad {
        __u32 id;
        __u32 entity_id;
        __u32 flags;
        __u32 reserved[5];
} __attribute__ ((packed));

This does not easily lend itself to a string. A pad type u16 would be easy
to add though. That said, adding a pad type is a completely separate issue
and needs an RFC or something to define this. If there such an RFC was posted
in the past, can you provide a link to refresh my memory?

Regards,

	Hans



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux