Re: MEDIA_IOC_G_TOPOLOGY and pad indices

[Mauro Carvalho Chehab <mchehab@kernel.org>]

Em Mon, 5 Feb 2018 12:38:29 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> 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@xs4all.nl> 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.  
> 
> I'll read up on this and reply to this later.
> 
> <snip>
> 
> >> 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.  
> > 
> > We added a test tool for G_TOPOLOGY on that time.
> > 
> > I doubt that writing test/compliance tools in advance will solve all API
> > gaps. The thing is that new features will take some time to be used on
> > real apps. Some things are only visible when real apps start using the
> > new API features.  
> 
> This is no excuse whatsoever NOT to write compliance tests. Sure, they will
> never find everything, but for sure they find a LOT! Especially all the
> little stupid things that can make something unusable for certain use-cases.

I agree that either a compliance or a test app is important.

> And equally important, driver developers can use it to check that they didn't
> forget anything.
> 
> A media-ctl/v4l2-ctl/whatever utility is no substitute for proper compliance
> testing. The media API is complex because video is complex. It is impossible
> to review code and catch all the little mistakes, but compliance tests can
> go far in verifying driver code and also catching core code regressions.
> 
> I have yet to see a new driver that didn't fail at least one v4l2-compliance
> test, and I very much doubt that existing subdev/media drivers would do any
> different.
> 
> It would be very interesting if you would test it as well on DVB devices.
> You should be able to run v4l2-compliance on the media device. There may
> well be bugs in the tests for DVB devices. But that might also be caused
> by incomplete documentation in the spec.

I can surely do some tests on DVB devices too. Please remind me after a
couple of weeks. I'm very busy this week, and usually the first week
after a merge window is also a busy one.


Regards,
Mauro