Em Sun, 04 Feb 2018 15:20:55 +0200
Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> escreveu:
> Hi Hans,
>
> On Sunday, 4 February 2018 15:16:26 EET Hans Verkuil wrote:
> > On 02/04/2018 02:13 PM, Laurent Pinchart wrote:
> > > On Sunday, 4 February 2018 15:06:42 EET Hans Verkuil wrote:
> > >> 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.
> > >>
> > >> Next time we add new public API features I want to see compliance tests
> > >> before accepting it. It's much too easy to overlook something, either in
> > >> the design or in a driver or in the documentation, so this is really,
> > >> really needed IMHO.
> > >
> > > I agree with you, and would even like to go one step beyond by requiring
> > > an implementation in a real use case, not just in a compliance or test
> > > tool.
We could require an open source real application and some hardware to
allow us to test new APIs before allowing adding them, but I suspect that
this will just stall the development, as, on most companies, one development
team work on writing a new Kernel feature. Once done, a separate team
starts to implement userspace tools. On embedded world, this doesn't even
envolve writing any open source apps.
> > > On the topic of the G_TOPOLOGY API, it's pretty clear it was merged too
> > > hastily. We could try to fix it, but given all the issues we haven't
> > > solved yet, I believe a new version of the API would be better.
> >
> > It's actually not too bad as an API speaking as an end-user. It's simple and
> > efficient. But this pad issue is a real problem.
>
> We have other issues such as connector support
The thing with connector is unrelated with the API that reports entities.
>From API PoV, a connector is just a new entity type.
The discussions around it were purely related about how to deal with the
case where a single physical connector carries multiple signals on it,
but require different settings, depending on how this is physically wired.
This is a very common problem with S-Video connectors
(MEDIA_ENT_F_CONN_SVIDEO), as lots of boards are shipped with a cable to
allow using a S-Video input for composite video.
At proprietary software that comes with those boards, it identifies a
single S-Video connector as if they were two different physical connectors
(e. g. it looks at the final connector after the cabling).
In other words S-Video input physical connectors at the board can be used
on two different scenarios:
1) using S-Video/S-Video cable, using 4 pins, 2 for chrominance, 2 for
luminance. The end connector is a S-Video plug.
2) using a S-Video to composite video cable, using just 2 pins of the
board input connector. The end connector is RCA composite plug.
There are even some scenarios (very common on Hauppauge devices) where
a single multiple pin proprietary connector has multiple physical
connectors at their ends for both Audio and Video. Among them, there
is a separate S-Video plug and a separate Composite RCA plug.
On such cables, the Composite connector is usually physically wired
to the S-Video Y signal, just like if it had a S-Video to composite
cable.
Depending if the physical connector is connected using a S-Video/S-Video
or a S-Video/Composite cable, the settings at the driver should be
different (not only enabling input pins but also changing some demod
parameters in order to provide enhanced quality for S-Video).
So, while physically there is just one connector at the board, logically,
there are two different V4L2 device/subdev settings, depending on the type
of cable/signals connected to it.
Any way, such discussion is completely orthogonal to G_TOPOLOGY.
No matter what API we use, we should have a way to allow userspace
to select between "S-Video" and "composite" on devices that provide
S-Video physical connectors.
As discussed, the main alternatives are:
1) Have one pad for Y signal and one pad for C signal.
If both are linked to a S-Video connector, it is in S-Video mode.
If just Y signal is linked, it is in composite mode.
2) Have one pad for S-Video, one pad for Composite.
If composite pad is linked to a S-Video connector, it is composite;
if s-video pad is linked to a S-Video connector, it is S-Video.
We didn't reach a consensus if either (1) or (2) would be the better
alternative. The main reason for not reaching a consensus is how
to map it to DT. So, it was decided to not expose connectors:
93125094c07d ("[media] media.h: postpone connectors entities")
Once we reach an agreement, all we need is to revert the above
patch and adjust the drivers that use MEDIA_ENT_F_CONN_SVIDEO to
do the right thing. That won't affect the ioctl.
> and entities function vs. types that we have never solved.
Huh? This was solved. The legacy API was per type, while G_TOPOLOGY is
per function.
What we don't have yet is a case where a single entity provide multiple
functions. Nobody ever submitted any patchset covering such scenario.
What it was agreed is that, if we ever have have such kind of entities,
the other functions would be exposed via the properties API.
> The G_TOPOLOGY ioctl moves in the right direction
> but has clearly been merged too early. It might be possible to fix it, I
> haven't checked yet, but I really don't want to see this mistake being
> repeated in the future.
Thanks,
Mauro
