Re: [PATCH v2 16/30] v4l: subdev: Add [GS]_ROUTING subdev ioctls and operations

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

 



Hi,

On 22/02/2019 11:29, Sakari Ailus wrote:
Hi Jacopo,

On Fri, Feb 22, 2019 at 12:17:47PM +0100, Jacopo Mondi wrote:
Hi Sakari,
     thanks for your suggestions.

On Fri, Feb 22, 2019 at 01:04:29PM +0200, Sakari Ailus wrote:
Hi Jacopo,

[snip]

On the previous example, I thought about GMSL-like devices, that can
output the video streams received from different remotes in a
separate virtual channel, at the same time.

A possible routing table in that case would be like:

Pads 0, 1, 2, 3 = SINKS
Pad 4 = SOURCE with 4 streams (1 for each VC)

0/0 -> 4/0
0/0 -> 4/1
0/0 -> 4/2
0/0 -> 4/3
1/0 -> 4/0
1/0 -> 4/1
1/0 -> 4/2
1/0 -> 4/3
2/0 -> 4/0
2/0 -> 4/1
2/0 -> 4/2
2/0 -> 4/3
3/0 -> 4/0
3/0 -> 4/1
3/0 -> 4/2
3/0 -> 4/3

If more than one pad can handle multiplexed streams, then you may end up in
a situation like that. Indeed.


Please note that in this case there is only one pad that can handle
multiplexed stream. The size of the routing table is the
multiplication of the total number of pads by the product of all
streams per pad. In this case (4 * (1 * 1 * 1 * 4))

Oh, good point, that's the case for G_ROUTING. I thought of S_ROUTING only.
:-)



With only one route per virtual channel active at a time.

[snip]


Thanks, I had a look at the MEDIA_ ioctls yesterday, G_TOPOLOGY in
particular, which uses several pointers to arrays.

Unfortunately, I didn't come up with anything better than using a
translation structure, from the IOCTL layer to the subdevice
operations layer:
https://paste.debian.net/hidden/b192969d/
(sharing a link for early comments, I can send v3 and you can comment
there directly if you prefer to :)

Hmm. That is a downside indeed. It's still a lesser problem than the compat
code in general --- which has been a source for bugs as well as nasty
security problems over time.


Good!

I think we need a naming scheme for such structs. How about just
calling that struct e.g. v4l2_subdev_krouting instead? It's simple, easy to
understand and it includes a suggestion which one is the kernel-only
variant.


I kind of like that! thanks!

You can btw. zero the struct memory by assigning { 0 } to it in
declaration. memset() in general is much more trouble. In this case you
could even do the assignments in delaration as well.


Thanks, noted. I have been lazy and copied memset from other places in
the ioctl handling code. I should check on your suggestions because I
remember one of the many 0-initialization statement was a GCC specific one,
don't remember which...

{} is GCC specific whereas { 0 } is not. But there have been long-standing
GCC bugs related to the use of { 0 } which is quite unfortunate --- they've
produced warnings or errors from code that is valid C...


Using = {} to intialise structs is all over the kernel and quite accepted. Eg. recent discussion with Mauro at [1].

[1] https://lore.kernel.org/linux-media/20181207105816.4c53aeaa@xxxxxxxx/

Regards,
Ian



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux